From 9d3ef89230ab0aa9670e448772f5d9d523d43d78 Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Wed, 5 Jan 2022 10:03:59 +0000 Subject: [PATCH 01/52] configure: post-release version bump --- configure.ac | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 93b98cb9..879fe97f 100644 --- a/configure.ac +++ b/configure.ac @@ -1,10 +1,10 @@ AC_PREREQ([2.63]) dnl To perform a release, follow the instructions in `docs/CONTRIBUTING.md`. m4_define([year_version], [2022]) -m4_define([release_version], [1]) +m4_define([release_version], [2]) m4_define([package_version], [year_version.release_version]) AC_INIT([libostree], [package_version], [walters@verbum.org]) -is_release_build=yes +is_release_build=no AC_CONFIG_HEADER([config.h]) AC_CONFIG_MACRO_DIR([buildutil]) AC_CONFIG_AUX_DIR([build-aux]) From 0b1a0856925c32fffa93c526b9f415d455d71a1d Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Mon, 10 Jan 2022 09:30:11 +0000 Subject: [PATCH 02/52] libotutil: avoid leaking builder memory on error This swaps the order of a couple of input sanity checks, in order to fix a minor memory leak due to an early-return on the error path. Memory for the result is now allocated only after input has been sanity-checked. It fixes a static analysis warning highlighted by Coverity. --- src/libotutil/ot-variant-builder.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libotutil/ot-variant-builder.c b/src/libotutil/ot-variant-builder.c index 92ac8ede..754b9323 100644 --- a/src/libotutil/ot-variant-builder.c +++ b/src/libotutil/ot-variant-builder.c @@ -760,10 +760,10 @@ ot_variant_builder_info_new (OtVariantBuilder *builder, const GVariantType *type { OtVariantBuilderInfo *info; - info = (OtVariantBuilderInfo *) g_slice_new0 (OtVariantBuilderInfo); - g_return_val_if_fail (g_variant_type_is_container (type), NULL); + info = (OtVariantBuilderInfo *) g_slice_new0 (OtVariantBuilderInfo); + info->builder = builder; info->type = g_variant_type_copy (type); info->type_info = g_variant_type_info_get (type); @@ -845,10 +845,10 @@ ot_variant_builder_new (const GVariantType *type, { OtVariantBuilder *builder; - builder = (OtVariantBuilder *) g_slice_new0 (OtVariantBuilder); - g_return_val_if_fail (g_variant_type_is_container (type), NULL); + builder = (OtVariantBuilder *) g_slice_new0 (OtVariantBuilder); + builder->head = ot_variant_builder_info_new (builder, type); builder->ref_count = 1; builder->fd = fd; From 0bdba574d735ae37447d4c1e8bd0b6a4e622da79 Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Mon, 10 Jan 2022 10:22:28 +0000 Subject: [PATCH 03/52] ostree: check g_setenv return value This adds proper return-value checks on g_setenv calls. It fixes a static analysis warning highlighted by Coverity. --- src/libostree/ostree-sepolicy.c | 5 ++++- src/ostree/ot-main.c | 6 +++++- tests/libostreetest.c | 3 ++- tests/test-gpg-verify-result.c | 3 ++- tests/test-rollsum-cli.c | 3 ++- tests/test-varint.c | 3 ++- 6 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/libostree/ostree-sepolicy.c b/src/libostree/ostree-sepolicy.c index d8ff35cb..0fed6457 100644 --- a/src/libostree/ostree-sepolicy.c +++ b/src/libostree/ostree-sepolicy.c @@ -422,7 +422,10 @@ initable_init (GInitable *initable, { const char *policy_rootpath = gs_file_get_path_cached (policy_root); - g_setenv ("LIBSELINUX_DISABLE_PCRE_PRECOMPILED", "1", FALSE); + /* TODO(lucab): get rid of this setenv(), it may be unsafe in a multi-thread context. */ + if (!g_setenv ("LIBSELINUX_DISABLE_PCRE_PRECOMPILED", "1", FALSE)) + return glnx_throw (error, "Failed to set environment variable LIBSELINUX_DISABLE_PCRE_PRECOMPILED"); + if (selinux_set_policy_root (policy_rootpath) != 0) return glnx_throw_errno_prefix (error, "selinux_set_policy_root(%s)", policy_rootpath); diff --git a/src/ostree/ot-main.c b/src/ostree/ot-main.c index 8ee73038..b72fa9d4 100644 --- a/src/ostree/ot-main.c +++ b/src/ostree/ot-main.c @@ -253,7 +253,11 @@ ostree_run (int argc, int in, out; /* avoid gvfs (http://bugzilla.gnome.org/show_bug.cgi?id=526454) */ - g_setenv ("GIO_USE_VFS", "local", TRUE); + if (!g_setenv ("GIO_USE_VFS", "local", TRUE)) + { + (void) glnx_throw (res_error, "Failed to set environment variable GIO_USE_FVS"); + return 1; + } g_log_set_handler (G_LOG_DOMAIN, G_LOG_LEVEL_MESSAGE, message_handler, NULL); diff --git a/tests/libostreetest.c b/tests/libostreetest.c index d5671a1e..08abb9f1 100644 --- a/tests/libostreetest.c +++ b/tests/libostreetest.c @@ -155,7 +155,8 @@ ot_test_setup_sysroot (GCancellable *cancellable, } /* Make sure deployments are mutable */ - g_setenv ("OSTREE_SYSROOT_DEBUG", buf->str, TRUE); + if (!g_setenv ("OSTREE_SYSROOT_DEBUG", buf->str, TRUE)) + return glnx_null_throw (error, "Failed to set environment variable OSTREE_SYSROOT_DEBUG"); g_autoptr(GFile) sysroot_path = g_file_new_for_path ("sysroot"); return ostree_sysroot_new (sysroot_path); diff --git a/tests/test-gpg-verify-result.c b/tests/test-gpg-verify-result.c index d49224ec..8485b888 100644 --- a/tests/test-gpg-verify-result.c +++ b/tests/test-gpg-verify-result.c @@ -78,7 +78,8 @@ test_fixture_setup (TestFixture *fixture, * certificates for certain test cases. */ homedir = g_test_build_filename (G_TEST_DIST, "tests/gpg-verify-data", NULL); - g_setenv ("GNUPGHOME", homedir, TRUE); + gboolean is_ok = g_setenv ("GNUPGHOME", homedir, TRUE); + g_assert (is_ok == TRUE); result = g_initable_new (OSTREE_TYPE_GPG_VERIFY_RESULT, NULL, &local_error, NULL); diff --git a/tests/test-rollsum-cli.c b/tests/test-rollsum-cli.c index 44e3390c..2cf730d3 100644 --- a/tests/test-rollsum-cli.c +++ b/tests/test-rollsum-cli.c @@ -35,7 +35,8 @@ main (int argc, char **argv) OstreeRollsumMatches *matches; GMappedFile *mfile; - g_setenv ("GIO_USE_VFS", "local", TRUE); + gboolean is_ok = g_setenv ("GIO_USE_VFS", "local", TRUE); + g_assert (is_ok == TRUE); if (argc < 3) return 1; diff --git a/tests/test-varint.c b/tests/test-varint.c index e86c008f..3fea805e 100644 --- a/tests/test-varint.c +++ b/tests/test-varint.c @@ -58,7 +58,8 @@ int main (int argc, char **argv) { - g_setenv ("GIO_USE_VFS", "local", TRUE); + gboolean is_ok = g_setenv ("GIO_USE_VFS", "local", TRUE); + g_assert (is_ok == TRUE); g_test_init (&argc, &argv, NULL); From 840cd7ab2da3e9992a6b91ba312a702e3ba55e0d Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Tue, 11 Jan 2022 15:39:30 +0000 Subject: [PATCH 04/52] libostree/sepolicy: get rid of a g_setenv() call This removes a 'g_setenv()' call, which could potentially be unsafe in a multi-thread context. The current libselinux codebase does not seem to check for `LIBSELINUX_DISABLE_PCRE_PRECOMPILED`, so I think this has no effects nowadays. Additionally, I could not find any reference to it in libselinux git history, so I'm not sure if it ever played any role at all. My current understanding is that this is coming from version incompatibilities between an older libselinux in the build environment and a newer policy (with precompiled regexs) in the target. But from the ML discussion I found, I think it eventually got solved in a different way, possibly by avoiding the policy binary caches. Refs: * https://www.spinics.net/lists/selinux/msg14822.html * https://github.com/ostreedev/ostree/pull/2513#discussion_r781042884 --- src/libostree/ostree-sepolicy.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/libostree/ostree-sepolicy.c b/src/libostree/ostree-sepolicy.c index 0fed6457..5fd59a82 100644 --- a/src/libostree/ostree-sepolicy.c +++ b/src/libostree/ostree-sepolicy.c @@ -422,10 +422,6 @@ initable_init (GInitable *initable, { const char *policy_rootpath = gs_file_get_path_cached (policy_root); - /* TODO(lucab): get rid of this setenv(), it may be unsafe in a multi-thread context. */ - if (!g_setenv ("LIBSELINUX_DISABLE_PCRE_PRECOMPILED", "1", FALSE)) - return glnx_throw (error, "Failed to set environment variable LIBSELINUX_DISABLE_PCRE_PRECOMPILED"); - if (selinux_set_policy_root (policy_rootpath) != 0) return glnx_throw_errno_prefix (error, "selinux_set_policy_root(%s)", policy_rootpath); From 998154f8ffc2f471bf4f3b59602332ba15ef6d07 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 11 Jan 2022 16:46:49 -0500 Subject: [PATCH 05/52] main: Also support CLI extensions in `/usr/libexec/libostree/ext` In fixing https://github.com/coreos/rpm-ostree/pull/3323 I felt that it was a bit ugly we're installing `/usr/bin/ostree-container`. It's kind of an implementation detail. We want users to use `ostree container`. Let's support values outside of $PATH too. For example, this also ensures that TAB completion for `ost` expands to `ostree ` with a space. --- configure.ac | 4 ++++ src/libostree/ostree-1.pc.in | 1 + src/ostree/ot-main.c | 16 ++++++++++++++-- tests/kolainst/destructive/basic-misc.sh | 23 +++++++++++++++++++++++ tests/test-cli-extensions.sh | 3 +++ 5 files changed, 45 insertions(+), 2 deletions(-) create mode 100644 tests/kolainst/destructive/basic-misc.sh diff --git a/configure.ac b/configure.ac index 879fe97f..763ffb04 100644 --- a/configure.ac +++ b/configure.ac @@ -25,6 +25,10 @@ AC_SUBST([YEAR_VERSION], [year_version]) AC_SUBST([RELEASE_VERSION], [release_version]) AC_SUBST([PACKAGE_VERSION], [package_version]) +dnl automake variables we want in pkg-config +pkglibexecdir=$libexecdir/$PACKAGE +AC_SUBST(pkglibexecdir) + AS_IF([echo "$CFLAGS" | grep -q -E -e '-Werror($| )'], [], [ CC_CHECK_FLAGS_APPEND([WARN_CFLAGS], [CFLAGS], [\ -pipe \ diff --git a/src/libostree/ostree-1.pc.in b/src/libostree/ostree-1.pc.in index 9a4debce..cbf0a13c 100644 --- a/src/libostree/ostree-1.pc.in +++ b/src/libostree/ostree-1.pc.in @@ -3,6 +3,7 @@ exec_prefix=@exec_prefix@ libdir=@libdir@ includedir=@includedir@ features=@OSTREE_FEATURES@ +cliextdir=@pkglibexecdir@/ext Name: OSTree Description: Git for operating system binaries diff --git a/src/ostree/ot-main.c b/src/ostree/ot-main.c index b72fa9d4..7a4c4707 100644 --- a/src/ostree/ot-main.c +++ b/src/ostree/ot-main.c @@ -41,6 +41,10 @@ static gboolean opt_verbose; static gboolean opt_version; static gboolean opt_print_current_dir; +// TODO: make this public? But no one sane wants to use our C headers +// to find where to put files. Maybe we can make it printed by the CLI? +#define _OSTREE_EXT_DIR PKGLIBEXECDIR "/ext" + static GOptionEntry global_entries[] = { { "verbose", 'v', 0, G_OPTION_ARG_NONE, &opt_verbose, "Print debug information during command processing", NULL }, { "version", 0, 0, G_OPTION_ARG_NONE, &opt_version, "Print version information and exit", NULL }, @@ -188,7 +192,7 @@ ostree_command_lookup_external (int argc, // Find the first verb (ignoring all earlier flags), then // check if it is a known native command. Otherwise, try to look it - // up in $PATH. + // up in /usr/lib/ostree/ostree-$cmd or $PATH. // We ignore argv[0] here, the ostree binary itself is not multicall. for (guint arg_index = 1; arg_index < argc; arg_index++) { @@ -204,10 +208,18 @@ ostree_command_lookup_external (int argc, return NULL; } + g_autofree gchar *ext_command = g_strdup_printf ("ostree-%s", current_arg); + + /* First, search in our libdir /usr/lib/ostree/ostree-$cmd */ + g_autofree char *ext_lib = g_strconcat (_OSTREE_EXT_DIR, "/", ext_command, NULL); + struct stat stbuf; + if (stat (ext_lib, &stbuf) == 0) + return g_steal_pointer (&ext_lib); + + /* Otherwise, look in $PATH */ if (g_find_program_in_path (ext_command) == NULL) return NULL; - return g_steal_pointer (&ext_command); } diff --git a/tests/kolainst/destructive/basic-misc.sh b/tests/kolainst/destructive/basic-misc.sh new file mode 100644 index 00000000..6d14cc04 --- /dev/null +++ b/tests/kolainst/destructive/basic-misc.sh @@ -0,0 +1,23 @@ +#!/bin/bash + +# Random misc tests + +set -xeuo pipefail + +. ${KOLA_EXT_DATA}/libinsttest.sh + +echo "1..1" +date + +# Test CLI extensions installed alongside the system +extdir=/usr/libexec/libostree/ext/ +mkdir -p "${extdir}" +ln -sr /usr/bin/env ${extdir}/ostree-env + +env TESTENV=foo ostree env > out.txt +assert_file_has_content out.text TESTENV=foo +rm -vf "${extdir}/ostree-env" +echo "ok env" + +# End test +date diff --git a/tests/test-cli-extensions.sh b/tests/test-cli-extensions.sh index 1fe9c037..e1916036 100755 --- a/tests/test-cli-extensions.sh +++ b/tests/test-cli-extensions.sh @@ -9,6 +9,9 @@ set -euo pipefail echo '1..2' +# Test CLI extensions via $PATH. If you change this, you may +# also want to change the corresponding destructive version in +# tests/kolainst/destructive/basic-misc.sh mkdir -p ./localbin ORIG_PATH="${PATH}" export PATH="./localbin/:${PATH}" From 0ff4bee74348e6894795ba3a5ac2bfab8aab7f28 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 12 Jan 2022 12:47:11 -0500 Subject: [PATCH 06/52] sysroot: Add a public `#define OSTREE_PATH_BOOTED` This is public API. Motivated by https://github.com/coreos/rpm-ostree/pull/3325/files#diff-56528694f6f3213d6fb88d872f77291412dceec263b57166519843b13eca9a4dR30 --- src/libostree/ostree-sysroot.c | 6 +++--- src/libostree/ostree-sysroot.h | 8 ++++++++ src/ostree/ot-admin-builtin-finalize-staged.c | 2 +- src/ostree/ot-main.c | 2 +- 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/libostree/ostree-sysroot.c b/src/libostree/ostree-sysroot.c index eccf9375..9cb40e66 100644 --- a/src/libostree/ostree-sysroot.c +++ b/src/libostree/ostree-sysroot.c @@ -973,7 +973,7 @@ ostree_sysroot_initialize (OstreeSysroot *self, * we'll use it to sanity check that we found a booted deployment for example. * Second, we also find out whether sysroot == /. */ - if (!glnx_fstatat_allow_noent (AT_FDCWD, "/run/ostree-booted", NULL, 0, error)) + if (!glnx_fstatat_allow_noent (AT_FDCWD, OSTREE_PATH_BOOTED, NULL, 0, error)) return FALSE; const gboolean ostree_booted = (errno == 0); @@ -1106,11 +1106,11 @@ sysroot_load_from_bootloader_configs (OstreeSysroot *self, return FALSE; if (errno == ENOENT) { - return glnx_throw (error, "Unexpected state: /run/ostree-booted found, but no /boot/loader directory"); + return glnx_throw (error, "Unexpected state: %s found, but no /boot/loader directory", OSTREE_PATH_BOOTED); } else { - return glnx_throw (error, "Unexpected state: /run/ostree-booted found and in / sysroot, but bootloader entry not found"); + return glnx_throw (error, "Unexpected state: %s found and in / sysroot, but bootloader entry not found", OSTREE_PATH_BOOTED); } } diff --git a/src/libostree/ostree-sysroot.h b/src/libostree/ostree-sysroot.h index 41361716..c240aaa0 100644 --- a/src/libostree/ostree-sysroot.h +++ b/src/libostree/ostree-sysroot.h @@ -24,6 +24,14 @@ G_BEGIN_DECLS +/** + * OSTREE_PATH_BOOTED: + * Filesystem path that is created on an ostree-booted system. + * + * Since: 2022.2 + */ +#define OSTREE_PATH_BOOTED "/run/ostree-booted" + #define OSTREE_TYPE_SYSROOT ostree_sysroot_get_type() #define OSTREE_SYSROOT(obj) \ (G_TYPE_CHECK_INSTANCE_CAST ((obj), OSTREE_TYPE_SYSROOT, OstreeSysroot)) diff --git a/src/ostree/ot-admin-builtin-finalize-staged.c b/src/ostree/ot-admin-builtin-finalize-staged.c index 80d8a9b7..17b6a625 100644 --- a/src/ostree/ot-admin-builtin-finalize-staged.c +++ b/src/ostree/ot-admin-builtin-finalize-staged.c @@ -45,7 +45,7 @@ ot_admin_builtin_finalize_staged (int argc, char **argv, OstreeCommandInvocation /* Just a sanity check; we shouldn't be called outside of the service though. */ struct stat stbuf; - if (fstatat (AT_FDCWD, "/run/ostree-booted", &stbuf, 0) < 0) + if (fstatat (AT_FDCWD, OSTREE_PATH_BOOTED, &stbuf, 0) < 0) return TRUE; g_autoptr(GOptionContext) context = g_option_context_new (""); diff --git a/src/ostree/ot-main.c b/src/ostree/ot-main.c index 7a4c4707..b7b50d67 100644 --- a/src/ostree/ot-main.c +++ b/src/ostree/ot-main.c @@ -120,7 +120,7 @@ maybe_setup_mount_namespace (gboolean *out_ns, return TRUE; /* If the system isn't booted via libostree, also nothing to do */ - if (!glnx_fstatat_allow_noent (AT_FDCWD, "/run/ostree-booted", NULL, 0, error)) + if (!glnx_fstatat_allow_noent (AT_FDCWD, OSTREE_PATH_BOOTED, NULL, 0, error)) return FALSE; if (errno == ENOENT) return TRUE; From de1870df8cf6345be5f497d6a5129d5abc7398e5 Mon Sep 17 00:00:00 2001 From: Dan Nicholson Date: Wed, 12 Jan 2022 17:10:56 -0700 Subject: [PATCH 07/52] github: Workaround glib/seccomp issue on Ubuntu impish The ubuntu-latest VMs are currently based on 20.04 (focal). In focal, libseccomp2 doesn't know about the close_range syscall[1], but g_spawn_sync in impish tries to use close_range since it's defined in glibc. That causes libseccomp2 to return EPERM as it does for any unknown syscalls. g_spawn_sync carries on silently instead of falling back to other means of setting CLOEXEC on open FDs. Eventually it causes some tests to hang since once side of a pipe is never closed. Remove this when libseccomp2 in focal is updated or glib in impish handles the EPERM better. 1. https://bugs.launchpad.net/ubuntu/+source/libseccomp/+bug/1944436 Fixes: #2495 --- .github/workflows/tests.yml | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 5fd14bde..76b3967b 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -22,6 +22,8 @@ jobs: # # image: The Docker image to use. # + # container-options: Additional Docker command line options. + # # pre-checkout-setup: Commands to run before the git repo checkout. # If git is not in the Docker image, it must be installed here. # Otherwise, the checkout action uses the GitHub REST API, which @@ -100,6 +102,21 @@ jobs: - name: Ubuntu Latest Release image: ubuntu:rolling + # FIXME: The ubuntu-latest VMs are currently based on 20.04 + # (focal). In focal, libseccomp2 doesn't know about the + # close_range syscall, but g_spawn_sync in impish tries to + # use close_range since it's defined in glibc. That causes + # libseccomp2 to return EPERM as it does for any unknown + # syscalls. g_spawn_sync carries on silently instead of + # falling back to other means of setting CLOEXEC on open + # FDs. Eventually it causes some tests to hang since once + # side of a pipe is never closed. Remove this when + # libseccomp2 in focal is updated or glib in impish handles + # the EPERM better. + # + # https://github.com/ostreedev/ostree/issues/2495 + # https://bugs.launchpad.net/ubuntu/+source/libseccomp/+bug/1944436 + container-options: --security-opt seccomp=unconfined pre-checkout-setup: | apt-get update apt-get install -y git @@ -108,6 +125,9 @@ jobs: runs-on: ubuntu-latest container: image: ${{ matrix.image }} + # An empty string isn't valid, so a dummy --label option is always + # added. + options: --label ostree ${{ matrix.container-options }} steps: - name: Pre-checkout setup From cb731294837736e957ee595ce11ab115277dbb36 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 17 Jan 2022 11:46:04 -0500 Subject: [PATCH 08/52] deploy: Add a 5s max timeout on global filesystem `sync()` https://bugzilla.redhat.com/show_bug.cgi?id=2003532 Basically there's a systemd bug where it's losing the `_netdev` aspect of Ceph filesystem mounts. This means the network is taken down before Ceph is unmounted. In turn, our invocation of `sync()` blocks on Ceph, which won't succeed. And this in turn manifests as a failure to transition to the new deployment. I initially did this patch to just rip out the global `sync()`. I am pretty sure we don't need it anymore. We've been doing individual `syncfs()` on `/sysroot` and `/boot` for a while now, and those are the only filesystems we should be touching. But *proving* that is a whole other thing of course. To be conservative, let's instead just add a timeout of 5s on our invocation of `sync()`. It doesn't return any information on success/error anyways. To allow testing without the `sync()` invocation, we also support a new `OSTREE_SYSROOT_OPT_SKIP_SYNC=1` environment variable. For staged deployments, this needs to be injected via e.g. systemd unit overrides into `ostree-finalize-staged.service`. Implementing this is a bit hairy - we need to spawn a thread. I debated blocking in arecursive mainloop, but I think `g_cond_wait_until()` is also fine here. --- src/libostree/ostree-sysroot-deploy.c | 68 +++++++++++++++++++++++++- src/libostree/ostree-sysroot-private.h | 6 +++ src/libostree/ostree-sysroot.c | 5 ++ 3 files changed, 78 insertions(+), 1 deletion(-) diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index c4ae86d5..6ee62410 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -1557,6 +1558,39 @@ typedef struct { guint64 extra_syncfs_msec; } SyncStats; +typedef struct { + int ref_count; /* atomic */ + bool success; + GMutex mutex; + GCond cond; +} SyncData; + +static void +sync_data_unref (SyncData *data) +{ + if (!g_atomic_int_dec_and_test (&data->ref_count)) + return; + g_mutex_clear (&data->mutex); + g_cond_clear (&data->cond); + g_free (data); +} + +// An asynchronous sync() call. See below. +static void * +sync_in_thread (void *ptr) +{ + SyncData *syncdata = ptr; + // Ensure that the caller is blocked waiting + g_mutex_lock (&syncdata->mutex); + sync (); + // Signal success + syncdata->success = true; + g_cond_broadcast (&syncdata->cond); + g_mutex_unlock (&syncdata->mutex); + sync_data_unref (syncdata); + return NULL; +} + /* First, sync the root directory as well as /var and /boot which may * be separate mount points. Then *in addition*, do a global * `sync()`. @@ -1589,9 +1623,41 @@ full_system_sync (OstreeSysroot *self, * actually get error codes out of that API, and we more clearly * delineate what we actually want to sync in the future when this * global sync call is removed. + * + * Due to https://bugzilla.redhat.com/show_bug.cgi?id=2003532 which is + * a bug in the interaction between Ceph and systemd, we have a capped + * timeout of 5s on the sync here. In general, we don't actually want + * to "own" flushing data on *all* mounted filesystems as part of + * our process. Again, we're only keeping the `sync` here out of + * conservatisim. We could likely just remove it and e.g. systemd + * would take care of sync as part of unmounting individual filesystems. */ start_msec = g_get_monotonic_time () / 1000; - sync (); + if (!(self->opt_flags & OSTREE_SYSROOT_GLOBAL_OPT_SKIP_SYNC)) + { + SyncData *syncdata = g_new (SyncData, 1); + syncdata->ref_count = 2; // One for us, one for thread + syncdata->success = FALSE; + g_mutex_init (&syncdata->mutex); + g_cond_init (&syncdata->cond); + g_mutex_lock (&syncdata->mutex); + GThread *worker = g_thread_new ("ostree sync", sync_in_thread, syncdata); + // Wait up to 5 seconds for sync() to finish + gint64 end_time = g_get_monotonic_time () + (5 * G_TIME_SPAN_SECOND); + while (!syncdata->success) + { + if (!g_cond_wait_until (&syncdata->cond, &syncdata->mutex, end_time)) + break; + } + g_mutex_unlock (&syncdata->mutex); + sync_data_unref (syncdata); + // We can't join the thread here, for two reasons. First, the whole + // point here is to avoid blocking on `sync`. The other reason is + // today there is no return value on success from `sync`, so there's + // no point to joining the thread even if we had a nonblocking mechanism + // to do so. + g_thread_unref (worker); + } end_msec = g_get_monotonic_time () / 1000; out_stats->extra_syncfs_msec = (end_msec - start_msec); diff --git a/src/libostree/ostree-sysroot-private.h b/src/libostree/ostree-sysroot-private.h index 9cc4023f..cb34eeb3 100644 --- a/src/libostree/ostree-sysroot-private.h +++ b/src/libostree/ostree-sysroot-private.h @@ -38,6 +38,11 @@ typedef enum { OSTREE_SYSROOT_DEBUG_TEST_NO_DTB = 1 << 3, /* https://github.com/ostreedev/ostree/issues/2154 */ } OstreeSysrootDebugFlags; +typedef enum { + /* Skip invoking `sync()` */ + OSTREE_SYSROOT_GLOBAL_OPT_SKIP_SYNC = 1 << 0, +} OstreeSysrootGlobalOptFlags; + typedef enum { OSTREE_SYSROOT_LOAD_STATE_NONE, /* ostree_sysroot_new() was called */ OSTREE_SYSROOT_LOAD_STATE_INIT, /* We've loaded basic sysroot state and have an fd */ @@ -75,6 +80,7 @@ struct OstreeSysroot { /* Only access through ostree_sysroot_[_get]repo() */ OstreeRepo *repo; + OstreeSysrootGlobalOptFlags opt_flags; OstreeSysrootDebugFlags debug_flags; }; diff --git a/src/libostree/ostree-sysroot.c b/src/libostree/ostree-sysroot.c index 9cb40e66..266a2975 100644 --- a/src/libostree/ostree-sysroot.c +++ b/src/libostree/ostree-sysroot.c @@ -184,6 +184,9 @@ ostree_sysroot_class_init (OstreeSysrootClass *klass) static void ostree_sysroot_init (OstreeSysroot *self) { + const GDebugKey globalopt_keys[] = { + { "skip-sync", OSTREE_SYSROOT_GLOBAL_OPT_SKIP_SYNC }, + }; const GDebugKey keys[] = { { "mutable-deployments", OSTREE_SYSROOT_DEBUG_MUTABLE_DEPLOYMENTS }, { "test-fifreeze", OSTREE_SYSROOT_DEBUG_TEST_FIFREEZE }, @@ -191,6 +194,8 @@ ostree_sysroot_init (OstreeSysroot *self) { "no-dtb", OSTREE_SYSROOT_DEBUG_TEST_NO_DTB }, }; + self->opt_flags = g_parse_debug_string (g_getenv ("OSTREE_SYSROOT_OPTS"), + globalopt_keys, G_N_ELEMENTS (globalopt_keys)); self->debug_flags = g_parse_debug_string (g_getenv ("OSTREE_SYSROOT_DEBUG"), keys, G_N_ELEMENTS (keys)); From 6230b3eeab52b132d5b7e4e164380389325db040 Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Wed, 19 Jan 2022 13:44:10 +0000 Subject: [PATCH 09/52] lib/commit: always validate metadata This tweaks commit logic in order to always validate metadata, including on commits where the expected checksum is already known. --- src/libostree/ostree-repo-commit.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index e2c86d96..a5aa63b0 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -2478,12 +2478,8 @@ ostree_repo_write_metadata (OstreeRepo *self, normalized = g_variant_get_normal_form (object); } - /* For untrusted objects, verify their structure here */ - if (expected_checksum) - { - if (!_ostree_validate_structureof_metadata (objtype, object, error)) - return FALSE; - } + if (!_ostree_validate_structureof_metadata (objtype, object, error)) + return FALSE; g_autoptr(GBytes) vdata = g_variant_get_data_as_bytes (normalized); if (!write_metadata_object (self, objtype, expected_checksum, From da72c245f4b730d2ff41db996ec14a7f21f097e9 Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Thu, 20 Jan 2022 10:54:30 +0000 Subject: [PATCH 10/52] lib/commit: reject empty metadata keys This adds one more check to the metadata validation logic in order to reject empty metadata keys. --- src/libostree/ostree-core.c | 13 +++++++++++++ src/ostree/ot-builtin-commit.c | 9 +++++---- tests/test-basic-user-only.sh | 13 ++++++++++++- 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/src/libostree/ostree-core.c b/src/libostree/ostree-core.c index 0abd90a4..038606e9 100644 --- a/src/libostree/ostree-core.c +++ b/src/libostree/ostree-core.c @@ -2197,6 +2197,19 @@ ostree_validate_structureof_commit (GVariant *commit, if (!validate_variant (commit, OSTREE_COMMIT_GVARIANT_FORMAT, error)) return FALSE; + g_autoptr(GVariant) metadata = NULL; + g_variant_get_child (commit, 0, "@a{sv}", &metadata); + g_assert (metadata != NULL); + g_autoptr(GVariantIter) metadata_iter = g_variant_iter_new (metadata); + g_assert (metadata_iter != NULL); + g_autoptr(GVariant) metadata_entry = NULL; + const gchar *metadata_key = NULL; + while (g_variant_iter_loop (metadata_iter, "{sv}", &metadata_key, NULL)) + { + if (metadata_key == NULL || strlen (metadata_key) == 0) + return glnx_throw (error, "Empty metadata key"); + } + g_autoptr(GVariant) parent_csum_v = NULL; g_variant_get_child (commit, 1, "@ay", &parent_csum_v); gsize n_elts; diff --git a/src/ostree/ot-builtin-commit.c b/src/ostree/ot-builtin-commit.c index 845013ed..c43f9b3c 100644 --- a/src/ostree/ot-builtin-commit.c +++ b/src/ostree/ot-builtin-commit.c @@ -335,17 +335,18 @@ parse_keyvalue_strings (GVariantBuilder *builder, if (!eq) return glnx_throw (error, "Missing '=' in KEY=VALUE metadata '%s'", s); g_autofree char *key = g_strndup (s, eq - s); + const char *value = eq + 1; if (is_gvariant_print) { - g_autoptr(GVariant) value = g_variant_parse (NULL, eq + 1, NULL, NULL, error); - if (!value) + g_autoptr(GVariant) variant = g_variant_parse (NULL, value, NULL, NULL, error); + if (!variant) return glnx_prefix_error (error, "Parsing %s", s); - g_variant_builder_add (builder, "{sv}", key, value); + g_variant_builder_add (builder, "{sv}", key, variant); } else g_variant_builder_add (builder, "{sv}", key, - g_variant_new_string (eq + 1)); + g_variant_new_string (value)); } return TRUE; diff --git a/tests/test-basic-user-only.sh b/tests/test-basic-user-only.sh index 368abf0d..f6e8606d 100755 --- a/tests/test-basic-user-only.sh +++ b/tests/test-basic-user-only.sh @@ -23,7 +23,7 @@ set -euo pipefail mode="bare-user-only" setup_test_repository "$mode" -extra_basic_tests=6 +extra_basic_tests=7 . $(dirname $0)/basic-test.sh $CMD_PREFIX ostree --version > version.yaml @@ -54,6 +54,17 @@ fi assert_file_has_content err.txt "Content object.*invalid mode.*with bits 040.*" echo "ok failed to commit suid" +cd ${test_tmpdir} +rm repo-input -rf +ostree_repo_init repo-input init --mode=archive +rm files -rf && mkdir files +if $CMD_PREFIX ostree --repo=repo-input commit -b metadata --tree=dir=files --add-metadata-string='=FOO' 2>err.txt; then + assert_not_reached "committed an empty metadata key" +fi +assert_file_has_content err.txt "Empty metadata key" +$CMD_PREFIX ostree --repo=repo-input commit -b metadata --tree=dir=files --add-metadata-string='FOO=' +echo "ok rejected invalid metadata" + cd ${test_tmpdir} rm repo-input -rf ostree_repo_init repo-input init --mode=archive From 8c58195cc4b1c6d1c90cea3f7f6457e15f4f75bb Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 20 Jan 2022 10:50:07 -0500 Subject: [PATCH 11/52] deploy: Also log to journal if we time out global sync() We do implicitly have this data because we log timings via structured metadata in a later journal entry, but it's quite common to lose the structured metadata because a lot of tooling just grabs the default syslog-compatible text from `journalctl`. Let's be louder when we hit this case as a general rule too; I think most people shipping ostree systems want to see if it's happening. --- src/libostree/ostree-sysroot-deploy.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index 6ee62410..726379d6 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -1647,7 +1647,10 @@ full_system_sync (OstreeSysroot *self, while (!syncdata->success) { if (!g_cond_wait_until (&syncdata->cond, &syncdata->mutex, end_time)) - break; + { + ot_journal_print (LOG_INFO, "Timed out waiting for global sync()"); + break; + } } g_mutex_unlock (&syncdata->mutex); sync_data_unref (syncdata); From 3f491a60c36168995222ab9c849673f06f38b7de Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Fri, 21 Jan 2022 12:26:32 +0000 Subject: [PATCH 12/52] builtin: use GCancellable and GError everywhere This reworks `ostree ls` top-level logic so that cancellation tokens and error details are plumbed through all codepaths. It also gets rid of all previous goto jumps. --- src/ostree/ot-builtin-ls.c | 126 +++++++++++++++++++------------------ 1 file changed, 66 insertions(+), 60 deletions(-) diff --git a/src/ostree/ot-builtin-ls.c b/src/ostree/ot-builtin-ls.c index a8056a86..5fa080ae 100644 --- a/src/ostree/ot-builtin-ls.c +++ b/src/ostree/ot-builtin-ls.c @@ -47,17 +47,19 @@ static GOptionEntry options[] = { { NULL } }; -static void -print_one_file_text (GFile *f, - GFileInfo *file_info) +static gboolean +print_one_file_text (GFile *f, + GFileInfo *file_info, + GCancellable *cancellable, + GError **error) { g_autoptr(GString) buf = g_string_new (""); char type_c; guint32 mode; guint32 type; - if (!ostree_repo_file_ensure_resolved ((OstreeRepoFile*)f, NULL)) - g_assert_not_reached (); + if (!ostree_repo_file_ensure_resolved ((OstreeRepoFile*)f, error)) + return FALSE; type_c = '?'; mode = g_file_info_get_attribute_uint32 (file_info, "unix::mode"); @@ -82,8 +84,7 @@ print_one_file_text (GFile *f, case G_FILE_TYPE_UNKNOWN: case G_FILE_TYPE_SHORTCUT: case G_FILE_TYPE_MOUNTABLE: - g_assert_not_reached (); - break; + return glnx_throw (error, "Invalid file type"); } g_string_append_c (buf, type_c); g_string_append_printf (buf, "0%04o %u %u %6" G_GUINT64_FORMAT " ", @@ -104,8 +105,8 @@ print_one_file_text (GFile *f, GVariant *xattrs; char *formatted; - if (!ostree_repo_file_get_xattrs ((OstreeRepoFile*)f, &xattrs, NULL, NULL)) - g_assert_not_reached (); + if (!ostree_repo_file_get_xattrs ((OstreeRepoFile*)f, &xattrs, cancellable, error)) + return FALSE; formatted = g_variant_print (xattrs, TRUE); g_string_append (buf, "{ "); @@ -121,39 +122,47 @@ print_one_file_text (GFile *f, g_string_append_printf (buf, " -> %s", g_file_info_get_attribute_byte_string (file_info, "standard::symlink-target")); g_print ("%s\n", buf->str); + + return TRUE; } -static void -print_one_file_binary (GFile *f, - GFileInfo *file_info) +static gboolean +print_one_file_binary (GFile *f, + GFileInfo *file_info, + GCancellable *cancellable, + GError **error) { const char *path; - if (!ostree_repo_file_ensure_resolved ((OstreeRepoFile*)f, NULL)) - g_assert_not_reached (); + if (!ostree_repo_file_ensure_resolved ((OstreeRepoFile*)f, error)) + return FALSE; path = gs_file_get_path_cached (f); fwrite (path, 1, strlen (path), stdout); fwrite ("\0", 1, 1, stdout); -} -static void -print_one_file (GFile *f, - GFileInfo *file_info) -{ - if (opt_nul_filenames_only) - print_one_file_binary (f, file_info); - else - print_one_file_text (f, file_info); + return TRUE; } static gboolean -print_directory_recurse (GFile *f, - int depth, - GError **error) +print_one_file (GFile *f, + GFileInfo *file_info, + GCancellable *cancellable, + GError **error) +{ + if (opt_nul_filenames_only) + return print_one_file_binary (f, file_info, cancellable, error); + else + return print_one_file_text (f, file_info, cancellable, error); +} + +static gboolean +print_directory_recurse (GFile *f, + int depth, + GCancellable *cancellable, + GError **error) { - gboolean ret = FALSE; g_autoptr(GFileEnumerator) dir_enum = NULL; g_autoptr(GFile) child = NULL; g_autoptr(GFileInfo) child_info = NULL; @@ -170,20 +179,21 @@ print_directory_recurse (GFile *f, G_FILE_QUERY_INFO_NOFOLLOW_SYMLINKS, NULL, error); - if (!dir_enum) - goto out; + if (dir_enum == NULL) + return FALSE; while ((child_info = g_file_enumerator_next_file (dir_enum, NULL, &temp_error)) != NULL) { g_clear_object (&child); child = g_file_get_child (f, g_file_info_get_name (child_info)); - print_one_file (child, child_info); + if (!print_one_file (child, child_info, cancellable, error)) + return FALSE; if (g_file_info_get_file_type (child_info) == G_FILE_TYPE_DIRECTORY) { - if (!print_directory_recurse (child, depth, error)) - goto out; + if (!print_directory_recurse (child, depth, cancellable, error)) + return FALSE; } g_clear_object (&child_info); @@ -191,12 +201,10 @@ print_directory_recurse (GFile *f, if (temp_error) { g_propagate_error (error, temp_error); - goto out; + return FALSE; } - ret = TRUE; - out: - return ret; + return TRUE; } static gboolean @@ -206,37 +214,38 @@ print_one_argument (OstreeRepo *repo, GCancellable *cancellable, GError **error) { - gboolean ret = FALSE; - g_autoptr(GFile) f = NULL; - g_autoptr(GFileInfo) file_info = NULL; + g_assert (root != NULL); + g_assert (arg != NULL); - f = g_file_resolve_relative_path (root, arg); - + g_autoptr(GFile) f = g_file_resolve_relative_path (root, arg); + if (f == NULL) + return glnx_throw (error, "Failed to resolve path '%s'", arg); + + g_autoptr(GFileInfo) file_info = NULL; file_info = g_file_query_info (f, OSTREE_GIO_FAST_QUERYINFO, G_FILE_QUERY_INFO_NOFOLLOW_SYMLINKS, cancellable, error); - if (!file_info) - goto out; + if (file_info == NULL) + return FALSE; - print_one_file (f, file_info); + if (!print_one_file (f, file_info, cancellable, error)) + return FALSE; if (g_file_info_get_file_type (file_info) == G_FILE_TYPE_DIRECTORY) { if (opt_recursive) { - if (!print_directory_recurse (f, -1, error)) - goto out; + if (!print_directory_recurse (f, -1, cancellable, error)) + return FALSE; } else if (!opt_dironly) { - if (!print_directory_recurse (f, 1, error)) - goto out; + if (!print_directory_recurse (f, 1, cancellable, error)) + return FALSE; } } - ret = TRUE; - out: - return ret; + return TRUE; } gboolean @@ -244,7 +253,6 @@ ostree_builtin_ls (int argc, char **argv, OstreeCommandInvocation *invocation, G { g_autoptr(GOptionContext) context = NULL; g_autoptr(OstreeRepo) repo = NULL; - gboolean ret = FALSE; const char *rev; int i; g_autoptr(GFile) root = NULL; @@ -252,33 +260,31 @@ ostree_builtin_ls (int argc, char **argv, OstreeCommandInvocation *invocation, G context = g_option_context_new ("COMMIT [PATH...]"); if (!ostree_option_context_parse (context, options, &argc, &argv, invocation, &repo, cancellable, error)) - goto out; + return FALSE; if (argc <= 1) { ot_util_usage_error (context, "An COMMIT argument is required", error); - goto out; + return FALSE; } rev = argv[1]; if (!ostree_repo_read_commit (repo, rev, &root, NULL, cancellable, error)) - goto out; + return FALSE; if (argc > 2) { for (i = 2; i < argc; i++) { if (!print_one_argument (repo, root, argv[i], cancellable, error)) - goto out; + return glnx_prefix_error (error, "Inspecting path '%s'", argv[i]); } } else { if (!print_one_argument (repo, root, "/", cancellable, error)) - goto out; + return FALSE; } - ret = TRUE; - out: - return ret; + return TRUE; } From 88dca03967ede45c4be475eafd005b6c881e6a56 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Fri, 21 Jan 2022 11:28:25 -0500 Subject: [PATCH 13/52] lib/deploy: When deleting staged deployment, delete any lock Otherwise, any future staged deployment will also automatically be locked even if not requested. Likely we should fold the locking into the primary `staged-deployment` serialized GVariant instead. --- src/libostree/ostree-sysroot-deploy.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index 726379d6..d3f277a7 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -2493,6 +2493,10 @@ ostree_sysroot_write_deployments_with_options (OstreeSysroot *self, if (!_ostree_sysroot_rmrf_deployment (self, self->staged_deployment, cancellable, error)) return FALSE; + /* Delete the lock if there was any. */ + if (!ot_ensure_unlinked_at (AT_FDCWD, _OSTREE_SYSROOT_RUNSTATE_STAGED_LOCKED, error)) + return FALSE; + /* Clear it out of the *current* deployments list to maintain invariants */ self->staged_deployment = NULL; g_ptr_array_remove_index (self->deployments, 0); From baf838de229a7934e4ee0cf27a10a54d6aac28fd Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Fri, 21 Jan 2022 13:47:02 -0500 Subject: [PATCH 14/52] ostree/deploy: Test finalization locking Support for that file was added previously, but the testing lived in rpm-ostree only. Let's add it here too. In the process add a hidden `--lock-finalization` to `ostree admin deploy` to make testing easier (though it could also be useful to update managers driving OSTree via the CLI). --- src/ostree/ot-admin-builtin-deploy.c | 18 ++++++++++++++++++ tests/kolainst/destructive/staged-deploy.sh | 12 ++++++++++-- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/ostree/ot-admin-builtin-deploy.c b/src/ostree/ot-admin-builtin-deploy.c index 97351b14..d4743890 100644 --- a/src/ostree/ot-admin-builtin-deploy.c +++ b/src/ostree/ot-admin-builtin-deploy.c @@ -21,6 +21,8 @@ #include "config.h" +#include "ostree-sysroot-private.h" + #include "ot-main.h" #include "ot-admin-builtins.h" #include "ot-admin-functions.h" @@ -31,6 +33,7 @@ static gboolean opt_retain; static gboolean opt_stage; +static gboolean opt_lock_finalization; static gboolean opt_retain_pending; static gboolean opt_retain_rollback; static gboolean opt_not_as_default; @@ -51,6 +54,7 @@ static GOptionEntry options[] = { { "no-merge", 0, 0, G_OPTION_ARG_NONE, &opt_no_merge, "Do not apply configuration (/etc and kernel arguments) from booted deployment", NULL}, { "retain", 0, 0, G_OPTION_ARG_NONE, &opt_retain, "Do not delete previous deployments", NULL }, { "stage", 0, 0, G_OPTION_ARG_NONE, &opt_stage, "Complete deployment at OS shutdown", NULL }, + { "lock-finalization", 0, G_OPTION_FLAG_HIDDEN, G_OPTION_ARG_NONE, &opt_lock_finalization, "Prevent automatic deployment finalization on shutdown", NULL }, { "retain-pending", 0, 0, G_OPTION_ARG_NONE, &opt_retain_pending, "Do not delete pending deployments", NULL }, { "retain-rollback", 0, 0, G_OPTION_ARG_NONE, &opt_retain_rollback, "Do not delete rollback deployments", NULL }, { "not-as-default", 0, 0, G_OPTION_ARG_NONE, &opt_not_as_default, "Append rather than prepend new deployment", NULL }, @@ -202,6 +206,20 @@ ot_admin_builtin_deploy (int argc, char **argv, OstreeCommandInvocation *invocat return glnx_throw (error, "--stage cannot currently be combined with --retain arguments"); if (opt_not_as_default) return glnx_throw (error, "--stage cannot currently be combined with --not-as-default"); + /* touch file *before* we stage to avoid races */ + if (opt_lock_finalization) + { + if (!glnx_shutil_mkdir_p_at (AT_FDCWD, + dirname (strdupa (_OSTREE_SYSROOT_RUNSTATE_STAGED_LOCKED)), + 0755, cancellable, error)) + return FALSE; + + glnx_autofd int fd = open (_OSTREE_SYSROOT_RUNSTATE_STAGED_LOCKED, + O_CREAT | O_WRONLY | O_NOCTTY | O_CLOEXEC, 0640); + if (fd == -1) + return glnx_throw_errno_prefix (error, "touch(%s)", + _OSTREE_SYSROOT_RUNSTATE_STAGED_LOCKED); + } /* use old API if we can to exercise it in CI */ if (!overlay_initrd_chksums) { diff --git a/tests/kolainst/destructive/staged-deploy.sh b/tests/kolainst/destructive/staged-deploy.sh index baadb3d8..406911ca 100755 --- a/tests/kolainst/destructive/staged-deploy.sh +++ b/tests/kolainst/destructive/staged-deploy.sh @@ -78,10 +78,18 @@ EOF # And there should not be a staged deployment test '!' -f /run/ostree/staged-deployment - # Upgrade with staging test '!' -f /run/ostree/staged-deployment - ostree admin deploy --stage staged-deploy + ostree admin deploy --stage staged-deploy --lock-finalization test -f /run/ostree/staged-deployment + test -f /run/ostree/staged-deployment-locked + # check that we can cleanup the staged deployment + ostree admin undeploy 0 + test ! -f /run/ostree/staged-deployment + test ! -f /run/ostree/staged-deployment-locked + echo "ok cleanup staged" + + # Upgrade with staging + ostree admin deploy --stage staged-deploy origcommit=$(ostree rev-parse staged-deploy) cd /ostree/repo/tmp ostree checkout -H "${origcommit}" t From b27792ade385130e9d0c2924c51107b5f263cb57 Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Mon, 24 Jan 2022 16:46:40 +0000 Subject: [PATCH 15/52] lib/repo: open file only if required This tightens up the logic for opening a file while inspecting its xattrs. The only codepath fetching xattrs from a FD is the one handling 'bare' mode. It also rearranges the else-assert flow, mostly for future-proofing. --- src/libostree/ostree-repo.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 86948ee2..64a8fea9 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -4218,8 +4218,9 @@ _ostree_repo_load_file_bare (OstreeRepo *self, cancellable, error); } - const gboolean need_open = - (out_fd || out_xattrs || self->mode == OSTREE_REPO_MODE_BARE_USER); + const gboolean need_open = (out_fd || + (out_xattrs && self->mode == OSTREE_REPO_MODE_BARE) || + self->mode == OSTREE_REPO_MODE_BARE_USER); /* If it's a regular file and we're requested to return the fd, do it now. As * a special case in bare-user, we always do an open, since the stat() metadata * lives there. @@ -4284,10 +4285,8 @@ _ostree_repo_load_file_bare (OstreeRepo *self, ret_xattrs = g_variant_ref_sink (g_variant_builder_end (&builder)); } } - else + else if (self->mode == OSTREE_REPO_MODE_BARE) { - g_assert (self->mode == OSTREE_REPO_MODE_BARE); - if (S_ISREG (stbuf.st_mode) && out_xattrs) { if (self->disable_xattrs) @@ -4306,6 +4305,10 @@ _ostree_repo_load_file_bare (OstreeRepo *self, return FALSE; } } + else + { + g_assert_not_reached (); + } if (out_fd) *out_fd = glnx_steal_fd (&fd); From 5bf57ec062b446a45de60de639e974ce06f028cd Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Fri, 21 Jan 2022 13:41:32 -0500 Subject: [PATCH 16/52] tests/kolainst: Avoid recursive symlinks `kola` now follows symlinks when archiving an external test's `data/` dir. So the recursive `data` symlink we have here breaks it. Let's just move the shared files in its own directory and update the symlinks. --- tests/kolainst/{ => data-shared}/libinsttest.sh | 0 tests/kolainst/{ => data-shared}/libtest-core.sh | 0 tests/kolainst/destructive/data | 2 +- tests/kolainst/nondestructive/data | 2 +- tests/kolainst/nondestructive/libtest-core.sh | 2 +- tests/libtest-core.sh | 2 +- 6 files changed, 4 insertions(+), 4 deletions(-) rename tests/kolainst/{ => data-shared}/libinsttest.sh (100%) rename tests/kolainst/{ => data-shared}/libtest-core.sh (100%) diff --git a/tests/kolainst/libinsttest.sh b/tests/kolainst/data-shared/libinsttest.sh similarity index 100% rename from tests/kolainst/libinsttest.sh rename to tests/kolainst/data-shared/libinsttest.sh diff --git a/tests/kolainst/libtest-core.sh b/tests/kolainst/data-shared/libtest-core.sh similarity index 100% rename from tests/kolainst/libtest-core.sh rename to tests/kolainst/data-shared/libtest-core.sh diff --git a/tests/kolainst/destructive/data b/tests/kolainst/destructive/data index a96aa0ea..8bd12e74 120000 --- a/tests/kolainst/destructive/data +++ b/tests/kolainst/destructive/data @@ -1 +1 @@ -.. \ No newline at end of file +../data-shared \ No newline at end of file diff --git a/tests/kolainst/nondestructive/data b/tests/kolainst/nondestructive/data index a96aa0ea..8bd12e74 120000 --- a/tests/kolainst/nondestructive/data +++ b/tests/kolainst/nondestructive/data @@ -1 +1 @@ -.. \ No newline at end of file +../data-shared \ No newline at end of file diff --git a/tests/kolainst/nondestructive/libtest-core.sh b/tests/kolainst/nondestructive/libtest-core.sh index d26203e2..bd5f8277 120000 --- a/tests/kolainst/nondestructive/libtest-core.sh +++ b/tests/kolainst/nondestructive/libtest-core.sh @@ -1 +1 @@ -../libtest-core.sh \ No newline at end of file +../data-shared/libtest-core.sh \ No newline at end of file diff --git a/tests/libtest-core.sh b/tests/libtest-core.sh index 3c181880..9bb1d3f1 120000 --- a/tests/libtest-core.sh +++ b/tests/libtest-core.sh @@ -1 +1 @@ -kolainst/libtest-core.sh \ No newline at end of file +kolainst/data-shared/libtest-core.sh \ No newline at end of file From 920f85cabc656e4a7c07574aa9af211b6153756d Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Fri, 28 Jan 2022 11:08:00 +0000 Subject: [PATCH 17/52] libotutil: Avoid infinite recursion during error unwinding When we clean up from an error, for example copy_file_range() failing while we generate a static delta (perhaps caused by https://gitlab.gnome.org/GNOME/libglnx/-/issues/3 or by a genuine write error), we might free a variant builder that has a non-null parent. Previously, this caused infinite recursion and a stack overflow, repeatedly freeing the same object, but Luca Bruno suggested that the intention here appears to have been to free the parent object. Partially resolves https://github.com/ostreedev/ostree/issues/2525 (the other bug reported in that issue needs to be resolved by updating libglnx to a version where libglnx#3 has been fixed). Signed-off-by: Simon McVittie --- src/libotutil/ot-variant-builder.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libotutil/ot-variant-builder.c b/src/libotutil/ot-variant-builder.c index 754b9323..4d3dcbe5 100644 --- a/src/libotutil/ot-variant-builder.c +++ b/src/libotutil/ot-variant-builder.c @@ -830,7 +830,7 @@ static void ot_variant_builder_info_free (OtVariantBuilderInfo *info) { if (info->parent) - ot_variant_builder_info_free (info); + ot_variant_builder_info_free (info->parent); g_variant_type_free (info->type); g_array_unref (info->child_ends); From 0ebf9d9f64647d0b8b95cda112c7854be6f58ed3 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Fri, 28 Jan 2022 12:20:39 +0000 Subject: [PATCH 18/52] Update submodule: libglnx Resolves: https://gitlab.gnome.org/GNOME/libglnx/-/issues/3 Signed-off-by: Simon McVittie --- libglnx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libglnx b/libglnx index ef502aab..803adaf4 160000 --- a/libglnx +++ b/libglnx @@ -1 +1 @@ -Subproject commit ef502aabf7d3a0d37f9c4d228f870ac93404447b +Subproject commit 803adaf4883f237d0a3658fa16e72eb1c41ac828 From e6e9f14985a52248a03d3e7202f876cadee1c7c8 Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Tue, 1 Feb 2022 17:33:28 +0000 Subject: [PATCH 19/52] lib/commit: clean up assertions This aligns all the assertion in the module. In particular, it gets rid of all `g_return_val_if_fail` instances which may fail without properly setting GError to the caller. --- src/libostree/ostree-repo-commit.c | 62 +++++++++++++++++++++--------- 1 file changed, 43 insertions(+), 19 deletions(-) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index a5aa63b0..21ce288f 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -912,8 +912,9 @@ write_content_object (OstreeRepo *self, GCancellable *cancellable, GError **error) { + g_assert (expected_checksum != NULL || out_csum != NULL); + GLNX_AUTO_PREFIX_ERROR ("Writing content object", error); - g_return_val_if_fail (expected_checksum || out_csum, FALSE); if (g_cancellable_set_error_if_cancelled (cancellable, error)) return FALSE; @@ -1244,9 +1245,10 @@ adopt_and_commit_regfile (OstreeRepo *self, GCancellable *cancellable, GError **error) { + g_assert (G_IN_SET (self->mode, OSTREE_REPO_MODE_BARE, OSTREE_REPO_MODE_BARE_USER_ONLY)); + GLNX_AUTO_PREFIX_ERROR ("Commit regfile (adopt)", error); - g_assert (G_IN_SET (self->mode, OSTREE_REPO_MODE_BARE, OSTREE_REPO_MODE_BARE_USER_ONLY)); g_autoptr(GBytes) header = _ostree_file_header_new (finfo, xattrs); g_auto(OtChecksum) hasher = { 0, }; @@ -1341,9 +1343,9 @@ write_metadata_object (OstreeRepo *self, GCancellable *cancellable, GError **error) { - GLNX_AUTO_PREFIX_ERROR ("Writing metadata object", error); + g_assert (expected_checksum != NULL || out_csum != NULL); - g_return_val_if_fail (expected_checksum || out_csum, FALSE); + GLNX_AUTO_PREFIX_ERROR ("Writing metadata object", error); if (g_cancellable_set_error_if_cancelled (cancellable, error)) return FALSE; @@ -1629,7 +1631,11 @@ ostree_repo_scan_hardlinks (OstreeRepo *self, GCancellable *cancellable, GError **error) { - g_return_val_if_fail (self->in_transaction == TRUE, FALSE); + g_assert (self != NULL); + g_assert (OSTREE_IS_REPO (self)); + + if (!self->in_transaction) + return glnx_throw (error, "Failed to scan hardlinks, not in a transaction"); if (!self->loose_object_devino_hash) self->loose_object_devino_hash = (GHashTable*)ostree_repo_devino_cache_new (); @@ -1671,10 +1677,10 @@ ostree_repo_prepare_transaction (OstreeRepo *self, GError **error) { g_assert (self != NULL); + g_assert (OSTREE_IS_REPO (self)); - guint64 reserved_bytes = 0; - - g_return_val_if_fail (self->in_transaction == FALSE, FALSE); + if (self->in_transaction) + return glnx_throw (error, "Failed to prepare transaction, another transaction is in progress"); g_debug ("Preparing transaction in repository %p", self); @@ -1700,6 +1706,7 @@ ostree_repo_prepare_transaction (OstreeRepo *self, g_mutex_lock (&self->txn_lock); self->txn.blocksize = stvfsbuf.f_bsize; + guint64 reserved_bytes = 0; if (!ostree_repo_get_min_free_space_bytes (self, &reserved_bytes, error)) { g_mutex_unlock (&self->txn_lock); @@ -2077,7 +2084,9 @@ ostree_repo_transaction_set_refspec (OstreeRepo *self, const char *refspec, const char *checksum) { - g_return_if_fail (self->in_transaction == TRUE); + g_assert (self != NULL); + g_assert (OSTREE_IS_REPO (self)); + g_assert (self->in_transaction == TRUE); g_mutex_lock (&self->txn_lock); ensure_txn_refs (self); @@ -2120,7 +2129,9 @@ ostree_repo_transaction_set_ref (OstreeRepo *self, const char *ref, const char *checksum) { - g_return_if_fail (self->in_transaction == TRUE); + g_assert (self != NULL); + g_assert (OSTREE_IS_REPO (self)); + g_assert (self->in_transaction == TRUE); char *refspec; if (remote) @@ -2160,9 +2171,13 @@ ostree_repo_transaction_set_collection_ref (OstreeRepo *self, const OstreeCollectionRef *ref, const char *checksum) { - g_return_if_fail (OSTREE_IS_REPO (self)); - g_return_if_fail (self->in_transaction == TRUE); - g_return_if_fail (ref != NULL); + g_assert (self != NULL); + g_assert (OSTREE_IS_REPO (self)); + g_assert (self->in_transaction == TRUE); + g_assert (ref != NULL); + + // TODO(lucab): introduce a method with error-returning in order to deprecate + // this one, because it can silently fail. g_return_if_fail (checksum == NULL || ostree_validate_checksum_string (checksum, NULL)); g_mutex_lock (&self->txn_lock); @@ -2248,11 +2263,13 @@ ostree_repo_set_collection_ref_immediate (OstreeRepo *self, GCancellable *cancellable, GError **error) { - g_return_val_if_fail (OSTREE_IS_REPO (self), FALSE); - g_return_val_if_fail (ref != NULL, FALSE); - g_return_val_if_fail (checksum == NULL || ostree_validate_checksum_string (checksum, NULL), FALSE); - g_return_val_if_fail (cancellable == NULL || G_IS_CANCELLABLE (cancellable), FALSE); - g_return_val_if_fail (error == NULL || *error == NULL, FALSE); + g_assert (self != NULL); + g_assert (OSTREE_IS_REPO (self)); + g_assert (ref != NULL); + + /* If a checksum was provided, validate it upfront. */ + if (checksum != NULL && !ostree_validate_checksum_string (checksum, error)) + return FALSE; return _ostree_repo_write_ref (self, NULL, ref, checksum, NULL, cancellable, error); @@ -2283,7 +2300,11 @@ ostree_repo_commit_transaction (OstreeRepo *self, GCancellable *cancellable, GError **error) { - g_return_val_if_fail (self->in_transaction == TRUE, FALSE); + g_assert (self != NULL); + g_assert (OSTREE_IS_REPO (self)); + + if (!self->in_transaction) + return glnx_throw (error, "Failed to commit transaction, no transaction in progress"); g_debug ("Committing transaction in repository %p", self); @@ -2370,6 +2391,9 @@ ostree_repo_abort_transaction (OstreeRepo *self, GCancellable *cancellable, GError **error) { + g_assert (self != NULL); + g_assert (OSTREE_IS_REPO (self)); + g_autoptr(GError) cleanup_error = NULL; /* Always ignore the cancellable to avoid the chance that, if it gets From 8e445cb957ecb6a3b428e8a4088a023300fdbd8b Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Thu, 3 Feb 2022 13:08:22 -0500 Subject: [PATCH 20/52] ci/libbuild.sh: drop yum/CentOS support `dnf` is present in all the platforms we care about now, and the CentOS bit is out of date. We can re-add it if we add e.g. C[89]S support with the updated list of packages. Motivated by noticing that the `yum` symlink isn't always present. --- ci/libbuild.sh | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/ci/libbuild.sh b/ci/libbuild.sh index dece8d09..ffc65585 100644 --- a/ci/libbuild.sh +++ b/ci/libbuild.sh @@ -6,7 +6,7 @@ OS_ID=$(. /etc/os-release; echo $ID) OS_VERSION_ID=$(. /etc/os-release; echo $VERSION_ID) pkg_upgrade() { - yum -y distro-sync + dnf -y distro-sync } make() { @@ -20,7 +20,7 @@ build() { } pkg_install() { - yum -y install "$@" + dnf -y install "$@" } pkg_install_if_os() { @@ -39,25 +39,15 @@ pkg_install_buildroot() { # https://github.com/projectatomic/rpm-ostree/pull/1889/commits/9ff611758bea22b0ad4892cc16182dd1f7f47e89 # https://fedoraproject.org/wiki/Common_F30_bugs#Conflicts_between_fedora-release_packages_when_installing_package_groups if rpm -q fedora-release-container; then - yum -y swap fedora-release{-container,} + dnf -y swap fedora-release{-container,} fi pkg_install dnf-plugins-core @buildsys-build;; - centos) pkg_install yum-utils - # Base buildroot, copied from the mock config sadly - pkg_install bash bzip2 coreutils cpio diffutils system-release findutils gawk gcc gcc-c++ \ - grep gzip info make patch redhat-rpm-config rpm-build sed shadow-utils tar \ - unzip util-linux which xz;; *) fatal "pkg_install_buildroot(): Unhandled OS ${OS_ID}";; esac } pkg_builddep() { - # This is sadly the only case where it's a different command - if test -x /usr/bin/dnf; then - dnf builddep -y "$@" - else - yum-builddep -y "$@" - fi + dnf builddep -y "$@" } # Install both build and runtime dependencies for $pkg From 8d45298a2d4ff77a5d70f871873cd099cd49f489 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Thu, 3 Feb 2022 13:21:54 -0500 Subject: [PATCH 21/52] ci/make-git-snapshot.sh: fix archive name The archive name is libostree even though the project name is ostree, so we can't rely on the directory name. Just hardcode it. --- ci/make-git-snapshot.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/make-git-snapshot.sh b/ci/make-git-snapshot.sh index 67cf14c9..391af64d 100755 --- a/ci/make-git-snapshot.sh +++ b/ci/make-git-snapshot.sh @@ -5,7 +5,7 @@ TOP=$(git rev-parse --show-toplevel) GITREV=$(git rev-parse HEAD) gitdescribe=$(git describe --always --tags $GITREV) version=$(echo "$gitdescribe" | sed -e 's,-,\.,g' -e 's,^v,,') -name=$(basename $(pwd)) +name=libostree PKG_VER="${name}-${version}" TARFILE=${PKG_VER}.tar From 92c396c82bb2a4041bac7f5d14c77f7d8c0bf23a Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Thu, 3 Feb 2022 13:22:47 -0500 Subject: [PATCH 22/52] ci/make-git-snapshot.sh: auto-initialize submodules Matches `autogen.sh`. --- ci/make-git-snapshot.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ci/make-git-snapshot.sh b/ci/make-git-snapshot.sh index 391af64d..1b17b93f 100755 --- a/ci/make-git-snapshot.sh +++ b/ci/make-git-snapshot.sh @@ -11,6 +11,10 @@ PKG_VER="${name}-${version}" TARFILE=${PKG_VER}.tar TARFILE_TMP=${TARFILE}.tmp +if ! test -f ${TOP}/libglnx/README.md || ! test -f ${TOP}/bsdiff/README.md; then + git submodule update --init +fi + echo "Archiving ${PKG_VER} at ${GITREV} to ${TARFILE_TMP}" (cd ${TOP}; git archive --format=tar --prefix=${PKG_VER}/ ${GITREV}) > ${TARFILE_TMP} ls -al ${TARFILE_TMP} From a51ae1ed736c8a3f97de5bc153472c9fd3c265d6 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Thu, 3 Feb 2022 13:23:43 -0500 Subject: [PATCH 23/52] ci/make-git-snapshot.sh: xz the archive This matches `make dist` and what the spec file expects. --- ci/make-git-snapshot.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/ci/make-git-snapshot.sh b/ci/make-git-snapshot.sh index 1b17b93f..acfdd6c2 100755 --- a/ci/make-git-snapshot.sh +++ b/ci/make-git-snapshot.sh @@ -26,3 +26,4 @@ ls -al ${TARFILE_TMP} rm submodule.tar done mv ${TARFILE_TMP} ${TARFILE} +xz "${TARFILE}" From 1e663baad723f4b948b696d7585a4e15bef43461 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Thu, 3 Feb 2022 13:25:07 -0500 Subject: [PATCH 24/52] Add COPR integration Makefile I'd like to enable auto-builds of this repo to https://copr.fedorainfracloud.org/coprs/g/CoreOS/continuous/ so it could eventually feed into https://github.com/coreos/fedora-coreos-tracker/issues/910. --- .copr/Makefile | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 .copr/Makefile diff --git a/.copr/Makefile b/.copr/Makefile new file mode 100644 index 00000000..81a02213 --- /dev/null +++ b/.copr/Makefile @@ -0,0 +1,8 @@ +srpm: + dnf install -y git + ci/make-git-snapshot.sh + curl -LO https://src.fedoraproject.org/rpms/ostree/raw/rawhide/f/ostree.spec + sed -ie "s,^Version:.*,Version: $$(git describe --always --tags | sed -e 's,-,\.,g' -e 's,^v,,')," ostree.spec + sed -ie 's/^Patch/# Patch/g' ostree.spec # we don't want any downstream patches + rpmbuild -bs --define "_sourcedir ${PWD}" --define "_specdir ${PWD}" --define "_builddir ${PWD}" --define "_srcrpmdir ${PWD}" --define "_rpmdir ${PWD}" --define "_buildrootdir ${PWD}/.build" ostree.spec + mv *.src.rpm $$outdir From 6fbf759279ca9908e0c7ceb2a3af75b1180fa8a1 Mon Sep 17 00:00:00 2001 From: Dan Nicholson Date: Fri, 4 Feb 2022 14:11:06 -0700 Subject: [PATCH 25/52] lib/repo: Add commit version metadata to summary metadata The commit metadata `version` key is well established but getting it for a remote commit is cumbersome since the commit object needs to be fetched and loaded. Including it in the summary additional metadata allows a much more convenient view of what each of the remote refs represents. --- man/ostree-summary.xml | 1 + src/libostree/ostree-core.h | 2 ++ src/libostree/ostree-repo-private.h | 1 + src/libostree/ostree-repo.c | 7 ++++++- src/ostree/ot-dump.c | 5 +++++ tests/test-summary-view.sh | 1 + 6 files changed, 16 insertions(+), 1 deletion(-) diff --git a/man/ostree-summary.xml b/man/ostree-summary.xml index 4c9652cc..e853e8cd 100644 --- a/man/ostree-summary.xml +++ b/man/ostree-summary.xml @@ -183,6 +183,7 @@ License along with this library. If not, see . Latest Commit (4.2 MB): 9828ab80f357459b4ab50f0629beab2ae3b67318fc3d161d10a89fae353afa90 Timestamp (ostree.commit.timestamp): 2017-11-21T01:41:10-08 + Version (ostree.commit.version): 1.2.3 Last-Modified (ostree.summary.last-modified): 2018-01-12T22:06:38-08 diff --git a/src/libostree/ostree-core.h b/src/libostree/ostree-core.h index 36e61290..48a75f92 100644 --- a/src/libostree/ostree-core.h +++ b/src/libostree/ostree-core.h @@ -164,6 +164,8 @@ typedef enum { * The currently defined keys for the `a{sv}` of additional metadata for each commit are: * - key: `ostree.commit.timestamp`, value: `t`, timestamp (seconds since the * Unix epoch in UTC, big-endian) when the commit was committed + * - key: `ostree.commit.version`, value: `s`, the `version` value from the + * commit's metadata if it was defined. Since: 2022.2 */ #define OSTREE_SUMMARY_GVARIANT_STRING "(a(s(taya{sv}))a{sv})" #define OSTREE_SUMMARY_GVARIANT_FORMAT G_VARIANT_TYPE (OSTREE_SUMMARY_GVARIANT_STRING) diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h index 6d8f0193..988c2179 100644 --- a/src/libostree/ostree-repo-private.h +++ b/src/libostree/ostree-repo-private.h @@ -63,6 +63,7 @@ G_BEGIN_DECLS /* Well-known keys for the additional metadata field in a commit in a ref entry * in a summary file. */ #define OSTREE_COMMIT_TIMESTAMP "ostree.commit.timestamp" +#define OSTREE_COMMIT_VERSION "ostree.commit.version" typedef enum { OSTREE_REPO_TEST_ERROR_PRE_COMMIT = (1 << 0), diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 64a8fea9..6c541029 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -6067,10 +6067,11 @@ summary_add_ref_entry (OstreeRepo *self, g_autoptr(GVariant) commit_obj = NULL; if (!ostree_repo_load_variant (self, OSTREE_OBJECT_TYPE_COMMIT, checksum, &commit_obj, error)) return FALSE; + g_autoptr(GVariant) orig_metadata = g_variant_get_child_value (commit_obj, 0); g_variant_dict_init (&commit_metadata_builder, NULL); - /* Forward the commit’s timestamp if it’s valid. */ + /* Forward the commit’s timestamp and version if they're valid. */ guint64 commit_timestamp = ostree_commit_get_timestamp (commit_obj); g_autoptr(GDateTime) dt = g_date_time_new_from_unix_utc (commit_timestamp); @@ -6078,6 +6079,10 @@ summary_add_ref_entry (OstreeRepo *self, g_variant_dict_insert_value (&commit_metadata_builder, OSTREE_COMMIT_TIMESTAMP, g_variant_new_uint64 (GUINT64_TO_BE (commit_timestamp))); + const char *version = NULL; + if (g_variant_lookup (orig_metadata, OSTREE_COMMIT_META_KEY_VERSION, "&s", &version)) + g_variant_dict_insert (&commit_metadata_builder, OSTREE_COMMIT_VERSION, "s", version); + g_variant_builder_add_value (refs_builder, g_variant_new ("(s(t@ay@a{sv}))", ref, (guint64) g_variant_get_size (commit_obj), diff --git a/src/ostree/ot-dump.c b/src/ostree/ot-dump.c index b874db0f..509eb792 100644 --- a/src/ostree/ot-dump.c +++ b/src/ostree/ot-dump.c @@ -249,6 +249,11 @@ dump_summary_ref (const char *collection_id, pretty_key = "Timestamp"; value_str = uint64_secs_to_iso8601 (GUINT64_FROM_BE (g_variant_get_uint64 (value))); } + else if (g_strcmp0 (key, OSTREE_COMMIT_VERSION) == 0) + { + pretty_key = "Version"; + value_str = g_strdup (g_variant_get_string (value, NULL)); + } else { value_str = g_variant_print (value, FALSE); diff --git a/tests/test-summary-view.sh b/tests/test-summary-view.sh index 088628d8..9dfc74f4 100755 --- a/tests/test-summary-view.sh +++ b/tests/test-summary-view.sh @@ -56,6 +56,7 @@ assert_file_has_content_literal summary.txt "* main" assert_file_has_content_literal summary.txt "* other" assert_file_has_content_literal summary.txt "ostree.summary.last-modified" assert_file_has_content_literal summary.txt "Timestamp (ostree.commit.timestamp): " +assert_file_has_content_literal summary.txt "Version (ostree.commit.version): 3.2" echo "ok view summary" # Check the summary can be viewed raw too. From c5ecef0aa453a1ba2e9fb9a2319bf4a9999a5180 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Wed, 9 Feb 2022 14:29:50 -0500 Subject: [PATCH 26/52] lib/gpg-verify-result: Add missing floating annotation I think I'm hitting issues due to this while using the Rust bindings: https://github.com/coreos/rpm-ostree/pull/3406#issuecomment-1033084956 The bindings for those APIs use `from_glib_full` which says: > Because ownership can only be transferred if something is already > referenced, this is unsuitable for floating references. --- src/libostree/ostree-gpg-verify-result-dummy.c | 4 ++-- src/libostree/ostree-gpg-verify-result.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libostree/ostree-gpg-verify-result-dummy.c b/src/libostree/ostree-gpg-verify-result-dummy.c index e0116518..382b7b92 100644 --- a/src/libostree/ostree-gpg-verify-result-dummy.c +++ b/src/libostree/ostree-gpg-verify-result-dummy.c @@ -143,7 +143,7 @@ ostree_gpg_verify_result_lookup (OstreeGpgVerifyResult *result, * an invalid @signature_index. Use ostree_gpg_verify_result_count_all() to * find the number of signatures in @result. * - * Returns: a new, floating, #GVariant tuple + * Returns: (transfer floating): a new, floating, #GVariant tuple **/ GVariant * ostree_gpg_verify_result_get (OstreeGpgVerifyResult *result, @@ -184,7 +184,7 @@ ostree_gpg_verify_result_get (OstreeGpgVerifyResult *result, * ostree_gpg_verify_result_count_all() to find the number of signatures in * @result. * - * Returns: a new, floating, #GVariant tuple + * Returns: (transfer floating): a new, floating, #GVariant tuple **/ GVariant * ostree_gpg_verify_result_get_all (OstreeGpgVerifyResult *result, diff --git a/src/libostree/ostree-gpg-verify-result.c b/src/libostree/ostree-gpg-verify-result.c index 7bf45aac..3b6a7da4 100644 --- a/src/libostree/ostree-gpg-verify-result.c +++ b/src/libostree/ostree-gpg-verify-result.c @@ -298,7 +298,7 @@ ostree_gpg_verify_result_lookup (OstreeGpgVerifyResult *result, * an invalid @signature_index. Use ostree_gpg_verify_result_count_all() to * find the number of signatures in @result. * - * Returns: a new, floating, #GVariant tuple + * Returns: (transfer floating): a new, floating, #GVariant tuple **/ GVariant * ostree_gpg_verify_result_get (OstreeGpgVerifyResult *result, @@ -490,7 +490,7 @@ ostree_gpg_verify_result_get (OstreeGpgVerifyResult *result, * ostree_gpg_verify_result_count_all() to find the number of signatures in * @result. * - * Returns: a new, floating, #GVariant tuple + * Returns: (transfer floating): a new, floating, #GVariant tuple **/ GVariant * ostree_gpg_verify_result_get_all (OstreeGpgVerifyResult *result, From c27b98a2cba73b03fba7d6baa2ff4f49a2fb3f31 Mon Sep 17 00:00:00 2001 From: Nikita Dubrovskii Date: Wed, 17 Nov 2021 13:10:20 +0100 Subject: [PATCH 27/52] s390x: add "IBM Secure Execution for Linux" support If system contains ibm-z-hostkey (fetched during ignition), than ostree generates 'sd-boot' image and reboots into Secure Execution Signed-off-by: Nikita Dubrovskii --- src/libostree/ostree-bootloader-zipl.c | 170 ++++++++++++++++++++++++- src/libostree/ostree-bootloader-zipl.h | 1 - src/libostree/ostree-bootloader.c | 3 +- src/libostree/ostree-bootloader.h | 2 + src/libostree/ostree-sysroot-deploy.c | 2 +- 5 files changed, 174 insertions(+), 4 deletions(-) diff --git a/src/libostree/ostree-bootloader-zipl.c b/src/libostree/ostree-bootloader-zipl.c index a7078aea..7358671b 100644 --- a/src/libostree/ostree-bootloader-zipl.c +++ b/src/libostree/ostree-bootloader-zipl.c @@ -19,10 +19,15 @@ #include "ostree-sysroot-private.h" #include "ostree-bootloader-zipl.h" +#include "ostree-deployment-private.h" #include "otutil.h" - +#include #include +#define SECURE_EXECUTION_BOOT_IMAGE "/boot/sd-boot" +#define SECURE_EXECUTION_HOSTKEY_PATH "/etc/se-hostkeys/" +#define SECURE_EXECUTION_HOSTKEY_PREFIX "ibm-z-hostkey" + /* This is specific to zipl today, but in the future we could also * use it for the grub2-mkconfig case. */ @@ -78,8 +83,163 @@ _ostree_bootloader_zipl_write_config (OstreeBootloader *bootloader, return TRUE; } +static gboolean +_ostree_secure_execution_get_keys (GPtrArray **keys, + GCancellable *cancellable, + GError **error) +{ + g_auto (GLnxDirFdIterator) it = { 0,}; + if ( !glnx_dirfd_iterator_init_at (-1, SECURE_EXECUTION_HOSTKEY_PATH, TRUE, &it, error)) + return glnx_prefix_error (error, "s390x SE: looking for SE keys"); + + g_autoptr(GPtrArray) ret_keys = g_ptr_array_new_with_free_func (g_free); + while (TRUE) + { + struct dirent *dent = NULL; + if (!glnx_dirfd_iterator_next_dent (&it, &dent, cancellable, error)) + return FALSE; + + if (!dent) + break; + + if (g_str_has_prefix (dent->d_name, SECURE_EXECUTION_HOSTKEY_PREFIX)) + g_ptr_array_add (ret_keys, g_build_filename (SECURE_EXECUTION_HOSTKEY_PATH, dent->d_name, NULL)); + } + + *keys = g_steal_pointer (&ret_keys); + return TRUE; +} + +static gboolean +_ostree_secure_execution_get_bls_config (OstreeBootloaderZipl *self, + int bootversion, + gchar **vmlinuz, + gchar **initramfs, + gchar **options, + GCancellable *cancellable, + GError **error) +{ + g_autoptr (GPtrArray) configs = NULL; + if ( !_ostree_sysroot_read_boot_loader_configs (self->sysroot, bootversion, &configs, cancellable, error)) + return glnx_prefix_error (error, "s390x SE: loading bls configs"); + + if (!configs || configs->len == 0) + return glnx_throw (error, "s390x SE: no bls config"); + + OstreeBootconfigParser *parser = (OstreeBootconfigParser *) g_ptr_array_index (configs, 0); + const gchar *val = NULL; + + val = ostree_bootconfig_parser_get (parser, "linux"); + if (!val) + return glnx_throw (error, "s390x SE: no \"linux\" key in bootloader config"); + *vmlinuz = g_build_filename ("/boot", val, NULL); + + val = ostree_bootconfig_parser_get (parser, "initrd"); + if (!val) + return glnx_throw (error, "s390x SE: no \"initrd\" key in bootloader config"); + *initramfs = g_build_filename ("/boot", val, NULL); + + val = ostree_bootconfig_parser_get (parser, "options"); + if (!val) + return glnx_throw (error, "s390x SE: no \"options\" key in bootloader config"); + *options = g_strdup(val); + + return TRUE; +} + +static gboolean +_ostree_secure_execution_generate_sdboot (gchar *vmlinuz, + gchar *initramfs, + gchar *options, + GPtrArray *keys, + GError **error) +{ + g_assert (vmlinuz && initramfs && options && keys && keys->len); + sd_journal_print(LOG_INFO, "s390x SE: kernel: %s", vmlinuz); + sd_journal_print(LOG_INFO, "s390x SE: initrd: %s", initramfs); + sd_journal_print(LOG_INFO, "s390x SE: kargs: %s", options); + + pid_t self = getpid(); + + // Store kernel options to temp file, so `genprotimg` can later embed it + g_auto(GLnxTmpfile) cmdline = { 0, }; + if (!glnx_open_anonymous_tmpfile (O_RDWR | O_CLOEXEC, &cmdline, error)) + return glnx_prefix_error(error, "s390x SE: opening cmdline file"); + if (glnx_loop_write (cmdline.fd, options, strlen (options)) < 0) + return glnx_throw_errno_prefix (error, "s390x SE: writting cmdline file"); + g_autofree gchar *cmdline_filename = g_strdup_printf ("/proc/%d/fd/%d", self, cmdline.fd); + + g_autoptr(GPtrArray) argv = g_ptr_array_new (); + g_ptr_array_add (argv, "genprotimg"); + g_ptr_array_add (argv, "-i"); + g_ptr_array_add (argv, vmlinuz); + g_ptr_array_add (argv, "-r"); + g_ptr_array_add (argv, initramfs); + g_ptr_array_add (argv, "-p"); + g_ptr_array_add (argv, cmdline_filename); + for (guint i = 0; i < keys->len; ++i) + { + gchar *key = g_ptr_array_index (keys, i); + g_ptr_array_add (argv, "-k"); + g_ptr_array_add (argv, key); + sd_journal_print(LOG_INFO, "s390x SE: key[%d]: %s", i + 1, key); + } + g_ptr_array_add (argv, "--no-verify"); + g_ptr_array_add (argv, "-o"); + g_ptr_array_add (argv, SECURE_EXECUTION_BOOT_IMAGE); + g_ptr_array_add (argv, NULL); + + gint status = 0; + if (!g_spawn_sync (NULL, (char**)argv->pdata, NULL, G_SPAWN_SEARCH_PATH, + NULL, NULL, NULL, NULL, &status, error)) + return glnx_prefix_error(error, "s390x SE: spawning genprotimg"); + + if (!g_spawn_check_exit_status (status, error)) + return glnx_prefix_error(error, "s390x SE: `genprotimg` failed"); + + sd_journal_print(LOG_INFO, "s390x SE: `%s` generated", SECURE_EXECUTION_BOOT_IMAGE); + return TRUE; +} + +static gboolean +_ostree_secure_execution_call_zipl (GError **error) +{ + int status = 0; + const char *const zipl_argv[] = {"zipl", "-V", "-t", "/boot", "-i", SECURE_EXECUTION_BOOT_IMAGE, NULL}; + if (!g_spawn_sync (NULL, (char**)zipl_argv, NULL, G_SPAWN_SEARCH_PATH, + NULL, NULL, NULL, NULL, &status, error)) + return glnx_prefix_error(error, "s390x SE: spawning zipl"); + + if (!g_spawn_check_exit_status (status, error)) + return glnx_prefix_error(error, "s390x SE: `zipl` failed"); + + sd_journal_print(LOG_INFO, "s390x SE: `sd-boot` zipled"); + return TRUE; +} + +static gboolean +_ostree_secure_execution_enable (OstreeBootloaderZipl *self, + int bootversion, + GPtrArray *keys, + GCancellable *cancellable, + GError **error) +{ + g_autofree gchar* vmlinuz = NULL; + g_autofree gchar* initramfs = NULL; + g_autofree gchar* options = NULL; + + gboolean rc = + _ostree_secure_execution_get_bls_config (self, bootversion, &vmlinuz, &initramfs, &options, cancellable, error) && + _ostree_secure_execution_generate_sdboot (vmlinuz, initramfs, options, keys, error) && + _ostree_secure_execution_call_zipl (error); + + return rc; +} + + static gboolean _ostree_bootloader_zipl_post_bls_sync (OstreeBootloader *bootloader, + int bootversion, GCancellable *cancellable, GError **error) { @@ -97,6 +257,14 @@ _ostree_bootloader_zipl_post_bls_sync (OstreeBootloader *bootloader, if (errno == ENOENT) return TRUE; + /* Try with Secure Execution */ + g_autoptr(GPtrArray) keys = NULL; + if (!_ostree_secure_execution_get_keys (&keys, cancellable, error)) + return FALSE; + if (keys && keys->len) + return _ostree_secure_execution_enable (self, bootversion, keys, cancellable, error); + + /* Fallback to non-SE setup */ const char *const zipl_argv[] = {"zipl", NULL}; int estatus; if (!g_spawn_sync (NULL, (char**)zipl_argv, NULL, G_SPAWN_SEARCH_PATH, diff --git a/src/libostree/ostree-bootloader-zipl.h b/src/libostree/ostree-bootloader-zipl.h index 3584feb2..e3f0b2b3 100644 --- a/src/libostree/ostree-bootloader-zipl.h +++ b/src/libostree/ostree-bootloader-zipl.h @@ -30,5 +30,4 @@ typedef struct _OstreeBootloaderZipl OstreeBootloaderZipl; GType _ostree_bootloader_zipl_get_type (void) G_GNUC_CONST; OstreeBootloaderZipl * _ostree_bootloader_zipl_new (OstreeSysroot *sysroot); - G_END_DECLS diff --git a/src/libostree/ostree-bootloader.c b/src/libostree/ostree-bootloader.c index f221b608..785fd233 100644 --- a/src/libostree/ostree-bootloader.c +++ b/src/libostree/ostree-bootloader.c @@ -65,13 +65,14 @@ _ostree_bootloader_write_config (OstreeBootloader *self, gboolean _ostree_bootloader_post_bls_sync (OstreeBootloader *self, + int bootversion, GCancellable *cancellable, GError **error) { g_return_val_if_fail (OSTREE_IS_BOOTLOADER (self), FALSE); if (OSTREE_BOOTLOADER_GET_IFACE (self)->post_bls_sync) - return OSTREE_BOOTLOADER_GET_IFACE (self)->post_bls_sync (self, cancellable, error); + return OSTREE_BOOTLOADER_GET_IFACE (self)->post_bls_sync (self, bootversion, cancellable, error); return TRUE; } diff --git a/src/libostree/ostree-bootloader.h b/src/libostree/ostree-bootloader.h index 6e0f6f88..ca1b453e 100644 --- a/src/libostree/ostree-bootloader.h +++ b/src/libostree/ostree-bootloader.h @@ -46,6 +46,7 @@ struct _OstreeBootloaderInterface GCancellable *cancellable, GError **error); gboolean (* post_bls_sync) (OstreeBootloader *self, + int bootversion, GCancellable *cancellable, GError **error); gboolean (* is_atomic) (OstreeBootloader *self); @@ -68,6 +69,7 @@ gboolean _ostree_bootloader_write_config (OstreeBootloader *self, GError **error); gboolean _ostree_bootloader_post_bls_sync (OstreeBootloader *self, + int bootversion, GCancellable *cancellable, GError **error); diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index c4ae86d5..c1e19db3 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -2097,7 +2097,7 @@ swap_bootloader (OstreeSysroot *sysroot, **/ if (bootloader) { - if (!_ostree_bootloader_post_bls_sync (bootloader, cancellable, error)) + if (!_ostree_bootloader_post_bls_sync (bootloader, new_bootversion, cancellable, error)) return FALSE; } From d1ab18f327634fe9175848237eac8a1cde9361ac Mon Sep 17 00:00:00 2001 From: Nikita Dubrovskii Date: Mon, 17 Jan 2022 15:59:54 +0100 Subject: [PATCH 28/52] s390x: add LUKS keyfile to 'sd-boot' This allows to use Secure Execution with LUKS encrypted boot disk, key and cryptab are stored only in 'sd-boot' encrypted image. Signed-off-by: Nikita Dubrovskii --- Makefile-libostree.am | 9 +++-- src/libostree/ostree-bootloader-zipl.c | 50 ++++++++++++++++++++++++-- src/libostree/s390x-se-luks-gencpio | 22 ++++++++++++ 3 files changed, 77 insertions(+), 4 deletions(-) create mode 100755 src/libostree/s390x-se-luks-gencpio diff --git a/Makefile-libostree.am b/Makefile-libostree.am index c9511fe3..02ae9c6a 100644 --- a/Makefile-libostree.am +++ b/Makefile-libostree.am @@ -184,7 +184,8 @@ EXTRA_DIST += \ libostree_1_la_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/bsdiff -I$(srcdir)/libglnx -I$(srcdir)/src/libotutil -I$(srcdir)/src/libostree -I$(builddir)/src/libostree \ $(OT_INTERNAL_GIO_UNIX_CFLAGS) $(OT_INTERNAL_GPGME_CFLAGS) $(OT_DEP_LZMA_CFLAGS) $(OT_DEP_ZLIB_CFLAGS) $(OT_DEP_CRYPTO_CFLAGS) \ - -fvisibility=hidden '-D_OSTREE_PUBLIC=__attribute__((visibility("default"))) extern' + -fvisibility=hidden '-D_OSTREE_PUBLIC=__attribute__((visibility("default"))) extern' \ + -DPKGLIBEXECDIR=\"$(pkglibexecdir)\" libostree_1_la_LDFLAGS = -version-number 1:0:0 -Bsymbolic-functions $(addprefix $(wl_versionscript_arg),$(symbol_files)) libostree_1_la_LIBADD = libotutil.la libglnx.la libbsdiff.la $(OT_INTERNAL_GIO_UNIX_LIBS) $(OT_INTERNAL_GPGME_LIBS) \ $(OT_DEP_LZMA_LIBS) $(OT_DEP_ZLIB_LIBS) $(OT_DEP_CRYPTO_LIBS) @@ -292,8 +293,12 @@ EXTRA_DIST += src/libostree/README-gpg src/libostree/bupsplit.h \ src/libostree/ostree-enumtypes.c.template \ src/libostree/ostree-deployment-private.h \ src/libostree/ostree-repo-deprecated.h \ - src/libostree/ostree-version.h + src/libostree/ostree-version.h \ + src/libostree/s390x-se-luks-gencpio install-mkdir-remotes-d-hook: mkdir -p $(DESTDIR)$(sysconfdir)/ostree/remotes.d INSTALL_DATA_HOOKS += install-mkdir-remotes-d-hook + +# Secure Execution: script for creating new initramdisk with LUKS key and config +pkglibexec_SCRIPTS += src/libostree/s390x-se-luks-gencpio diff --git a/src/libostree/ostree-bootloader-zipl.c b/src/libostree/ostree-bootloader-zipl.c index 7358671b..14c2762e 100644 --- a/src/libostree/ostree-bootloader-zipl.c +++ b/src/libostree/ostree-bootloader-zipl.c @@ -27,6 +27,9 @@ #define SECURE_EXECUTION_BOOT_IMAGE "/boot/sd-boot" #define SECURE_EXECUTION_HOSTKEY_PATH "/etc/se-hostkeys/" #define SECURE_EXECUTION_HOSTKEY_PREFIX "ibm-z-hostkey" +#define SECURE_EXECUTION_LUKS_ROOT_KEY "/etc/luks/root" +#define SECURE_EXECUTION_LUKS_CONFIG "/etc/crypttab" +#define SECURE_EXECUTION_RAMDISK_TOOL PKGLIBEXECDIR "/s390x-se-luks-gencpio" /* This is specific to zipl today, but in the future we could also * use it for the grub2-mkconfig case. @@ -147,6 +150,37 @@ _ostree_secure_execution_get_bls_config (OstreeBootloaderZipl *self, return TRUE; } +static gboolean +_ostree_secure_execution_luks_key_exists (void) +{ + return (access(SECURE_EXECUTION_LUKS_ROOT_KEY, F_OK) == 0 && + access(SECURE_EXECUTION_LUKS_CONFIG, F_OK) == 0); +} + +static gboolean +_ostree_secure_execution_enable_luks(const gchar *oldramfs, + const gchar *newramfs, + GError **error) +{ + const char *const argv[] = {SECURE_EXECUTION_RAMDISK_TOOL, oldramfs, newramfs, NULL}; + g_autofree gchar *out = NULL; + g_autofree gchar *err = NULL; + int status = 0; + if (!g_spawn_sync (NULL, (char**)argv, NULL, G_SPAWN_SEARCH_PATH, + NULL, NULL, &out, &err, &status, error)) + return glnx_prefix_error(error, "s390x SE: spawning %s", SECURE_EXECUTION_RAMDISK_TOOL); + + if (!g_spawn_check_exit_status (status, error)) + { + g_printerr("s390x SE: `%s` stdout: %s\n", SECURE_EXECUTION_RAMDISK_TOOL, out); + g_printerr("s390x SE: `%s` stderr: %s\n", SECURE_EXECUTION_RAMDISK_TOOL, err); + return glnx_prefix_error(error, "s390x SE: `%s` failed", SECURE_EXECUTION_RAMDISK_TOOL); + } + + sd_journal_print(LOG_INFO, "s390x SE: luks key added to initrd"); + return TRUE; +} + static gboolean _ostree_secure_execution_generate_sdboot (gchar *vmlinuz, gchar *initramfs, @@ -169,12 +203,24 @@ _ostree_secure_execution_generate_sdboot (gchar *vmlinuz, return glnx_throw_errno_prefix (error, "s390x SE: writting cmdline file"); g_autofree gchar *cmdline_filename = g_strdup_printf ("/proc/%d/fd/%d", self, cmdline.fd); + // Copy initramfs to temp file and embed LUKS key and config into it + g_auto(GLnxTmpfile) ramdisk = { 0, }; + g_autofree gchar *ramdisk_filename = NULL; + if (_ostree_secure_execution_luks_key_exists ()) + { + if (!glnx_open_anonymous_tmpfile (O_RDWR | O_CLOEXEC, &ramdisk, error)) + return glnx_prefix_error(error, "s390x SE: creating new ramdisk"); + ramdisk_filename = g_strdup_printf ("/proc/%d/fd/%d", self, ramdisk.fd); + if (!_ostree_secure_execution_enable_luks (initramfs, ramdisk_filename, error)) + return FALSE; + } + g_autoptr(GPtrArray) argv = g_ptr_array_new (); g_ptr_array_add (argv, "genprotimg"); g_ptr_array_add (argv, "-i"); g_ptr_array_add (argv, vmlinuz); g_ptr_array_add (argv, "-r"); - g_ptr_array_add (argv, initramfs); + g_ptr_array_add (argv, (ramdisk_filename == NULL) ? initramfs: ramdisk_filename); g_ptr_array_add (argv, "-p"); g_ptr_array_add (argv, cmdline_filename); for (guint i = 0; i < keys->len; ++i) @@ -191,7 +237,7 @@ _ostree_secure_execution_generate_sdboot (gchar *vmlinuz, gint status = 0; if (!g_spawn_sync (NULL, (char**)argv->pdata, NULL, G_SPAWN_SEARCH_PATH, - NULL, NULL, NULL, NULL, &status, error)) + NULL, NULL, NULL, NULL, &status, error)) return glnx_prefix_error(error, "s390x SE: spawning genprotimg"); if (!g_spawn_check_exit_status (status, error)) diff --git a/src/libostree/s390x-se-luks-gencpio b/src/libostree/s390x-se-luks-gencpio new file mode 100755 index 00000000..f0ad24eb --- /dev/null +++ b/src/libostree/s390x-se-luks-gencpio @@ -0,0 +1,22 @@ + #!/usr/bin/bash + # This script creates new initramdisk with LUKS config within +set -euo pipefail + +old_initrd=$1 +new_initrd=$2 + +# Unpacking existing initramdisk +workdir=$(mktemp -d -p /tmp se-initramfs-XXXXXX) +cd ${workdir} +gzip -cd ${old_initrd} | cpio -imd --quiet + +# Adding LUKS root key and crypttab config +mkdir -p etc/luks +cp -f /etc/luks/root etc/luks/ +cp -f /etc/crypttab etc/ + +# Creating new initramdisk image +find . | cpio --quiet -H newc -o | gzip -9 -n >> ${new_initrd} + +# Cleanup +rm -rf ${workdir} From ea5f7b0f38ee0b7a56cd5e3bfe2025207dd44a12 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 11 Feb 2022 14:09:49 -0500 Subject: [PATCH 29/52] core: Mark `ostree_create_directory_metadata` as `(not nullable)` So I can drop an unnecessary use of `unwrap()` in Rust. --- src/libostree/ostree-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libostree/ostree-core.c b/src/libostree/ostree-core.c index 038606e9..0671ed35 100644 --- a/src/libostree/ostree-core.c +++ b/src/libostree/ostree-core.c @@ -1141,7 +1141,7 @@ _ostree_compare_object_checksum (OstreeObjectType objtype, * @dir_info: a #GFileInfo containing directory information * @xattrs: (allow-none): Optional extended attributes * - * Returns: (transfer full): A new #GVariant containing %OSTREE_OBJECT_TYPE_DIR_META + * Returns: (transfer full) (not nullable): A new #GVariant containing %OSTREE_OBJECT_TYPE_DIR_META */ GVariant * ostree_create_directory_metadata (GFileInfo *dir_info, From 5d08032aec50dadb9a4646c96885eaefa4206049 Mon Sep 17 00:00:00 2001 From: Christian Hergert Date: Fri, 11 Feb 2022 14:31:12 -0800 Subject: [PATCH 30/52] lib/util: add syslog.h for ot_journal_print() If we aren't including sd-journal, we may need this too. --- src/libotutil/otutil.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/libotutil/otutil.h b/src/libotutil/otutil.h index 1b543062..4279bc15 100644 --- a/src/libotutil/otutil.h +++ b/src/libotutil/otutil.h @@ -23,8 +23,13 @@ #include #include /* Yeah...let's just do that here. */ +#include #include +#ifdef HAVE_LIBSYSTEMD +#include +#endif + /* https://bugzilla.gnome.org/show_bug.cgi?id=766370 */ #if !GLIB_CHECK_VERSION(2, 49, 3) #define OT_VARIANT_BUILDER_INITIALIZER {{0,}} From 6419c323348b15aa8e4e7b417b7406c4f76e1e44 Mon Sep 17 00:00:00 2001 From: Christian Hergert Date: Fri, 11 Feb 2022 15:12:15 -0800 Subject: [PATCH 31/52] lib/bootloader: use ot_journal_print() instead of sd-journal This needs to use the helper so that USE_LIBSYSTEMD still works as expected. --- src/libostree/ostree-bootloader-zipl.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/libostree/ostree-bootloader-zipl.c b/src/libostree/ostree-bootloader-zipl.c index 14c2762e..02c10826 100644 --- a/src/libostree/ostree-bootloader-zipl.c +++ b/src/libostree/ostree-bootloader-zipl.c @@ -21,7 +21,6 @@ #include "ostree-bootloader-zipl.h" #include "ostree-deployment-private.h" #include "otutil.h" -#include #include #define SECURE_EXECUTION_BOOT_IMAGE "/boot/sd-boot" @@ -177,7 +176,7 @@ _ostree_secure_execution_enable_luks(const gchar *oldramfs, return glnx_prefix_error(error, "s390x SE: `%s` failed", SECURE_EXECUTION_RAMDISK_TOOL); } - sd_journal_print(LOG_INFO, "s390x SE: luks key added to initrd"); + ot_journal_print(LOG_INFO, "s390x SE: luks key added to initrd"); return TRUE; } @@ -189,9 +188,9 @@ _ostree_secure_execution_generate_sdboot (gchar *vmlinuz, GError **error) { g_assert (vmlinuz && initramfs && options && keys && keys->len); - sd_journal_print(LOG_INFO, "s390x SE: kernel: %s", vmlinuz); - sd_journal_print(LOG_INFO, "s390x SE: initrd: %s", initramfs); - sd_journal_print(LOG_INFO, "s390x SE: kargs: %s", options); + ot_journal_print(LOG_INFO, "s390x SE: kernel: %s", vmlinuz); + ot_journal_print(LOG_INFO, "s390x SE: initrd: %s", initramfs); + ot_journal_print(LOG_INFO, "s390x SE: kargs: %s", options); pid_t self = getpid(); @@ -228,7 +227,7 @@ _ostree_secure_execution_generate_sdboot (gchar *vmlinuz, gchar *key = g_ptr_array_index (keys, i); g_ptr_array_add (argv, "-k"); g_ptr_array_add (argv, key); - sd_journal_print(LOG_INFO, "s390x SE: key[%d]: %s", i + 1, key); + ot_journal_print(LOG_INFO, "s390x SE: key[%d]: %s", i + 1, key); } g_ptr_array_add (argv, "--no-verify"); g_ptr_array_add (argv, "-o"); @@ -243,7 +242,7 @@ _ostree_secure_execution_generate_sdboot (gchar *vmlinuz, if (!g_spawn_check_exit_status (status, error)) return glnx_prefix_error(error, "s390x SE: `genprotimg` failed"); - sd_journal_print(LOG_INFO, "s390x SE: `%s` generated", SECURE_EXECUTION_BOOT_IMAGE); + ot_journal_print(LOG_INFO, "s390x SE: `%s` generated", SECURE_EXECUTION_BOOT_IMAGE); return TRUE; } @@ -259,7 +258,7 @@ _ostree_secure_execution_call_zipl (GError **error) if (!g_spawn_check_exit_status (status, error)) return glnx_prefix_error(error, "s390x SE: `zipl` failed"); - sd_journal_print(LOG_INFO, "s390x SE: `sd-boot` zipled"); + ot_journal_print(LOG_INFO, "s390x SE: `sd-boot` zipled"); return TRUE; } From 99e01c8b7b4cf55b992daf88e4e6e9fbc91e1ce4 Mon Sep 17 00:00:00 2001 From: Marco Melorio Date: Sat, 12 Feb 2022 18:48:35 +0100 Subject: [PATCH 32/52] man: Fix typo in ostree-admin-switch --- man/ostree-admin-switch.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/man/ostree-admin-switch.xml b/man/ostree-admin-switch.xml index 720cabdd..6795c49b 100644 --- a/man/ostree-admin-switch.xml +++ b/man/ostree-admin-switch.xml @@ -57,7 +57,7 @@ License along with this library. If not, see . Description - Choose a different REF forom the current remote to track. This is the ref that will be "tracked" and upgraded with ostree admin upgrade. Like an upgrade, the operating system state will be preserved. + Choose a different REF from the current remote to track. This is the ref that will be "tracked" and upgraded with ostree admin upgrade. Like an upgrade, the operating system state will be preserved. From 6264c6decab4edcc9a45722f4998dd0c9a0efe50 Mon Sep 17 00:00:00 2001 From: Marco Melorio Date: Sat, 12 Feb 2022 19:00:28 +0100 Subject: [PATCH 33/52] man: Fix typo in ostree-find-remotes --- man/ostree-find-remotes.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/man/ostree-find-remotes.xml b/man/ostree-find-remotes.xml index 95664a89..5c5b2412 100644 --- a/man/ostree-find-remotes.xml +++ b/man/ostree-find-remotes.xml @@ -57,7 +57,7 @@ License along with this library. If not, see . Description - OSTree has the ability do pulls not just from configured remote + OSTree has the ability to pull not just from the configured remote servers but also from peer computers on the LAN and from mounted filesystems such as USB drives. This functionality requires the use of collection IDs and GPG verification. From 48104f76ee848e9b8f07b9209906d20a51c66628 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 15 Feb 2022 14:17:20 -0500 Subject: [PATCH 34/52] lib/tar: Add some error prefixing We're trying to debug a problem with a tar stream with hardlinks, and I think this will be helpful. --- src/libostree/ostree-repo-libarchive.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/libostree/ostree-repo-libarchive.c b/src/libostree/ostree-repo-libarchive.c index aacf7c64..679aa44d 100644 --- a/src/libostree/ostree-repo-libarchive.c +++ b/src/libostree/ostree-repo-libarchive.c @@ -364,6 +364,7 @@ aic_ensure_parent_dir (OstreeRepoArchiveImportContext *ctx, GCancellable *cancellable, GError **error) { + GLNX_AUTO_PREFIX_ERROR ("ostree-tar: Failed to create parent", error); /* Who should own the parent dir? Since it's not in the archive, it's up to * us. Here, we use the heuristic of simply creating it as the same user as * the owner of the archive entry for which we're creating the dir. This is OK @@ -452,6 +453,7 @@ aic_get_xattrs (OstreeRepoArchiveImportContext *ctx, GCancellable *cancellable, GError **error) { + GLNX_AUTO_PREFIX_ERROR ("ostree-tar: Failed to get xattrs", error); g_autofree char *abspath = g_build_filename ("/", path, NULL); g_autoptr(GVariant) xattrs = NULL; const char *cb_path = abspath; @@ -526,6 +528,7 @@ aic_handle_dir (OstreeRepoArchiveImportContext *ctx, GCancellable *cancellable, GError **error) { + GLNX_AUTO_PREFIX_ERROR ("ostree-tar: Failed to handle directory", error); const char *name = glnx_basename (path); g_autoptr(GVariant) xattrs = NULL; @@ -575,6 +578,7 @@ aic_import_file (OstreeRepoArchiveImportContext *ctx, GCancellable *cancellable, GError **error) { + GLNX_AUTO_PREFIX_ERROR ("ostree-tar: Failed to import file", error); const char *name = glnx_basename (path); g_autoptr(GVariant) xattrs = NULL; g_autofree char *csum = NULL; @@ -631,6 +635,7 @@ aic_handle_file (OstreeRepoArchiveImportContext *ctx, GCancellable *cancellable, GError **error) { + GLNX_AUTO_PREFIX_ERROR ("ostree-tar: Failed to handle file", error); /* The wonderful world of hardlinks and archives. We have to be very careful * here. Do not assume that if a file is a hardlink, it will have size 0 (e.g. * cpio). Do not assume that if a file will have hardlinks to it, it will have @@ -784,10 +789,10 @@ aic_import_deferred_hardlinks_for (OstreeRepoArchiveImportContext *ctx, /* rewrite the target so it points to the csum of the payload hardlink */ if (payload) if (!aic_import_from_hardlink (ctx, target, payload->data, error)) - return FALSE; + return glnx_prefix_error (error, "Failed importing hardlink"); if (!aic_lookup_file_csum (ctx, target, &csum, error)) - return FALSE; + return glnx_prefix_error (error, "Failed to find object"); /* import all the hardlinks */ for (GSList *hl = hardlinks; hl != NULL; hl = g_slist_next (hl)) @@ -799,7 +804,7 @@ aic_import_deferred_hardlinks_for (OstreeRepoArchiveImportContext *ctx, continue; /* small optimization; no need to redo this one */ if (!ostree_mutable_tree_replace_file (df->parent, name, csum, error)) - return FALSE; + return glnx_prefix_error (error, "Failed to replace file"); } return TRUE; @@ -813,7 +818,7 @@ aic_import_deferred_hardlinks (OstreeRepoArchiveImportContext *ctx, GLNX_HASH_TABLE_FOREACH_KV (ctx->deferred_hardlinks, const char*, target, GSList*, links) { if (!aic_import_deferred_hardlinks_for (ctx, target, links, error)) - return FALSE; + return glnx_prefix_error (error, "ostree-tar: Processing deferred hardlink %s", target); } return TRUE; } From b91f29ca39eba1a3ec0ff59d8a07da827e01ba4a Mon Sep 17 00:00:00 2001 From: Dan Nicholson Date: Thu, 17 Feb 2022 10:13:03 -0700 Subject: [PATCH 35/52] .lgtm.yml: Fix gpgme dependency Since Ubuntu 18.04, libgpgme-dev is the real package and libgpgme11-dev is a virtual package provided by it. Apparently LGTM running on Ubuntu 20.04 no longer resolves the virtual package: ``` WARNING: Package 'libgpgme11-dev' requested by configuration file was not found ``` That ends up causing the build to fail: ``` configure: error: Need GPGME_PTHREAD version 1.1.8 or later ``` --- .lgtm.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.lgtm.yml b/.lgtm.yml index 8eec4480..7df3bb0d 100644 --- a/.lgtm.yml +++ b/.lgtm.yml @@ -29,7 +29,7 @@ extraction: - "libfuse-dev" - "libgirepository1.0-dev" - "libglib2.0-dev" - - "libgpgme11-dev" + - "libgpgme-dev" - "liblzma-dev" - "libmount-dev" - "libselinux1-dev" From 71ae1c35e3ad8479f4cec86fa13f5fb42702139d Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 17 Feb 2022 14:22:27 -0500 Subject: [PATCH 36/52] build-sys: Drop `-Werror=aggregate-return` This is failing for me as of recently but only when I build without optimization. I don't think we've ever written any code that returned a large structure by value. Let's just drop this one. --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 763ffb04..69e03982 100644 --- a/configure.ac +++ b/configure.ac @@ -49,7 +49,7 @@ CC_CHECK_FLAGS_APPEND([WARN_CFLAGS], [CFLAGS], [\ -Werror=undef \ -Werror=incompatible-pointer-types \ -Werror=misleading-indentation \ - -Werror=missing-include-dirs -Werror=aggregate-return \ + -Werror=missing-include-dirs \ -Wstrict-aliasing=2 \ -Werror=unused-result \ ])]) From 929f62c59eff22c546208375193d97a7926e2294 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 18 Feb 2022 10:22:26 -0500 Subject: [PATCH 37/52] mtree: Use declare-and-initialize style Prep for further work. --- src/libostree/ostree-mutable-tree.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/libostree/ostree-mutable-tree.c b/src/libostree/ostree-mutable-tree.c index 91023ca9..e0e73f37 100644 --- a/src/libostree/ostree-mutable-tree.c +++ b/src/libostree/ostree-mutable-tree.c @@ -463,13 +463,11 @@ ostree_mutable_tree_ensure_parent_dirs (OstreeMutableTree *self, OstreeMutableTree *subdir = self; /* nofree */ for (guint i = 0; i+1 < split_path->len; i++) { - OstreeMutableTree *next; const char *name = split_path->pdata[i]; - if (g_hash_table_lookup (subdir->files, name)) return glnx_throw (error, "Can't replace file with directory: %s", name); - next = g_hash_table_lookup (subdir->subdirs, name); + OstreeMutableTree *next = g_hash_table_lookup (subdir->subdirs, name); if (!next) { invalidate_contents_checksum (subdir); @@ -580,11 +578,9 @@ ostree_mutable_tree_walk (OstreeMutableTree *self, } else { - OstreeMutableTree *subdir; if (!_ostree_mutable_tree_make_whole (self, NULL, error)) return FALSE; - - subdir = g_hash_table_lookup (self->subdirs, split_path->pdata[start]); + OstreeMutableTree *subdir = g_hash_table_lookup (self->subdirs, split_path->pdata[start]); if (!subdir) return set_error_noent (error, (char*)split_path->pdata[start]); From f36a940ed04eb3fd2f68565ebbe46e17a9d00f53 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 18 Feb 2022 10:24:13 -0500 Subject: [PATCH 38/52] mtree: Load traversed subdirs when creating parents I'm working on enhancing the ostree-rs-ext test suite and I hit a bug where walking a mtree and creating a parent would fail to load lazy intermediate directories, e.g.: / -> usr -> bin If we walked we'd load `/` but keep `usr` lazy, and then invalidation would crash because it wasn't loaded. If we're going to mutate a subdir, we need to have all the parents loaded. I know this is missing tests, but...it's a bit tedious to do with the existing C tests. Eventually soon we'll execute on merging all 3 repos, and better share test suites. --- src/libostree/ostree-mutable-tree.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libostree/ostree-mutable-tree.c b/src/libostree/ostree-mutable-tree.c index e0e73f37..4b3460e3 100644 --- a/src/libostree/ostree-mutable-tree.c +++ b/src/libostree/ostree-mutable-tree.c @@ -477,6 +477,9 @@ ostree_mutable_tree_ensure_parent_dirs (OstreeMutableTree *self, } subdir = next; + g_assert (subdir); + if (!_ostree_mutable_tree_make_whole (subdir, NULL, error)) + return FALSE; } if (out_parent) From 5d3b1ca37a508e9f80702b7ef7383fe95253ec6a Mon Sep 17 00:00:00 2001 From: Phaedrus Leeds Date: Sat, 19 Feb 2022 07:55:02 -0600 Subject: [PATCH 39/52] Fix marking static delta commits as partial This patch makes it so that we mark the .commit file from a static delta as partial before writing the commit to the staging directory. This exactly mirrors what we do in meta_fetch_on_complete() when writing the commit on that codepath, which should lend some credibility to the correctness of this patch. I have checked that this fixes an issue Flatpak users have been encountering (https://github.com/flatpak/flatpak/issues/3479) which results in error messages like "error: Failed to install org.freedesktop.Sdk.Extension.texlive: Failed to read commit c7958d966cfa8b80a42877d1d6124831d7807f93c89461a2a586956aa28d438a: No such metadata object 8bdaa943b957f3cf14d19301c59c7eec076e57389e0fbb3ef5d30082e47a178f.dirtree" Here's the sequence of events that lead to the error: 1. An install operation is started that fetches static deltas. 2. The fetch is interrupted for some reason such as network connectivity dropping. 3. The .commit and .commitmeta files for the commit being pulled are left in the staging dir, e.g. "~/.local/share/flatpak/repo/tmp/staging-dfe862b2-13fc-49a2-ac92-5a59cc0d8e18-RURckd" 4. There is no `.commitpartial` file for the commit in "~/.local/share/flatpak/repo/state/" 5. The next time the user attempts the install, libostree reuses the existing staging dir, pulls the commit and commitmeta objects into the repo from the staging dir on the assumption that it's a complete commit. 6. Flatpak then tries to deploy the commit but fails in ostree_repo_read_commit() in flatpak_dir_deploy(), leading to the error message "Failed to read commit ..." 7. This happens again any subsequent time the user attempts the install, until the incomplete commit is removed with "flatpak repair --user". I will try to also add a workaround in Flatpak so this is fixed even when Flatpak links against affected versions of libostree. --- src/libostree/ostree-repo-pull.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 65b56789..e8918cf6 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -2244,6 +2244,9 @@ process_one_static_delta (OtPullData *pull_data, ref, cancellable, error)) return FALSE; + if (!ostree_repo_mark_commit_partial (pull_data->repo, to_revision, TRUE, error)) + return FALSE; + if (detached_data && !ostree_repo_write_commit_detached_metadata (pull_data->repo, to_revision, detached_data, From c213dd3a84bf901656297bbecc341a70cf0706cb Mon Sep 17 00:00:00 2001 From: Phaedrus Leeds Date: Sat, 19 Feb 2022 14:46:02 -0600 Subject: [PATCH 40/52] lib/repo-refs: Remove misleading newline --- src/libostree/ostree-repo-refs.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libostree/ostree-repo-refs.c b/src/libostree/ostree-repo-refs.c index 8d010729..86bd27c5 100644 --- a/src/libostree/ostree-repo-refs.c +++ b/src/libostree/ostree-repo-refs.c @@ -396,7 +396,6 @@ _ostree_repo_resolve_rev_internal (OstreeRepo *self, { ret_rev = g_strdup (refspec); } - else if (!ostree_repo_resolve_partial_checksum (self, refspec, &ret_rev, error)) return FALSE; From f73434f1b38bd8f3d9b23f6465c88feaf97609f3 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 22 Feb 2022 15:51:55 +0000 Subject: [PATCH 41/52] build(deps): bump libglnx from `803adaf` to `88da8dd` Bumps libglnx from `803adaf` to `88da8dd`. --- updated-dependencies: - dependency-name: libglnx dependency-type: direct:production ... Signed-off-by: dependabot[bot] --- libglnx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libglnx b/libglnx index 803adaf4..88da8ddd 160000 --- a/libglnx +++ b/libglnx @@ -1 +1 @@ -Subproject commit 803adaf4883f237d0a3658fa16e72eb1c41ac828 +Subproject commit 88da8ddd60bf0401f37c12b7bdcd20f8a9e955b5 From 725d50a3b53de47deef53a7c9120d304cb9a625a Mon Sep 17 00:00:00 2001 From: Saqib Ali Date: Mon, 7 Feb 2022 10:53:08 -0500 Subject: [PATCH 42/52] src/ostree: Add --commit-only option to ostree prune Recently we have noticed exceedingly long execution times for multiple invocations of ostree prune. This is a result of calculating full reachability on each invocation. The --commit-only flag provides an alternative strategy. It will only traverse and delete commit objects to avoid the more expensive reachability calculations. This allows us to chain multiple --commit-only commands cheaply, and then follow with a more expensive ostree prune invocation at the end to clean up orphaned meta and content objects. --- Makefile-libostree.am | 6 +- apidoc/ostree-sections.txt | 1 + src/libostree/libostree-devel.sym | 5 ++ src/libostree/ostree-repo-prune.c | 109 +++++++++++++++++---------- src/libostree/ostree-repo-traverse.c | 74 +++++++++++++----- src/libostree/ostree-repo.h | 27 ++++++- src/ostree/ot-builtin-prune.c | 28 +++++-- 7 files changed, 179 insertions(+), 71 deletions(-) diff --git a/Makefile-libostree.am b/Makefile-libostree.am index 02ae9c6a..9b48a308 100644 --- a/Makefile-libostree.am +++ b/Makefile-libostree.am @@ -171,9 +171,9 @@ endif # USE_GPGME symbol_files = $(top_srcdir)/src/libostree/libostree-released.sym # Uncomment this include when adding new development symbols. -# if BUILDOPT_IS_DEVEL_BUILD -# symbol_files += $(top_srcdir)/src/libostree/libostree-devel.sym -# endif +if BUILDOPT_IS_DEVEL_BUILD +symbol_files += $(top_srcdir)/src/libostree/libostree-devel.sym +endif # http://blog.jgc.org/2007/06/escaping-comma-and-space-in-gnu-make.html wl_versionscript_arg = -Wl,--version-script= diff --git a/apidoc/ostree-sections.txt b/apidoc/ostree-sections.txt index aa74c839..577ee808 100644 --- a/apidoc/ostree-sections.txt +++ b/apidoc/ostree-sections.txt @@ -451,6 +451,7 @@ ostree_repo_traverse_parents_get_commits ostree_repo_traverse_commit ostree_repo_traverse_commit_union ostree_repo_traverse_commit_union_with_parents +ostree_repo_traverse_commit_with_flags ostree_repo_commit_traverse_iter_cleanup ostree_repo_commit_traverse_iter_clear ostree_repo_commit_traverse_iter_get_dir diff --git a/src/libostree/libostree-devel.sym b/src/libostree/libostree-devel.sym index 9168db73..74f9b9a9 100644 --- a/src/libostree/libostree-devel.sym +++ b/src/libostree/libostree-devel.sym @@ -20,6 +20,11 @@ - uncomment the include in Makefile-libostree.am */ +LIBOSTREE_2022.2 { +global: + ostree_repo_traverse_commit_with_flags; +} LIBOSTREE_2021.5; + /* Stub section for the stable release *after* this development one; don't * edit this other than to update the year. This is just a copy/paste * source. Replace $LASTSTABLE with the last stable version, and $NEWVERSION diff --git a/src/libostree/ostree-repo-prune.c b/src/libostree/ostree-repo-prune.c index 175765fd..0c702dc9 100644 --- a/src/libostree/ostree-repo-prune.c +++ b/src/libostree/ostree-repo-prune.c @@ -48,6 +48,10 @@ maybe_prune_loose_object (OtPruneData *data, OstreeObjectType objtype; ostree_object_name_deserialize (key, &checksum, &objtype); + /* Return if we only want to delete commits and this object is not a commit object. */ + gboolean commit_only = flags & OSTREE_REPO_PRUNE_FLAGS_COMMIT_ONLY; + if (commit_only && (objtype != OSTREE_OBJECT_TYPE_COMMIT)) + goto exit; if (g_hash_table_lookup_extended (data->reachable, key, NULL, NULL)) reachable = TRUE; @@ -125,7 +129,11 @@ maybe_prune_loose_object (OtPruneData *data, else data->n_reachable_content++; } - + if (commit_only && (objtype != OSTREE_OBJECT_TYPE_COMMIT)) + { + g_debug ("Keeping object (not commit) %s.%s", checksum, + ostree_object_type_to_string (objtype)); + } return TRUE; } @@ -299,6 +307,52 @@ repo_prune_internal (OstreeRepo *self, return TRUE; } +static gboolean +traverse_reachable_internal (OstreeRepo *self, + OstreeRepoCommitTraverseFlags flags, + guint depth, + GHashTable *reachable, + GCancellable *cancellable, + GError **error) +{ + g_autoptr(OstreeRepoAutoLock) lock = + ostree_repo_auto_lock_push (self, OSTREE_REPO_LOCK_SHARED, cancellable, error); + if (!lock) + return FALSE; + + /* Ignoring collections. */ + g_autoptr(GHashTable) all_refs = NULL; /* (element-type utf8 utf8) */ + + if (!ostree_repo_list_refs (self, NULL, &all_refs, + cancellable, error)) + return FALSE; + + GLNX_HASH_TABLE_FOREACH_V (all_refs, const char*, checksum) + { + g_debug ("Finding objects to keep for commit %s", checksum); + if (!ostree_repo_traverse_commit_with_flags (self, flags, checksum, depth, reachable, + NULL, cancellable, error)) + return FALSE; + } + + /* Using collections. */ + g_autoptr(GHashTable) all_collection_refs = NULL; /* (element-type OstreeChecksumRef utf8) */ + + if (!ostree_repo_list_collection_refs (self, NULL, &all_collection_refs, + OSTREE_REPO_LIST_REFS_EXT_EXCLUDE_REMOTES, cancellable, error)) + return FALSE; + + GLNX_HASH_TABLE_FOREACH_V (all_collection_refs, const char*, checksum) + { + g_debug ("Finding objects to keep for commit %s", checksum); + if (!ostree_repo_traverse_commit_with_flags (self, flags, checksum, depth, reachable, + NULL, cancellable, error)) + return FALSE; + } + + return TRUE; +} + /** * ostree_repo_traverse_reachable_refs: * @self: Repo @@ -319,42 +373,10 @@ ostree_repo_traverse_reachable_refs (OstreeRepo *self, GCancellable *cancellable, GError **error) { - g_autoptr(OstreeRepoAutoLock) lock = - ostree_repo_auto_lock_push (self, OSTREE_REPO_LOCK_SHARED, cancellable, error); - if (!lock) - return FALSE; - - /* Ignoring collections. */ - g_autoptr(GHashTable) all_refs = NULL; /* (element-type utf8 utf8) */ - - if (!ostree_repo_list_refs (self, NULL, &all_refs, - cancellable, error)) - return FALSE; - - GLNX_HASH_TABLE_FOREACH_V (all_refs, const char*, checksum) - { - g_debug ("Finding objects to keep for commit %s", checksum); - if (!ostree_repo_traverse_commit_union (self, checksum, depth, reachable, - cancellable, error)) - return FALSE; - } - - /* Using collections. */ - g_autoptr(GHashTable) all_collection_refs = NULL; /* (element-type OstreeChecksumRef utf8) */ - - if (!ostree_repo_list_collection_refs (self, NULL, &all_collection_refs, - OSTREE_REPO_LIST_REFS_EXT_EXCLUDE_REMOTES, cancellable, error)) - return FALSE; - - GLNX_HASH_TABLE_FOREACH_V (all_collection_refs, const char*, checksum) - { - g_debug ("Finding objects to keep for commit %s", checksum); - if (!ostree_repo_traverse_commit_union (self, checksum, depth, reachable, - cancellable, error)) - return FALSE; - } - - return TRUE; + return traverse_reachable_internal (self, + OSTREE_REPO_COMMIT_TRAVERSE_FLAG_NONE, + depth, reachable, + cancellable, error); } /** @@ -401,6 +423,7 @@ ostree_repo_prune (OstreeRepo *self, g_autoptr(GHashTable) objects = NULL; gboolean refs_only = flags & OSTREE_REPO_PRUNE_FLAGS_REFS_ONLY; + gboolean commit_only = flags & OSTREE_REPO_PRUNE_FLAGS_COMMIT_ONLY; g_autoptr(GHashTable) reachable = ostree_repo_traverse_new_reachable (); @@ -409,9 +432,15 @@ ostree_repo_prune (OstreeRepo *self, * the deletion. */ + OstreeRepoCommitTraverseFlags traverse_flags = OSTREE_REPO_COMMIT_TRAVERSE_FLAG_NONE; + if (commit_only) + traverse_flags |= OSTREE_REPO_COMMIT_TRAVERSE_FLAG_COMMIT_ONLY; + if (refs_only) { - if (!ostree_repo_traverse_reachable_refs (self, depth, reachable, cancellable, error)) + if (!traverse_reachable_internal (self, traverse_flags, + depth, reachable, + cancellable, error)) return FALSE; } @@ -432,8 +461,8 @@ ostree_repo_prune (OstreeRepo *self, continue; g_debug ("Finding objects to keep for commit %s", checksum); - if (!ostree_repo_traverse_commit_union (self, checksum, depth, reachable, - cancellable, error)) + if (!ostree_repo_traverse_commit_with_flags (self, traverse_flags, checksum, depth, reachable, + NULL, cancellable, error)) return FALSE; } } diff --git a/src/libostree/ostree-repo-traverse.c b/src/libostree/ostree-repo-traverse.c index c5c204d7..5efed100 100644 --- a/src/libostree/ostree-repo-traverse.c +++ b/src/libostree/ostree-repo-traverse.c @@ -542,8 +542,9 @@ traverse_dirtree (OstreeRepo *repo, } /** - * ostree_repo_traverse_commit_union_with_parents: (skip) + * ostree_repo_traverse_commit_with_flags: (skip) * @repo: Repo + * @flags: change traversal behaviour according to these flags * @commit_checksum: ASCII SHA256 checksum * @maxdepth: Traverse this many parent commits, -1 for unlimited * @inout_reachable: Set of reachable objects @@ -561,15 +562,17 @@ traverse_dirtree (OstreeRepo *repo, * Since: 2018.5 */ gboolean -ostree_repo_traverse_commit_union_with_parents (OstreeRepo *repo, - const char *commit_checksum, - int maxdepth, - GHashTable *inout_reachable, - GHashTable *inout_parents, - GCancellable *cancellable, - GError **error) +ostree_repo_traverse_commit_with_flags (OstreeRepo *repo, + OstreeRepoCommitTraverseFlags flags, + const char *commit_checksum, + int maxdepth, + GHashTable *inout_reachable, + GHashTable *inout_parents, + GCancellable *cancellable, + GError **error) { g_autofree char *tmp_checksum = NULL; + gboolean commit_only = flags & OSTREE_REPO_COMMIT_TRAVERSE_FLAG_COMMIT_ONLY; while (TRUE) { @@ -603,16 +606,20 @@ ostree_repo_traverse_commit_union_with_parents (OstreeRepo *repo, g_hash_table_add (inout_reachable, g_variant_ref (key)); - g_debug ("Traversing commit %s", commit_checksum); - ostree_cleanup_repo_commit_traverse_iter - OstreeRepoCommitTraverseIter iter = { 0, }; - if (!ostree_repo_commit_traverse_iter_init_commit (&iter, repo, commit, - OSTREE_REPO_COMMIT_TRAVERSE_FLAG_NONE, - error)) - return FALSE; + /* Save time by skipping traversal of non-commit objects */ + if (!commit_only) + { + g_debug ("Traversing commit %s", commit_checksum); + ostree_cleanup_repo_commit_traverse_iter + OstreeRepoCommitTraverseIter iter = { 0, }; + if (!ostree_repo_commit_traverse_iter_init_commit (&iter, repo, commit, + OSTREE_REPO_COMMIT_TRAVERSE_FLAG_NONE, + error)) + return FALSE; - if (!traverse_iter (repo, &iter, key, inout_reachable, inout_parents, ignore_missing_dirs, cancellable, error)) - return FALSE; + if (!traverse_iter (repo, &iter, key, inout_reachable, inout_parents, ignore_missing_dirs, cancellable, error)) + return FALSE; + } gboolean recurse = FALSE; if (maxdepth == -1 || maxdepth > 0) @@ -634,6 +641,39 @@ ostree_repo_traverse_commit_union_with_parents (OstreeRepo *repo, return TRUE; } +/** + * ostree_repo_traverse_commit_union_with_parents: (skip) + * @repo: Repo + * @commit_checksum: ASCII SHA256 checksum + * @maxdepth: Traverse this many parent commits, -1 for unlimited + * @inout_reachable: Set of reachable objects + * @inout_parents: Map from object to parent object + * @cancellable: Cancellable + * @error: Error + * + * Update the set @inout_reachable containing all objects reachable + * from @commit_checksum, traversing @maxdepth parent commits. + * + * Additionally this constructs a mapping from each object to the parents + * of the object, which can be used to track which commits an object + * belongs to. + * + * Since: 2018.5 + */ +gboolean +ostree_repo_traverse_commit_union_with_parents (OstreeRepo *repo, + const char *commit_checksum, + int maxdepth, + GHashTable *inout_reachable, + GHashTable *inout_parents, + GCancellable *cancellable, + GError **error) +{ + return ostree_repo_traverse_commit_with_flags(repo, OSTREE_REPO_COMMIT_TRAVERSE_FLAG_NONE, + commit_checksum, maxdepth, inout_reachable, inout_parents, + cancellable, error); +} + /** * ostree_repo_traverse_commit_union: (skip) * @repo: Repo diff --git a/src/libostree/ostree-repo.h b/src/libostree/ostree-repo.h index 8a5c3b33..98571170 100644 --- a/src/libostree/ostree-repo.h +++ b/src/libostree/ostree-repo.h @@ -1118,6 +1118,16 @@ typedef enum { OSTREE_STATIC_DELTA_INDEX_FLAGS_NONE = 0, } OstreeStaticDeltaIndexFlags; +/** + * OstreeRepoCommitTraverseFlags: + * @OSTREE_REPO_COMMIT_TRAVERSE_FLAG_NONE: No special options for traverse + * @OSTREE_REPO_COMMIT_TRAVERSE_FLAG_COMMIT_ONLY: Traverse and retrieve only commit objects. (Since: 2022.2) + */ +typedef enum { + OSTREE_REPO_COMMIT_TRAVERSE_FLAG_NONE = (1 << 0), + OSTREE_REPO_COMMIT_TRAVERSE_FLAG_COMMIT_ONLY = (1 << 1), +} OstreeRepoCommitTraverseFlags; + _OSTREE_PUBLIC gboolean ostree_repo_static_delta_reindex (OstreeRepo *repo, OstreeStaticDeltaIndexFlags flags, @@ -1171,6 +1181,7 @@ gboolean ostree_repo_traverse_commit_union (OstreeRepo *repo, GHashTable *inout_reachable, GCancellable *cancellable, GError **error); + _OSTREE_PUBLIC gboolean ostree_repo_traverse_commit_union_with_parents (OstreeRepo *repo, const char *commit_checksum, @@ -1180,6 +1191,16 @@ gboolean ostree_repo_traverse_commit_union_with_parents (OstreeRepo *rep GCancellable *cancellable, GError **error); +_OSTREE_PUBLIC +gboolean ostree_repo_traverse_commit_with_flags (OstreeRepo *repo, + OstreeRepoCommitTraverseFlags flags, + const char *commit_checksum, + int maxdepth, + GHashTable *inout_reachable, + GHashTable *inout_parents, + GCancellable *cancellable, + GError **error); + struct _OstreeRepoCommitTraverseIter { gboolean initialized; /* 4 byte hole on 64 bit */ @@ -1189,10 +1210,6 @@ struct _OstreeRepoCommitTraverseIter { typedef struct _OstreeRepoCommitTraverseIter OstreeRepoCommitTraverseIter; -typedef enum { - OSTREE_REPO_COMMIT_TRAVERSE_FLAG_NONE = (1 << 0) -} OstreeRepoCommitTraverseFlags; - _OSTREE_PUBLIC gboolean ostree_repo_commit_traverse_iter_init_commit (OstreeRepoCommitTraverseIter *iter, @@ -1245,11 +1262,13 @@ void ostree_repo_commit_traverse_iter_cleanup (void *p); * @OSTREE_REPO_PRUNE_FLAGS_NONE: No special options for pruning * @OSTREE_REPO_PRUNE_FLAGS_NO_PRUNE: Don't actually delete objects * @OSTREE_REPO_PRUNE_FLAGS_REFS_ONLY: Do not traverse individual commit objects, only follow refs + * @OSTREE_REPO_PRUNE_FLAGS_COMMIT_ONLY: Only traverse commit objects. (Since 2022.2) */ typedef enum { OSTREE_REPO_PRUNE_FLAGS_NONE = 0, OSTREE_REPO_PRUNE_FLAGS_NO_PRUNE = (1 << 0), OSTREE_REPO_PRUNE_FLAGS_REFS_ONLY = (1 << 1), + OSTREE_REPO_PRUNE_FLAGS_COMMIT_ONLY = (1 << 2), } OstreeRepoPruneFlags; _OSTREE_PUBLIC diff --git a/src/ostree/ot-builtin-prune.c b/src/ostree/ot-builtin-prune.c index d133bbd8..b2dd407a 100644 --- a/src/ostree/ot-builtin-prune.c +++ b/src/ostree/ot-builtin-prune.c @@ -35,6 +35,7 @@ static char *opt_delete_commit; static char *opt_keep_younger_than; static char **opt_retain_branch_depth; static char **opt_only_branches; +static gboolean opt_commit_only; /* ATTENTION: * Please remember to update the bash-completion script (bash/ostree) and @@ -50,6 +51,7 @@ static GOptionEntry options[] = { { "static-deltas-only", 0, 0, G_OPTION_ARG_NONE, &opt_static_deltas_only, "Change the behavior of delete-commit and keep-younger-than to prune only static deltas" }, { "retain-branch-depth", 0, 0, G_OPTION_ARG_STRING_ARRAY, &opt_retain_branch_depth, "Additionally retain BRANCH=DEPTH commits", "BRANCH=DEPTH" }, { "only-branch", 0, 0, G_OPTION_ARG_STRING_ARRAY, &opt_only_branches, "Only prune BRANCH (may be specified multiple times)", "BRANCH" }, + { "commit-only", 0, 0, G_OPTION_ARG_NONE, &opt_commit_only, "Only traverse and delete commit objects.", NULL }, { NULL } }; @@ -99,12 +101,15 @@ traverse_keep_younger_than (OstreeRepo *repo, const char *checksum, GCancellable *cancellable, GError **error) { g_autofree char *next_checksum = g_strdup (checksum); + OstreeRepoCommitTraverseFlags traverse_flags = OSTREE_REPO_COMMIT_TRAVERSE_FLAG_NONE; + if (opt_commit_only) + traverse_flags |= OSTREE_REPO_COMMIT_TRAVERSE_FLAG_COMMIT_ONLY; /* This is the first commit in our loop, which has a ref pointing to it. We * don't want to auto-prune it. */ - if (!ostree_repo_traverse_commit_union (repo, checksum, 0, reachable, - cancellable, error)) + if (!ostree_repo_traverse_commit_with_flags (repo, traverse_flags, checksum, 0, reachable, + NULL, cancellable, error)) return FALSE; while (TRUE) @@ -121,8 +126,8 @@ traverse_keep_younger_than (OstreeRepo *repo, const char *checksum, if (commit_timestamp >= ts->tv_sec) { /* It's newer, traverse it */ - if (!ostree_repo_traverse_commit_union (repo, next_checksum, 0, reachable, - cancellable, error)) + if (!ostree_repo_traverse_commit_with_flags (repo, traverse_flags, next_checksum, 0, reachable, + NULL, cancellable, error)) return FALSE; g_free (next_checksum); @@ -183,6 +188,8 @@ ostree_builtin_prune (int argc, char **argv, OstreeCommandInvocation *invocation pruneflags |= OSTREE_REPO_PRUNE_FLAGS_REFS_ONLY; if (opt_no_prune) pruneflags |= OSTREE_REPO_PRUNE_FLAGS_NO_PRUNE; + if (opt_commit_only) + pruneflags |= OSTREE_REPO_PRUNE_FLAGS_COMMIT_ONLY; /* If no newer more complex options are specified, drop down to the original * prune API - both to avoid code duplication, and to keep it run from the @@ -285,6 +292,10 @@ ostree_builtin_prune (int argc, char **argv, OstreeCommandInvocation *invocation /* Traverse each ref, and gather all objects pointed to by it up to a * specific depth (if configured). */ + OstreeRepoCommitTraverseFlags traverse_flags = OSTREE_REPO_COMMIT_TRAVERSE_FLAG_NONE; + if (opt_commit_only) + /** We can avoid looking at all objects if --commit-only is specified **/ + traverse_flags |= OSTREE_REPO_COMMIT_TRAVERSE_FLAG_COMMIT_ONLY; g_hash_table_iter_init (&hash_iter, all_refs); while (g_hash_table_iter_next (&hash_iter, &key, &value)) { @@ -316,8 +327,8 @@ ostree_builtin_prune (int argc, char **argv, OstreeCommandInvocation *invocation the global default */ g_debug ("Finding objects to keep for commit %s", checksum); - if (!ostree_repo_traverse_commit_union (repo, checksum, depth, reachable, - cancellable, error)) + if (!ostree_repo_traverse_commit_with_flags (repo, traverse_flags, checksum, depth, reachable, + NULL, cancellable, error)) return FALSE; } @@ -333,7 +344,10 @@ ostree_builtin_prune (int argc, char **argv, OstreeCommandInvocation *invocation } g_autofree char *formatted_freed_size = g_format_size_full (objsize_total, 0); - g_print ("Total objects: %u\n", n_objects_total); + if (opt_commit_only) + g_print("Total (commit only) objects: %u\n", n_objects_total); + else + g_print ("Total objects: %u\n", n_objects_total); if (n_objects_pruned == 0) g_print ("No unreachable objects\n"); else if (pruneflags & OSTREE_REPO_PRUNE_FLAGS_NO_PRUNE) From ce44b1907e6ef68dec4f80859e799891bfec2be4 Mon Sep 17 00:00:00 2001 From: Saqib Ali Date: Tue, 8 Feb 2022 09:08:44 -0500 Subject: [PATCH 43/52] man/prune, bash: Add --commit-only flag for ostree prune Update the man page and the auto-complete script to include the --commit-only flag --- bash/ostree | 1 + man/ostree-prune.xml | 11 +++++++++++ 2 files changed, 12 insertions(+) diff --git a/bash/ostree b/bash/ostree index c990462f..46363315 100644 --- a/bash/ostree +++ b/bash/ostree @@ -805,6 +805,7 @@ _ostree_prune() { --no-prune --refs-only --static-deltas-only + --commit-only " local options_with_args=" diff --git a/man/ostree-prune.xml b/man/ostree-prune.xml index e7e028ab..341ac7fb 100644 --- a/man/ostree-prune.xml +++ b/man/ostree-prune.xml @@ -120,6 +120,17 @@ License along with this library. If not, see . . + + + + + + Only traverse and delete commit objects. This leaves orphaned meta and + content objects, which can be cleaned up with another prune invocation. + One may want to use this option to cheaply delete multiple commits, + and then clean up with a more expensive prune at the end. + + From 18ab5361b99ccc88043d2f7d9ac8e99fcf19508c Mon Sep 17 00:00:00 2001 From: Saqib Ali Date: Tue, 22 Feb 2022 19:00:25 -0500 Subject: [PATCH 44/52] tests/test-prune.sh: expand testing for --commit-only Let's add additional tests to expand the test suite for the new --commit-only functionality. --- tests/test-prune.sh | 73 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 72 insertions(+), 1 deletion(-) diff --git a/tests/test-prune.sh b/tests/test-prune.sh index e33ab1e7..b029b359 100755 --- a/tests/test-prune.sh +++ b/tests/test-prune.sh @@ -25,7 +25,7 @@ skip_without_user_xattrs setup_fake_remote_repo1 "archive" -echo '1..12' +echo '1..17' cd ${test_tmpdir} mkdir repo @@ -50,6 +50,12 @@ assert_repo_has_n_commits() { assert_streq "$(find ${repo}/objects -name '*.commit' | wc -l)" "${count}" } +assert_repo_has_n_non_commit_objects() { + repo=$1 + count=$2 + assert_streq "$(find ${repo}/objects -name '*.*' ! -name '*.commit' ! -name '*.tombstone-commit'| wc -l)" "${count}" +} + # Test --no-prune objectcount_orig=$(find repo/objects | wc -l) ${CMD_PREFIX} ostree prune --repo=repo --refs-only --depth=0 --no-prune | tee noprune.txt @@ -294,3 +300,68 @@ if ${CMD_PREFIX} ostree --repo=repo prune --only-branch=BACON 2>err.txt; then fi assert_file_has_content err.txt "Refspec.*BACON.*not found" echo "ok --only-branch=BACON" + +# We will use the same principle as datesnap repo +# to create a snapshot to test --commit-only +rm -rf commit-only-test-repo +ostree_repo_init commit-only-test-repo --mode=archive +# Older commits w/ content objects +echo 'message' > tree/file.txt +${CMD_PREFIX} ostree --repo=commit-only-test-repo commit --branch=stable -m test -s "new stable build 1" tree --timestamp="October 15 1985" +${CMD_PREFIX} ostree --repo=commit-only-test-repo commit --branch=dev -m test -s "new dev build 1" tree --timestamp="October 15 1985" +# Commits without any content objects +${CMD_PREFIX} ostree --repo=commit-only-test-repo commit --branch=stable -m test -s "new stable build 2" tree +${CMD_PREFIX} ostree --repo=commit-only-test-repo commit --branch=dev -m test -s "new dev build 2" tree +# Commits with content objects +echo 'message2' > tree/file2.txt +${CMD_PREFIX} ostree --repo=commit-only-test-repo commit --branch=stable -m test -s "new stable build 2" tree +${CMD_PREFIX} ostree --repo=commit-only-test-repo commit --branch=dev -m test -s "new dev build 2" tree +assert_repo_has_n_commits commit-only-test-repo 6 +reinitialize_commit_only_test_repo() { + rm repo -rf + ostree_repo_init repo --mode=archive + ${CMD_PREFIX} ostree --repo=repo pull-local --depth=-1 commit-only-test-repo +} +orig_obj_count=$(find commit-only-test-repo/objects -name '*.*' ! -name '*.commit' | wc -l) + +# --commit-only tests +# Test single branch +reinitialize_commit_only_test_repo +${CMD_PREFIX} ostree --repo=repo prune --commit-only --only-branch=dev --depth=0 +assert_repo_has_n_commits repo 4 +assert_repo_has_n_non_commit_objects repo ${orig_obj_count} +echo 'ok --commit-only and --only-branch' + +# Test multiple branches (and depth > 0) +reinitialize_commit_only_test_repo +${CMD_PREFIX} ostree --repo=repo prune --commit-only --refs-only --depth=1 +assert_repo_has_n_commits repo 4 +assert_repo_has_n_non_commit_objects repo ${orig_obj_count} +echo 'ok --commit-only and multiple branches (depth > 0)' + +# Test --delete-commit with --commit-only +reinitialize_commit_only_test_repo +# this commit does not have a parent commit +COMMIT_TO_DELETE=$(${CMD_PREFIX} ostree --repo=repo log dev | grep ^commit | cut -f 2 -d' ' | tail -n 1) +${CMD_PREFIX} ostree --repo=repo prune --commit-only --delete-commit=$COMMIT_TO_DELETE +assert_repo_has_n_commits repo 5 +# We gain an extra +assert_repo_has_n_non_commit_objects repo ${orig_obj_count} +echo 'ok --commit-only and --delete-commit' + +# Test --delete-commit when it creates orphaned commits +reinitialize_commit_only_test_repo +# get the current HEAD's parent on dev branch +COMMIT_TO_DELETE=$(${CMD_PREFIX} ostree --repo=repo log dev | grep ^commit | cut -f 2 -d' ' | head -n 2 | tail -n 1) +${CMD_PREFIX} ostree --repo=repo prune --commit-only --refs-only --delete-commit=$COMMIT_TO_DELETE +# we deleted a commit that orphaned another, so we lose two commits +assert_repo_has_n_commits repo 4 +assert_repo_has_n_non_commit_objects repo ${orig_obj_count} +echo 'ok --commit-only and --delete-commit with orphaned commits' + +# Test --keep-younger-than with --commit-only +reinitialize_commit_only_test_repo +${CMD_PREFIX} ostree --repo=repo prune --commit-only --keep-younger-than="1 week ago" +assert_repo_has_n_commits repo 4 +assert_repo_has_n_non_commit_objects repo ${orig_obj_count} +echo 'ok --commit-only and --keep-younger-than' From cf66eaccee710a53cb1e80a65b9b2f74e26e39c9 Mon Sep 17 00:00:00 2001 From: Saqib Ali Date: Thu, 24 Feb 2022 16:57:23 -0500 Subject: [PATCH 45/52] tests/test-prune.sh: Use TAP API Change tests to use the newer TAP API introduced in https://github.com/ostreedev/ostree/pull/2440 --- tests/test-prune.sh | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/tests/test-prune.sh b/tests/test-prune.sh index b029b359..20904f31 100755 --- a/tests/test-prune.sh +++ b/tests/test-prune.sh @@ -25,8 +25,6 @@ skip_without_user_xattrs setup_fake_remote_repo1 "archive" -echo '1..17' - cd ${test_tmpdir} mkdir repo ostree_repo_init repo @@ -150,7 +148,7 @@ if ${CMD_PREFIX} ostree --repo=repo prune --static-deltas-only --keep-younger-th fi assert_file_has_content_literal err.txt "--static-deltas-only requires --delete-commit" -echo "ok prune" +tap_ok prune rm repo -rf ostree_repo_init repo --mode=bare-user @@ -158,7 +156,7 @@ ${CMD_PREFIX} ostree --repo=repo remote add --set=gpg-verify=false origin $(cat ${CMD_PREFIX} ostree --repo=repo pull --depth=-1 --commit-metadata-only origin test ${CMD_PREFIX} ostree --repo=repo prune -echo "ok prune with partial repo" +tap_ok prune with partial repo assert_has_n_objects() { find $1/objects -name '*.filez' | wc -l > object-count @@ -194,7 +192,7 @@ ${CMD_PREFIX} ostree --repo=repo refs --delete test ${CMD_PREFIX} ostree --repo=child-repo prune --refs-only --depth=0 assert_has_n_objects child-repo 3 -echo "ok prune with parent repo" +tap_ok prune with parent repo # Delete all the above since I can't be bothered to think about how new tests # would interact. We make a new repo test suite, then clone it @@ -239,7 +237,7 @@ $OSTREE fsck ${CMD_PREFIX} ostree --repo=repo prune --keep-younger-than="1 week ago" --retain-branch-depth=stable=5 assert_repo_has_n_commits repo 9 $OSTREE fsck -echo "ok retain branch depth and keep-younger-than" +tap_ok retain branch depth and keep-younger-than # Just stable branch ref, we should prune everything except the tip of dev, # so 8 stable + 1 dev = 9 @@ -248,7 +246,7 @@ ${CMD_PREFIX} ostree --repo=repo prune --depth=0 --retain-branch-depth=stable=-1 assert_repo_has_n_commits repo 9 $OSTREE fsck -echo "ok retain branch depth (alone)" +tap_ok retain branch depth [alone] # Test --only-branch with --depth=0; this should be exactly identical to the # above with a result of 9. @@ -256,13 +254,13 @@ reinitialize_datesnap_repo ${CMD_PREFIX} ostree --repo=repo prune --only-branch=dev --depth=0 assert_repo_has_n_commits repo 9 $OSTREE fsck -echo "ok --only-branch --depth=0" +tap_ok --only-branch --depth=0 # Test --only-branch with --depth=1; should just add 1 to the above, for 10. reinitialize_datesnap_repo ${CMD_PREFIX} ostree --repo=repo prune --only-branch=dev --depth=1 assert_repo_has_n_commits repo 10 -echo "ok --only-branch --depth=1" +tap_ok --only-branch --depth=1 # Test --only-branch with all branches reinitialize_datesnap_repo @@ -271,27 +269,27 @@ assert_repo_has_n_commits repo 2 reinitialize_datesnap_repo ${CMD_PREFIX} ostree --repo=repo prune --only-branch=dev --only-branch=stable --depth=1 assert_repo_has_n_commits repo 4 -echo "ok --only-branch (all) --depth=1" +tap_ok --only-branch [all] --depth=1 # Test --only-branch and --retain-branch-depth overlap reinitialize_datesnap_repo ${CMD_PREFIX} ostree --repo=repo prune --only-branch=dev --only-branch=stable --depth=0 \ --retain-branch-depth=stable=2 assert_repo_has_n_commits repo 4 -echo "ok --only-branch and --retain-branch-depth overlap" +tap_ok --only-branch and --retain-branch-depth overlap # Test --only-branch and --retain-branch-depth together reinitialize_datesnap_repo ${CMD_PREFIX} ostree --repo=repo prune --only-branch=dev --depth=0 --retain-branch-depth=stable=2 assert_repo_has_n_commits repo 4 -echo "ok --only-branch and --retain-branch-depth together" +tap_ok --only-branch and --retain-branch-depth together # Test --only-branch with --keep-younger-than; this should be identical to the test # above for --retain-branch-depth=stable=-1 reinitialize_datesnap_repo ${CMD_PREFIX} ostree --repo=repo prune --only-branch=stable --keep-younger-than="1 week ago" assert_repo_has_n_commits repo 11 -echo "ok --only-branch --keep-younger-than" +tap_ok --only-branch --keep-younger-than # Test --only-branch with a nonexistent ref reinitialize_datesnap_repo @@ -299,7 +297,7 @@ if ${CMD_PREFIX} ostree --repo=repo prune --only-branch=BACON 2>err.txt; then fatal "we pruned BACON?" fi assert_file_has_content err.txt "Refspec.*BACON.*not found" -echo "ok --only-branch=BACON" +tap_ok --only-branch=BACON # We will use the same principle as datesnap repo # to create a snapshot to test --commit-only @@ -330,14 +328,14 @@ reinitialize_commit_only_test_repo ${CMD_PREFIX} ostree --repo=repo prune --commit-only --only-branch=dev --depth=0 assert_repo_has_n_commits repo 4 assert_repo_has_n_non_commit_objects repo ${orig_obj_count} -echo 'ok --commit-only and --only-branch' +tap_ok --commit-only and --only-branch # Test multiple branches (and depth > 0) reinitialize_commit_only_test_repo ${CMD_PREFIX} ostree --repo=repo prune --commit-only --refs-only --depth=1 assert_repo_has_n_commits repo 4 assert_repo_has_n_non_commit_objects repo ${orig_obj_count} -echo 'ok --commit-only and multiple branches (depth > 0)' +tap_ok --commit-only and multiple branches depth=1 # Test --delete-commit with --commit-only reinitialize_commit_only_test_repo @@ -347,7 +345,7 @@ ${CMD_PREFIX} ostree --repo=repo prune --commit-only --delete-commit=$COMMIT_TO_ assert_repo_has_n_commits repo 5 # We gain an extra assert_repo_has_n_non_commit_objects repo ${orig_obj_count} -echo 'ok --commit-only and --delete-commit' +tap_ok --commit-only and --delete-commit # Test --delete-commit when it creates orphaned commits reinitialize_commit_only_test_repo @@ -357,11 +355,12 @@ ${CMD_PREFIX} ostree --repo=repo prune --commit-only --refs-only --delete-commit # we deleted a commit that orphaned another, so we lose two commits assert_repo_has_n_commits repo 4 assert_repo_has_n_non_commit_objects repo ${orig_obj_count} -echo 'ok --commit-only and --delete-commit with orphaned commits' +tap_ok --commit-only and --delete-commit with orphaned commits # Test --keep-younger-than with --commit-only reinitialize_commit_only_test_repo ${CMD_PREFIX} ostree --repo=repo prune --commit-only --keep-younger-than="1 week ago" assert_repo_has_n_commits repo 4 assert_repo_has_n_non_commit_objects repo ${orig_obj_count} -echo 'ok --commit-only and --keep-younger-than' +tap_ok --commit-only and --keep-younger-than +tap_end From 65ccf2951d8116c961d04ea28160a07a9b4a542e Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 2 Mar 2022 12:30:34 +0000 Subject: [PATCH 46/52] build(deps): bump libglnx from `88da8dd` to `c71f7ae` Bumps libglnx from `88da8dd` to `c71f7ae`. --- updated-dependencies: - dependency-name: libglnx dependency-type: direct:production ... Signed-off-by: dependabot[bot] --- libglnx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libglnx b/libglnx index 88da8ddd..c71f7aef 160000 --- a/libglnx +++ b/libglnx @@ -1 +1 @@ -Subproject commit 88da8ddd60bf0401f37c12b7bdcd20f8a9e955b5 +Subproject commit c71f7aefa142c444210f1021d1af42f365ec3a7b From 2c60f302f91e3aa9ba343261b09b5a0e3d188b28 Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Wed, 2 Mar 2022 16:44:59 +0000 Subject: [PATCH 47/52] lib/core: introduce two new object types for split xattrs This adds two new object types for storing xattrs separately from content objects. `.file-xattrs` are regular files storing xattrs content, encoded as GVariant. Each object is keyed by the checksum of its content, allowing for multiple references. `.file-xattrs-link` are hardlinks which are associated to file objects. Each object is keyed by the same checksum of the corresponding file object. The target of the hardlink is an existing file-xattrs object. In case of reaching the limit of too many links, this object could be a plain file too. --- src/libostree/ostree-core.c | 10 ++++++++++ src/libostree/ostree-core.h | 8 ++++++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/libostree/ostree-core.c b/src/libostree/ostree-core.c index 0671ed35..2ff72086 100644 --- a/src/libostree/ostree-core.c +++ b/src/libostree/ostree-core.c @@ -1228,6 +1228,10 @@ ostree_object_type_to_string (OstreeObjectType objtype) return "commitmeta"; case OSTREE_OBJECT_TYPE_PAYLOAD_LINK: return "payload-link"; + case OSTREE_OBJECT_TYPE_FILE_XATTRS: + return "file-xattrs"; + case OSTREE_OBJECT_TYPE_FILE_XATTRS_LINK: + return "file-xattrs-link"; default: g_assert_not_reached (); return NULL; @@ -1257,6 +1261,10 @@ ostree_object_type_from_string (const char *str) return OSTREE_OBJECT_TYPE_COMMIT_META; else if (!strcmp (str, "payload-link")) return OSTREE_OBJECT_TYPE_PAYLOAD_LINK; + else if (!strcmp (str, "file-xattrs")) + return OSTREE_OBJECT_TYPE_FILE_XATTRS; + else if (!strcmp (str, "file-xattrs-link")) + return OSTREE_OBJECT_TYPE_FILE_XATTRS_LINK; g_assert_not_reached (); return 0; } @@ -2141,6 +2149,8 @@ _ostree_validate_structureof_metadata (OstreeObjectType objtype, /* TODO */ break; case OSTREE_OBJECT_TYPE_FILE: + case OSTREE_OBJECT_TYPE_FILE_XATTRS: + case OSTREE_OBJECT_TYPE_FILE_XATTRS_LINK: g_assert_not_reached (); break; } diff --git a/src/libostree/ostree-core.h b/src/libostree/ostree-core.h index 48a75f92..638c40ac 100644 --- a/src/libostree/ostree-core.h +++ b/src/libostree/ostree-core.h @@ -67,6 +67,8 @@ G_BEGIN_DECLS * @OSTREE_OBJECT_TYPE_TOMBSTONE_COMMIT: Toplevel object, refers to a deleted commit * @OSTREE_OBJECT_TYPE_COMMIT_META: Detached metadata for a commit * @OSTREE_OBJECT_TYPE_PAYLOAD_LINK: Symlink to a .file given its checksum on the payload only. + * @OSTREE_OBJECT_TYPE_FILE_XATTRS: Detached xattrs content, for 'bare-split-xattrs' mode. + * @OSTREE_OBJECT_TYPE_FILE_XATTRS_LINK: Hardlink to a .file-xattrs given the checksum of its .file object. * * Enumeration for core object types; %OSTREE_OBJECT_TYPE_FILE is for * content, the other types are metadata. @@ -78,7 +80,9 @@ typedef enum { OSTREE_OBJECT_TYPE_COMMIT = 4, /* .commit */ OSTREE_OBJECT_TYPE_TOMBSTONE_COMMIT = 5, /* .commit-tombstone */ OSTREE_OBJECT_TYPE_COMMIT_META = 6, /* .commitmeta */ - OSTREE_OBJECT_TYPE_PAYLOAD_LINK = 7, /* .payload-link */ + OSTREE_OBJECT_TYPE_PAYLOAD_LINK = 7, /* .payload-link */ + OSTREE_OBJECT_TYPE_FILE_XATTRS = 8, /* .file-xattrs */ + OSTREE_OBJECT_TYPE_FILE_XATTRS_LINK = 9, /* .file-xattrs-link */ } OstreeObjectType; /** @@ -94,7 +98,7 @@ typedef enum { * * Last valid object type; use this to validate ranges. */ -#define OSTREE_OBJECT_TYPE_LAST OSTREE_OBJECT_TYPE_PAYLOAD_LINK +#define OSTREE_OBJECT_TYPE_LAST OSTREE_OBJECT_TYPE_FILE_XATTRS_LINK /** * OSTREE_DIRMETA_GVARIANT_FORMAT: From 08e98e904289464e00cd68ac7aa675d23d78971f Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Wed, 2 Mar 2022 16:45:00 +0000 Subject: [PATCH 48/52] lib/core: introduce 'bare-split-xattrs' mode --- Makefile-tests.am | 1 + docs/formats.md | 24 +++++++++++++++----- docs/repo.md | 32 ++++++++++++++++++++++----- src/libostree/ostree-core-private.h | 3 ++- src/libostree/ostree-core.c | 1 + src/libostree/ostree-core.h | 2 ++ src/libostree/ostree-repo.c | 5 +++++ tests/test-basic-bare-split-xattrs.sh | 25 +++++++++++++++++++++ 8 files changed, 82 insertions(+), 11 deletions(-) create mode 100755 tests/test-basic-bare-split-xattrs.sh diff --git a/Makefile-tests.am b/Makefile-tests.am index 6bae65cf..5c97bd84 100644 --- a/Makefile-tests.am +++ b/Makefile-tests.am @@ -59,6 +59,7 @@ test_programs = \ $(NULL) _installed_or_uninstalled_test_scripts = \ tests/test-basic.sh \ + tests/test-basic-bare-split-xattrs.sh \ tests/test-basic-user.sh \ tests/test-basic-user-only.sh \ tests/test-basic-root.sh \ diff --git a/docs/formats.md b/docs/formats.md index 0943aafa..c3723279 100644 --- a/docs/formats.md +++ b/docs/formats.md @@ -63,18 +63,32 @@ Other disadvantages of `archive`: - One doesn't know the total size (compressed or uncompressed) of content before downloading everything -## Aside: the bare and bare-user formats +## Aside: bare formats -The most common operation is to pull from an `archive` repository -into a `bare` or `bare-user` formatted repository. These latter two -are not compressed on disk. In other words, pulling to them is -similar to unpacking (but not installing) an RPM/deb package. +The most common operation is to pull from a remote `archive` repository +into a local one. This latter is not compressed on disk. In other +words, pulling to a local repository is similar to unpacking (but not +installing) the content of an RPM/deb package. + +The `bare` repository format is the simplest one. In this mode regular files +are directly stored to disk, and all metadata (e.g. uid/gid and xattrs) is +reflected to the filesystem. +It allows further direct access to content and metadata, but it may require +elevated privileges when writing objects to the repository. The `bare-user` format is a bit special in that the uid/gid and xattrs from the content are ignored. This is primarily useful if you want to have the same OSTree-managed content that can be run on a host system or an unprivileged container. +Similarly, the `bare-split-xattrs` format is a special mode where xattrs +are stored as separate repository objects, and not directly reflected to +the filesystem. +This is primarily useful when transporting xattrs through lossy environments +(e.g. tar streams and containerized environments). It also allows carrying +security-sensitive xattrs (e.g. SELinux labels) out-of-band without involving +OS filesystem logic. + ## Static deltas OSTree itself was originally focused on a continuous delivery model, where diff --git a/docs/repo.md b/docs/repo.md index 69f26172..580281ca 100644 --- a/docs/repo.md +++ b/docs/repo.md @@ -81,15 +81,37 @@ warnings such as GNU Tar emitting "implausibly old time stamp" with 0; however, until we have a mechanism to transition cleanly to 1, for compatibilty OSTree is reverted to use zero again. +### Xattrs objects + +In some repository modes (e.g. `bare-split-xattrs`), xattrs are stored on the +side of the content objects they refer to. This is done via two dedicated +object types, `file-xattrs` and `file-xattrs-link`. + +`file-xattrs` store xattrs data, encoded as GVariant. Each object is keyed by +the checksum of the xattrs content, allowing for multiple references. + +`file-xattrs-link` are hardlinks which are associated to file objects. +Each object is keyed by the same checksum of the corresponding file +object. The target of the hardlink is an existing `file-xattrs` object. +In case of reaching the limit of too many links, this object could be +a plain file too. + # Repository types and locations -Also unlike git, an OSTree repository can be in one of four separate -modes: `bare`, `bare-user`, `bare-user-only`, and `archive`. A bare repository is -one where content files are just stored as regular files; it's -designed to be the source of a "hardlink farm", where each operating -system checkout is merely links into it. If you want to store files +Also unlike git, an OSTree repository can be in one of five separate +modes: `bare`, `bare-split-xattrs, ``bare-user`, `bare-user-only`, and +`archive`. + +A `bare` repository is one where content files are just stored as regular +files; it's designed to be the source of a "hardlink farm", where each +operating system checkout is merely links into it. If you want to store files owned by e.g. root in this mode, you must run OSTree as root. +The `bare-split-xattrs` mode is similar to the above one, but it does store +xattrs as separate objects. This is meant to avoid conflicts with +kernel-enforced constraints (e.g. on SELinux labels) and with other softwares +that may perform ephemeral changes to xattrs (e.g. container runtimes). + The `bare-user` mode is a later addition that is like `bare` in that files are unpacked, but it can (and should generally) be created as non-root. In this mode, extended metadata such as owner uid, gid, and diff --git a/src/libostree/ostree-core-private.h b/src/libostree/ostree-core-private.h index 34f86a6c..2bd2f984 100644 --- a/src/libostree/ostree-core-private.h +++ b/src/libostree/ostree-core-private.h @@ -197,7 +197,8 @@ _ostree_repo_mode_is_bare (OstreeRepoMode mode) return mode == OSTREE_REPO_MODE_BARE || mode == OSTREE_REPO_MODE_BARE_USER || - mode == OSTREE_REPO_MODE_BARE_USER_ONLY; + mode == OSTREE_REPO_MODE_BARE_USER_ONLY || + mode == OSTREE_REPO_MODE_BARE_SPLIT_XATTRS; } #ifndef OSTREE_DISABLE_GPGME diff --git a/src/libostree/ostree-core.c b/src/libostree/ostree-core.c index 2ff72086..f0d0e698 100644 --- a/src/libostree/ostree-core.c +++ b/src/libostree/ostree-core.c @@ -39,6 +39,7 @@ G_STATIC_ASSERT(OSTREE_REPO_MODE_ARCHIVE_Z2 == 1); G_STATIC_ASSERT(OSTREE_REPO_MODE_ARCHIVE == OSTREE_REPO_MODE_ARCHIVE_Z2); G_STATIC_ASSERT(OSTREE_REPO_MODE_BARE_USER == 2); G_STATIC_ASSERT(OSTREE_REPO_MODE_BARE_USER_ONLY == 3); +G_STATIC_ASSERT(OSTREE_REPO_MODE_BARE_SPLIT_XATTRS == 4); static GBytes *variant_to_lenprefixed_buffer (GVariant *variant); diff --git a/src/libostree/ostree-core.h b/src/libostree/ostree-core.h index 638c40ac..9a14192d 100644 --- a/src/libostree/ostree-core.h +++ b/src/libostree/ostree-core.h @@ -193,6 +193,7 @@ typedef enum { * @OSTREE_REPO_MODE_ARCHIVE_Z2: Legacy alias for `OSTREE_REPO_MODE_ARCHIVE` * @OSTREE_REPO_MODE_BARE_USER: Files are stored as themselves, except ownership; can be written by user. Hardlinks work only in user checkouts. * @OSTREE_REPO_MODE_BARE_USER_ONLY: Same as BARE_USER, but all metadata is not stored, so it can only be used for user checkouts. Does not need xattrs. + * @OSTREE_REPO_MODE_BARE_SPLIT_XATTRS: Same as BARE_USER, but xattrs are stored separately from file content, with dedicated object types. * * See the documentation of #OstreeRepo for more information about the * possible modes. @@ -203,6 +204,7 @@ typedef enum { OSTREE_REPO_MODE_ARCHIVE_Z2 = OSTREE_REPO_MODE_ARCHIVE, OSTREE_REPO_MODE_BARE_USER, OSTREE_REPO_MODE_BARE_USER_ONLY, + OSTREE_REPO_MODE_BARE_SPLIT_XATTRS, } OstreeRepoMode; /** diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 6c541029..71a01d59 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -2704,6 +2704,9 @@ ostree_repo_mode_to_string (OstreeRepoMode mode, /* Legacy alias */ ret_mode ="archive-z2"; break; + case OSTREE_REPO_MODE_BARE_SPLIT_XATTRS: + ret_mode = "bare-split-xattrs"; + break; default: return glnx_throw (error, "Invalid mode '%d'", mode); } @@ -2734,6 +2737,8 @@ ostree_repo_mode_from_string (const char *mode, else if (strcmp (mode, "archive-z2") == 0 || strcmp (mode, "archive") == 0) ret_mode = OSTREE_REPO_MODE_ARCHIVE; + else if (strcmp (mode, "bare-split-xattrs") == 0) + ret_mode = OSTREE_REPO_MODE_BARE_SPLIT_XATTRS; else return glnx_throw (error, "Invalid mode '%s' in repository configuration", mode); diff --git a/tests/test-basic-bare-split-xattrs.sh b/tests/test-basic-bare-split-xattrs.sh new file mode 100755 index 00000000..8bd6430d --- /dev/null +++ b/tests/test-basic-bare-split-xattrs.sh @@ -0,0 +1,25 @@ +#!/usr/bin/env bash +# +# SPDX-License-Identifier: LGPL-2.0+ + +set -euo pipefail + +. $(dirname $0)/libtest.sh + +mode="bare-split-xattrs" +OSTREE="${CMD_PREFIX} ostree --repo=${test_tmpdir}/repo" + +cd ${test_tmpdir} +${OSTREE} init --mode "${mode}" +${OSTREE} config get core.mode > mode.txt +assert_file_has_content mode.txt "${mode}" +tap_ok "repo init" +rm -rf -- repo mode.txt + +cd ${test_tmpdir} +${OSTREE} init --mode "${mode}" +${OSTREE} fsck --all +tap_ok "repo fsck" +rm -rf -- repo + +tap_end From 14a6e6d8d081b11217785d7c37b0c0a8c661000c Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Wed, 2 Mar 2022 16:45:01 +0000 Subject: [PATCH 49/52] lib/repo: read split xattrs content from file-xattrs-link objects --- src/libostree/ostree-repo.c | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 71a01d59..a27591b3 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -4176,6 +4176,32 @@ repo_load_file_archive (OstreeRepo *self, } } +static GVariant * +_ostree_repo_read_xattrs_file_link (OstreeRepo *self, + const char *checksum, + GCancellable *cancellable, + GError **error) +{ + g_assert (self != NULL); + g_assert (checksum != NULL); + + char xattr_path[_OSTREE_LOOSE_PATH_MAX]; + _ostree_loose_path (xattr_path, checksum, OSTREE_OBJECT_TYPE_FILE_XATTRS_LINK, self->mode); + + g_autoptr(GVariant) xattrs = NULL; + glnx_autofd int fd = -1; + if (!glnx_openat_rdonly (self->objects_dir_fd, xattr_path, FALSE, &fd, error)) + return FALSE; + + g_assert (fd >= 0); + if (!ot_variant_read_fd (fd, 0, G_VARIANT_TYPE ("a(ayay)"), TRUE, + &xattrs, error)) + return glnx_prefix_error_null (error, "Deserializing xattrs content"); + + g_assert (xattrs != NULL); + return g_steal_pointer (&xattrs); +} + gboolean _ostree_repo_load_file_bare (OstreeRepo *self, const char *checksum, @@ -4310,6 +4336,15 @@ _ostree_repo_load_file_bare (OstreeRepo *self, return FALSE; } } + else if (self->mode == OSTREE_REPO_MODE_BARE_SPLIT_XATTRS) + { + if (out_xattrs) + { + ret_xattrs = _ostree_repo_read_xattrs_file_link(self, checksum, cancellable, error); + if (ret_xattrs == NULL) + return FALSE; + } + } else { g_assert_not_reached (); From 7e79b82ff8b3c3aa7fbe5edfcfa237f9447e2f18 Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Wed, 2 Mar 2022 16:45:02 +0000 Subject: [PATCH 50/52] lib/commit: disallow writing content in 'bare-split-xattrs' mode This prevents writing content into 'bare-split-xattrs` repository, while carving some space for experimenting via a temporary `OSTREE_EXP_WRITE_BARE_SPLIT_XATTRS` environment flag. --- src/libostree/ostree-repo-commit.c | 3 +++ tests/test-basic-bare-split-xattrs.sh | 22 ++++++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index 21ce288f..5b16be5b 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -920,6 +920,9 @@ write_content_object (OstreeRepo *self, return FALSE; OstreeRepoMode repo_mode = ostree_repo_get_mode (self); + if (repo_mode == OSTREE_REPO_MODE_BARE_SPLIT_XATTRS && + g_getenv ("OSTREE_EXP_WRITE_BARE_SPLIT_XATTRS") == NULL) + return glnx_throw (error, "Not allowed due to repo mode"); GInputStream *file_input; /* Unowned alias */ g_autoptr(GInputStream) file_input_owned = NULL; /* We need a temporary for bare-user symlinks */ diff --git a/tests/test-basic-bare-split-xattrs.sh b/tests/test-basic-bare-split-xattrs.sh index 8bd6430d..1eeb3039 100755 --- a/tests/test-basic-bare-split-xattrs.sh +++ b/tests/test-basic-bare-split-xattrs.sh @@ -22,4 +22,26 @@ ${OSTREE} fsck --all tap_ok "repo fsck" rm -rf -- repo +cd ${test_tmpdir} +mkdir -p "${test_tmpdir}/files" +touch files/foo +${OSTREE} init --mode "${mode}" +if ${OSTREE} commit --orphan -m "not implemented" files; then + assert_not_reached "commit to bare-split-xattrs should have failed" +fi +${OSTREE} fsck --all +tap_ok "commit not implemented" +rm -rf -- repo files + +cd ${test_tmpdir} +mkdir -p "${test_tmpdir}/files" +touch files/foo +${OSTREE} init --mode "${mode}" +OSTREE_EXP_WRITE_BARE_SPLIT_XATTRS=true ${OSTREE} commit --orphan -m "experimental" files +if ${OSTREE} fsck --all; then + assert_not_reached "fsck should have failed" +fi +tap_ok "commit exp override" +rm -rf -- repo files + tap_end From aca5671eb19fdf5e43784ffb5223e7358e5a6537 Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Thu, 3 Mar 2022 11:12:00 +0000 Subject: [PATCH 51/52] tests/basic-bare-split-xattrs: add fixture, check read logic --- Makefile-tests.am | 1 + tests/fixtures/bare-split-xattrs/basic.tar.xz | Bin 0 -> 2504 bytes tests/test-basic-bare-split-xattrs.sh | 28 ++++++++++++++++++ 3 files changed, 29 insertions(+) create mode 100644 tests/fixtures/bare-split-xattrs/basic.tar.xz diff --git a/Makefile-tests.am b/Makefile-tests.am index 5c97bd84..5d39ee5e 100644 --- a/Makefile-tests.am +++ b/Makefile-tests.am @@ -202,6 +202,7 @@ dist_installed_test_data = tests/archive-test.sh \ tests/ostree-path-traverse.tar.gz \ tests/pre-signed-pull-data.tar.gz \ tests/libtest-core.sh \ + tests/fixtures/bare-split-xattrs/basic.tar.xz \ $(NULL) EXTRA_DIST += tests/libtest.sh diff --git a/tests/fixtures/bare-split-xattrs/basic.tar.xz b/tests/fixtures/bare-split-xattrs/basic.tar.xz new file mode 100644 index 0000000000000000000000000000000000000000..cec6717e4498565ccf027af48cbe870e14c83545 GIT binary patch literal 2504 zcmV;(2{-orH+ooF000E$*0e?f03iV!0000G&sfajSN{ozT>v>5N@un}Hi1L#y~)Z+ z#*g6p3nvV2T!s$WJjx_8;BxI-gB3s`y-{9IvVag3zm$$VCOkhgCf zB=Q8y!4F7AcKU3wrb9DGEfgPip}P@&`-Y_!4sHF71FCe19{+PzVL`g7|Hij^< zhl)zd?T;d`rI~{XpT1T2*#0{DpMuh!nAHcU_XEse5+wn z8qa6BgKb^>dI4c3mhgXQk{F$xrGX_pkakLR%peh)1oA`h)62=*+MO2=X zZgaf=^a=s%5-#~T?Hn!pd&ZsE5fz3UXs}SW`0k0QG5#O?Q=*nwdRofeyk|&!;a->N z*-?QjG9a z$H4OA5m0Zx&VgYesg@alTbP5EI*4X^3=Z-7Kq;IK2`Vcw${gt^ID-9ylyP1>Ie%eT ztcS1Hr-|;2UVOp+Zj=cjd0Yd9o^`>)RGw~C2iZUoSy;tgq`f&O&JCN2=lp4)%aFJQ zdcHnq#E*z=tGg~Yly0SSn3z`WRD@1T5ipvwILEcy+Jc)$ ziv;aNwaC63UaJBv#AP*|-y2uuwrI?S&AO7KKf9_{>Nb+WoT%I712H76weCk?Z|eJk ze6txAfBP&P<~m&F2kHPOn{B_}j;iNTzdf-fF1iw6FP!@U%ic~s?*IgSE(&|BX9%HG zor#}o^t28(p!eE;?=^JmG&Tu|N*DHmAvH}Nd1z75%>Oq_uz3H-LhOm_irn-wm{6PL z$K_!vz^td9qC5pX+(f&0eIHvdLqL7q5e)9$TP)l7^(-6^np0XDHSF4?P{y=@K@VLC zAvbp5EfR3b2+dqDcM$3GX6ixi<#6-m5D86q*B2ekKb8lp(~9tmN59{qcUd`F5?z%J7vxi-4`LG&?>Z73im z#o)IelbW1BwUfW*-o!lM)g!0`uH*mF!@)0TtdRH%-YB&(WOJ4s_Kczdu-FB+E%fy{ zx(k7Xra$d-qUaI5bnTyOa&Maxp52aIg#ak?0Ni(`TlDiv!=ol2tgm{2wmH^!#Emh+ zXt~LzFG!eV`_3cMQL-WS5 zqw}=7r+qMkCU0%wLInSI7Z+#WpVTXsuA(aC;JRZKH+30vidBpxWq4(%ZQ~jtL`SzE z2k&6~dlXtQU`V^FXCkq>5qE?=e_+WSVS-#SBao!xK4ut4x=CKjO^q+@=1;DNGK&v1F&}@V z@3xwiRVw+^Ds)zZ<$G2DyAi*ViN3yb1m30R3i&;eLRec47(%@fYZc9#X|6!fv7~HT z?mi0jj=&r31wpYe3W< zabK^iQQ5EGG^~r);tEK1|7v;ca2Gl$i*Q!r@|f3h#2Df<;&%>c%6x1u*k2e}d?` zXx&>~>_P8MZn#`?bKhbf(*ZFvamxq=ilSs`3^Szi8%}IQrwvdqheqF4aw|$L^P!_EO+_LhWPb!tTQ4Yfr*7t2wAo%i0!(as7HC@yQe@Ql zzXr5H3ZKyeQ3We?`X4G zDhXs;K)8}$r%~9uKYRx#SJ$d;65cTwl z+g1^d5rG?~@G+1doL5jvH*=Q(VHxvx)`Gnt4s*s-U&IZ&7c@25L}1G;*=|5#0000v&z mode.txt @@ -44,4 +53,23 @@ fi tap_ok "commit exp override" rm -rf -- repo files +if [ "${PRIVILEGED}" = "true" ]; then + COMMIT="d614c428015227259031b0f19b934dade908942fd71c49047e0daa70e7800a5d" + cd ${test_tmpdir} + ${SUDO} tar --same-permissions --same-owner -xaf ${test_srcdir}/fixtures/bare-split-xattrs/basic.tar.xz + ${SUDO} ${OSTREE} fsck --all + ${OSTREE} log ${COMMIT} > out.txt + assert_file_has_content_literal out.txt "fixtures: bare-split-xattrs repo" + ${OSTREE} ls ${COMMIT} -X /foo > out.txt + assert_file_has_content_literal out.txt "{ @a(ayay) [] } /foo" + ${OSTREE} ls ${COMMIT} -X /bar > out.txt + assert_file_has_content_literal out.txt "{ [(b'user.mykey', [byte 0x6d, 0x79, 0x76, 0x61, 0x6c, 0x75, 0x65])] } /bar" + ${OSTREE} ls ${COMMIT} /foolink > out.txt + assert_file_has_content_literal out.txt "/foolink -> foo" + tap_ok "reading simple fixture" + ${SUDO} rm -rf -- repo log.txt +else + tap_ok "reading simple fixture # skip Unable to sudo" +fi + tap_end From fbc6d21c2f71099fbab44cbdd74222b91f61c667 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 3 Mar 2022 16:29:11 -0500 Subject: [PATCH 52/52] Release 2022.2 --- Makefile-libostree.am | 6 +++--- configure.ac | 2 +- src/libostree/libostree-devel.sym | 5 +---- src/libostree/libostree-released.sym | 5 +++++ tests/test-symbols.sh | 2 +- 5 files changed, 11 insertions(+), 9 deletions(-) diff --git a/Makefile-libostree.am b/Makefile-libostree.am index 9b48a308..f125adb8 100644 --- a/Makefile-libostree.am +++ b/Makefile-libostree.am @@ -171,9 +171,9 @@ endif # USE_GPGME symbol_files = $(top_srcdir)/src/libostree/libostree-released.sym # Uncomment this include when adding new development symbols. -if BUILDOPT_IS_DEVEL_BUILD -symbol_files += $(top_srcdir)/src/libostree/libostree-devel.sym -endif +#if BUILDOPT_IS_DEVEL_BUILD +#symbol_files += $(top_srcdir)/src/libostree/libostree-devel.sym +#endif # http://blog.jgc.org/2007/06/escaping-comma-and-space-in-gnu-make.html wl_versionscript_arg = -Wl,--version-script= diff --git a/configure.ac b/configure.ac index 69e03982..3bd735ed 100644 --- a/configure.ac +++ b/configure.ac @@ -4,7 +4,7 @@ m4_define([year_version], [2022]) m4_define([release_version], [2]) m4_define([package_version], [year_version.release_version]) AC_INIT([libostree], [package_version], [walters@verbum.org]) -is_release_build=no +is_release_build=yes AC_CONFIG_HEADER([config.h]) AC_CONFIG_MACRO_DIR([buildutil]) AC_CONFIG_AUX_DIR([build-aux]) diff --git a/src/libostree/libostree-devel.sym b/src/libostree/libostree-devel.sym index 74f9b9a9..c15ae9fa 100644 --- a/src/libostree/libostree-devel.sym +++ b/src/libostree/libostree-devel.sym @@ -20,10 +20,7 @@ - uncomment the include in Makefile-libostree.am */ -LIBOSTREE_2022.2 { -global: - ostree_repo_traverse_commit_with_flags; -} LIBOSTREE_2021.5; + /* Stub section for the stable release *after* this development one; don't * edit this other than to update the year. This is just a copy/paste diff --git a/src/libostree/libostree-released.sym b/src/libostree/libostree-released.sym index 5349de63..dbff16a0 100644 --- a/src/libostree/libostree-released.sym +++ b/src/libostree/libostree-released.sym @@ -672,6 +672,11 @@ global: ostree_mutable_tree_new_from_commit; } LIBOSTREE_2021.4; +LIBOSTREE_2022.2 { +global: + ostree_repo_traverse_commit_with_flags; +} LIBOSTREE_2021.5; + /* NOTE: Only add more content here in release commits! See the * comments at the top of this file. */ diff --git a/tests/test-symbols.sh b/tests/test-symbols.sh index 18e13e9f..76e63ba1 100755 --- a/tests/test-symbols.sh +++ b/tests/test-symbols.sh @@ -54,7 +54,7 @@ echo 'ok documented symbols' # ONLY update this checksum in release commits! cat > released-sha256.txt <