From 35821c9505aad1cbbdee537762c65076f90b9acb Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Thu, 22 Jan 2015 01:05:13 +0100 Subject: [PATCH 1/2] pull: use a single per-transaction syncfs instead of fsync Do not write directly to objects/ but maintain pulled files under tmp/ with a "tmpobject-$CHECKSUM.$OBJTYPE" name until they are syncfs'ed to disk. Move them under objects/ at ostree_repo_commit_transaction cleanup time. Before (test done on a local network): $ LANG=C sudo time ./ostree --repo=repo pull origin master 0 metadata, 3 content objects fetched; 83820 KiB; 4 delta parts fetched, transferred in 417 seconds 16.42user 6.73system 6:57.19elapsed 5%CPU (0avgtext+0avgdata 248428maxresident)k 24inputs+794472outputs (0major+233968minor)pagefaults 0swaps After: $ LANG=C sudo time ./ostree --repo=repo pull origin master 0 metadata, 3 content objects fetched; 83820 KiB; 4 delta parts fetched, transferred in 9 seconds 14.70user 2.87system 0:09.99elapsed 175%CPU (0avgtext+0avgdata 256168maxresident)k 0inputs+794472outputs (0major+164333minor)pagefaults 0swaps https://bugzilla.gnome.org/show_bug.cgi?id=728065 Signed-off-by: Giuseppe Scrivano --- src/libostree/ostree-repo-commit.c | 112 +++++++++++++++++++++++----- src/libostree/ostree-repo-private.h | 5 ++ src/libostree/ostree-repo.c | 68 +++++++++++------ 3 files changed, 144 insertions(+), 41 deletions(-) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index a149c2e0d1..67ffb34eb0 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -35,6 +35,7 @@ #include "ostree-mutable-tree.h" #include "ostree-varint.h" #include +#include gboolean _ostree_repo_ensure_loose_objdir_at (int dfd, @@ -59,6 +60,17 @@ _ostree_repo_ensure_loose_objdir_at (int dfd, return TRUE; } +void +_ostree_repo_get_tmpobject_path (char *output, + const char *checksum, + OstreeObjectType objtype) +{ + g_sprintf (output, + "tmpobject-%s.%s", + checksum, + ostree_object_type_to_string (objtype)); +} + static GVariant * create_file_metadata (GFileInfo *file_info, GVariant *xattrs) @@ -108,6 +120,7 @@ write_file_metadata_to_xattr (int fd, static gboolean commit_loose_object_trusted (OstreeRepo *self, + const char *checksum, OstreeObjectType objtype, const char *loose_path, GFile *temp_file, @@ -251,7 +264,7 @@ commit_loose_object_trusted (OstreeRepo *self, /* Ensure that in case of a power cut, these files have the data we * want. See http://lwn.net/Articles/322823/ */ - if (!self->disable_fsync) + if (!self->in_transaction && !self->disable_fsync) { if (fsync (fd) == -1) { @@ -267,20 +280,39 @@ commit_loose_object_trusted (OstreeRepo *self, if (!_ostree_repo_ensure_loose_objdir_at (self->objects_dir_fd, loose_path, cancellable, error)) goto out; - - if (G_UNLIKELY (renameat (self->tmp_dir_fd, temp_filename, - self->objects_dir_fd, loose_path) == -1)) - { - if (errno != EEXIST) - { - gs_set_error_from_errno (error, errno); - g_prefix_error (error, "Storing file '%s': ", temp_filename); - goto out; - } - else - (void) unlinkat (self->tmp_dir_fd, temp_filename, 0); - } + { + gs_free gchar *tmp_dest = NULL; + int dir; + const char *dest; + + if (self->in_transaction) + { + char tmpbuf[_OSTREE_LOOSE_PATH_MAX]; + _ostree_repo_get_tmpobject_path (tmpbuf, checksum, objtype); + tmp_dest = g_strdup (tmpbuf); + dir = self->tmp_dir_fd; + dest = tmp_dest; + } + else + { + dir = self->objects_dir_fd; + dest = loose_path; + } + + if (G_UNLIKELY (renameat (self->tmp_dir_fd, temp_filename, + dir, dest) == -1)) + { + if (errno != EEXIST) + { + gs_set_error_from_errno (error, errno); + g_prefix_error (error, "Storing file '%s': ", temp_filename); + goto out; + } + else + (void) unlinkat (self->tmp_dir_fd, temp_filename, 0); + } + } ret = TRUE; out: return ret; @@ -474,7 +506,7 @@ write_object (OstreeRepo *self, { if (!_ostree_repo_has_loose_object (self, expected_checksum, objtype, &have_obj, loose_objpath, - cancellable, error)) + NULL, cancellable, error)) goto out; if (have_obj) { @@ -655,7 +687,7 @@ write_object (OstreeRepo *self, } if (!_ostree_repo_has_loose_object (self, actual_checksum, objtype, - &have_obj, loose_objpath, + &have_obj, loose_objpath, NULL, cancellable, error)) goto out; @@ -663,7 +695,8 @@ write_object (OstreeRepo *self, if (do_commit) { - if (!commit_loose_object_trusted (self, objtype, loose_objpath, + if (!commit_loose_object_trusted (self, actual_checksum, + objtype, loose_objpath, temp_file, temp_filename, object_is_symlink, file_info, xattrs, temp_out, @@ -944,6 +977,7 @@ ostree_repo_prepare_transaction (OstreeRepo *self, static gboolean cleanup_tmpdir (OstreeRepo *self, + gboolean move_tmpobject, GCancellable *cancellable, GError **error) { @@ -966,6 +1000,7 @@ cleanup_tmpdir (OstreeRepo *self, GFile *path; guint64 mtime; guint64 delta; + gs_free char *basename = NULL; if (!gs_file_enumerator_iterate (enumerator, &file_info, &path, cancellable, error)) @@ -973,6 +1008,39 @@ cleanup_tmpdir (OstreeRepo *self, if (file_info == NULL) break; + if (move_tmpobject) + { + basename = g_file_get_basename (path); + if (strncmp (basename, "tmpobject-", 10) == 0) + { + char loose_path[_OSTREE_LOOSE_PATH_MAX]; + gs_free gchar *checksum = NULL; + OstreeObjectType type; + ostree_object_from_string (basename + 10, + &checksum, + &type); + + _ostree_loose_path (loose_path, checksum, type, self->mode); + + if (!_ostree_repo_ensure_loose_objdir_at (self->objects_dir_fd, loose_path, + cancellable, error)) + goto out; + + if (G_UNLIKELY (renameat (self->tmp_dir_fd, basename, + self->objects_dir_fd, loose_path) < 0)) + { + (void) unlinkat (self->tmp_dir_fd, basename, 0); + if (errno != EEXIST) + { + gs_set_error_from_errno (error, errno); + g_prefix_error (error, "Storing file '%s': ", loose_path); + goto out; + } + } + continue; + } + } + mtime = g_file_info_get_attribute_uint64 (file_info, "time::modified"); if (mtime > curtime_secs) continue; @@ -1109,7 +1177,13 @@ ostree_repo_commit_transaction (OstreeRepo *self, g_return_val_if_fail (self->in_transaction == TRUE, FALSE); - if (!cleanup_tmpdir (self, cancellable, error)) + if (syncfs (self->tmp_dir_fd) < 0) + { + gs_set_error_from_errno (error, errno); + goto out; + } + + if (!cleanup_tmpdir (self, TRUE, cancellable, error)) goto out; if (self->loose_object_devino_hash) @@ -1143,7 +1217,7 @@ ostree_repo_abort_transaction (OstreeRepo *self, if (!self->in_transaction) return TRUE; - if (!cleanup_tmpdir (self, cancellable, error)) + if (!cleanup_tmpdir (self, FALSE, cancellable, error)) goto out; if (self->loose_object_devino_hash) diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h index b36e6d9405..d569a28008 100644 --- a/src/libostree/ostree-repo-private.h +++ b/src/libostree/ostree-repo-private.h @@ -82,6 +82,10 @@ _ostree_repo_ensure_loose_objdir_at (int dfd, const char *loose_path, GCancellable *cancellable, GError **error); +void +_ostree_repo_get_tmpobject_path (char *output, + const char *checksum, + OstreeObjectType objtype); gboolean _ostree_repo_find_object (OstreeRepo *self, @@ -101,6 +105,7 @@ _ostree_repo_has_loose_object (OstreeRepo *self, OstreeObjectType objtype, gboolean *out_is_stored, char *loose_path_buf, + GFile **out_stored_path, GCancellable *cancellable, GError **error); diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index f27911dea1..d54150c23e 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -1715,6 +1715,13 @@ load_metadata_internal (OstreeRepo *self, cancellable, error)) goto out; + if (self->in_transaction && fd < 0) + { + _ostree_repo_get_tmpobject_path (loose_path_buf, sha256, objtype); + if (!openat_allow_noent (self->tmp_dir_fd, loose_path_buf, &fd, cancellable, error)) + goto out; + } + if (fd != -1) { if (out_variant) @@ -2111,27 +2118,55 @@ _ostree_repo_has_loose_object (OstreeRepo *self, OstreeObjectType objtype, gboolean *out_is_stored, char *loose_path_buf, + GFile **out_stored_path, GCancellable *cancellable, GError **error) { gboolean ret = FALSE; struct stat stbuf; - int res; + int res = -1; + gboolean tmp_file = FALSE; - _ostree_loose_path (loose_path_buf, checksum, objtype, self->mode); + if (self->in_transaction) + { + _ostree_repo_get_tmpobject_path (loose_path_buf, checksum, objtype); + do + res = fstatat (self->tmp_dir_fd, loose_path_buf, &stbuf, AT_SYMLINK_NOFOLLOW); + while (G_UNLIKELY (res == -1 && errno == EINTR)); + if (res == -1 && errno != ENOENT) + { + gs_set_error_from_errno (error, errno); + goto out; + } + } - do - res = fstatat (self->objects_dir_fd, loose_path_buf, &stbuf, AT_SYMLINK_NOFOLLOW); - while (G_UNLIKELY (res == -1 && errno == EINTR)); - if (res == -1 && errno != ENOENT) + if (res == 0) + tmp_file = TRUE; + else { - gs_set_error_from_errno (error, errno); - goto out; + _ostree_loose_path (loose_path_buf, checksum, objtype, self->mode); + + do + res = fstatat (self->objects_dir_fd, loose_path_buf, &stbuf, AT_SYMLINK_NOFOLLOW); + while (G_UNLIKELY (res == -1 && errno == EINTR)); + if (res == -1 && errno != ENOENT) + { + gs_set_error_from_errno (error, errno); + goto out; + } } ret = TRUE; *out_is_stored = (res != -1); - out: + + if (out_stored_path) + { + if (res != -1) + *out_stored_path = g_file_resolve_relative_path (tmp_file ? self->tmp_dir : self->objects_dir, loose_path_buf); + else + *out_stored_path = NULL; + } +out: return ret; } @@ -2143,21 +2178,10 @@ _ostree_repo_find_object (OstreeRepo *self, GCancellable *cancellable, GError **error) { - gboolean ret = FALSE; gboolean has_object; char loose_path[_OSTREE_LOOSE_PATH_MAX]; - - if (!_ostree_repo_has_loose_object (self, checksum, objtype, &has_object, loose_path, - cancellable, error)) - goto out; - - ret = TRUE; - if (has_object) - *out_stored_path = g_file_resolve_relative_path (self->objects_dir, loose_path); - else - *out_stored_path = NULL; -out: - return ret; + return _ostree_repo_has_loose_object (self, checksum, objtype, &has_object, loose_path, + out_stored_path, cancellable, error); } /** From c88ea80ce90030754c31761ff25cd89157bc0f6c Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Wed, 28 Jan 2015 12:31:51 +0100 Subject: [PATCH 2/2] transactions: move all tmpobject-%s files under a per-boot subdir This prevents to use files after a kernel crash or power failure and that can be not completely synced to disk. Signed-off-by: Giuseppe Scrivano --- src/libostree/ostree-repo-commit.c | 168 +++++++++++++++++++++------- src/libostree/ostree-repo-private.h | 6 +- src/libostree/ostree-repo.c | 8 +- 3 files changed, 137 insertions(+), 45 deletions(-) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index 67ffb34eb0..5117cf35f2 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -60,15 +60,42 @@ _ostree_repo_ensure_loose_objdir_at (int dfd, return TRUE; } -void +static const gchar * +ostree_repo_get_tmpobject_bootid (GError **error) +{ + static gchar *contents; + static gsize bootid_initialized; + if (g_once_init_enter (&bootid_initialized)) + { + if (g_file_get_contents ("/proc/sys/kernel/random/boot_id", + &contents, + NULL, + error)) + g_strdelimit (contents, "\n", '\0'); + + g_once_init_leave (&bootid_initialized, 1); + } + + return contents; +} + +gboolean _ostree_repo_get_tmpobject_path (char *output, const char *checksum, - OstreeObjectType objtype) + OstreeObjectType objtype, + GCancellable *cancellable, + GError **error) { + const char *boot_id; + if ((boot_id = ostree_repo_get_tmpobject_bootid (error)) == NULL) + return FALSE; + g_sprintf (output, - "tmpobject-%s.%s", + "%s/tmpobject-%s.%s", + boot_id, checksum, ostree_object_type_to_string (objtype)); + return TRUE; } static GVariant * @@ -289,7 +316,9 @@ commit_loose_object_trusted (OstreeRepo *self, if (self->in_transaction) { char tmpbuf[_OSTREE_LOOSE_PATH_MAX]; - _ostree_repo_get_tmpobject_path (tmpbuf, checksum, objtype); + if (! _ostree_repo_get_tmpobject_path (tmpbuf, checksum, objtype, + cancellable, error)) + goto out; tmp_dest = g_strdup (tmpbuf); dir = self->tmp_dir_fd; dest = tmp_dest; @@ -944,6 +973,7 @@ ostree_repo_prepare_transaction (OstreeRepo *self, gboolean ret = FALSE; gboolean ret_transaction_resume = FALSE; gs_free char *transaction_str = NULL; + const char *boot_id; g_return_val_if_fail (self->in_transaction == FALSE, FALSE); @@ -963,6 +993,20 @@ ostree_repo_prepare_transaction (OstreeRepo *self, if (!ot_gfile_ensure_unlinked (self->transaction_lock_path, cancellable, error)) goto out; } + + if ((boot_id = ostree_repo_get_tmpobject_bootid (error)) == NULL) + goto out; + + if (mkdirat (self->tmp_dir_fd, boot_id, 0777) == -1) + { + int errsv = errno; + if (G_UNLIKELY (errsv != EEXIST)) + { + gs_set_error_from_errno (error, errsv); + goto out; + } + } + transaction_str = g_strdup_printf ("pid=%llu", (unsigned long long) getpid ()); if (!g_file_make_symbolic_link (self->transaction_lock_path, transaction_str, cancellable, error)) @@ -975,9 +1019,82 @@ ostree_repo_prepare_transaction (OstreeRepo *self, return ret; } +static gboolean +rename_pending_loose_objects (OstreeRepo *self, + GCancellable *cancellable, + GError **error) +{ + gboolean ret = FALSE; + const char *boot_id; + gs_unref_object GFile *tmpobjectsdir = NULL; + int dir_fd = -1; + gs_dirfd_iterator_cleanup GSDirFdIterator child_dfd_iter = { 0, }; + + if ((boot_id = ostree_repo_get_tmpobject_bootid (error)) == NULL) + goto out; + + tmpobjectsdir = g_file_get_child (self->tmp_dir, boot_id); + if (! tmpobjectsdir) + goto out; + + if (! gs_file_open_dir_fd (tmpobjectsdir, &dir_fd, cancellable, error)) + goto out; + + if (!gs_dirfd_iterator_init_at (self->tmp_dir_fd, boot_id, FALSE, &child_dfd_iter, error)) + goto out; + + while (TRUE) + { + struct dirent *out_dent; + + if (!gs_dirfd_iterator_next_dent (&child_dfd_iter, &out_dent, cancellable, error)) + goto out; + + if (out_dent == NULL) + break; + + if (strncmp (out_dent->d_name, "tmpobject-", 10) == 0) + { + char loose_path[_OSTREE_LOOSE_PATH_MAX]; + gs_free gchar *checksum = NULL; + OstreeObjectType type; + ostree_object_from_string (out_dent->d_name + 10, + &checksum, + &type); + + _ostree_loose_path (loose_path, checksum, type, self->mode); + + if (!_ostree_repo_ensure_loose_objdir_at (self->objects_dir_fd, loose_path, + cancellable, error)) + goto out; + + if (G_UNLIKELY (renameat (dir_fd, out_dent->d_name, + self->objects_dir_fd, loose_path) < 0)) + { + (void) unlinkat (self->tmp_dir_fd, out_dent->d_name, 0); + if (errno != EEXIST) + { + gs_set_error_from_errno (error, errno); + g_prefix_error (error, "Storing file '%s': ", loose_path); + goto out; + } + } + continue; + } + } + + if (!gs_shutil_rm_rf_at (self->tmp_dir_fd, boot_id, cancellable, error)) + goto out; + + ret = TRUE; + out: + if (dir_fd >= 0) + close (dir_fd); + return ret; +} + static gboolean cleanup_tmpdir (OstreeRepo *self, - gboolean move_tmpobject, GCancellable *cancellable, GError **error) { @@ -1000,7 +1117,6 @@ cleanup_tmpdir (OstreeRepo *self, GFile *path; guint64 mtime; guint64 delta; - gs_free char *basename = NULL; if (!gs_file_enumerator_iterate (enumerator, &file_info, &path, cancellable, error)) @@ -1008,39 +1124,6 @@ cleanup_tmpdir (OstreeRepo *self, if (file_info == NULL) break; - if (move_tmpobject) - { - basename = g_file_get_basename (path); - if (strncmp (basename, "tmpobject-", 10) == 0) - { - char loose_path[_OSTREE_LOOSE_PATH_MAX]; - gs_free gchar *checksum = NULL; - OstreeObjectType type; - ostree_object_from_string (basename + 10, - &checksum, - &type); - - _ostree_loose_path (loose_path, checksum, type, self->mode); - - if (!_ostree_repo_ensure_loose_objdir_at (self->objects_dir_fd, loose_path, - cancellable, error)) - goto out; - - if (G_UNLIKELY (renameat (self->tmp_dir_fd, basename, - self->objects_dir_fd, loose_path) < 0)) - { - (void) unlinkat (self->tmp_dir_fd, basename, 0); - if (errno != EEXIST) - { - gs_set_error_from_errno (error, errno); - g_prefix_error (error, "Storing file '%s': ", loose_path); - goto out; - } - } - continue; - } - } - mtime = g_file_info_get_attribute_uint64 (file_info, "time::modified"); if (mtime > curtime_secs) continue; @@ -1183,7 +1266,10 @@ ostree_repo_commit_transaction (OstreeRepo *self, goto out; } - if (!cleanup_tmpdir (self, TRUE, cancellable, error)) + if (! rename_pending_loose_objects (self, cancellable, error)) + goto out; + + if (!cleanup_tmpdir (self, cancellable, error)) goto out; if (self->loose_object_devino_hash) @@ -1217,7 +1303,7 @@ ostree_repo_abort_transaction (OstreeRepo *self, if (!self->in_transaction) return TRUE; - if (!cleanup_tmpdir (self, FALSE, cancellable, error)) + if (!cleanup_tmpdir (self, cancellable, error)) goto out; if (self->loose_object_devino_hash) diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h index d569a28008..0395fc947f 100644 --- a/src/libostree/ostree-repo-private.h +++ b/src/libostree/ostree-repo-private.h @@ -82,10 +82,12 @@ _ostree_repo_ensure_loose_objdir_at (int dfd, const char *loose_path, GCancellable *cancellable, GError **error); -void +gboolean _ostree_repo_get_tmpobject_path (char *output, const char *checksum, - OstreeObjectType objtype); + OstreeObjectType objtype, + GCancellable *cancellable, + GError **error); gboolean _ostree_repo_find_object (OstreeRepo *self, diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index d54150c23e..a1a16dbe78 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -1717,7 +1717,9 @@ load_metadata_internal (OstreeRepo *self, if (self->in_transaction && fd < 0) { - _ostree_repo_get_tmpobject_path (loose_path_buf, sha256, objtype); + if (! _ostree_repo_get_tmpobject_path (loose_path_buf, sha256, objtype, + cancellable, error)) + goto out; if (!openat_allow_noent (self->tmp_dir_fd, loose_path_buf, &fd, cancellable, error)) goto out; } @@ -2129,7 +2131,9 @@ _ostree_repo_has_loose_object (OstreeRepo *self, if (self->in_transaction) { - _ostree_repo_get_tmpobject_path (loose_path_buf, checksum, objtype); + if (! _ostree_repo_get_tmpobject_path (loose_path_buf, checksum, objtype, + cancellable, error)) + goto out; do res = fstatat (self->tmp_dir_fd, loose_path_buf, &stbuf, AT_SYMLINK_NOFOLLOW); while (G_UNLIKELY (res == -1 && errno == EINTR));