From b709c3c67bcaf3651ff2dd1d5a5f8680cf20d50c Mon Sep 17 00:00:00 2001 From: Jason Wessel Date: Wed, 10 Jul 2019 14:47:27 -0400 Subject: [PATCH] fsck: Implement a partial commit reason bitmask After the corruption has been fixed with "ostree fsck -a --delete", a second run of the "ostree fsck" command will print X partial commits not verified and exit with a zero. The zero exit code makes it hard to detect if a repair operation needs to be run. When ever fsck creates a partial commit it should add a reason for the partial commit to the state file found in state/.commitpartial. This will allow a future execution of the fsck to still return an error indicating that the repository is still in the damaged state, awaiting repair. Additional reason codes could be added in the future for why a partial commit exists. Text from: https://github.com/ostreedev/ostree/pull/1880 ==== cgwalters commented: To restate, the core issue is that it's valid to have partial commits for reasons other than fsck pruned bad objects, and libostree doesn't have a way to distinguish. Another option perhaps is to write e.g. fsck-partial into the statefile state/.commitpartial which would mean "partial, and expected to exist but was pruned by fsck" and fsck would continue to error out until the commit was re-pulled. Right now the partial stamp file is empty, so it'd be fully compatible to write a rationale into it. ==== Signed-off-by: Jason Wessel Closes: #1910 Approved by: cgwalters --- apidoc/ostree-sections.txt | 1 + src/libostree/libostree-devel.sym | 1 + src/libostree/ostree-repo-commit.c | 79 ++++++++++++++++++++++-------- src/libostree/ostree-repo.c | 15 ++++-- src/libostree/ostree-repo.h | 44 ++++++++++------- src/ostree/ot-builtin-fsck.c | 12 ++++- 6 files changed, 109 insertions(+), 43 deletions(-) diff --git a/apidoc/ostree-sections.txt b/apidoc/ostree-sections.txt index e8faeb10..252a563a 100644 --- a/apidoc/ostree-sections.txt +++ b/apidoc/ostree-sections.txt @@ -333,6 +333,7 @@ ostree_repo_set_cache_dir ostree_repo_sign_delta ostree_repo_has_object ostree_repo_mark_commit_partial +ostree_repo_mark_commit_partial_reason ostree_repo_write_metadata ostree_repo_write_metadata_async ostree_repo_write_metadata_finish diff --git a/src/libostree/libostree-devel.sym b/src/libostree/libostree-devel.sym index f552bcea..58d35f55 100644 --- a/src/libostree/libostree-devel.sym +++ b/src/libostree/libostree-devel.sym @@ -19,6 +19,7 @@ /* Add new symbols here. Release commits should copy this section into -released.sym. */ LIBOSTREE_2019.4 { + ostree_repo_mark_commit_partial_reason; } LIBOSTREE_2019.3; /* Stub section for the stable release *after* this development one; don't diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index e7bc9820..0aaebbdb 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -1873,6 +1873,57 @@ ensure_txn_refs (OstreeRepo *self) g_free); } +/** + * ostree_repo_mark_commit_partial_reason: + * @self: Repo + * @checksum: Commit SHA-256 + * @is_partial: Whether or not this commit is partial + * @in_state: Reason bitmask for partial commit + * @error: Error + * + * Allows the setting of a reason code for a partial commit. Presently + * it only supports setting reason bitmask to + * OSTREE_REPO_COMMIT_STATE_FSCK_PARTIAL, or + * OSTREE_REPO_COMMIT_STATE_NORMAL. This will allow successive ostree + * fsck operations to exit properly with an error code if the + * repository has been truncated as a result of fsck trying to repair + * it. + * + * Since: 2019.4 + */ +gboolean +ostree_repo_mark_commit_partial_reason (OstreeRepo *self, + const char *checksum, + gboolean is_partial, + OstreeRepoCommitState in_state, + GError **error) +{ + g_autofree char *commitpartial_path = _ostree_get_commitpartial_path (checksum); + if (is_partial) + { + glnx_autofd int fd = openat (self->repo_dir_fd, commitpartial_path, + O_EXCL | O_CREAT | O_WRONLY | O_CLOEXEC | O_NOCTTY, 0644); + if (fd == -1) + { + if (errno != EEXIST) + return glnx_throw_errno_prefix (error, "open(%s)", commitpartial_path); + } + else + { + if (in_state & OSTREE_REPO_COMMIT_STATE_FSCK_PARTIAL) + if (glnx_loop_write (fd, "f", 1) < 0) + return glnx_throw_errno_prefix (error, "write(%s)", commitpartial_path); + } + } + else + { + if (!ot_ensure_unlinked_at (self->repo_dir_fd, commitpartial_path, 0)) + return FALSE; + } + + return TRUE; +} + /** * ostree_repo_mark_commit_partial: * @self: Repo @@ -1880,9 +1931,10 @@ ensure_txn_refs (OstreeRepo *self) * @is_partial: Whether or not this commit is partial * @error: Error * - * Commits in "partial" state do not have all their child objects written. This - * occurs in various situations, such as during a pull, but also if a "subpath" - * pull is used, as well as "commit only" pulls. + * Commits in the "partial" state do not have all their child objects + * written. This occurs in various situations, such as during a pull, + * but also if a "subpath" pull is used, as well as "commit only" + * pulls. * * This function is used by ostree_repo_pull_with_options(); you * should use this if you are implementing a different type of transport. @@ -1895,24 +1947,9 @@ ostree_repo_mark_commit_partial (OstreeRepo *self, gboolean is_partial, GError **error) { - g_autofree char *commitpartial_path = _ostree_get_commitpartial_path (checksum); - if (is_partial) - { - glnx_autofd int fd = openat (self->repo_dir_fd, commitpartial_path, - O_EXCL | O_CREAT | O_WRONLY | O_CLOEXEC | O_NOCTTY, 0644); - if (fd == -1) - { - if (errno != EEXIST) - return glnx_throw_errno_prefix (error, "open(%s)", commitpartial_path); - } - } - else - { - if (!ot_ensure_unlinked_at (self->repo_dir_fd, commitpartial_path, 0)) - return FALSE; - } - - return TRUE; + return ostree_repo_mark_commit_partial_reason (self, checksum, is_partial, + OSTREE_REPO_COMMIT_STATE_NORMAL, + error); } /** diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index eb652bef..b654aff2 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -3768,10 +3768,19 @@ load_metadata_internal (OstreeRepo *self, g_autofree char *commitpartial_path = _ostree_get_commitpartial_path (sha256); *out_state = 0; - if (!glnx_fstatat_allow_noent (self->repo_dir_fd, commitpartial_path, NULL, 0, error)) + glnx_autofd int fd = -1; + if (!ot_openat_ignore_enoent (self->repo_dir_fd, commitpartial_path, &fd, error)) return FALSE; - if (errno == 0) - *out_state |= OSTREE_REPO_COMMIT_STATE_PARTIAL; + if (fd != -1) + { + *out_state |= OSTREE_REPO_COMMIT_STATE_PARTIAL; + char reason; + if (read (fd, &reason, 1) == 1) + { + if (reason == 'f') + *out_state |= OSTREE_REPO_COMMIT_STATE_FSCK_PARTIAL; + } + } } } else if (self->parent_repo) diff --git a/src/libostree/ostree-repo.h b/src/libostree/ostree-repo.h index 038bbd41..aaaaa622 100644 --- a/src/libostree/ostree-repo.h +++ b/src/libostree/ostree-repo.h @@ -248,6 +248,26 @@ gboolean ostree_repo_write_config (OstreeRepo *self, GKeyFile *new_config, GError **error); +/** + * OstreeRepoCommitState: + * @OSTREE_REPO_COMMIT_STATE_NORMAL: Commit is complete. This is the default. + * (Since: 2017.14.) + * @OSTREE_REPO_COMMIT_STATE_PARTIAL: One or more objects are missing from the + * local copy of the commit, but metadata is present. (Since: 2015.7.) + * @OSTREE_REPO_COMMIT_STATE_FSCK_PARTIAL: One or more objects are missing from the + * local copy of the commit, due to an fsck --delete. (Since: 2019.3.) + * + * Flags representing the state of a commit in the local repository, as returned + * by ostree_repo_load_commit(). + * + * Since: 2015.7 + */ +typedef enum { + OSTREE_REPO_COMMIT_STATE_NORMAL = 0, + OSTREE_REPO_COMMIT_STATE_PARTIAL = (1 << 0), + OSTREE_REPO_COMMIT_STATE_FSCK_PARTIAL = (1 << 1), +} OstreeRepoCommitState; + /** * OstreeRepoTransactionStats: * @metadata_objects_total: The total number of metadata objects @@ -315,6 +335,13 @@ gboolean ostree_repo_mark_commit_partial (OstreeRepo *self, gboolean is_partial, GError **error); +_OSTREE_PUBLIC +gboolean ostree_repo_mark_commit_partial_reason (OstreeRepo *self, + const char *checksum, + gboolean is_partial, + OstreeRepoCommitState in_state, + GError **error); + _OSTREE_PUBLIC void ostree_repo_transaction_set_refspec (OstreeRepo *self, const char *refspec, @@ -526,23 +553,6 @@ gboolean ostree_repo_load_variant_if_exists (OstreeRepo *self, GVariant **out_variant, GError **error); -/** - * OstreeRepoCommitState: - * @OSTREE_REPO_COMMIT_STATE_NORMAL: Commit is complete. This is the default. - * (Since: 2017.14.) - * @OSTREE_REPO_COMMIT_STATE_PARTIAL: One or more objects are missing from the - * local copy of the commit, but metadata is present. (Since: 2015.7.) - * - * Flags representing the state of a commit in the local repository, as returned - * by ostree_repo_load_commit(). - * - * Since: 2015.7 - */ -typedef enum { - OSTREE_REPO_COMMIT_STATE_NORMAL = 0, - OSTREE_REPO_COMMIT_STATE_PARTIAL = (1 << 0), -} OstreeRepoCommitState; - _OSTREE_PUBLIC gboolean ostree_repo_load_commit (OstreeRepo *self, const char *checksum, diff --git a/src/ostree/ot-builtin-fsck.c b/src/ostree/ot-builtin-fsck.c index a7ecd3d0..44e98a76 100644 --- a/src/ostree/ot-builtin-fsck.c +++ b/src/ostree/ot-builtin-fsck.c @@ -127,7 +127,7 @@ fsck_one_object (OstreeRepo *repo, if ((state & OSTREE_REPO_COMMIT_STATE_PARTIAL) == 0) { g_printerr ("Marking commit as partial: %s\n", parent_commit); - if (!ostree_repo_mark_commit_partial (repo, parent_commit, TRUE, error)) + if (!ostree_repo_mark_commit_partial_reason (repo, parent_commit, TRUE, OSTREE_REPO_COMMIT_STATE_FSCK_PARTIAL, error)) return FALSE; } } @@ -302,6 +302,7 @@ ostree_builtin_fsck (int argc, char **argv, OstreeCommandInvocation *invocation, opt_verify_bindings = TRUE; guint n_partial = 0; + guint n_fsck_partial = 0; g_hash_table_iter_init (&hash_iter, objects); while (g_hash_table_iter_next (&hash_iter, &key, &value)) { @@ -410,7 +411,11 @@ ostree_builtin_fsck (int argc, char **argv, OstreeCommandInvocation *invocation, } if (commitstate & OSTREE_REPO_COMMIT_STATE_PARTIAL) - n_partial++; + { + n_partial++; + if (commitstate & OSTREE_REPO_COMMIT_STATE_FSCK_PARTIAL) + n_fsck_partial++; + } else g_hash_table_add (commits, g_variant_ref (serialized_key)); } @@ -450,5 +455,8 @@ ostree_builtin_fsck (int argc, char **argv, OstreeCommandInvocation *invocation, if (found_corruption) return glnx_throw (error, "Repository corruption encountered"); + if (n_fsck_partial > 0) + return glnx_throw (error, "%u fsck deleted partial commits not verified", n_partial); + return TRUE; }