From 3594bb2d0f6818a97adc95655bd136267e27fe4c Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 5 Sep 2017 15:43:04 -0400 Subject: [PATCH] lib: Add a private helper to abort txns, use in sysroot cleanup Steal some code from flatpak for this, which allows porting a few more things to new style. I started on a public API version of this but was trying to roll some other things into it and it snowballed. Let's do this version since it's easy for now. While here I changed things so that `generate_deployment_refs()` now just uses `_set_ref_immediate()` rather than requring a txn. Also, AFAICS there was no test coverage of `generate_deployment_refs()`; I tried commenting it out and at least `admin-test.sh` passed. Add some coverage of this - I verified that with this commenting out bits of that function cause the test to fail. Closes: #1132 Approved by: jlebon --- src/libostree/ostree-repo-private.h | 21 +++++++++ src/libostree/ostree-sysroot-cleanup.c | 62 ++++++++++---------------- tests/admin-test.sh | 15 ++++++- 3 files changed, 57 insertions(+), 41 deletions(-) diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h index 121c2d5202..fdcd8f3f01 100644 --- a/src/libostree/ostree-repo-private.h +++ b/src/libostree/ostree-repo-private.h @@ -158,6 +158,27 @@ struct OstreeRepo { OstreeRepo *parent_repo; }; +/* Taken from flatpak; may be made into public API later */ +typedef OstreeRepo _OstreeRepoAutoTransaction; +static inline void +_ostree_repo_auto_transaction_cleanup (void *p) +{ + OstreeRepo *repo = p; + if (repo) + (void) ostree_repo_abort_transaction (repo, NULL, NULL); +} + +static inline _OstreeRepoAutoTransaction * +_ostree_repo_auto_transaction_start (OstreeRepo *repo, + GCancellable *cancellable, + GError **error) +{ + if (!ostree_repo_prepare_transaction (repo, NULL, cancellable, error)) + return NULL; + return (_OstreeRepoAutoTransaction *)repo; +} +G_DEFINE_AUTOPTR_CLEANUP_FUNC (_OstreeRepoAutoTransaction, _ostree_repo_auto_transaction_cleanup) + typedef struct { dev_t dev; ino_t ino; diff --git a/src/libostree/ostree-sysroot-cleanup.c b/src/libostree/ostree-sysroot-cleanup.c index d689f02efc..da239c7830 100644 --- a/src/libostree/ostree-sysroot-cleanup.c +++ b/src/libostree/ostree-sysroot-cleanup.c @@ -21,6 +21,7 @@ #include "config.h" #include "otutil.h" +#include "ostree-repo-private.h" #include "ostree-linuxfsutil.h" #include "ostree-sysroot-private.h" @@ -335,10 +336,7 @@ cleanup_old_deployments (OstreeSysroot *self, return TRUE; } -/* libostree holds a ref for each deployment's exact checksum to avoid it being - * GC'd even if the origin ref changes. This function resets those refs - * to match active deployments. - */ +/* Delete the ref bindings for a non-active boot version */ static gboolean cleanup_ref_prefix (OstreeRepo *repo, int bootversion, @@ -346,30 +344,24 @@ cleanup_ref_prefix (OstreeRepo *repo, GCancellable *cancellable, GError **error) { - gboolean ret = FALSE; g_autofree char *prefix = g_strdup_printf ("ostree/%d/%d", bootversion, subbootversion); - g_autoptr(GHashTable) refs = NULL; if (!ostree_repo_list_refs_ext (repo, prefix, &refs, OSTREE_REPO_LIST_REFS_EXT_NONE, cancellable, error)) - goto out; - - if (!ostree_repo_prepare_transaction (repo, NULL, cancellable, error)) - goto out; + return FALSE; GLNX_HASH_TABLE_FOREACH (refs, const char *, ref) { - ostree_repo_transaction_set_refspec (repo, ref, NULL); + if (!ostree_repo_set_ref_immediate (repo, NULL, ref, NULL, cancellable, error)) + return FALSE; } - if (!ostree_repo_commit_transaction (repo, NULL, cancellable, error)) - goto out; - - ret = TRUE; - out: - ostree_repo_abort_transaction (repo, cancellable, NULL); - return ret; + return TRUE; } +/* libostree holds a ref for each deployment's exact checksum to avoid it being + * GC'd even if the origin ref changes. This function resets those refs + * to match active deployments. + */ static gboolean generate_deployment_refs (OstreeSysroot *self, OstreeRepo *repo, @@ -379,46 +371,38 @@ generate_deployment_refs (OstreeSysroot *self, GCancellable *cancellable, GError **error) { - gboolean ret = FALSE; - int cleanup_bootversion; - int cleanup_subbootversion; - guint i; - - cleanup_bootversion = (bootversion == 0) ? 1 : 0; - cleanup_subbootversion = (subbootversion == 0) ? 1 : 0; + int cleanup_bootversion = (bootversion == 0) ? 1 : 0; + int cleanup_subbootversion = (subbootversion == 0) ? 1 : 0; if (!cleanup_ref_prefix (repo, cleanup_bootversion, 0, cancellable, error)) - goto out; + return FALSE; if (!cleanup_ref_prefix (repo, cleanup_bootversion, 1, cancellable, error)) - goto out; + return FALSE; if (!cleanup_ref_prefix (repo, bootversion, cleanup_subbootversion, cancellable, error)) - goto out; + return FALSE; - for (i = 0; i < deployments->len; i++) + g_autoptr(_OstreeRepoAutoTransaction) txn = + _ostree_repo_auto_transaction_start (repo, cancellable, error); + if (!txn) + return FALSE; + for (guint i = 0; i < deployments->len; i++) { OstreeDeployment *deployment = deployments->pdata[i]; g_autofree char *refname = g_strdup_printf ("ostree/%d/%d/%u", bootversion, subbootversion, i); - if (!ostree_repo_prepare_transaction (repo, NULL, cancellable, error)) - goto out; - ostree_repo_transaction_set_refspec (repo, refname, ostree_deployment_get_csum (deployment)); - - if (!ostree_repo_commit_transaction (repo, NULL, cancellable, error)) - goto out; } + if (!ostree_repo_commit_transaction (repo, NULL, cancellable, error)) + return FALSE; - ret = TRUE; - out: - ostree_repo_abort_transaction (repo, cancellable, NULL); - return ret; + return TRUE; } static gboolean diff --git a/tests/admin-test.sh b/tests/admin-test.sh index d556695979..805a71303d 100644 --- a/tests/admin-test.sh +++ b/tests/admin-test.sh @@ -35,6 +35,13 @@ function validate_bootloader() { cd - } +# Test generate_deployment_refs() +assert_ostree_deployment_refs() { + ${CMD_PREFIX} ostree --repo=sysroot/ostree/repo refs ostree | sort > ostree-refs.txt + (for v in "$@"; do echo $v; done) | sort > ostree-refs-expected.txt + diff -u ostree-refs{-expected,}.txt +} + orig_mtime=$(stat -c '%.Y' sysroot/ostree/deploy) ${CMD_PREFIX} ostree --repo=sysroot/ostree/repo pull-local --remote=testos testos-repo testos/buildmaster/x86_64-runtime rev=$(${CMD_PREFIX} ostree --repo=sysroot/ostree/repo rev-parse testos/buildmaster/x86_64-runtime) @@ -55,6 +62,7 @@ assert_file_has_content curdir ^`pwd`/sysroot/ostree/deploy/testos/deploy/${rev} echo "ok --print-current-dir" +# Test layout of bootloader config and refs assert_not_has_dir sysroot/boot/loader.0 assert_has_dir sysroot/boot/loader.1 assert_has_dir sysroot/ostree/boot.1.1 @@ -64,9 +72,8 @@ assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'option assert_file_has_content sysroot/boot/ostree/testos-${bootcsum}/vmlinuz-3.6.0 'a kernel' assert_file_has_content sysroot/ostree/deploy/testos/deploy/${rev}.0/etc/os-release 'NAME=TestOS' assert_file_has_content sysroot/ostree/boot.1/testos/${bootcsum}/0/etc/os-release 'NAME=TestOS' +assert_ostree_deployment_refs 1/1/0 ${CMD_PREFIX} ostree admin status - - echo "ok layout" orig_mtime=$(stat -c '%.Y' sysroot/ostree/deploy) @@ -84,6 +91,7 @@ assert_not_has_dir sysroot/ostree/boot.1.1 assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'options.* root=LABEL=MOO' assert_file_has_content sysroot/ostree/deploy/testos/deploy/${rev}.1/etc/os-release 'NAME=TestOS' assert_file_has_content sysroot/ostree/boot.0/testos/${bootcsum}/0/etc/os-release 'NAME=TestOS' +assert_ostree_deployment_refs 0/1/{0,1} ${CMD_PREFIX} ostree admin status validate_bootloader @@ -96,6 +104,7 @@ assert_not_has_dir sysroot/boot/loader.1 # But swap subbootversion assert_has_dir sysroot/ostree/boot.0.0 assert_not_has_dir sysroot/ostree/boot.0.1 +assert_ostree_deployment_refs 0/0/{0,1} ${CMD_PREFIX} ostree admin status validate_bootloader @@ -110,6 +119,7 @@ assert_has_file sysroot/boot/loader/entries/ostree-testos-1.conf assert_has_file sysroot/boot/loader/entries/ostree-otheros-0.conf assert_file_has_content sysroot/ostree/deploy/testos/deploy/${rev}.1/etc/os-release 'NAME=TestOS' assert_file_has_content sysroot/ostree/deploy/otheros/deploy/${rev}.0/etc/os-release 'NAME=TestOS' +assert_ostree_deployment_refs 1/1/{0,1,2} ${CMD_PREFIX} ostree admin status validate_bootloader @@ -123,6 +133,7 @@ assert_file_has_content sysroot/ostree/deploy/testos/deploy/${rev}.2/etc/os-rele assert_has_file sysroot/boot/loader/entries/ostree-testos-2.conf assert_file_has_content sysroot/ostree/deploy/testos/deploy/${rev}.3/etc/os-release 'NAME=TestOS' ${CMD_PREFIX} ostree admin status +assert_ostree_deployment_refs 0/1/{0,1,2,3} validate_bootloader echo "ok fourth deploy (retain)"