From 15d93bf038df1743ff57d19b17a19e09530205da Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 27 Apr 2017 14:24:20 -0400 Subject: [PATCH] checkout/commit: Use glnx_regfile_copy_bytes() if possible Rather than `g_output_stream_splice()`, where the input is a regular file. See https://github.com/GNOME/libglnx/pull/44 for some more information. I didn't try to measure the performance difference, but seeing the read()/write() to/from userspace mixed in with the pointless `poll()` annoyed me when reading strace. As a bonus, we will again start using reflinks (if available) for `/etc`, which is a regression from the https://github.com/ostreedev/ostree/pull/797 changes (which before used `glnx_file_copy_at()`). Also, for the first time we'll use reflinks when doing commits from file-backed content. This happens in `rpm-ostree compose tree` today for example. Update submodule: libglnx --- libglnx | 2 +- src/libostree/ostree-repo-checkout.c | 44 ++++++++++++-------- src/libostree/ostree-repo-commit.c | 61 +++++++++++++--------------- 3 files changed, 57 insertions(+), 50 deletions(-) diff --git a/libglnx b/libglnx index 602fdd93cb..3a4d0f4684 160000 --- a/libglnx +++ b/libglnx @@ -1 +1 @@ -Subproject commit 602fdd93cb7a339c6b5749eee73df926429a5ab8 +Subproject commit 3a4d0f4684f4653338c4756c8a1abfc6b5738467 diff --git a/src/libostree/ostree-repo-checkout.c b/src/libostree/ostree-repo-checkout.c index fa29699b53..0dbaf487ef 100644 --- a/src/libostree/ostree-repo-checkout.c +++ b/src/libostree/ostree-repo-checkout.c @@ -102,7 +102,7 @@ fsync_is_enabled (OstreeRepo *self, static gboolean write_regular_file_content (OstreeRepo *self, OstreeRepoCheckoutAtOptions *options, - GOutputStream *output, + int outfd, GFileInfo *file_info, GVariant *xattrs, GInputStream *input, @@ -110,40 +110,54 @@ write_regular_file_content (OstreeRepo *self, GError **error) { const OstreeRepoCheckoutMode mode = options->mode; + g_autoptr(GOutputStream) outstream = NULL; - if (g_output_stream_splice (output, input, 0, - cancellable, error) < 0) - return FALSE; + if (G_IS_FILE_DESCRIPTOR_BASED (input)) + { + int infd = g_file_descriptor_based_get_fd ((GFileDescriptorBased*) input); + guint64 len = g_file_info_get_size (file_info); - if (!g_output_stream_flush (output, cancellable, error)) - return FALSE; + if (glnx_regfile_copy_bytes (infd, outfd, (off_t)len, TRUE) < 0) + return glnx_throw_errno_prefix (error, "regfile copy"); + } + else + { + outstream = g_unix_output_stream_new (outfd, FALSE); + if (g_output_stream_splice (outstream, input, 0, + cancellable, error) < 0) + return FALSE; - int fd = g_file_descriptor_based_get_fd ((GFileDescriptorBased*)output); + if (!g_output_stream_flush (outstream, cancellable, error)) + return FALSE; + } if (mode != OSTREE_REPO_CHECKOUT_MODE_USER) { - if (TEMP_FAILURE_RETRY (fchown (fd, g_file_info_get_attribute_uint32 (file_info, "unix::uid"), + if (TEMP_FAILURE_RETRY (fchown (outfd, g_file_info_get_attribute_uint32 (file_info, "unix::uid"), g_file_info_get_attribute_uint32 (file_info, "unix::gid"))) < 0) return glnx_throw_errno (error); - if (TEMP_FAILURE_RETRY (fchmod (fd, g_file_info_get_attribute_uint32 (file_info, "unix::mode"))) < 0) + if (TEMP_FAILURE_RETRY (fchmod (outfd, g_file_info_get_attribute_uint32 (file_info, "unix::mode"))) < 0) return glnx_throw_errno (error); if (xattrs) { - if (!glnx_fd_set_all_xattrs (fd, xattrs, cancellable, error)) + if (!glnx_fd_set_all_xattrs (outfd, xattrs, cancellable, error)) return FALSE; } } if (fsync_is_enabled (self, options)) { - if (fsync (fd) == -1) + if (fsync (outfd) == -1) return glnx_throw_errno (error); } - if (!g_output_stream_close (output, cancellable, error)) - return FALSE; + if (outstream) + { + if (!g_output_stream_close (outstream, cancellable, error)) + return FALSE; + } return TRUE; } @@ -231,7 +245,6 @@ create_file_copy_from_input_at (OstreeRepo *repo, else if (g_file_info_get_file_type (file_info) == G_FILE_TYPE_REGULAR) { glnx_fd_close int temp_fd = -1; - g_autoptr(GOutputStream) temp_out = NULL; guint32 file_mode; GLnxLinkTmpfileReplaceMode replace_mode; @@ -244,7 +257,6 @@ create_file_copy_from_input_at (OstreeRepo *repo, &temp_fd, &temp_filename, error)) return FALSE; - temp_out = g_unix_output_stream_new (temp_fd, FALSE); if (sepolicy_enabled) { @@ -258,7 +270,7 @@ create_file_copy_from_input_at (OstreeRepo *repo, return glnx_throw_errno_prefix (error, "Setting security.selinux"); } - if (!write_regular_file_content (repo, options, temp_out, file_info, xattrs, input, + if (!write_regular_file_content (repo, options, temp_fd, file_info, xattrs, input, cancellable, error)) return FALSE; diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index 45577373de..6d01de605f 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -459,29 +459,19 @@ add_size_index_to_metadata (OstreeRepo *self, } static gboolean -fallocate_stream (GFileDescriptorBased *stream, - goffset size, - GCancellable *cancellable, - GError **error) +ot_fallocate (int fd, goffset size, GError **error) { - gboolean ret = FALSE; - int fd = g_file_descriptor_based_get_fd (stream); + if (size == 0) + return TRUE; - if (size > 0) + int r = posix_fallocate (fd, 0, size); + if (r != 0) { - int r = posix_fallocate (fd, 0, size); - if (r != 0) - { - /* posix_fallocate is a weird deviation from errno standards */ - errno = r; - glnx_set_error_from_errno (error); - goto out; - } + /* posix_fallocate is a weird deviation from errno standards */ + errno = r; + return glnx_throw_errno_prefix (error, "fallocate"); } - - ret = TRUE; - out: - return ret; + return TRUE; } gboolean @@ -511,11 +501,10 @@ _ostree_repo_open_content_bare (OstreeRepo *self, &fd, &temp_filename, error)) goto out; - ret_stream = g_unix_output_stream_new (fd, TRUE); - - if (!fallocate_stream ((GFileDescriptorBased*)ret_stream, content_len, - cancellable, error)) + if (!ot_fallocate (fd, content_len, error)) goto out; + + ret_stream = g_unix_output_stream_new (fd, TRUE); } ret = TRUE; @@ -570,7 +559,6 @@ create_regular_tmpfile_linkable_with_content (OstreeRepo *self, GCancellable *cancellable, GError **error) { - g_autoptr(GOutputStream) temp_out = NULL; glnx_fd_close int temp_fd = -1; g_autofree char *temp_filename = NULL; @@ -578,20 +566,27 @@ create_regular_tmpfile_linkable_with_content (OstreeRepo *self, &temp_fd, &temp_filename, error)) return FALSE; - temp_out = g_unix_output_stream_new (temp_fd, FALSE); - if (!fallocate_stream ((GFileDescriptorBased*)temp_out, length, - cancellable, error)) + if (!ot_fallocate (temp_fd, length, error)) return FALSE; - if (g_output_stream_splice (temp_out, input, 0, - cancellable, error) < 0) - return FALSE; - if (fchmod (temp_fd, 0644) < 0) + if (G_IS_FILE_DESCRIPTOR_BASED (input)) { - glnx_set_error_from_errno (error); - return FALSE; + int infd = g_file_descriptor_based_get_fd ((GFileDescriptorBased*) input); + if (glnx_regfile_copy_bytes (infd, temp_fd, (off_t)length, TRUE) < 0) + return glnx_throw_errno_prefix (error, "regfile copy"); } + else + { + g_autoptr(GOutputStream) temp_out = g_unix_output_stream_new (temp_fd, FALSE); + if (g_output_stream_splice (temp_out, input, 0, + cancellable, error) < 0) + return FALSE; + } + + if (fchmod (temp_fd, 0644) < 0) + return glnx_throw_errno_prefix (error, "fchmod"); + *out_fd = temp_fd; temp_fd = -1; *out_path = g_steal_pointer (&temp_filename); return TRUE;