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

diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h
index 121c2d52023a28dddb6f9d5fdc9901d3d2d6f1ac..fdcd8f3f0115285dab078c783e106762143d23f9 100644 (file)
--- 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 d689f02efce52728d4afccd0c37df8d024e959f7..da239c78309748435ccfd1f447cb2ac8e02d2eab 100644 (file)
--- 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 d5566959796872843adbec4d69998b3d17682aa8..805a71303d9da2f728f768f7af2e74261a38f018 100644 (file)
--- 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)"