From 603c1258cc44a1c218021f4e7d407bfe8f3270eb Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Thu, 21 Jun 2018 11:23:40 -0400 Subject: [PATCH 01/47] Post-release version bump --- configure.ac | 4 ++-- src/libostree/libostree-devel.sym | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index e3b15262..896e7806 100644 --- a/configure.ac +++ b/configure.ac @@ -4,10 +4,10 @@ dnl update libostree-released.sym from libostree-devel.sym, and update the check dnl in test-symbols.sh, and also set is_release_build=yes below. Then make dnl another post-release commit to bump the version, and set is_release_build=no. m4_define([year_version], [2018]) -m4_define([release_version], [6]) +m4_define([release_version], [7]) 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]) diff --git a/src/libostree/libostree-devel.sym b/src/libostree/libostree-devel.sym index a4040ba6..1e866922 100644 --- a/src/libostree/libostree-devel.sym +++ b/src/libostree/libostree-devel.sym @@ -18,6 +18,8 @@ ***/ /* Add new symbols here. Release commits should copy this section into -released.sym. */ +LIBOSTREE_2018.7 { +} LIBOSTREE_2018.6; /* Stub section for the stable release *after* this development one; don't * edit this other than to update the last number. This is just a copy/paste From 1174d9f5ba537562c67084caf0214544fbb14ffc Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 21 Jun 2018 14:17:28 +0000 Subject: [PATCH 02/47] lib/repo: Fix 32 bit format string error --- src/libostree/ostree-repo-commit.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index 37be748b..7e1d707b 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -903,7 +903,7 @@ write_content_object (OstreeRepo *self, return glnx_throw (error, "min-free-space-percent '%u%%' would be exceeded, %s more required", self->min_free_space_percent, formatted_required); else - return glnx_throw (error, "min-free-space-size %luMB would be exceeded, %s more required", + return glnx_throw (error, "min-free-space-size %" G_GUINT64_FORMAT "MB would be exceeded, %s more required", self->min_free_space_mb, formatted_required); } /* This is the main bit that needs mutex protection */ @@ -1617,7 +1617,7 @@ ostree_repo_prepare_transaction (OstreeRepo *self, return glnx_throw (error, "min-free-space-percent '%u%%' would be exceeded, %s available", self->min_free_space_percent, formatted_free); else - return glnx_throw (error, "min-free-space-size %luMB would be exceeded, %s available", + return glnx_throw (error, "min-free-space-size %" G_GUINT64_FORMAT "MB would be exceeded, %s available", self->min_free_space_mb, formatted_free); } g_mutex_unlock (&self->txn_lock); From 095376efa2ecaf538246cf2e3979a9606afdea5f Mon Sep 17 00:00:00 2001 From: Umang Jain Date: Wed, 20 Jun 2018 02:22:12 +0530 Subject: [PATCH 03/47] lib/repo: Enforce min-free-space-* size check for regfiles in deltas During the pull, there is an explicit check for free space on disk vs. the size of uncompressed delta; But while writing the new content objects that are generated, they have to honor min-free-space-* checks too. We enforce this check in _bare_content_commit as that is where we can know the final size of the new content object. Closes: #1614 Approved by: jlebon --- src/libostree/ostree-repo-commit.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index 7e1d707b..2516ba3d 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -509,6 +509,33 @@ _ostree_repo_bare_content_commit (OstreeRepo *self, { OstreeRealRepoBareContent *real = (OstreeRealRepoBareContent*) barewrite; g_assert (real->initialized); + + if ((self->min_free_space_percent > 0 || self->min_free_space_mb > 0) && self->in_transaction) + { + struct stat st_buf; + if (!glnx_fstat (real->tmpf.fd, &st_buf, error)) + return FALSE; + + g_mutex_lock (&self->txn_lock); + g_assert_cmpint (self->txn.blocksize, >, 0); + + const fsblkcnt_t object_blocks = (st_buf.st_size / self->txn.blocksize) + 1; + if (object_blocks > self->txn.max_blocks) + { + g_mutex_unlock (&self->txn_lock); + g_autofree char *formatted_required = g_format_size (st_buf.st_size); + if (self->min_free_space_percent > 0) + return glnx_throw (error, "min-free-space-percent '%u%%' would be exceeded, %s more required", + self->min_free_space_percent, formatted_required); + else + return glnx_throw (error, "min-free-space-size %luMB woulid be exceeded, %s more required", + self->min_free_space_mb, formatted_required); + } + /* This is the main bit that needs mutex protection */ + self->txn.max_blocks -= object_blocks; + g_mutex_unlock (&self->txn_lock); + } + ot_checksum_get_hexdigest (&real->checksum, checksum_buf, buflen); if (real->expected_checksum && From 8d97b552412de0baa109255c9d8e3bca04ed27ff Mon Sep 17 00:00:00 2001 From: Umang Jain Date: Fri, 22 Jun 2018 23:36:21 +0530 Subject: [PATCH 04/47] tests: Add tests for space checks during deltas codepath Closes: #1614 Approved by: jlebon --- .../nondestructive/itest-pull-space.sh | 66 ++++++++++++++++++- 1 file changed, 63 insertions(+), 3 deletions(-) diff --git a/tests/installed/nondestructive/itest-pull-space.sh b/tests/installed/nondestructive/itest-pull-space.sh index 8fcf1e8a..baf1042f 100755 --- a/tests/installed/nondestructive/itest-pull-space.sh +++ b/tests/installed/nondestructive/itest-pull-space.sh @@ -37,9 +37,6 @@ fi assert_file_has_content err.txt "min-free-space-size" echo "ok min-free-space-size (error)" -umount mnt -losetup -d ${blkdev} -rm testblk.img # min-free-space-size success ostree --repo=repo init --mode=bare-user @@ -48,4 +45,67 @@ echo 'min-free-space-size=1MB' >> repo/config ostree --repo=repo pull-local /ostree/repo ${host_commit} echo "ok min-free-space-size (success)" +rm -rf mnt/repo + +# Test min-free-space-size on deltas + +#helper function copied from test-delta.sh +get_assert_one_direntry_matching() { + local path=$1 + local r=$2 + local child="" + local bn + for p in ${path}/*; do + bn=$(basename $p) + if ! echo ${bn} | grep -q "$r"; then + continue + fi + if test -z "${child}"; then + child=${bn} + else + assert_not_reached "Expected only one child matching ${r} in ${path}"; + fi + done + if test -z "${child}"; then + assert_not_reached "Failed to find child matching ${r}" + fi + echo ${child} +} + +mkdir mnt/repo1 mnt/repo2 +ostree --repo=mnt/repo1 init --mode=bare-user +ostree --repo=mnt/repo2 init --mode=bare-user +mkdir files +echo "hello" >> files/test.txt +truncate -s 2MB files/test.txt + +host_commit=$(ostree --repo=mnt/repo1 commit -b test -s test --tree=dir=files) + +origrev=$(ostree --repo=mnt/repo1 rev-parse test) +ostree --repo=mnt/repo1 static-delta generate --empty --to=${origrev} +echo 'fsync=false' >> mnt/repo2/config +echo 'min-free-space-size=12MB' >> mnt/repo2/config + +deltaprefix=$(get_assert_one_direntry_matching mnt/repo1/deltas '.') +deltadir=$(get_assert_one_direntry_matching mnt/repo1/deltas/${deltaprefix} '.') + +# Try to pull delta and trigger error +if ostree --repo=mnt/repo2 static-delta apply-offline mnt/repo1/deltas/${deltaprefix}/${deltadir} 2>err.txt; then + fatal "succeeded in doing a delta pull with no free space" +fi +assert_file_has_content err.txt "min-free-space-size" +echo "OK min-free-space-size delta pull (error)" + +# Re-adjust min-free-space-size so that delta pull succeeds +sed -i s/min-free-space-size=12MB/min-free-space-size=1MB/g mnt/repo2/config +rm -rf mnt/repo2/deltas +ostree --repo=mnt/repo2 static-delta apply-offline mnt/repo1/deltas/${deltaprefix}/${deltadir} + +echo "OK min-free-space-size delta pull (success)" +rm -rf files + +umount mnt +losetup -d ${blkdev} +rm testblk.img + date From e120a6b1198aaa785533c76316898f15a804dae1 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Fri, 22 Jun 2018 01:10:55 +0100 Subject: [PATCH 05/47] avahi: Fail immediately if we can't talk to D-Bus or Avahi We special-case AVAHI_ERR_NO_DAEMON to not cause warnings, but if we pass AVAHI_CLIENT_NO_FAIL to avahi_client_new, we never actually see AVAHI_ERR_NO_DAEMON. Instead, we will get AVAHI_ERR_BAD_STATE when we try to use the client. Closes: #1618 Signed-off-by: Simon McVittie Closes: #1639 Approved by: cgwalters --- src/libostree/ostree-repo-finder-avahi.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libostree/ostree-repo-finder-avahi.c b/src/libostree/ostree-repo-finder-avahi.c index 223d8f0a..bc383769 100644 --- a/src/libostree/ostree-repo-finder-avahi.c +++ b/src/libostree/ostree-repo-finder-avahi.c @@ -1432,8 +1432,7 @@ ostree_repo_finder_avahi_start (OstreeRepoFinderAvahi *self, g_assert (self->client == NULL); - client = avahi_client_new (avahi_glib_poll_get (self->poll), - AVAHI_CLIENT_NO_FAIL, + client = avahi_client_new (avahi_glib_poll_get (self->poll), 0, client_cb, self, &failure); if (client == NULL) From ca8571a49b1719887b06017a6c77798bb7f3bb6b Mon Sep 17 00:00:00 2001 From: William Manley Date: Fri, 22 Jun 2018 11:37:37 +0100 Subject: [PATCH 06/47] OstreeMutableTree: Document each private member of `OstreeMutableTree` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A prelude to my understanding. Unfortunately `OstreeMutableTree` provides little encapsulation, as each member has setters† so it's difficult to come up with a list of invariants. † `files` and `subdirs` only have getters, but the getters return mutable references to the internals, so we still can't reason about invariants. Closes: #1645 Approved by: jlebon --- src/libostree/ostree-mutable-tree.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/libostree/ostree-mutable-tree.c b/src/libostree/ostree-mutable-tree.c index 5cff5d82..f46e36db 100644 --- a/src/libostree/ostree-mutable-tree.c +++ b/src/libostree/ostree-mutable-tree.c @@ -47,10 +47,20 @@ struct OstreeMutableTree { GObject parent_instance; + /* This is the checksum of the Dirtree object that corresponds to the current + * contents of this directory. contents_checksum can be NULL if the SHA was + * never calculated or contents of the mtree has been modified. Even if + * contents_checksum is not NULL it may be out of date. */ char *contents_checksum; + + /* This is the checksum of the DirMeta object that holds the uid, gid, mode + * and xattrs of this directory. This can be NULL. */ char *metadata_checksum; + /* const char* filename -> const char* checksum */ GHashTable *files; + + /* const char* filename -> OstreeMutableTree* subtree */ GHashTable *subdirs; }; From 5190f1df424a52fe84944852760fdb5be448320e Mon Sep 17 00:00:00 2001 From: William Manley Date: Fri, 22 Jun 2018 12:00:29 +0100 Subject: [PATCH 07/47] OstreeMutableTree: Document ostree_mutable_tree_ensure_dir Closes: #1645 Approved by: jlebon --- src/libostree/ostree-mutable-tree.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/libostree/ostree-mutable-tree.c b/src/libostree/ostree-mutable-tree.c index f46e36db..e5e282d4 100644 --- a/src/libostree/ostree-mutable-tree.c +++ b/src/libostree/ostree-mutable-tree.c @@ -186,6 +186,16 @@ ostree_mutable_tree_replace_file (OstreeMutableTree *self, return ret; } +/** + * ostree_mutable_tree_ensure_dir: + * @self: Tree + * @name: Name of subdirectory of self to retrieve/creates + * @out_subdir: (out) (transfer full): the subdirectory + * @error: a #GError + * + * Returns the subdirectory of self with filename @name, creating an empty one + * it if it doesn't exist. + */ gboolean ostree_mutable_tree_ensure_dir (OstreeMutableTree *self, const char *name, From 5d031ae78b2e6b6da57fd6dca2171e2471a04586 Mon Sep 17 00:00:00 2001 From: William Manley Date: Sat, 23 Jun 2018 13:11:07 +0100 Subject: [PATCH 08/47] Add test for composing trees in different ways In preparation for adding `ostree commit` optimisations. Closes: #1645 Approved by: jlebon --- tests/basic-test.sh | 48 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/tests/basic-test.sh b/tests/basic-test.sh index e0ed2c32..a0c2f1f7 100644 --- a/tests/basic-test.sh +++ b/tests/basic-test.sh @@ -435,6 +435,54 @@ $OSTREE checkout test3-combined checkout-test3-combined assert_has_file checkout-test3-combined/file-a assert_has_file checkout-test3-combined/file-b +mkdir -p tree-C/usr/share tree-C/usr/bin tree-C/etc tree-D/etc + +echo exe >tree-C/usr/bin/exe +echo sudoers1 >tree-C/etc/sudoers +echo mtab >tree-C/etc/mtab + +echo sudoers2 >tree-D/etc/sudoers + +$OSTREE commit ${COMMIT_ARGS} -b test3-C1 --tree=dir=tree-C +$OSTREE commit ${COMMIT_ARGS} -b test3-D --tree=dir=tree-D + +echo sudoers2 >tree-C/etc/sudoers +$OSTREE commit ${COMMIT_ARGS} -b test3-C2 --tree=dir=tree-C +echo sudoers1 >tree-C/etc/sudoers + +$OSTREE commit ${COMMIT_ARGS} -b test3-ref-ref --tree=ref=test3-C1 --tree=ref=test3-D +$OSTREE commit ${COMMIT_ARGS} -b test3-dir-ref --tree=dir=tree-C --tree=ref=test3-D +$OSTREE commit ${COMMIT_ARGS} -b test3-ref-dir --tree=ref=test3-C1 --tree=dir=tree-D +$OSTREE commit ${COMMIT_ARGS} -b test3-dir-dir --tree=dir=tree-C --tree=dir=tree-D + +assert_trees_identical() { + $OSTREE diff "$1" "$2" > "diff-$1-$2" + cat "diff-$1-$2" 1>&2 + assert_file_empty "diff-$1-$2" + rm "diff-$1-$2" +} + +for x in ref dir +do + for y in ref dir + do + assert_trees_identical test3-C2 "test3-$x-$y" + done +done + +# Regression test + +mkdir -p tree-E/etc +mkdir -p tree-F/etc/apt/sources.list.d/ +echo contents >tree-F/etc/apt/sources.list.d/universe.list + +$OSTREE commit ${COMMIT_ARGS} -b test3-E --tree=dir=tree-E +$OSTREE commit ${COMMIT_ARGS} -b test3-F --tree=dir=tree-F + +$OSTREE commit ${COMMIT_ARGS} -b test3-F2 --tree=ref=test3-E --tree=ref=test3-F + +assert_trees_identical test3-F test3-F2 + echo "ok commit combined ref trees" # NB: The + is optional, but we need to make sure we support it From f1133a2a0a47f6b206984eb1f840c84865ee5fb9 Mon Sep 17 00:00:00 2001 From: Matthew Leeds Date: Mon, 25 Jun 2018 22:50:58 -0700 Subject: [PATCH 09/47] man/ostree.repo-config: Document collection-id The collection-id option in the core section was recently made public but not documented. Closes: #1646 Approved by: cgwalters --- man/ostree.repo-config.xml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/man/ostree.repo-config.xml b/man/ostree.repo-config.xml index 5a95cafa..09312d5c 100644 --- a/man/ostree.repo-config.xml +++ b/man/ostree.repo-config.xml @@ -161,6 +161,15 @@ Boston, MA 02111-1307, USA. + + collection-id + A reverse DNS domain name under your control, which enables peer + to peer distribution of refs in this repository. See the + --collection-id section in + ostree-init1 + + + From 087ad1167aa58e841baa159a5a62ff72904588d7 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 27 Jun 2018 08:17:31 -0400 Subject: [PATCH 10/47] ci: Workaround getfedora.org/atomic_qcow2_latest being 404 Closes: #1652 Approved by: cgwalters --- ci/fah28-insttests.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ci/fah28-insttests.sh b/ci/fah28-insttests.sh index a045cf52..22560bec 100755 --- a/ci/fah28-insttests.sh +++ b/ci/fah28-insttests.sh @@ -4,5 +4,6 @@ set -xeuo pipefail ./tests/installed/provision.sh # TODO: enhance papr to have caching, a bit like https://docs.travis-ci.com/user/caching/ cd tests/installed -curl -Lo fedora-atomic-host.qcow2 https://getfedora.org/atomic_qcow2_latest +# This should be https://getfedora.org/atomic_qcow2_latest but that's broken +curl -Lo fedora-atomic-host.qcow2 https://kojipkgs.fedoraproject.org/compose/twoweek/Fedora-Atomic-28-20180626.0/compose/AtomicHost/x86_64/images/Fedora-AtomicHost-28-20180626.0.x86_64.qcow2 exec env "TEST_SUBJECTS=$(pwd)/fedora-atomic-host.qcow2" ./run.sh From 8dff04601b718869e67ac7f0ed8d535f89bedb58 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Mon, 25 Jun 2018 10:08:21 -0400 Subject: [PATCH 11/47] tests/installed: Wait a bit more for http.server And also print out the output if it still didn't start up in case there are error messages hidden in there. This should hopefully help with diagnosing the flakes we've been seeing in starting it up. Closes: #1652 Approved by: cgwalters --- tests/installed/libinsttest.sh | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/installed/libinsttest.sh b/tests/installed/libinsttest.sh index 1c192b8e..edf358f2 100644 --- a/tests/installed/libinsttest.sh +++ b/tests/installed/libinsttest.sh @@ -58,7 +58,8 @@ run_tmp_webserver() { cd - child_pid=$! - for x in $(seq 10); do + for x in $(seq 60); do + echo "Waiting for web server ($x/60)..." >&2 # Snapshot the output cp ${test_tmpdir}/httpd-output{,.tmp} # If it's non-empty, see whether it matches our regexp @@ -71,6 +72,12 @@ run_tmp_webserver() { fi sleep 1 done + + if [ ! -f ${test_tmpdir}/httpd-port ]; then + cat ${test_tmpdir}/httpd-output + fatal "can't start up httpd" + fi + port=$(cat ${test_tmpdir}/httpd-port) echo "http://127.0.0.1:${port}" > ${test_tmpdir}/httpd-address echo "$child_pid" > ${test_tmpdir}/httpd-pid From 25e17e0b62e26a0ca82cd2a42d06ddd04274d475 Mon Sep 17 00:00:00 2001 From: Javier Martinez Canillas Date: Wed, 27 Jun 2018 12:19:26 +0200 Subject: [PATCH 12/47] ostree-grub-generator: sort BLS files by version instead of alphabetically The ostree-grub-generator populates the grub.cfg menu entries using the BLS config files. But it uses the ls command that by default sorts the entries alphabetically, so the order won't be correct if there are more than 10 deployments, i.e: $ ls -1 /boot/loader/entries/ ostree-fedora-workstation-0.conf ostree-fedora-workstation-10.conf ostree-fedora-workstation-1.conf ... So instead the -v option should be used to make ls use version sorting: $ ls -1 -v /boot/loader/entries/ ostree-fedora-workstation-0.conf ostree-fedora-workstation-1.conf ... ostree-fedora-workstation-10.conf Signed-off-by: Javier Martinez Canillas Closes: #1653 Approved by: cgwalters --- src/boot/grub2/ostree-grub-generator | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/boot/grub2/ostree-grub-generator b/src/boot/grub2/ostree-grub-generator index 97ef4d69..bfae55a5 100644 --- a/src/boot/grub2/ostree-grub-generator +++ b/src/boot/grub2/ostree-grub-generator @@ -66,7 +66,7 @@ populate_menu() else boot_prefix="${OSTREE_BOOT_PARTITION}" fi - for config in $(ls $entries_path/*.conf); do + for config in $(ls -v $entries_path/*.conf); do read_config ${config} menu="${menu}menuentry '${title}' {\n" menu="${menu}\t linux ${boot_prefix}${linux} ${options}\n" From 47ae4f5c7e9ed72b79b39df71b620bf6ace2b88a Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Sun, 24 Jun 2018 12:56:49 +0100 Subject: [PATCH 13/47] OstreeRepoFinderConfig: Fix guint/gsize confusion If a function has a guint "out argument", passing a pointer to a gsize is not, in general, valid. On an ILP64 platform there is no problem since guint and gsize are identical, but on an LP64 platform it will overwrite only the first word of the gsize, leaving the second word unaffected. On little-endian machines, if the second word is zero-initialized (as it is here), the result is numerically equal to the guint, but on big-endian machines the result is around 4 billion times what it should be, resulting in ostree_repo_finder_config_resolve_async() reading past the end of the array and causing undefined behaviour. In practice this caused assertion failures (and consequently test failures) on Debian's s390x (z/Architecture), ppc64 (64-bit PowerPC) and sparc64 (64-bit SPARC) ports. Closes: #1640 Signed-off-by: Simon McVittie Closes: #1641 Approved by: cgwalters --- src/libostree/ostree-repo-finder-config.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libostree/ostree-repo-finder-config.c b/src/libostree/ostree-repo-finder-config.c index 4366d72a..06f61657 100644 --- a/src/libostree/ostree-repo-finder-config.c +++ b/src/libostree/ostree-repo-finder-config.c @@ -96,7 +96,7 @@ ostree_repo_finder_config_resolve_async (OstreeRepoFinder *find GHashTableIter iter; const gchar *remote_name; g_auto(GStrv) remotes = NULL; - gsize n_remotes = 0; + guint n_remotes = 0; task = g_task_new (finder, cancellable, callback, user_data); g_task_set_source_tag (task, ostree_repo_finder_config_resolve_async); @@ -106,9 +106,9 @@ ostree_repo_finder_config_resolve_async (OstreeRepoFinder *find /* List all remotes in this #OstreeRepo and see which of their ref lists * intersect with @refs. */ - remotes = ostree_repo_remote_list (parent_repo, (guint *) &n_remotes); + remotes = ostree_repo_remote_list (parent_repo, &n_remotes); - g_debug ("%s: Checking %" G_GSIZE_FORMAT " remotes", G_STRFUNC, n_remotes); + g_debug ("%s: Checking %u remotes", G_STRFUNC, n_remotes); for (i = 0; i < n_remotes; i++) { From 9f48e212a3bf9ed418fb3216e4f834d581bc520e Mon Sep 17 00:00:00 2001 From: Javier Martinez Canillas Date: Wed, 27 Jun 2018 16:45:43 +0200 Subject: [PATCH 14/47] deploy: Change BootLoaderSpec filenames so they can be used for sorting Currently the BLS snippets are named ostree-$ID-$VARIANT_ID-$index.conf, but the BLS config files are actually sorted by using the version field which is the inverse of the index. In most places, _ostree_sysroot_read_boot_loader_configs() is used to get the BLS files and this function already returns them sorted by the version field. The only place where the index trailing number is used is in the ostree-grub-generator script that lists the BLS files to populate the grub config file. But for some bootloaders the BLS filename is the criteria for sorting by taking the filename as a string version. So on these bootloaders the BLS entries will be listed in the reverse order. To avoid that, change the BLS snippets filename to have the version field instead of the index and also to have the version before deployment name. Make the filenames to be of the form ostree-$version-$ID-$VARIANT_ID.conf so the version is before the deployment name. Signed-off-by: Javier Martinez Canillas Closes: #1654 Approved by: cgwalters --- src/boot/grub2/ostree-grub-generator | 2 +- src/libostree/ostree-sysroot-deploy.c | 5 ++-- tests/admin-test.sh | 36 +++++++++++++------------- tests/test-admin-deploy-2.sh | 4 +-- tests/test-admin-deploy-karg.sh | 18 ++++++------- tests/test-admin-deploy-syslinux.sh | 6 ++--- tests/test-admin-instutil-set-kargs.sh | 22 ++++++++-------- tests/test-admin-pull-deploy-split.sh | 4 +-- tests/test-admin-upgrade-endoflife.sh | 2 +- tests/test-no-initramfs.sh | 26 +++++++++---------- 10 files changed, 63 insertions(+), 62 deletions(-) diff --git a/src/boot/grub2/ostree-grub-generator b/src/boot/grub2/ostree-grub-generator index bfae55a5..5b7ea1ab 100644 --- a/src/boot/grub2/ostree-grub-generator +++ b/src/boot/grub2/ostree-grub-generator @@ -66,7 +66,7 @@ populate_menu() else boot_prefix="${OSTREE_BOOT_PARTITION}" fi - for config in $(ls -v $entries_path/*.conf); do + for config in $(ls -v -r $entries_path/*.conf); do read_config ${config} menu="${menu}menuentry '${title}' {\n" menu="${menu}\t linux ${boot_prefix}${linux} ${options}\n" diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index 08bf4c0d..42128e72 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -1628,8 +1628,9 @@ install_deployment_kernel (OstreeSysroot *sysroot, g_assert_cmpstr (kernel_layout->bootcsum, ==, bootcsum); g_autofree char *bootcsumdir = g_strdup_printf ("ostree/%s-%s", osname, bootcsum); g_autofree char *bootconfdir = g_strdup_printf ("loader.%d/entries", new_bootversion); - g_autofree char *bootconf_name = g_strdup_printf ("ostree-%s-%d.conf", osname, - ostree_deployment_get_index (deployment)); + g_autofree char *bootconf_name = g_strdup_printf ("ostree-%d-%s.conf", + n_deployments - ostree_deployment_get_index (deployment), + osname); if (!glnx_shutil_mkdir_p_at (boot_dfd, bootcsumdir, 0775, cancellable, error)) return FALSE; diff --git a/tests/admin-test.sh b/tests/admin-test.sh index 7384d8f3..4f0b9e93 100644 --- a/tests/admin-test.sh +++ b/tests/admin-test.sh @@ -68,9 +68,9 @@ echo "ok --print-current-dir" assert_not_has_dir sysroot/boot/loader.0 assert_has_dir sysroot/boot/loader.1 assert_has_dir sysroot/ostree/boot.1.1 -assert_has_file sysroot/boot/loader/entries/ostree-testos-0.conf -assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'options.* root=LABEL=MOO' -assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'options.* quiet' +assert_has_file sysroot/boot/loader/entries/ostree-1-testos.conf +assert_file_has_content sysroot/boot/loader/entries/ostree-1-testos.conf 'options.* root=LABEL=MOO' +assert_file_has_content sysroot/boot/loader/entries/ostree-1-testos.conf 'options.* quiet' assert_file_has_content sysroot/boot/ostree/testos-${bootcsum}/vmlinuz-3.6.0 'a kernel' assert_file_has_content sysroot/ostree/deploy/testos/deploy/${rev}.0/etc/os-release 'NAME=TestOS' assert_file_has_content sysroot/ostree/boot.1/testos/${bootcsum}/0/etc/os-release 'NAME=TestOS' @@ -96,7 +96,7 @@ assert_not_has_dir sysroot/ostree/boot.0.0 assert_not_has_dir sysroot/ostree/boot.1.0 assert_not_has_dir sysroot/ostree/boot.1.1 # Ensure we propagated kernel arguments from previous deployment -assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'options.* root=LABEL=MOO' +assert_file_has_content sysroot/boot/loader/entries/ostree-2-testos.conf 'options.* root=LABEL=MOO' assert_file_has_content sysroot/ostree/deploy/testos/deploy/${rev}.1/etc/os-release 'NAME=TestOS' assert_file_has_content sysroot/ostree/boot.0/testos/${bootcsum}/0/etc/os-release 'NAME=TestOS' assert_ostree_deployment_refs 0/1/{0,1} @@ -123,8 +123,8 @@ ${CMD_PREFIX} ostree admin os-init otheros ${CMD_PREFIX} ostree admin deploy --os=otheros testos/buildmaster/x86_64-runtime assert_not_has_dir sysroot/boot/loader.0 assert_has_dir sysroot/boot/loader.1 -assert_has_file sysroot/boot/loader/entries/ostree-testos-1.conf -assert_has_file sysroot/boot/loader/entries/ostree-otheros-0.conf +assert_has_file sysroot/boot/loader/entries/ostree-2-testos.conf +assert_has_file sysroot/boot/loader/entries/ostree-3-otheros.conf assert_file_has_content sysroot/ostree/deploy/testos/deploy/${rev}.1/etc/os-release 'NAME=TestOS' assert_file_has_content sysroot/ostree/deploy/otheros/deploy/${rev}.0/etc/os-release 'NAME=TestOS' assert_ostree_deployment_refs 1/1/{0,1,2} @@ -136,9 +136,9 @@ echo "ok independent deploy" ${CMD_PREFIX} ostree admin deploy --retain --os=testos testos:testos/buildmaster/x86_64-runtime assert_has_dir sysroot/boot/loader.0 assert_not_has_dir sysroot/boot/loader.1 -assert_has_file sysroot/boot/loader/entries/ostree-testos-0.conf +assert_has_file sysroot/boot/loader/entries/ostree-4-testos.conf assert_file_has_content sysroot/ostree/deploy/testos/deploy/${rev}.2/etc/os-release 'NAME=TestOS' -assert_has_file sysroot/boot/loader/entries/ostree-testos-2.conf +assert_has_file sysroot/boot/loader/entries/ostree-2-testos.conf assert_file_has_content sysroot/ostree/deploy/testos/deploy/${rev}.3/etc/os-release 'NAME=TestOS' ${CMD_PREFIX} ostree admin status assert_ostree_deployment_refs 0/1/{0,1,2,3} @@ -172,14 +172,14 @@ echo "ok deploy with modified /etc" for i in $(seq 4); do ${CMD_PREFIX} ostree admin undeploy 0 done -assert_has_file sysroot/boot/loader/entries/ostree-testos-0.conf -assert_not_has_file sysroot/boot/loader/entries/ostree-testos-1.conf -assert_not_has_file sysroot/boot/loader/entries/ostree-otheros-1.conf +assert_has_file sysroot/boot/loader/entries/ostree-1-testos.conf +assert_not_has_file sysroot/boot/loader/entries/ostree-2-testos.conf +assert_not_has_file sysroot/boot/loader/entries/ostree-3-otheros.conf ${CMD_PREFIX} ostree admin deploy --not-as-default --os=otheros testos:testos/buildmaster/x86_64-runtime assert_has_dir sysroot/boot/loader.0 assert_not_has_dir sysroot/boot/loader.1 -assert_has_file sysroot/boot/loader/entries/ostree-testos-0.conf -assert_has_file sysroot/boot/loader/entries/ostree-otheros-1.conf +assert_has_file sysroot/boot/loader/entries/ostree-2-testos.conf +assert_has_file sysroot/boot/loader/entries/ostree-1-otheros.conf ${CMD_PREFIX} ostree admin status validate_bootloader @@ -188,9 +188,9 @@ echo "ok deploy --not-as-default" ${CMD_PREFIX} ostree admin deploy --retain-rollback --os=otheros testos:testos/buildmaster/x86_64-runtime assert_not_has_dir sysroot/boot/loader.0 assert_has_dir sysroot/boot/loader.1 -assert_has_file sysroot/boot/loader/entries/ostree-otheros-0.conf -assert_has_file sysroot/boot/loader/entries/ostree-testos-1.conf -assert_has_file sysroot/boot/loader/entries/ostree-otheros-2.conf +assert_has_file sysroot/boot/loader/entries/ostree-3-otheros.conf +assert_has_file sysroot/boot/loader/entries/ostree-2-testos.conf +assert_has_file sysroot/boot/loader/entries/ostree-1-otheros.conf ${CMD_PREFIX} ostree admin status validate_bootloader @@ -265,7 +265,7 @@ echo "ok deploy with unknown OS" ${CMD_PREFIX} ostree admin deploy --os=testos --karg-append=console=/dev/foo --karg-append=console=/dev/bar testos:testos/buildmaster/x86_64-runtime ${CMD_PREFIX} ostree admin deploy --os=testos testos:testos/buildmaster/x86_64-runtime -assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'console=/dev/foo.*console=/dev/bar' +assert_file_has_content sysroot/boot/loader/entries/ostree-4-testos.conf 'console=/dev/foo.*console=/dev/bar' validate_bootloader echo "ok deploy with multiple kernel args" @@ -275,7 +275,7 @@ os_repository_new_commit 0 "test upgrade multiple kernel args" ${CMD_PREFIX} ostree admin upgrade --os=testos newrev=$(${CMD_PREFIX} ostree --repo=sysroot/ostree/repo rev-parse testos/buildmaster/x86_64-runtime) assert_not_streq ${origrev} ${newrev} -assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'console=/dev/foo.*console=/dev/bar' +assert_file_has_content sysroot/boot/loader/entries/ostree-4-testos.conf 'console=/dev/foo.*console=/dev/bar' validate_bootloader echo "ok upgrade with multiple kernel args" diff --git a/tests/test-admin-deploy-2.sh b/tests/test-admin-deploy-2.sh index 7e69ec88..3e68ecf3 100755 --- a/tests/test-admin-deploy-2.sh +++ b/tests/test-admin-deploy-2.sh @@ -71,9 +71,9 @@ oldversion=${version} # another commit with *same* bootcsum but *new* content os_repository_new_commit "1" "2" newversion=${version} -assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf ${oldversion} +assert_file_has_content sysroot/boot/loader/entries/ostree-1-testos.conf ${oldversion} ${CMD_PREFIX} ostree admin upgrade --os=testos -assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf ${newversion} +assert_file_has_content sysroot/boot/loader/entries/ostree-2-testos.conf ${newversion} echo "ok new version same bootcsum" diff --git a/tests/test-admin-deploy-karg.sh b/tests/test-admin-deploy-karg.sh index 7dcf7dbd..aade011c 100755 --- a/tests/test-admin-deploy-karg.sh +++ b/tests/test-admin-deploy-karg.sh @@ -35,12 +35,12 @@ export rev ${CMD_PREFIX} ostree admin deploy --karg=root=LABEL=MOO --karg=quiet --os=testos testos:testos/buildmaster/x86_64-runtime ${CMD_PREFIX} ostree admin deploy --karg=FOO=BAR --os=testos testos:testos/buildmaster/x86_64-runtime ${CMD_PREFIX} ostree admin deploy --karg=TESTARG=TESTVALUE --os=testos testos:testos/buildmaster/x86_64-runtime -assert_file_has_content sysroot/boot/loader/entries/ostree-testos-1.conf 'options.*FOO=BAR' -assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'options.*FOO=BAR' -assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'options.*TESTARG=TESTVALUE' +assert_file_has_content sysroot/boot/loader/entries/ostree-1-testos.conf 'options.*FOO=BAR' +assert_file_has_content sysroot/boot/loader/entries/ostree-2-testos.conf 'options.*FOO=BAR' +assert_file_has_content sysroot/boot/loader/entries/ostree-2-testos.conf 'options.*TESTARG=TESTVALUE' ${CMD_PREFIX} ostree admin deploy --karg=ANOTHERARG=ANOTHERVALUE --os=testos testos:testos/buildmaster/x86_64-runtime -assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'options.*TESTARG=TESTVALUE' -assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'options.*ANOTHERARG=ANOTHERVALUE' +assert_file_has_content sysroot/boot/loader/entries/ostree-2-testos.conf 'options.*TESTARG=TESTVALUE' +assert_file_has_content sysroot/boot/loader/entries/ostree-2-testos.conf 'options.*ANOTHERARG=ANOTHERVALUE' echo "ok deploy with --karg, but same config" @@ -51,7 +51,7 @@ for arg in $(cat /proc/cmdline); do ;; initrd=*|BOOT_IMAGE=*) # Skip options set by bootloader that gets filtered out ;; - *) assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf "options.*$arg" + *) assert_file_has_content sysroot/boot/loader/entries/ostree-2-testos.conf "options.*$arg" ;; esac done @@ -62,8 +62,8 @@ ${CMD_PREFIX} ostree admin status ${CMD_PREFIX} ostree admin undeploy 0 ${CMD_PREFIX} ostree admin deploy --os=testos --karg-append=APPENDARG=VALAPPEND --karg-append=APPENDARG=2NDAPPEND testos:testos/buildmaster/x86_64-runtime -assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'options.*FOO=BAR' -assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'options.*TESTARG=TESTVALUE' -assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'options.*APPENDARG=VALAPPEND .*APPENDARG=2NDAPPEND' +assert_file_has_content sysroot/boot/loader/entries/ostree-2-testos.conf 'options.*FOO=BAR' +assert_file_has_content sysroot/boot/loader/entries/ostree-2-testos.conf 'options.*TESTARG=TESTVALUE' +assert_file_has_content sysroot/boot/loader/entries/ostree-2-testos.conf 'options.*APPENDARG=VALAPPEND .*APPENDARG=2NDAPPEND' echo "ok deploy --karg-append" diff --git a/tests/test-admin-deploy-syslinux.sh b/tests/test-admin-deploy-syslinux.sh index 46651763..4d4ac7ae 100755 --- a/tests/test-admin-deploy-syslinux.sh +++ b/tests/test-admin-deploy-syslinux.sh @@ -38,8 +38,8 @@ for test_bootdir in "boot" "usr/lib/ostree-boot"; do ${CMD_PREFIX} ostree --repo=sysroot/ostree/repo pull-local --remote=testos testos-repo testos/buildmaster/x86_64-runtime rev=$(${CMD_PREFIX} ostree --repo=sysroot/ostree/repo rev-parse testos/buildmaster/x86_64-runtime) ${CMD_PREFIX} ostree admin deploy --karg=root=LABEL=MOO --karg=quiet --os=testos testos:testos/buildmaster/x86_64-runtime - assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'options.* root=LABEL=MOO' - assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'options.* quiet' + assert_file_has_content sysroot/boot/loader/entries/ostree-1-testos.conf 'options.* root=LABEL=MOO' + assert_file_has_content sysroot/boot/loader/entries/ostree-1-testos.conf 'options.* quiet' assert_file_has_content sysroot/boot/ostree/testos-${bootcsum}/vmlinuz-3.6.0 'a kernel' assert_file_has_content sysroot/boot/ostree/testos-${bootcsum}/initramfs-3.6.0.img 'an initramfs' # kernel/initrams should also be in the tree's /boot with the checksum @@ -64,7 +64,7 @@ cd ${test_tmpdir} ${CMD_PREFIX} ostree --repo=sysroot/ostree/repo pull-local --remote=testos testos-repo testos/buildmaster/x86_64-runtime rev=$(${CMD_PREFIX} ostree --repo=sysroot/ostree/repo rev-parse testos/buildmaster/x86_64-runtime) ${CMD_PREFIX} ostree admin deploy --karg=root=LABEL=MOO --karg=quiet --os=testos testos:testos/buildmaster/x86_64-runtime -assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'options.* root=LABEL=MOO' +assert_file_has_content sysroot/boot/loader/entries/ostree-1-testos.conf 'options.* root=LABEL=MOO' assert_file_has_content sysroot/boot/ostree/testos-${bootcsum}/vmlinuz-3.6.0 'a kernel' assert_file_has_content sysroot/boot/ostree/testos-${bootcsum}/initramfs-3.6.0.img 'an initramfs' # Note this bootcsum shouldn't be the modules one diff --git a/tests/test-admin-instutil-set-kargs.sh b/tests/test-admin-instutil-set-kargs.sh index 9bfd2a08..5568f238 100755 --- a/tests/test-admin-instutil-set-kargs.sh +++ b/tests/test-admin-instutil-set-kargs.sh @@ -34,25 +34,25 @@ ${CMD_PREFIX} ostree admin deploy --os=testos testos:testos/buildmaster/x86_64-r ${CMD_PREFIX} ostree admin instutil set-kargs FOO=BAR ${CMD_PREFIX} ostree admin instutil set-kargs FOO=BAZ FOO=BIF TESTARG=TESTVALUE -assert_not_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'options.*FOO=BAR' -assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'options.*FOO=BAZ .*FOO=BIF' -assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'options.*TESTARG=TESTVALUE' +assert_not_file_has_content sysroot/boot/loader/entries/ostree-1-testos.conf 'options.*FOO=BAR' +assert_file_has_content sysroot/boot/loader/entries/ostree-1-testos.conf 'options.*FOO=BAZ .*FOO=BIF' +assert_file_has_content sysroot/boot/loader/entries/ostree-1-testos.conf 'options.*TESTARG=TESTVALUE' echo "ok instutil set-kargs (basic)" ${CMD_PREFIX} ostree admin instutil set-kargs --merge FOO=BAR -assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'options.*FOO=BAZ .*FOO=BIF .*FOO=BAR' -assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'options.*TESTARG=TESTVALUE' +assert_file_has_content sysroot/boot/loader/entries/ostree-1-testos.conf 'options.*FOO=BAZ .*FOO=BIF .*FOO=BAR' +assert_file_has_content sysroot/boot/loader/entries/ostree-1-testos.conf 'options.*TESTARG=TESTVALUE' echo "ok instutil set-kargs --merge" ${CMD_PREFIX} ostree admin instutil set-kargs --merge --replace=FOO=XXX -assert_not_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'options.*FOO=BAR' -assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'options.*FOO=XXX' -assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'options.*TESTARG=TESTVALUE' +assert_not_file_has_content sysroot/boot/loader/entries/ostree-1-testos.conf 'options.*FOO=BAR' +assert_file_has_content sysroot/boot/loader/entries/ostree-1-testos.conf 'options.*FOO=XXX' +assert_file_has_content sysroot/boot/loader/entries/ostree-1-testos.conf 'options.*TESTARG=TESTVALUE' echo "ok instutil set-kargs --replace" ${CMD_PREFIX} ostree admin instutil set-kargs --merge --append=FOO=BAR --append=APPENDARG=VALAPPEND --append=APPENDARG=2NDAPPEND testos:testos/buildmaster/x86_64-runtime -assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'options.*FOO=XXX.*FOO=BAR' -assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'options.*APPENDARG=VALAPPEND .*APPENDARG=2NDAPPEND' +assert_file_has_content sysroot/boot/loader/entries/ostree-1-testos.conf 'options.*FOO=XXX.*FOO=BAR' +assert_file_has_content sysroot/boot/loader/entries/ostree-1-testos.conf 'options.*APPENDARG=VALAPPEND .*APPENDARG=2NDAPPEND' echo "ok instutil set-kargs --append" ${CMD_PREFIX} ostree admin instutil set-kargs --import-proc-cmdline @@ -62,7 +62,7 @@ for arg in $(cat /proc/cmdline); do ;; initrd=*|BOOT_IMAGE=*) # Skip options set by bootloader that gets filtered out ;; - *) assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf "options.*$arg" + *) assert_file_has_content sysroot/boot/loader/entries/ostree-1-testos.conf "options.*$arg" ;; esac done diff --git a/tests/test-admin-pull-deploy-split.sh b/tests/test-admin-pull-deploy-split.sh index 602cdd6c..5e38d6fa 100755 --- a/tests/test-admin-pull-deploy-split.sh +++ b/tests/test-admin-pull-deploy-split.sh @@ -46,13 +46,13 @@ ${CMD_PREFIX} ostree admin upgrade --os=testos --pull-only --os=testos > out.txt assert_not_file_has_content out.txt 'No update available' assert_has_dir sysroot/ostree/deploy/testos/deploy/${parent_rev}.0 assert_not_has_dir sysroot/ostree/deploy/testos/deploy/${rev}.0 -assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'TestOS 42 1.0.9' +assert_file_has_content sysroot/boot/loader/entries/ostree-1-testos.conf 'TestOS 42 1.0.9' assert_streq "${rev}" $(${CMD_PREFIX} ostree --repo=sysroot/ostree/repo rev-parse testos/buildmaster/x86_64-runtime) # Now, generate new content upstream; we shouldn't pull it os_repository_new_commit ${CMD_PREFIX} ostree admin upgrade --os=testos --deploy-only --os=testos > out.txt assert_not_file_has_content out.txt 'No update available' -assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'TestOS 42 1.0.10' +assert_file_has_content sysroot/boot/loader/entries/ostree-2-testos.conf 'TestOS 42 1.0.10' assert_has_dir sysroot/ostree/deploy/testos/deploy/${parent_rev}.0 assert_has_dir sysroot/ostree/deploy/testos/deploy/${rev}.0 ${CMD_PREFIX} ostree admin upgrade --os=testos --deploy-only --os=testos > out.txt diff --git a/tests/test-admin-upgrade-endoflife.sh b/tests/test-admin-upgrade-endoflife.sh index b8a091f4..8a0dc9c2 100755 --- a/tests/test-admin-upgrade-endoflife.sh +++ b/tests/test-admin-upgrade-endoflife.sh @@ -65,7 +65,7 @@ ${CMD_PREFIX} ostree admin upgrade --os=testos --pull-only ${CMD_PREFIX} ostree admin upgrade --os=testos --deploy-only # Check we got redirected to the new branch -assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf "${bootcsum}" +assert_file_has_content sysroot/boot/loader/entries/ostree-2-testos.conf "${bootcsum}" rev=$(${CMD_PREFIX} ostree --repo=${test_tmpdir}/testos-repo rev-parse testos/buildmaster/newbranch) assert_file_has_content sysroot/ostree/deploy/testos/deploy/${rev}.0/usr/bin/content-iteration "1" diff --git a/tests/test-no-initramfs.sh b/tests/test-no-initramfs.sh index 52bb9d98..60e1f9a7 100755 --- a/tests/test-no-initramfs.sh +++ b/tests/test-no-initramfs.sh @@ -12,8 +12,8 @@ ${CMD_PREFIX} ostree --repo=sysroot/ostree/repo remote add --set=gpg-verify=fals ${CMD_PREFIX} ostree --repo=sysroot/ostree/repo pull testos testos/buildmaster/x86_64-runtime ${CMD_PREFIX} ostree admin deploy --karg=root=LABEL=rootfs --os=testos testos:testos/buildmaster/x86_64-runtime -assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'root=LABEL=rootfs' -assert_not_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'init=' +assert_file_has_content sysroot/boot/loader/entries/ostree-1-testos.conf 'root=LABEL=rootfs' +assert_not_file_has_content sysroot/boot/loader/entries/ostree-1-testos.conf 'init=' echo "ok deployment with initramfs" @@ -63,28 +63,28 @@ for layout in /usr/lib/modules /usr/lib/ostree-boot /boot; do pull_test_tree "the kernel only" ${CMD_PREFIX} ostree admin deploy --os=testos --karg=root=/dev/sda2 --karg=rootwait testos:testos/buildmaster/x86_64-runtime - assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'rootwait' - assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'init=' - assert_not_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'initrd' + assert_file_has_content sysroot/boot/loader/entries/ostree-2-testos.conf 'rootwait' + assert_file_has_content sysroot/boot/loader/entries/ostree-2-testos.conf 'init=' + assert_not_file_has_content sysroot/boot/loader/entries/ostree-2-testos.conf 'initrd' echo "ok switching to bootdir with no initramfs layout=$layout" pull_test_tree "the kernel" "initramfs to assist the kernel" ${CMD_PREFIX} ostree admin deploy --os=testos --karg-none --karg=root=LABEL=rootfs testos:testos/buildmaster/x86_64-runtime - assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'initrd' - assert_file_has_content sysroot/boot/$(get_key_from_bootloader_conf sysroot/boot/loader/entries/ostree-testos-0.conf "initrd") "initramfs to assist the kernel" - assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'root=LABEL=rootfs' - assert_not_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'rootwait' - assert_not_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'init=' + assert_file_has_content sysroot/boot/loader/entries/ostree-2-testos.conf 'initrd' + assert_file_has_content sysroot/boot/$(get_key_from_bootloader_conf sysroot/boot/loader/entries/ostree-2-testos.conf "initrd") "initramfs to assist the kernel" + assert_file_has_content sysroot/boot/loader/entries/ostree-2-testos.conf 'root=LABEL=rootfs' + assert_not_file_has_content sysroot/boot/loader/entries/ostree-2-testos.conf 'rootwait' + assert_not_file_has_content sysroot/boot/loader/entries/ostree-2-testos.conf 'init=' echo "ok switching from no initramfs to initramfs enabled sysroot layout=$layout" pull_test_tree "the kernel" "" "my .dtb file" ${CMD_PREFIX} ostree admin deploy --os=testos testos:testos/buildmaster/x86_64-runtime - assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'init=' - assert_file_has_content sysroot/boot/"$(get_key_from_bootloader_conf sysroot/boot/loader/entries/ostree-testos-0.conf 'devicetree')" "my .dtb file" - assert_not_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'initrd' + assert_file_has_content sysroot/boot/loader/entries/ostree-2-testos.conf 'init=' + assert_file_has_content sysroot/boot/"$(get_key_from_bootloader_conf sysroot/boot/loader/entries/ostree-2-testos.conf 'devicetree')" "my .dtb file" + assert_not_file_has_content sysroot/boot/loader/entries/ostree-2-testos.conf 'initrd' echo "ok switching from initramfs to no initramfs sysroot with devicetree layout=$layout" done From 1074668ede14345678fe8220814a8fed1d432475 Mon Sep 17 00:00:00 2001 From: Umang Jain Date: Wed, 27 Jun 2018 02:41:43 +0530 Subject: [PATCH 15/47] lib/repo: cleanup_tmpdir should be executed after releasing lock file Here's a subtle bug in abort_transaction(): One of the policies of cleaning up is to skip the current boot's staging directory. The responsible function for this is cleanup_tmpdir() which tries to lock each of the tmpdir before deleting it. When it comes to the current boot's staging dir, it tries to lock the directory(again!) but fails as there is already a lockfile present. Just because the current boot's staging dir was meant to be skipped, the bug never surfaced up and wasn't catastrohpic. if (!_ostree_repo_try_lock_tmpdir (dfd, path, &lockfile, &did_lock, error)) return FALSE; if (!did_lock) return TRUE; /* Note early return */ ... if (g_str_has_prefix (path, self->stagedir_prefix)) return TRUE; /* Note early return */ The actual check for skipping staging dir for current boot was never reached because the function returned at did_lock failure. Therefore, execute cleanup_tmpdir() after releasing the lockfile in abort_transaction() so that cleanup_tmpdir gets a chance to lock current boot's staging directory and succeed. Closes: #1602 Approved by: jlebon --- src/libostree/ostree-repo-commit.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index 2516ba3d..ce2f4deb 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -2229,10 +2229,6 @@ ostree_repo_abort_transaction (OstreeRepo *self, if (!self->in_transaction) return TRUE; - /* Do not propagate failures from cleanup_tmpdir() immediately, as we want - * to clean up the rest of the internal transaction state first. */ - cleanup_tmpdir (self, cancellable, &cleanup_error); - if (self->loose_object_devino_hash) g_hash_table_remove_all (self->loose_object_devino_hash); @@ -2242,6 +2238,10 @@ ostree_repo_abort_transaction (OstreeRepo *self, glnx_tmpdir_unset (&self->commit_stagedir); glnx_release_lock_file (&self->commit_stagedir_lock); + /* Do not propagate failures from cleanup_tmpdir() immediately, as we want + * to clean up the rest of the internal transaction state first. */ + cleanup_tmpdir (self, cancellable, &cleanup_error); + self->in_transaction = FALSE; if (self->txn_locked) From d686056254294fcabff472e71ebe486ec82e070e Mon Sep 17 00:00:00 2001 From: Umang Jain Date: Wed, 27 Jun 2018 02:40:15 +0530 Subject: [PATCH 16/47] lib/repo: Cleanup current boot's staging dir min-free-space-* checks are hit min-free-space-* act as a gating condition whether to we want hold onto caches in repo/tmp. If it is found that the free-disk space is going below this threshold, we flag it as an error and cleanup current boot's staging directory. Closes: #1602 Approved by: jlebon --- src/libostree/ostree-repo-commit.c | 9 +++++++-- src/libostree/ostree-repo-private.h | 1 + 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index ce2f4deb..a3a6df44 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -522,6 +522,7 @@ _ostree_repo_bare_content_commit (OstreeRepo *self, const fsblkcnt_t object_blocks = (st_buf.st_size / self->txn.blocksize) + 1; if (object_blocks > self->txn.max_blocks) { + self->cleanup_stagedir = TRUE; g_mutex_unlock (&self->txn_lock); g_autofree char *formatted_required = g_format_size (st_buf.st_size); if (self->min_free_space_percent > 0) @@ -924,6 +925,7 @@ write_content_object (OstreeRepo *self, if (object_blocks > self->txn.max_blocks) { guint64 bytes_required = (guint64)object_blocks * self->txn.blocksize; + self->cleanup_stagedir = TRUE; g_mutex_unlock (&self->txn_lock); g_autofree char *formatted_required = g_format_size (bytes_required); if (self->min_free_space_percent > 0) @@ -1623,6 +1625,7 @@ ostree_repo_prepare_transaction (OstreeRepo *self, return FALSE; self->in_transaction = TRUE; + self->cleanup_stagedir = FALSE; if (self->min_free_space_percent >= 0 || self->min_free_space_mb >= 0) { struct statvfs stvfsbuf; @@ -1638,6 +1641,7 @@ ostree_repo_prepare_transaction (OstreeRepo *self, else { guint64 bytes_required = bfree * self->txn.blocksize; + self->cleanup_stagedir = TRUE; g_mutex_unlock (&self->txn_lock); g_autofree char *formatted_free = g_format_size (bytes_required); if (self->min_free_space_percent > 0) @@ -1783,9 +1787,10 @@ cleanup_txn_dir (OstreeRepo *self, /* If however this is the staging directory for the *current* * boot, then don't delete it now - we may end up reusing it, as - * is the point. + * is the point. Delete *only if* we have hit min-free-space* checks + * as we don't want to hold onto caches in that case. */ - if (g_str_has_prefix (path, self->stagedir_prefix)) + if (g_str_has_prefix (path, self->stagedir_prefix) && !self->cleanup_stagedir) return TRUE; /* Note early return */ /* But, crucially we can now clean up staging directories diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h index 69f53921..0a447634 100644 --- a/src/libostree/ostree-repo-private.h +++ b/src/libostree/ostree-repo-private.h @@ -152,6 +152,7 @@ struct OstreeRepo { gid_t target_owner_gid; guint min_free_space_percent; /* See the min-free-space-percent config option */ guint64 min_free_space_mb; /* See the min-free-space-size config option */ + gboolean cleanup_stagedir; guint test_error_flags; /* OstreeRepoTestErrorFlags */ From 05d8ade563d14633fb101412d78b70a9e94d3f33 Mon Sep 17 00:00:00 2001 From: Matthew Leeds Date: Wed, 27 Jun 2018 19:13:33 -0700 Subject: [PATCH 17/47] create-usb: Tweak docs for --destination-repo Make it show up in the help output as --destination-repo=DEST so it's clear that it takes an argument. Closes: #1656 Approved by: jlebon --- src/ostree/ot-builtin-create-usb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ostree/ot-builtin-create-usb.c b/src/ostree/ot-builtin-create-usb.c index 57feeaa1..eee637c8 100644 --- a/src/ostree/ot-builtin-create-usb.c +++ b/src/ostree/ot-builtin-create-usb.c @@ -37,7 +37,7 @@ static char *opt_destination_repo = NULL; static GOptionEntry options[] = { { "disable-fsync", 0, 0, G_OPTION_ARG_NONE, &opt_disable_fsync, "Do not invoke fsync()", NULL }, - { "destination-repo", 0, 0, G_OPTION_ARG_FILENAME, &opt_destination_repo, "Use custom repository directory within the mount", NULL }, + { "destination-repo", 0, 0, G_OPTION_ARG_FILENAME, &opt_destination_repo, "Use custom repository directory within the mount", "DEST" }, { NULL } }; From bab3b2bd4c45ae02c08dc0e43d38d3b542946bac Mon Sep 17 00:00:00 2001 From: William Manley Date: Thu, 28 Jun 2018 12:10:13 +0100 Subject: [PATCH 18/47] tests: Save corefiles back to tests/ directory if one exists Makes it easier to debug failures from the tests. Closes: #1657 Approved by: cgwalters --- tests/libtest.sh | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/libtest.sh b/tests/libtest.sh index f6b6eb2f..388408df 100755 --- a/tests/libtest.sh +++ b/tests/libtest.sh @@ -34,6 +34,14 @@ else fi . ${test_srcdir}/libtest-core.sh +save_core() { + if [ -e core ]; then + cp core "$test_srcdir/core" + fi +} + +trap save_core EXIT; + test_tmpdir=$(pwd) # Sanity check that we're in a tmpdir that has From 7ead3c1aa83c1282bd583e2c3e1e610225987ac5 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 28 Jun 2018 15:18:27 -0400 Subject: [PATCH 19/47] sysroot: Reject attempts to pin the staged deployment From https://github.com/projectatomic/rpm-ostree/pull/1434#discussion_r198936674 To support it we'd have to actually write it to disk, which...let's not try that right now. Closes: #1660 Approved by: jlebon --- src/libostree/ostree-sysroot.c | 6 +++++- tests/installed/destructive/staged-deploy.yml | 4 ++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/libostree/ostree-sysroot.c b/src/libostree/ostree-sysroot.c index 707b161f..4b2c654c 100644 --- a/src/libostree/ostree-sysroot.c +++ b/src/libostree/ostree-sysroot.c @@ -1894,7 +1894,8 @@ ostree_sysroot_deployment_unlock (OstreeSysroot *self, * for e.g. older versions of libostree unaware of pinning to GC the deployment. * * This function does nothing and returns successfully if the deployment - * is already in the desired pinning state. + * is already in the desired pinning state. It is an error to try to pin + * the staged deployment (as it's not in the bootloader entries). * * Since: 2018.3 */ @@ -1908,6 +1909,9 @@ ostree_sysroot_deployment_set_pinned (OstreeSysroot *self, if (is_pinned == current_pin) return TRUE; + if (ostree_deployment_is_staged (deployment)) + return glnx_throw (error, "Cannot pin staged deployment"); + g_autoptr(OstreeDeployment) deployment_clone = ostree_deployment_clone (deployment); GKeyFile *origin_clone = ostree_deployment_get_origin (deployment_clone); diff --git a/tests/installed/destructive/staged-deploy.yml b/tests/installed/destructive/staged-deploy.yml index f34550a2..8802fb1b 100644 --- a/tests/installed/destructive/staged-deploy.yml +++ b/tests/installed/destructive/staged-deploy.yml @@ -21,6 +21,10 @@ done test -f deployment-ref-found rm deployment-ref-found + if ostree admin pin 0 2>err.txt; then + echo "Pinned staged deployment"; exit 1 + fi + grep -qFe 'Cannot pin staged deployment' err.txt environment: commit: "{{ rpmostree_status['deployments'][0]['checksum'] }}" - include_tasks: ../tasks/reboot.yml From be018ed70cf08a4bc0fb6b1885b4965e729b6d5c Mon Sep 17 00:00:00 2001 From: Marcus Folkesson Date: Fri, 29 Jun 2018 10:16:41 +0200 Subject: [PATCH 20/47] ci: exclude 'lib' from libsoup configure option The option used by configure script is actually --with-soup/--without-soup. Signed-off-by: Marcus Folkesson Closes: #1661 Approved by: jlebon --- .papr-ex.yaml | 2 +- .papr.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.papr-ex.yaml b/.papr-ex.yaml index 3ebbdb40..4b12f7d3 100644 --- a/.papr-ex.yaml +++ b/.papr-ex.yaml @@ -99,7 +99,7 @@ required: true context: f28-libsoup env: - CONFIGOPTS: "--without-curl --without-openssl --with-libsoup" + CONFIGOPTS: "--without-curl --without-openssl --with-soup" tests: - ci/build-check.sh diff --git a/.papr.yml b/.papr.yml index b99a1f70..ab2479d2 100644 --- a/.papr.yml +++ b/.papr.yml @@ -104,7 +104,7 @@ required: true context: f28-libsoup env: - CONFIGOPTS: "--without-curl --without-openssl --with-libsoup" + CONFIGOPTS: "--without-curl --without-openssl --with-soup" tests: - ci/build-check.sh From abff8b8cfac00203062362ad506ffdb14f1da6ef Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 26 Jun 2018 14:39:16 +0100 Subject: [PATCH 21/47] lib/repo-commit: Abort a transaction if preparing it fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If ostree_repo_prepare_transaction() fails, we should reset the repository’s state so that the failed call was essentially idempotent. Do that by calling ostree_repo_abort_transaction() on the failure path. Typically, the way for preparing a transaction to fail is for its GCancellable to be triggered, rather than because any of the operations involved in preparing a transaction are particularly failure prone. Signed-off-by: Philip Withnall Closes: #1647 Approved by: cgwalters --- src/libostree/ostree-repo-commit.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index a3a6df44..54bd8b66 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -1614,9 +1614,15 @@ ostree_repo_prepare_transaction (OstreeRepo *self, GCancellable *cancellable, GError **error) { + g_autoptr(_OstreeRepoAutoTransaction) txn = NULL; g_return_val_if_fail (self->in_transaction == FALSE, FALSE); + g_debug ("Preparing transaction in repository %p", self); + + /* Set up to abort the transaction if we return early from this function. */ + txn = self; + memset (&self->txn.stats, 0, sizeof (OstreeRepoTransactionStats)); self->txn_locked = _ostree_repo_lock_push (self, OSTREE_REPO_LOCK_SHARED, @@ -1631,6 +1637,7 @@ ostree_repo_prepare_transaction (OstreeRepo *self, struct statvfs stvfsbuf; if (TEMP_FAILURE_RETRY (fstatvfs (self->repo_dir_fd, &stvfsbuf)) < 0) return glnx_throw_errno_prefix (error, "fstatvfs"); + g_mutex_lock (&self->txn_lock); self->txn.blocksize = stvfsbuf.f_bsize; guint64 reserved_blocks = min_free_space_calculate_reserved_blocks (self, &stvfsbuf); @@ -1663,6 +1670,9 @@ ostree_repo_prepare_transaction (OstreeRepo *self, cancellable, error)) return FALSE; + /* Success: do not abort the transaction when returning. */ + txn = NULL; + if (out_transaction_resume) *out_transaction_resume = ret_transaction_resume; return TRUE; @@ -2150,6 +2160,8 @@ ostree_repo_commit_transaction (OstreeRepo *self, { g_return_val_if_fail (self->in_transaction == TRUE, FALSE); + g_debug ("Committing transaction in repository %p", self); + if ((self->test_error_flags & OSTREE_REPO_TEST_ERROR_PRE_COMMIT) > 0) return glnx_throw (error, "OSTREE_REPO_TEST_ERROR_PRE_COMMIT specified"); @@ -2234,6 +2246,8 @@ ostree_repo_abort_transaction (OstreeRepo *self, if (!self->in_transaction) return TRUE; + g_debug ("Aborting transaction in repository %p", self); + if (self->loose_object_devino_hash) g_hash_table_remove_all (self->loose_object_devino_hash); From f2e71c60ecd6c46c5c54d521c6dd23f339d7c67b Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 29 Jun 2018 13:53:54 -0400 Subject: [PATCH 22/47] ci/flatpak: Fix to use built ostree version Noticed as part of a random failure in this PR: https://github.com/ostreedev/ostree/pull/1655 that we weren't actually testing the version of ostree we built in git. Probably we should be generating RPMs but...later. Closes: #1662 Approved by: jlebon --- ci/flatpak.sh | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/ci/flatpak.sh b/ci/flatpak.sh index c47171f2..a0e556ac 100755 --- a/ci/flatpak.sh +++ b/ci/flatpak.sh @@ -3,24 +3,38 @@ set -xeuo pipefail -FLATPAK_TAG=master +# Keep this pinned to avoid arbitrary change for now; it's also +# good to test building older code against newer ostree as it helps +# us notice any API breaks. +FLATPAK_TAG=0.99.2 dn=$(dirname $0) . ${dn}/libpaprci/libbuild.sh codedir=$(pwd) -# Build and install ostree +# Build ostree, but we don't install it yet cd ${codedir} ci/build.sh -make install # Build flatpak tmpd=$(mktemp -d) cd ${tmpd} git clone --recursive --depth=1 -b ${FLATPAK_TAG} https://github.com/flatpak/flatpak cd ${tmpd}/flatpak -ci/build.sh +# This is a copy of flatpak/ci/build.sh, but we can't use that as we want to install +# our built ostree over it. +pkg_install sudo which attr fuse bison \ + libubsan libasan libtsan clang python2 \ + elfutils git gettext-devel libappstream-glib-devel hicolor-icon-theme \ + /usr/bin/{update-mime-database,update-desktop-database,gtk-update-icon-cache} +pkg_builddep flatpak +# Now install ostree over the package version +cd ${codedir} +make install +cd - +# And build flatpak +build # We want to capture automake results from flatpak cleanup() { mv test-suite.log ${codedir} || true From 5b0dd1002e29d6ea7a768a66b7fee92c27dd1bf8 Mon Sep 17 00:00:00 2001 From: William Manley Date: Mon, 25 Jun 2018 21:53:23 +0100 Subject: [PATCH 23/47] OstreeMutableTree: Refactor: Add `parent` pointer This implements a TODO item from `ostree_mutable_tree_get_contents_checksum`. We now no-longer invalidate the dirtree contents checksum at `get_contents_checksum` time - we invalidate it when the mtree is modified. This is implemented by keeping a pointer to the parent directory in each `OstreeMutableTree`. This gives us stronger invariants on `contents_checksum`. For even stronger guarantees about invariants we could make `ostree_repo_write_mtree` or similar a member of `OstreeMutableTree` and remove `ostree_mutable_tree_set_metadata_checksum`. I think I've fixed a bug here too. We now invalidate parent's contents checksum when our metadata checksum changes, whereas we didn't before. Closes: #1655 Approved by: cgwalters --- src/libostree/ostree-mutable-tree.c | 96 +++++++++++++++++++++-------- 1 file changed, 69 insertions(+), 27 deletions(-) diff --git a/src/libostree/ostree-mutable-tree.c b/src/libostree/ostree-mutable-tree.c index e5e282d4..7ae915ff 100644 --- a/src/libostree/ostree-mutable-tree.c +++ b/src/libostree/ostree-mutable-tree.c @@ -47,10 +47,24 @@ struct OstreeMutableTree { GObject parent_instance; + /* The parent directory to this one. We don't hold a ref because this mtree + * is owned by the parent. We can be certain that any mtree only has one + * parent because external users can't set this, it's only set when we create + * a child from within this file (see insert_child_mtree). We ensure that the + * parent pointer is either valid or NULL because when the parent is destroyed + * it sets parent = NULL on all its children (see remove_child_mtree) */ + OstreeMutableTree *parent; + /* This is the checksum of the Dirtree object that corresponds to the current * contents of this directory. contents_checksum can be NULL if the SHA was - * never calculated or contents of the mtree has been modified. Even if - * contents_checksum is not NULL it may be out of date. */ + * never calculated or contents of this mtree or any subdirectory has been + * modified. If a contents_checksum is NULL then all the parent's checksums + * will be NULL (see `invalidate_contents_checksum`). + * + * Note: This invariant is partially maintained externally - we + * rely on the callers of `ostree_mutable_tree_set_contents_checksum` to have + * first ensured that the mtree contents really does correspond to this + * checksum */ char *contents_checksum; /* This is the checksum of the DirMeta object that holds the uid, gid, mode @@ -66,6 +80,8 @@ struct OstreeMutableTree G_DEFINE_TYPE (OstreeMutableTree, ostree_mutable_tree, G_TYPE_OBJECT) +static void invalidate_contents_checksum (OstreeMutableTree *self); + static void ostree_mutable_tree_finalize (GObject *object) { @@ -90,13 +106,52 @@ ostree_mutable_tree_class_init (OstreeMutableTreeClass *klass) gobject_class->finalize = ostree_mutable_tree_finalize; } +/* This must not be made public or we can't maintain the invariant that any + * OstreeMutableTree has only one parent. + * + * Ownership of @child is transferred from the caller to @self */ +static void +insert_child_mtree (OstreeMutableTree *self, const gchar* name, + OstreeMutableTree *child) +{ + g_assert_null (child->parent); + g_hash_table_insert (self->subdirs, g_strdup (name), child); + child->parent = self; + invalidate_contents_checksum (self); +} + +static void +remove_child_mtree (gpointer data) +{ + /* Each mtree has shared ownership of its children and each child has a + * non-owning reference back to parent. If the parent goes out of scope the + * children may still be alive because they're reference counted. This + * removes the reference to the parent before it goes stale. */ + OstreeMutableTree *child = (OstreeMutableTree*) data; + child->parent = NULL; + g_object_unref (child); +} + static void ostree_mutable_tree_init (OstreeMutableTree *self) { self->files = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free); self->subdirs = g_hash_table_new_full (g_str_hash, g_str_equal, - g_free, (GDestroyNotify)g_object_unref); + g_free, remove_child_mtree); +} + +static void +invalidate_contents_checksum (OstreeMutableTree *self) +{ + while (self) { + if (!self->contents_checksum) + break; + + g_free (self->contents_checksum); + g_clear_pointer (&self->contents_checksum, g_free); + self = self->parent; + } } void @@ -117,6 +172,14 @@ void ostree_mutable_tree_set_contents_checksum (OstreeMutableTree *self, const char *checksum) { + if (g_strcmp0 (checksum, self->contents_checksum) == 0) + return; + + if (checksum && self->contents_checksum) + g_warning ("Setting a contents checksum on an OstreeMutableTree that " + "already has a checksum set. Old checksum %s, new checksum %s", + self->contents_checksum, checksum); + g_free (self->contents_checksum); self->contents_checksum = g_strdup (checksum); } @@ -124,26 +187,6 @@ ostree_mutable_tree_set_contents_checksum (OstreeMutableTree *self, const char * ostree_mutable_tree_get_contents_checksum (OstreeMutableTree *self) { - if (!self->contents_checksum) - return NULL; - - /* Ensure the cache is valid; this implementation is a bit - * lame in that we walk the whole tree every time this - * getter is called; a better approach would be to invalidate - * all of the parents whenever a child is modified. - * - * However, we only call this function once right now. - */ - GLNX_HASH_TABLE_FOREACH_V (self->subdirs, OstreeMutableTree*, subdir) - { - if (!ostree_mutable_tree_get_contents_checksum (subdir)) - { - g_free (self->contents_checksum); - self->contents_checksum = NULL; - return NULL; - } - } - return self->contents_checksum; } @@ -176,7 +219,7 @@ ostree_mutable_tree_replace_file (OstreeMutableTree *self, goto out; } - ostree_mutable_tree_set_contents_checksum (self, NULL); + invalidate_contents_checksum (self); g_hash_table_replace (self->files, g_strdup (name), g_strdup (checksum)); @@ -221,8 +264,7 @@ ostree_mutable_tree_ensure_dir (OstreeMutableTree *self, if (!ret_dir) { ret_dir = ostree_mutable_tree_new (); - ostree_mutable_tree_set_contents_checksum (self, NULL); - g_hash_table_insert (self->subdirs, g_strdup (name), g_object_ref (ret_dir)); + insert_child_mtree (self, name, g_object_ref (ret_dir)); } ret = TRUE; @@ -305,7 +347,7 @@ ostree_mutable_tree_ensure_parent_dirs (OstreeMutableTree *self, { next = ostree_mutable_tree_new (); ostree_mutable_tree_set_metadata_checksum (next, metadata_checksum); - g_hash_table_insert (subdir->subdirs, g_strdup (name), next); + insert_child_mtree (subdir, g_strdup (name), next); } subdir = next; From 488365f9bf296bf8e56d571b0abfcae638ac0a3f Mon Sep 17 00:00:00 2001 From: William Manley Date: Mon, 25 Jun 2018 22:56:10 +0100 Subject: [PATCH 24/47] OstreeMutableTree: Invalidate parent contents checksum when metadata changes This bug has existed before the previous commit, but thanks to the previous commit it is now easy to fix. Closes: #1655 Approved by: cgwalters --- src/libostree/ostree-mutable-tree.c | 5 ++++- tests/test-mutable-tree.c | 25 +++++++++++++++++++++++-- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/libostree/ostree-mutable-tree.c b/src/libostree/ostree-mutable-tree.c index 7ae915ff..25dc901e 100644 --- a/src/libostree/ostree-mutable-tree.c +++ b/src/libostree/ostree-mutable-tree.c @@ -148,7 +148,6 @@ invalidate_contents_checksum (OstreeMutableTree *self) if (!self->contents_checksum) break; - g_free (self->contents_checksum); g_clear_pointer (&self->contents_checksum, g_free); self = self->parent; } @@ -158,6 +157,10 @@ void ostree_mutable_tree_set_metadata_checksum (OstreeMutableTree *self, const char *checksum) { + if (g_strcmp0 (checksum, self->metadata_checksum) == 0) + return; + + invalidate_contents_checksum (self->parent); g_free (self->metadata_checksum); self->metadata_checksum = g_strdup (checksum); } diff --git a/tests/test-mutable-tree.c b/tests/test-mutable-tree.c index c03bddba..d0dfdd74 100644 --- a/tests/test-mutable-tree.c +++ b/tests/test-mutable-tree.c @@ -31,6 +31,7 @@ static void test_metadata_checksum (void) { + g_autoptr(GError) error = NULL; const char *checksum = "12345678901234567890123456789012"; glnx_unref_object OstreeMutableTree *tree = ostree_mutable_tree_new (); @@ -39,6 +40,27 @@ test_metadata_checksum (void) ostree_mutable_tree_set_metadata_checksum (tree, checksum); g_assert_cmpstr (checksum, ==, ostree_mutable_tree_get_metadata_checksum (tree)); + + /* If a child tree's metadata changes the parent tree's contents needs to be + * recalculated */ + glnx_unref_object OstreeMutableTree *subdir = NULL; + g_assert (ostree_mutable_tree_ensure_dir (tree, "subdir", &subdir, &error)); + g_assert_nonnull (subdir); + + ostree_mutable_tree_set_contents_checksum ( + subdir, "11111111111111111111111111111111"); + ostree_mutable_tree_set_metadata_checksum ( + subdir, "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"); + ostree_mutable_tree_set_contents_checksum ( + tree, "abcdefabcdefabcdefabcdefabcdefab"); + + g_assert_cmpstr (ostree_mutable_tree_get_contents_checksum (tree), ==, + "abcdefabcdefabcdefabcdefabcdefab"); + ostree_mutable_tree_set_metadata_checksum ( + subdir, "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"); + g_assert_null (ostree_mutable_tree_get_contents_checksum (tree)); + g_assert_cmpstr (ostree_mutable_tree_get_contents_checksum (subdir), ==, + "11111111111111111111111111111111"); } static void @@ -162,13 +184,12 @@ test_contents_checksum (void) const char *subdir_checksum = "ABCD0123456789012345678901234567"; glnx_unref_object OstreeMutableTree *tree = ostree_mutable_tree_new (); glnx_unref_object OstreeMutableTree *subdir = NULL; - g_autoptr(GError) error = NULL; g_assert_null (ostree_mutable_tree_get_contents_checksum (tree)); ostree_mutable_tree_set_contents_checksum (tree, checksum); g_assert_cmpstr (checksum, ==, ostree_mutable_tree_get_contents_checksum (tree)); - g_assert (ostree_mutable_tree_ensure_dir (tree, "subdir", &subdir, &error)); + g_assert (ostree_mutable_tree_ensure_dir (tree, "subdir", &subdir, NULL)); g_assert_nonnull (subdir); ostree_mutable_tree_set_contents_checksum (subdir, subdir_checksum); From 0c8b86ea09e31b5c3b7138854cf42dc845b6f5c8 Mon Sep 17 00:00:00 2001 From: Umang Jain Date: Sat, 30 Jun 2018 02:10:12 +0530 Subject: [PATCH 25/47] lib/repo: Minor fixes around min-free-space Summary: * Remove a useless if condition in prepare_transaction() * Fix glnx_throw error propagation * Integer overflow check while parsing min-free-space-size config * Documentation fixes Closes: #1663 Approved by: jlebon --- man/ostree.repo-config.xml | 6 ++-- src/libostree/ostree-repo-commit.c | 48 ++++++++++++++---------------- src/libostree/ostree-repo.c | 10 +++++-- 3 files changed, 33 insertions(+), 31 deletions(-) diff --git a/man/ostree.repo-config.xml b/man/ostree.repo-config.xml index 09312d5c..602d412e 100644 --- a/man/ostree.repo-config.xml +++ b/man/ostree.repo-config.xml @@ -126,10 +126,10 @@ Boston, MA 02111-1307, USA. min-free-space-size - Value (in MB, GB or TB) that specifies a minimum space (in blocks) - in the underlying filesystem to keep free. Also, note that min-free-space-percent + Value (in MB, GB or TB) that specifies a minimum space in the + underlying filesystem to keep free. Also, note that min-free-space-percent and min-free-space-size are mutually exclusive. Examples of acceptable values: - 500MB, 1GB etc. + 500MB, 1GB etc. The default value is 0MB, which disables this check. diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index 54bd8b66..05635afd 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -1632,34 +1632,32 @@ ostree_repo_prepare_transaction (OstreeRepo *self, self->in_transaction = TRUE; self->cleanup_stagedir = FALSE; - if (self->min_free_space_percent >= 0 || self->min_free_space_mb >= 0) - { - struct statvfs stvfsbuf; - if (TEMP_FAILURE_RETRY (fstatvfs (self->repo_dir_fd, &stvfsbuf)) < 0) - return glnx_throw_errno_prefix (error, "fstatvfs"); - g_mutex_lock (&self->txn_lock); - self->txn.blocksize = stvfsbuf.f_bsize; - guint64 reserved_blocks = min_free_space_calculate_reserved_blocks (self, &stvfsbuf); - /* Use the appropriate free block count if we're unprivileged */ - guint64 bfree = (getuid () != 0 ? stvfsbuf.f_bavail : stvfsbuf.f_bfree); - if (bfree > reserved_blocks) - self->txn.max_blocks = bfree - reserved_blocks; - else - { - guint64 bytes_required = bfree * self->txn.blocksize; - self->cleanup_stagedir = TRUE; - g_mutex_unlock (&self->txn_lock); - g_autofree char *formatted_free = g_format_size (bytes_required); - if (self->min_free_space_percent > 0) - return glnx_throw (error, "min-free-space-percent '%u%%' would be exceeded, %s available", - self->min_free_space_percent, formatted_free); - else - return glnx_throw (error, "min-free-space-size %" G_GUINT64_FORMAT "MB would be exceeded, %s available", - self->min_free_space_mb, formatted_free); - } + struct statvfs stvfsbuf; + if (TEMP_FAILURE_RETRY (fstatvfs (self->repo_dir_fd, &stvfsbuf)) < 0) + return glnx_throw_errno_prefix (error, "fstatvfs"); + + g_mutex_lock (&self->txn_lock); + self->txn.blocksize = stvfsbuf.f_bsize; + guint64 reserved_blocks = min_free_space_calculate_reserved_blocks (self, &stvfsbuf); + /* Use the appropriate free block count if we're unprivileged */ + guint64 bfree = (getuid () != 0 ? stvfsbuf.f_bavail : stvfsbuf.f_bfree); + if (bfree > reserved_blocks) + self->txn.max_blocks = bfree - reserved_blocks; + else + { + guint64 bytes_required = bfree * self->txn.blocksize; + self->cleanup_stagedir = TRUE; g_mutex_unlock (&self->txn_lock); + g_autofree char *formatted_free = g_format_size (bytes_required); + if (self->min_free_space_percent > 0) + return glnx_throw (error, "min-free-space-percent '%u%%' would be exceeded, %s available", + self->min_free_space_percent, formatted_free); + else + return glnx_throw (error, "min-free-space-size %" G_GUINT64_FORMAT "MB would be exceeded, %s available", + self->min_free_space_mb, formatted_free); } + g_mutex_unlock (&self->txn_lock); gboolean ret_transaction_resume = FALSE; if (!_ostree_repo_allocate_tmpdir (self->tmp_dir_fd, diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index d64e4125..53d5cfda 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -2670,7 +2670,7 @@ min_free_space_size_validate_and_convert (OstreeRepo *self, g_autoptr(GMatchInfo) match = NULL; if (!g_regex_match (regex, min_free_space_size_str, 0, &match)) - return glnx_prefix_error (error, "Failed to parse min-free-space-size parameter: '%s'", min_free_space_size_str); + return glnx_throw (error, "Failed to match '^[0-9]+[GMT]B$'"); g_autofree char *size_str = g_match_info_fetch (match, 1); g_autofree char *unit = g_match_info_fetch (match, 2); @@ -2691,7 +2691,11 @@ min_free_space_size_validate_and_convert (OstreeRepo *self, g_assert_not_reached (); } - self->min_free_space_mb = g_ascii_strtoull (size_str, NULL, 10) << shifts; + guint64 min_free_space = g_ascii_strtoull (size_str, NULL, 10); + if (shifts > 0 && g_bit_nth_lsf (min_free_space, 63 - shifts) != -1) + return glnx_throw (error, "Integer overflow detected"); + + self->min_free_space_mb = min_free_space << shifts; return TRUE; } @@ -2829,7 +2833,7 @@ reload_core_config (OstreeRepo *self, /* Validate the string and convert the size to MBs */ if (!min_free_space_size_validate_and_convert (self, min_free_space_size_str, error)) - return glnx_throw (error, "Invalid min-free-space-size '%s'", min_free_space_size_str); + return glnx_prefix_error (error, "Invalid min-free-space-size '%s'", min_free_space_size_str); } else { From a0527e70866bbb29f98e72b3e697f886b3aa8393 Mon Sep 17 00:00:00 2001 From: Alex Kiernan Date: Sat, 30 Jun 2018 17:03:51 +0000 Subject: [PATCH 26/47] boot: Use emergency.target, not emergency.service Follow systemd units in using emergency.target, not emergency.service (which is the sole unit, by default, in emergency.target) so we can easily reconfigure the units which are actived when entering emergency mode. Signed-off-by: Alex Kiernan Closes: #1665 Approved by: cgwalters --- src/boot/ostree-prepare-root.service | 2 +- src/boot/ostree-remount.service | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/boot/ostree-prepare-root.service b/src/boot/ostree-prepare-root.service index e0659d10..52cf8863 100644 --- a/src/boot/ostree-prepare-root.service +++ b/src/boot/ostree-prepare-root.service @@ -20,7 +20,7 @@ Description=OSTree Prepare OS/ DefaultDependencies=no ConditionKernelCommandLine=ostree ConditionPathExists=/etc/initrd-release -OnFailure=emergency.service +OnFailure=emergency.target After=initrd-switch-root.target Before=initrd-switch-root.service Before=plymouth-switch-root.service diff --git a/src/boot/ostree-remount.service b/src/boot/ostree-remount.service index 8439b495..68209f96 100644 --- a/src/boot/ostree-remount.service +++ b/src/boot/ostree-remount.service @@ -19,7 +19,7 @@ Description=OSTree Remount OS/ bind mounts DefaultDependencies=no ConditionKernelCommandLine=ostree -OnFailure=emergency.service +OnFailure=emergency.target Conflicts=umount.target After=-.mount After=systemd-remount-fs.service From 8af389b758970ce73929c84315505ac8410f355b Mon Sep 17 00:00:00 2001 From: Marcus Folkesson Date: Tue, 3 Jul 2018 10:02:46 +0200 Subject: [PATCH 27/47] build: add ostree-soup-* to build process when configured with avahi Avoid getting these link errors: ./.libs/libostree-1.so: undefined reference to `soup_uri_set_path' ./.libs/libostree-1.so: undefined reference to `soup_uri_new' ./.libs/libostree-1.so: undefined reference to `soup_uri_free' ./.libs/libostree-1.so: undefined reference to `soup_uri_set_scheme' ./.libs/libostree-1.so: undefined reference to `soup_uri_to_string' ./.libs/libostree-1.so: undefined reference to `soup_uri_set_host' ./.libs/libostree-1.so: undefined reference to `soup_uri_set_port' collect2: error: ld returned 1 exit status Reproduce with: ./configure --with-avahi --without-soup Signed-off-by: Marcus Folkesson Closes: #1666 Approved by: cgwalters --- Makefile-libostree.am | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Makefile-libostree.am b/Makefile-libostree.am index 01a209d1..dbc9ebb8 100644 --- a/Makefile-libostree.am +++ b/Makefile-libostree.am @@ -238,6 +238,13 @@ if USE_LIBSOUP libostree_1_la_SOURCES += src/libostree/ostree-fetcher-soup.c libostree_1_la_CFLAGS += $(OT_INTERNAL_SOUP_CFLAGS) libostree_1_la_LIBADD += $(OT_INTERNAL_SOUP_LIBS) +else +if USE_AVAHI +libostree_1_la_SOURCES += src/libostree/ostree-soup-uri.h \ + src/libostree/ostree-soup-uri.c \ + src/libostree/ostree-soup-form.c \ + $(NULL) +endif endif endif From a13ea6497e91920e9d6297d14a132123a5e48b43 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Tue, 3 Jul 2018 17:41:45 -0400 Subject: [PATCH 28/47] switchroot: Fix regression for separately mounted /var I made a logical error in #1617 which resulted in the exact *opposite* behaviour we want when `/var` is a separate mount. Split this out and lower the number of negations to make it more obvious that it's correct. Closes: #1667 Closes: #1668 Approved by: cgwalters --- src/switchroot/ostree-prepare-root.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/switchroot/ostree-prepare-root.c b/src/switchroot/ostree-prepare-root.c index 53df463c..0131d246 100644 --- a/src/switchroot/ostree-prepare-root.c +++ b/src/switchroot/ostree-prepare-root.c @@ -151,14 +151,17 @@ main(int argc, char *argv[]) if (chdir (deploy_path) < 0) err (EXIT_FAILURE, "failed to chdir to deploy_path"); + /* Default to true, but in the systemd case, default to false because it's handled by + * ostree-system-generator. */ bool mount_var = true; - /* In the systemd case, this is handled by ostree-system-generator by default */ -#ifndef HAVE_SYSTEMD_AND_LIBMOUNT - /* file in /run can override that behaviour */ - if (lstat (INITRAMFS_MOUNT_VAR, &stbuf) < 0) - mount_var = false; +#ifdef HAVE_SYSTEMD_AND_LIBMOUNT + mount_var = false; #endif + /* file in /run can override the default behaviour so that we definitely mount /var */ + if (lstat (INITRAMFS_MOUNT_VAR, &stbuf) == 0) + mount_var = true; + /* Link to the deployment's /var */ if (mount_var && mount ("../../var", "var", NULL, MS_MGC_VAL|MS_BIND, NULL) < 0) err (EXIT_FAILURE, "failed to bind mount ../../var to var"); From 61ba4e7e5ae257b0278e5be2ac0e0af7e3049418 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 4 Jul 2018 11:57:50 -0400 Subject: [PATCH 29/47] tests/installed: Add var-mount.yml destructive test Closes: #1668 Approved by: cgwalters --- tests/installed/destructive-ansible.yml | 1 + tests/installed/destructive/var-mount.yml | 13 +++++++++++++ tests/installed/tasks/install-git.yml | 3 +++ 3 files changed, 17 insertions(+) create mode 100644 tests/installed/destructive/var-mount.yml diff --git a/tests/installed/destructive-ansible.yml b/tests/installed/destructive-ansible.yml index c72f6b82..50c8fd77 100644 --- a/tests/installed/destructive-ansible.yml +++ b/tests/installed/destructive-ansible.yml @@ -13,3 +13,4 @@ when: use_git_build - import_tasks: tasks/query-host.yml - import_tasks: destructive/staged-deploy.yml + - import_tasks: destructive/var-mount.yml diff --git a/tests/installed/destructive/var-mount.yml b/tests/installed/destructive/var-mount.yml new file mode 100644 index 00000000..3fe041af --- /dev/null +++ b/tests/installed/destructive/var-mount.yml @@ -0,0 +1,13 @@ +# https://github.com/ostreedev/ostree/issues/1667 +- name: Set up /var as a mountpoint + shell: | + set -xeuo pipefail + cp -a /var /sysroot/myvar + touch /sysroot/myvar/somenewfile + echo '/sysroot/myvar /var none bind 0 0' >> /etc/fstab +- include_tasks: ../tasks/reboot.yml +- name: Check that /var mountpoint worked + shell: | + set -xeuo pipefail + systemctl status var.mount + test -f /var/somenewfile diff --git a/tests/installed/tasks/install-git.yml b/tests/installed/tasks/install-git.yml index 8216afeb..3374cce7 100644 --- a/tests/installed/tasks/install-git.yml +++ b/tests/installed/tasks/install-git.yml @@ -8,6 +8,9 @@ synchronize: src=build/x86_64/ dest=/root/x86_64/ archive=yes - name: Install RPMs shell: rpm-ostree override replace /root/x86_64/*.rpm +# Regenerate to make sure new ostree binaries also make it to the initrd +- name: Regenerate initramfs + shell: rpm-ostree initramfs --enable - import_tasks: ../tasks/reboot.yml - import_tasks: ../tasks/query-host.yml - command: ostree --version From 2cb2571127e3d82f59133f63fca97951691859aa Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Wed, 4 Jul 2018 11:59:18 -0400 Subject: [PATCH 30/47] tests/installed: Add NOTE when re-using RPMs One gotcha here is that we don't invalidate the RPMs if we're not sitting on the same commit anymore. Shouldn't be too hard to fix, though let's at least make a note of it for now. Closes: #1668 Approved by: cgwalters --- tests/installed/playbook-run.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/installed/playbook-run.sh b/tests/installed/playbook-run.sh index 37572bd4..af906c3d 100755 --- a/tests/installed/playbook-run.sh +++ b/tests/installed/playbook-run.sh @@ -7,6 +7,10 @@ dn=$(cd $(dirname $0) && pwd) if ! test -d build; then mkdir -p build (cd build && ${dn}/../../ci/build-rpm.sh) +else + # XXX: we should invalidate based on `git describe` or something + echo "NOTE: Re-using prebuilt RPMs:" + find build -name '*.rpm' fi # https://fedoraproject.org/wiki/CI/Tests From 3696e1cfdd1f0109c7cd9e157763ecef23ae7adc Mon Sep 17 00:00:00 2001 From: Alex Kiernan Date: Wed, 4 Jul 2018 19:19:41 +0000 Subject: [PATCH 31/47] build: Use ostree_prepare_root_CPPFLAGS for ostree-prepare-root Swap from AM_CPPFLAGS to ostree_prepare_root_CPPFLAGS when compiling ostree-prepare-root statically. This fixes a problem when you have systemd and libmount, but only ostree_prepare_root_CPPFLAGS includes -DHAVE_SYSTEMD_AND_LIBMOUNT=1. Signed-off-by: Alex Kiernan Closes: #1670 Approved by: jlebon --- Makefile-switchroot.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile-switchroot.am b/Makefile-switchroot.am index 8a8ec570..4423fdcd 100644 --- a/Makefile-switchroot.am +++ b/Makefile-switchroot.am @@ -45,7 +45,7 @@ if BUILDOPT_USE_STATIC_COMPILER ostree_boot_SCRIPTS = ostree-prepare-root ostree-prepare-root : $(ostree_prepare_root_SOURCES) - $(STATIC_COMPILER) -o $@ -static $(top_srcdir)/src/switchroot/ostree-prepare-root.c $(AM_CPPFLAGS) $(AM_CFLAGS) $(DEFAULT_INCLUDES) + $(STATIC_COMPILER) -o $@ -static $(top_srcdir)/src/switchroot/ostree-prepare-root.c $(ostree_prepare_root_CPPFLAGS) $(AM_CFLAGS) $(DEFAULT_INCLUDES) else ostree_boot_PROGRAMS += ostree-prepare-root ostree_prepare_root_CFLAGS = $(AM_CFLAGS) -Isrc/switchroot From 4f096c8f11c06883c5eec03c0ad6d962f3d82a4d Mon Sep 17 00:00:00 2001 From: Robert Fairley Date: Thu, 5 Jul 2018 12:30:05 -0400 Subject: [PATCH 32/47] tests: Move assert_fail function to tests/libtest.sh This moves the assert_fail function definition which was defined and called in tests/test-remote-headers.sh. Done in preparation for use of the assert_fail function in other test files. Closes: #1669 Approved by: jlebon --- tests/libtest.sh | 9 +++++++++ tests/test-remote-headers.sh | 9 --------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/libtest.sh b/tests/libtest.sh index 388408df..f09f4a2c 100755 --- a/tests/libtest.sh +++ b/tests/libtest.sh @@ -648,3 +648,12 @@ assert_not_ref () { fatal "rev-parse $2 unexpectedly succeeded!" fi } + +assert_fail () { + set +e + $@ + if [ $? = 0 ] ; then + echo 1>&2 "$@ did not fail"; exit 1 + fi + set -euo pipefail +} diff --git a/tests/test-remote-headers.sh b/tests/test-remote-headers.sh index a4ee386f..a41d087a 100755 --- a/tests/test-remote-headers.sh +++ b/tests/test-remote-headers.sh @@ -33,15 +33,6 @@ setup_fake_remote_repo1 "archive" "" \ --expected-header baz=badger \ --expected-header "User-Agent=libostree/$V dodo/2.15" -assert_fail (){ - set +e - $@ - if [ $? = 0 ] ; then - echo 1>&2 "$@ did not fail"; exit 1 - fi - set -euo pipefail -} - cd ${test_tmpdir} rm repo -rf mkdir repo From 7baf1678810c090206a8bee21ccebb2ee498fe60 Mon Sep 17 00:00:00 2001 From: Robert Fairley Date: Thu, 5 Jul 2018 12:35:30 -0400 Subject: [PATCH 33/47] ostree/pull: Add network-retries command line option This exposes a way to specify from the command line the number of times to retry each download after a network error. If a negative value is given, then the default number of retries (5) is used. If 0 is given, then errors are returned without retrying. closes #1659 Closes: #1669 Approved by: jlebon --- bash/ostree | 1 + man/ostree-pull.xml | 8 ++++++++ src/ostree/ot-builtin-pull.c | 6 ++++++ tests/test-pull-repeated.sh | 38 +++++++++++++++++++++++++++++++++--- 4 files changed, 50 insertions(+), 3 deletions(-) diff --git a/bash/ostree b/bash/ostree index f3aef686..d7b54373 100644 --- a/bash/ostree +++ b/bash/ostree @@ -908,6 +908,7 @@ _ostree_pull() { --depth --http-header --localcache-repo -L + --network-retries --repo --subpath --update-frequency diff --git a/man/ostree-pull.xml b/man/ostree-pull.xml index 04ca4aaa..84d75be3 100644 --- a/man/ostree-pull.xml +++ b/man/ostree-pull.xml @@ -129,6 +129,14 @@ Boston, MA 02111-1307, USA. Traverse DEPTH parents (-1=infinite) (default: 0). + + + =N + + + Specifies how many times each download should be retried upon error (default: 5) + + diff --git a/src/ostree/ot-builtin-pull.c b/src/ostree/ot-builtin-pull.c index e2d1f09a..8c6569cf 100644 --- a/src/ostree/ot-builtin-pull.c +++ b/src/ostree/ot-builtin-pull.c @@ -44,6 +44,7 @@ static char* opt_cache_dir; static char* opt_append_user_agent; static int opt_depth = 0; static int opt_frequency = 0; +static int opt_network_retries = -1; static char* opt_url; static char** opt_localcache_repos; @@ -68,6 +69,7 @@ static GOptionEntry options[] = { { "url", 0, 0, G_OPTION_ARG_STRING, &opt_url, "Pull objects from this URL instead of the one from the remote config", NULL }, { "http-header", 0, 0, G_OPTION_ARG_STRING_ARRAY, &opt_http_headers, "Add NAME=VALUE as HTTP header to all requests", "NAME=VALUE" }, { "update-frequency", 0, 0, G_OPTION_ARG_INT, &opt_frequency, "Sets the update frequency, in milliseconds (0=1000ms) (default: 0)", "FREQUENCY" }, + { "network-retries", 0, 0, G_OPTION_ARG_INT, &opt_network_retries, "Specifies how many times each download should be retried upon error (default: 5)", "N"}, { "localcache-repo", 'L', 0, G_OPTION_ARG_FILENAME_ARRAY, &opt_localcache_repos, "Add REPO as local cache source for objects during this pull", "REPO" }, { "timestamp-check", 'T', 0, G_OPTION_ARG_NONE, &opt_timestamp_check, "Require fetched commits to have newer timestamps", NULL }, /* let's leave this hidden for now; we just need it for tests */ @@ -293,6 +295,10 @@ ostree_builtin_pull (int argc, char **argv, OstreeCommandInvocation *invocation, g_variant_builder_add (&builder, "{s@v}", "update-frequency", g_variant_new_variant (g_variant_new_uint32 (opt_frequency))); + + if (opt_network_retries >= 0) + g_variant_builder_add (&builder, "{s@v}", "n-network-retries", + g_variant_new_variant (g_variant_new_uint32 (opt_network_retries))); g_variant_builder_add (&builder, "{s@v}", "disable-static-deltas", g_variant_new_variant (g_variant_new_boolean (opt_disable_static_deltas))); diff --git a/tests/test-pull-repeated.sh b/tests/test-pull-repeated.sh index bfc7148a..306d48e5 100755 --- a/tests/test-pull-repeated.sh +++ b/tests/test-pull-repeated.sh @@ -23,7 +23,7 @@ set -euo pipefail . $(dirname $0)/libtest.sh -echo "1..2" +echo "1..4" COMMIT_SIGN="--gpg-homedir=${TEST_GPG_KEYHOME} --gpg-sign=${TEST_GPG_KEYID_1}" @@ -47,7 +47,20 @@ ${CMD_PREFIX} ostree --repo=repo rev-parse main popd echo "ok repeated pull after 500s" -# Now test from a repo which gives error 408 (request timeout) a lot of the time. +# Sanity check with no network retries and 408s given, pull should fail. +rm ostree-srv httpd repo -rf +setup_fake_remote_repo1 "archive" "${COMMIT_SIGN}" --random-408s=99 + +pushd ${test_tmpdir} +ostree_repo_init repo --mode=archive +${CMD_PREFIX} ostree --repo=repo remote add --set=gpg-verify=false origin $(cat httpd-address)/ostree/gnomerepo +assert_fail ${CMD_PREFIX} ostree --repo=repo pull --mirror origin --network-retries=0 main 2>err.txt +assert_file_has_content err.txt "\(408.*Request Timeout\)\|\(HTTP 408\)" + +popd +echo "ok no retries after a 408" + +# Test pulling a repo which gives error 408 (request timeout) a lot of the time. rm ostree-srv httpd repo -rf setup_fake_remote_repo1 "archive" "${COMMIT_SIGN}" --random-408s=50 @@ -55,7 +68,7 @@ pushd ${test_tmpdir} ostree_repo_init repo --mode=archive ${CMD_PREFIX} ostree --repo=repo remote add --set=gpg-verify=false origin $(cat httpd-address)/ostree/gnomerepo for x in $(seq 40); do - if ${CMD_PREFIX} ostree --repo=repo pull --mirror origin main 2>err.txt; then + if ${CMD_PREFIX} ostree --repo=repo pull --mirror origin --network-retries=2 main 2>err.txt; then echo "Success on iteration ${x}" break; fi @@ -67,3 +80,22 @@ ${CMD_PREFIX} ostree --repo=repo rev-parse main popd echo "ok repeated pull after 408s" + +# Test pulling a repo that gives 408s a lot of the time, with many network retries. +rm ostree-srv httpd repo -rf +setup_fake_remote_repo1 "archive" "${COMMIT_SIGN}" --random-408s=50 + +pushd ${test_tmpdir} +ostree_repo_init repo --mode=archive +${CMD_PREFIX} ostree --repo=repo remote add --set=gpg-verify=false origin $(cat httpd-address)/ostree/gnomerepo + +# Using 8 network retries gives error rate of <0.5%, when --random-408s=50 +if ${CMD_PREFIX} ostree --repo=repo pull --mirror origin --network-retries=8 main 2>err.txt; then + echo "Success with big number of network retries" +fi + +${CMD_PREFIX} ostree --repo=repo fsck +${CMD_PREFIX} ostree --repo=repo rev-parse main + +popd +echo "ok big number of retries with one 408" From 10c2fc33f68fbc7d8b9d29888d76cc0b44a170be Mon Sep 17 00:00:00 2001 From: Robert Fairley Date: Thu, 5 Jul 2018 14:59:24 -0400 Subject: [PATCH 34/47] tests: Run network retries test for many retries directly This runs a test involving many retries for the --network-retries option directly rather than inside a conditional statement, so that the command does not silently fail and allow the test to continue running. Closes: #1673 Approved by: jlebon --- tests/test-pull-repeated.sh | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/test-pull-repeated.sh b/tests/test-pull-repeated.sh index 306d48e5..a2707d6d 100755 --- a/tests/test-pull-repeated.sh +++ b/tests/test-pull-repeated.sh @@ -90,9 +90,8 @@ ostree_repo_init repo --mode=archive ${CMD_PREFIX} ostree --repo=repo remote add --set=gpg-verify=false origin $(cat httpd-address)/ostree/gnomerepo # Using 8 network retries gives error rate of <0.5%, when --random-408s=50 -if ${CMD_PREFIX} ostree --repo=repo pull --mirror origin --network-retries=8 main 2>err.txt; then - echo "Success with big number of network retries" -fi +${CMD_PREFIX} ostree --repo=repo pull --mirror origin --network-retries=8 main +echo "Success with big number of network retries" ${CMD_PREFIX} ostree --repo=repo fsck ${CMD_PREFIX} ostree --repo=repo rev-parse main From 7468600029d01b6fe2db81ebbfb86f7648b2b120 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 3 Jul 2018 17:28:48 -0400 Subject: [PATCH 35/47] deploy: Retain staged by default For `rpm-ostree ex livefs` we have a use case of pushing a rollback deployment. There's no reason this should require deleting the staged deployment (and doing so actually breaks livefs which tries to access it as a data source). I was initially very conservative here, but I think it ends up being fairly easy to retain the staged deployment. We need to handle two cases: First, when the staged is *intentionally* deleted; here, we just need to unlink the `/run` file, and then everything will be sync'd up after reloading. Second, (as in the livefs case) where we're retaining it, e.g. adding a deployment to the end. What I realized here is that we can have the code keep `new_deployments` as view without staged, and then when we do the final reload we'll end up re-reading it from disk anyways. Closes: #1672 Approved by: jlebon --- src/libostree/ostree-sysroot-deploy.c | 66 ++++++++++++------- tests/installed/destructive/staged-deploy.yml | 16 ++++- 2 files changed, 57 insertions(+), 25 deletions(-) diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index 42128e72..a4387ab2 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -2182,39 +2182,56 @@ ostree_sysroot_write_deployments_with_options (OstreeSysroot *self, { g_assert (self->loaded); - /* It dramatically simplifies a lot of the logic below if we - * drop the staged deployment from both the source deployment list, - * as well as the target list. We don't want to write it to the bootloader - * now, which is mostly what this function is concerned with. - * In the future we though should probably adapt things to keep it. + /* Dealing with the staged deployment is quite tricky here. This function is + * primarily concerned with writing out "finalized" deployments which have + * bootloader entries. Originally, we simply dropped the staged deployment + * here unconditionally. Now, the high level strategy is to retain it, but + * *only* if it's the first item in the new deployment list - otherwise, it's + * silently dropped. */ - gboolean removed_staged = FALSE; - if (self->staged_deployment) + + g_autoptr(GPtrArray) new_deployments_copy = g_ptr_array_new (); + gboolean removed_staged = (self->staged_deployment != NULL); + if (new_deployments->len > 0) { + OstreeDeployment *first = new_deployments->pdata[0]; + /* If the first deployment is the staged, we filter it out for now */ + g_assert (first); + if (first == self->staged_deployment) + { + g_assert (ostree_deployment_is_staged (first)); + + /* In this case note staged was retained */ + removed_staged = FALSE; + } + + /* Create a copy without any staged deployments */ + for (guint i = 0; i < new_deployments->len; i++) + { + OstreeDeployment *deployment = new_deployments->pdata[i]; + if (!ostree_deployment_is_staged (deployment)) + g_ptr_array_add (new_deployments_copy, deployment); + } + new_deployments = new_deployments_copy; + } + + /* Take care of removing the staged deployment's on-disk state if we should */ + if (removed_staged) + { + g_assert (self->staged_deployment); + g_assert (self->staged_deployment == self->deployments->pdata[0]); + if (!glnx_unlinkat (AT_FDCWD, _OSTREE_SYSROOT_RUNSTATE_STAGED, 0, error)) return FALSE; if (!_ostree_sysroot_rmrf_deployment (self, self->staged_deployment, cancellable, error)) return FALSE; - g_assert (self->staged_deployment == self->deployments->pdata[0]); + /* Clear it out of the *current* deployments list to maintain invariants */ + self->staged_deployment = NULL; g_ptr_array_remove_index (self->deployments, 0); - removed_staged = TRUE; } - /* First new deployment; we'll see if it's staged */ - OstreeDeployment *first_new = - (new_deployments->len > 0 ? new_deployments->pdata[0] : NULL); - g_autoptr(GPtrArray) new_deployments_copy = NULL; - if (first_new && ostree_deployment_is_staged (first_new)) - { - g_assert_cmpint (new_deployments->len, >, 0); - new_deployments_copy = g_ptr_array_sized_new (new_deployments->len - 1); - for (guint i = 1; i < new_deployments->len; i++) - g_ptr_array_add (new_deployments_copy, new_deployments->pdata[i]); - } - else - new_deployments_copy = g_ptr_array_ref (new_deployments); - new_deployments = new_deployments_copy; + const guint nonstaged_current_len = self->deployments->len - (self->staged_deployment ? 1 : 0); /* Assign a bootserial to each new deployment. */ @@ -2226,7 +2243,8 @@ ostree_sysroot_write_deployments_with_options (OstreeSysroot *self, * subbootversion bootlinks. */ gboolean requires_new_bootversion = FALSE; - if (new_deployments->len != self->deployments->len) + + if (new_deployments->len != nonstaged_current_len) requires_new_bootversion = TRUE; else { diff --git a/tests/installed/destructive/staged-deploy.yml b/tests/installed/destructive/staged-deploy.yml index 8802fb1b..bc53d06d 100644 --- a/tests/installed/destructive/staged-deploy.yml +++ b/tests/installed/destructive/staged-deploy.yml @@ -70,7 +70,7 @@ grep -vqFe '(staged)' status.txt test '!' -f /run/ostree/staged-deployment -- name: Staged should be overwritten by non-staged +- name: Staged should be overwritten by non-staged as first shell: | set -xeuo pipefail ostree admin deploy --stage staged-deploy @@ -84,5 +84,19 @@ environment: commit: "{{ rpmostree_status['deployments'][0]['checksum'] }}" +- name: Staged is retained when pushing rollback + shell: | + set -xeuo pipefail + ostree admin deploy --stage staged-deploy + test -f /run/ostree/staged-deployment + ostree admin deploy --retain-pending --not-as-default nonstaged-deploy + test -f /run/ostree/staged-deployment + ostree admin status > status.txt + grep -qFe '(staged)' status.txt + ostree admin undeploy 0 + ostree admin undeploy 1 + environment: + commit: "{{ rpmostree_status['deployments'][0]['checksum'] }}" + - name: Cleanup refs shell: ostree refs --delete staged-deploy nonstaged-deploy From 4c023a95857e69129ef55a56bbccd29bf126717e Mon Sep 17 00:00:00 2001 From: Umang Jain Date: Thu, 5 Jul 2018 16:31:48 +0530 Subject: [PATCH 36/47] lib/repo-commit: Factor out min-free-space-size error reporting Improves code readability. Closes: #1671 Approved by: jlebon --- src/libostree/ostree-repo-commit.c | 47 ++++++++++++++++-------------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index 05635afd..0b48323e 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -429,6 +429,28 @@ add_size_index_to_metadata (OstreeRepo *self, return g_variant_ref_sink (g_variant_builder_end (builder)); } +static gboolean +throw_min_free_space_error (OstreeRepo *self, + guint64 bytes_required, + GError **error) +{ + const char *err_msg = NULL; + g_autofree char *err_msg_owned = NULL; + + if (bytes_required > 0) + { + g_autofree char *formatted_required = g_format_size (bytes_required); + err_msg = err_msg_owned = g_strdup_printf ("would be exceeded, at least %s requested", formatted_required); + } + else + err_msg = "would be exceeded"; + + if (self->min_free_space_percent > 0) + return glnx_throw (error, "min-free-space-percent '%u%%' %s", self->min_free_space_percent, err_msg); + else + return glnx_throw (error, "min-free-space-size %" G_GUINT64_FORMAT "MB %s", self->min_free_space_mb, err_msg); +} + typedef struct { gboolean initialized; GLnxTmpfile tmpf; @@ -524,13 +546,7 @@ _ostree_repo_bare_content_commit (OstreeRepo *self, { self->cleanup_stagedir = TRUE; g_mutex_unlock (&self->txn_lock); - g_autofree char *formatted_required = g_format_size (st_buf.st_size); - if (self->min_free_space_percent > 0) - return glnx_throw (error, "min-free-space-percent '%u%%' would be exceeded, %s more required", - self->min_free_space_percent, formatted_required); - else - return glnx_throw (error, "min-free-space-size %luMB woulid be exceeded, %s more required", - self->min_free_space_mb, formatted_required); + return throw_min_free_space_error (self, st_buf.st_size, error); } /* This is the main bit that needs mutex protection */ self->txn.max_blocks -= object_blocks; @@ -927,13 +943,7 @@ write_content_object (OstreeRepo *self, guint64 bytes_required = (guint64)object_blocks * self->txn.blocksize; self->cleanup_stagedir = TRUE; g_mutex_unlock (&self->txn_lock); - g_autofree char *formatted_required = g_format_size (bytes_required); - if (self->min_free_space_percent > 0) - return glnx_throw (error, "min-free-space-percent '%u%%' would be exceeded, %s more required", - self->min_free_space_percent, formatted_required); - else - return glnx_throw (error, "min-free-space-size %" G_GUINT64_FORMAT "MB would be exceeded, %s more required", - self->min_free_space_mb, formatted_required); + return throw_min_free_space_error (self, bytes_required, error); } /* This is the main bit that needs mutex protection */ self->txn.max_blocks -= object_blocks; @@ -1646,16 +1656,9 @@ ostree_repo_prepare_transaction (OstreeRepo *self, self->txn.max_blocks = bfree - reserved_blocks; else { - guint64 bytes_required = bfree * self->txn.blocksize; self->cleanup_stagedir = TRUE; g_mutex_unlock (&self->txn_lock); - g_autofree char *formatted_free = g_format_size (bytes_required); - if (self->min_free_space_percent > 0) - return glnx_throw (error, "min-free-space-percent '%u%%' would be exceeded, %s available", - self->min_free_space_percent, formatted_free); - else - return glnx_throw (error, "min-free-space-size %" G_GUINT64_FORMAT "MB would be exceeded, %s available", - self->min_free_space_mb, formatted_free); + return throw_min_free_space_error (self, 0, error); } g_mutex_unlock (&self->txn_lock); From eeacbc6b29fe1ab38471ed4362c0fdefd7298d19 Mon Sep 17 00:00:00 2001 From: Umang Jain Date: Thu, 5 Jul 2018 17:07:34 +0530 Subject: [PATCH 37/47] repo: Reword min-free-space-size option's error strings It is important that we use user-friendly error strings. The reason being error strings are seen by users such as in GNOME Software's error banner. Closes: #1671 Approved by: jlebon --- src/libostree/ostree-repo.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 53d5cfda..cae38e6d 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -2670,7 +2670,7 @@ min_free_space_size_validate_and_convert (OstreeRepo *self, g_autoptr(GMatchInfo) match = NULL; if (!g_regex_match (regex, min_free_space_size_str, 0, &match)) - return glnx_throw (error, "Failed to match '^[0-9]+[GMT]B$'"); + return glnx_throw (error, "It should be of the format '123MB', '123GB' or '123TB'"); g_autofree char *size_str = g_match_info_fetch (match, 1); g_autofree char *unit = g_match_info_fetch (match, 2); @@ -2693,7 +2693,7 @@ min_free_space_size_validate_and_convert (OstreeRepo *self, guint64 min_free_space = g_ascii_strtoull (size_str, NULL, 10); if (shifts > 0 && g_bit_nth_lsf (min_free_space, 63 - shifts) != -1) - return glnx_throw (error, "Integer overflow detected"); + return glnx_throw (error, "Value was too high"); self->min_free_space_mb = min_free_space << shifts; From d6327f9dd9c4c787a5df46b339c1b0e045ec8134 Mon Sep 17 00:00:00 2001 From: Alex Kiernan Date: Sat, 7 Jul 2018 13:18:40 +0000 Subject: [PATCH 38/47] switchroot: Fix typo in comment ENINVAL => EINVAL Closes: #1676 Approved by: cgwalters --- src/switchroot/ostree-remount.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/switchroot/ostree-remount.c b/src/switchroot/ostree-remount.c index c3e39c0b..cd09fc15 100644 --- a/src/switchroot/ostree-remount.c +++ b/src/switchroot/ostree-remount.c @@ -94,7 +94,7 @@ main(int argc, char *argv[]) /* It's a mounted, read-only fs; remount it */ if (mount (target, target, NULL, MS_REMOUNT | MS_SILENT, NULL) < 0) { - /* Also ignore ENINVAL - if the target isn't a mountpoint + /* Also ignore EINVAL - if the target isn't a mountpoint * already, then assume things are OK. */ if (errno != EINVAL) From 11eb0bd227ffa1a3300e4dbde3da288a7d9d41ae Mon Sep 17 00:00:00 2001 From: Alex Kiernan Date: Sat, 7 Jul 2018 21:35:35 +0000 Subject: [PATCH 39/47] switchroot: Move late /run/ostree-booted creation to ostree-system-generator When ostree-prepare-root is pid 1, ostree-prepare-boot defers creation of /run/ostree-booted, which happens in ostree-remount, but that's too late if we need ostree-system-generator to bind /var. Add the creation of the /run/ostree-booted marker to ostree-system-generator based on the existence of the ostree= kernel command line argument (which matches the condition that ostree-remount uses). Signed-off-by: Alex Kiernan Closes: #1675 Approved by: cgwalters --- src/switchroot/ostree-prepare-root.c | 3 ++- src/switchroot/ostree-remount.c | 9 ++------- src/switchroot/ostree-system-generator.c | 23 ++++++++++++++--------- 3 files changed, 18 insertions(+), 17 deletions(-) diff --git a/src/switchroot/ostree-prepare-root.c b/src/switchroot/ostree-prepare-root.c index 0131d246..01a85569 100644 --- a/src/switchroot/ostree-prepare-root.c +++ b/src/switchroot/ostree-prepare-root.c @@ -210,8 +210,9 @@ main(int argc, char *argv[]) /* We only stamp /run now if we're running in an initramfs, i.e. we're - * not pid 1. Otherwise it's handled later via ostree-remount.service. + * not pid 1. Otherwise it's handled later via ostree-system-generator. * https://mail.gnome.org/archives/ostree-list/2018-March/msg00012.html + * https://github.com/ostreedev/ostree/pull/1675 */ if (!running_as_pid1) touch_run_ostree (); diff --git a/src/switchroot/ostree-remount.c b/src/switchroot/ostree-remount.c index cd09fc15..71b7b395 100644 --- a/src/switchroot/ostree-remount.c +++ b/src/switchroot/ostree-remount.c @@ -46,13 +46,8 @@ main(int argc, char *argv[]) struct stat stbuf; int i; - /* See comments in ostree-prepare-root.c for this. - * - * This service is triggered via - * ConditionKernelCommandLine=ostree - * but it's a lot easier for various bits of userspace to check for - * a file versus parsing the kernel cmdline. So let's ensure - * the stamp file is created here too. + /* When systemd is in use this is normally created via the generator, but + * we ensure it's created here as well for redundancy. */ touch_run_ostree (); diff --git a/src/switchroot/ostree-system-generator.c b/src/switchroot/ostree-system-generator.c index 799a3104..78bca7c4 100644 --- a/src/switchroot/ostree-system-generator.c +++ b/src/switchroot/ostree-system-generator.c @@ -41,14 +41,6 @@ static const char *arg_dest_late = "/tmp"; int main(int argc, char *argv[]) { - /* Important: if this isn't an ostree-booted system, do nothing; people could - * have the package installed as a dependency for flatpak or whatever. - */ - { struct stat stbuf; - if (fstatat (AT_FDCWD, "/run/ostree-booted", &stbuf, 0) < 0) - exit (EXIT_SUCCESS); - } - /* We conflict with the magic ostree-mount-deployment-var file for ostree-prepare-root */ { struct stat stbuf; if (fstatat (AT_FDCWD, INITRAMFS_MOUNT_VAR, &stbuf, 0) == 0) @@ -67,9 +59,22 @@ main(int argc, char *argv[]) if (argc > 3) arg_dest_late = argv[3]; + /* If we're installed on a system which isn't using OSTree for boot (e.g. + * package installed as a dependency for flatpak or whatever), silently + * exit so that we don't error, but at the same time work where switchroot + * is PID 1 (and so hasn't created /run/ostree-booted). + */ char *ostree_cmdline = read_proc_cmdline_ostree (); if (!ostree_cmdline) - errx (EXIT_FAILURE, "Failed to find ostree= kernel argument"); + exit (EXIT_SUCCESS); + + /* See comments in ostree-prepare-root.c for this. + * + * It's a lot easier for various bits of userspace to check for + * a file versus parsing the kernel cmdline. So let's ensure + * the stamp file is created here too. + */ + touch_run_ostree (); { g_autoptr(GError) local_error = NULL; if (!ostree_cmd__private__()->ostree_system_generator (ostree_cmdline, arg_dest, NULL, arg_dest_late, &local_error)) From c7b12a8730260c66bc96363a3a3c5805b325bac8 Mon Sep 17 00:00:00 2001 From: William Manley Date: Fri, 22 Jun 2018 15:28:49 +0100 Subject: [PATCH 40/47] ostree repo commit: Speed up composing trees with `--tree=ref` Running `ostree commit --tree=ref=a --tree=dir=b` involves reading the whole of a into an `OstreeMutableTree` before composing `b` on top. This is inefficient if a is a complete rootfs and b is just touching one file. We process O(size of a + size of b) directories rather than O(number of touched dirs). This commit makes `ostree commit` more efficient at composing multiple directories together. With `ostree_mutable_tree_fill_empty_from_dirtree` we create a lazy `OstreeMutableTree` which only reads the underlying information from disk when needed. We don't need to read all the subdirectories just to get the checksum of a tree already checked into the repo. This provides great speedups when composing a rootfs out of multiple other rootfs as we do in our build system. We compose multiple containers together with: ostree commit --tree=ref=base-rootfs --tree=ref=container1 --tree=ref=container2 and it is much faster now. As a test I ran time ostree --repo=... commit --orphan --tree=ref=big-rootfs --tree=dir=modified_etc Where modified_etc contained a modified sudoers file under /etc. I used `strace` to count syscalls and I seperatly took timing measurements. To test with a cold cache I ran sync && echo 3 | sudo tee /proc/sys/vm/drop_caches Results: | | Before | After | | -------------------- | ------ | ----- | | Time (cold cache) | 8.1s | 0.12s | | Time (warm cache) | 3.7s | 0.08s | | `openat` calls | 53589 | 246 | | `fgetxattr` calls | 78916 | 0 | I'm not sure if this will have some negative interaction with the `_ostree_repo_commit_modifier_apply` which is short-circuited here. I think it was disabled for `--tree=ref=` anyway, but I'm not certain. All the tests pass anyway. I originally implemented this in terms of the `OstreeRepoFile` APIs, but it was *way* less efficient, opening and reading many files unnecessarily. Closes: #1643 Approved by: cgwalters --- apidoc/ostree-sections.txt | 3 + src/libostree/libostree-devel.sym | 4 + src/libostree/ostree-mutable-tree.c | 237 +++++++++++++++++++++++++++- src/libostree/ostree-mutable-tree.h | 16 ++ src/libostree/ostree-repo-commit.c | 11 ++ 5 files changed, 267 insertions(+), 4 deletions(-) diff --git a/apidoc/ostree-sections.txt b/apidoc/ostree-sections.txt index 74c1fba0..2e174244 100644 --- a/apidoc/ostree-sections.txt +++ b/apidoc/ostree-sections.txt @@ -250,6 +250,8 @@ OstreeLzmaDecompressorClass ostree-mutable-tree OstreeMutableTree ostree_mutable_tree_new +ostree_mutable_tree_new_from_checksum +ostree_mutable_tree_check_error ostree_mutable_tree_set_metadata_checksum ostree_mutable_tree_get_metadata_checksum ostree_mutable_tree_set_contents_checksum @@ -261,6 +263,7 @@ ostree_mutable_tree_ensure_parent_dirs ostree_mutable_tree_walk ostree_mutable_tree_get_subdirs ostree_mutable_tree_get_files +ostree_mutable_tree_fill_empty_from_dirtree OSTREE_MUTABLE_TREE OSTREE_IS_MUTABLE_TREE diff --git a/src/libostree/libostree-devel.sym b/src/libostree/libostree-devel.sym index 1e866922..49aa2b97 100644 --- a/src/libostree/libostree-devel.sym +++ b/src/libostree/libostree-devel.sym @@ -19,6 +19,10 @@ /* Add new symbols here. Release commits should copy this section into -released.sym. */ LIBOSTREE_2018.7 { +global: + ostree_mutable_tree_fill_empty_from_dirtree; + ostree_mutable_tree_new_from_checksum; + ostree_mutable_tree_check_error; } LIBOSTREE_2018.6; /* Stub section for the stable release *after* this development one; don't diff --git a/src/libostree/ostree-mutable-tree.c b/src/libostree/ostree-mutable-tree.c index 25dc901e..cd18927a 100644 --- a/src/libostree/ostree-mutable-tree.c +++ b/src/libostree/ostree-mutable-tree.c @@ -26,6 +26,8 @@ #include "otutil.h" #include "ostree.h" +#include "ostree-core-private.h" + /** * SECTION:ostree-mutable-tree * @title: In-memory modifiable filesystem tree @@ -38,6 +40,15 @@ * programmatically. */ +typedef enum { + MTREE_STATE_WHOLE, + + /* MTREE_STATE_LAZY allows us to not read files and subdirs from the objects + * on disk until they're actually needed - often they won't be needed at + * all. */ + MTREE_STATE_LAZY +} OstreeMutableTreeState; + /** * OstreeMutableTree: * @@ -55,6 +66,8 @@ struct OstreeMutableTree * it sets parent = NULL on all its children (see remove_child_mtree) */ OstreeMutableTree *parent; + OstreeMutableTreeState state; + /* This is the checksum of the Dirtree object that corresponds to the current * contents of this directory. contents_checksum can be NULL if the SHA was * never calculated or contents of this mtree or any subdirectory has been @@ -71,7 +84,16 @@ struct OstreeMutableTree * and xattrs of this directory. This can be NULL. */ char *metadata_checksum; - /* const char* filename -> const char* checksum */ + /* ======== Valid for state LAZY: =========== */ + + /* The repo so we can look up the checksums. */ + OstreeRepo *repo; + + GError *cached_error; + + /* ======== Valid for state WHOLE: ========== */ + + /* const char* filename -> const char* checksum. */ GHashTable *files; /* const char* filename -> OstreeMutableTree* subtree */ @@ -80,8 +102,6 @@ struct OstreeMutableTree G_DEFINE_TYPE (OstreeMutableTree, ostree_mutable_tree, G_TYPE_OBJECT) -static void invalidate_contents_checksum (OstreeMutableTree *self); - static void ostree_mutable_tree_finalize (GObject *object) { @@ -92,9 +112,12 @@ ostree_mutable_tree_finalize (GObject *object) g_free (self->contents_checksum); g_free (self->metadata_checksum); + g_clear_pointer (&self->cached_error, g_error_free); g_hash_table_destroy (self->files); g_hash_table_destroy (self->subdirs); + g_clear_object (&self->repo); + G_OBJECT_CLASS (ostree_mutable_tree_parent_class)->finalize (object); } @@ -117,7 +140,6 @@ insert_child_mtree (OstreeMutableTree *self, const gchar* name, g_assert_null (child->parent); g_hash_table_insert (self->subdirs, g_strdup (name), child); child->parent = self; - invalidate_contents_checksum (self); } static void @@ -139,6 +161,7 @@ ostree_mutable_tree_init (OstreeMutableTree *self) g_free, g_free); self->subdirs = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, remove_child_mtree); + self->state = MTREE_STATE_WHOLE; } static void @@ -153,6 +176,78 @@ invalidate_contents_checksum (OstreeMutableTree *self) } } +/* Go from state LAZY to state WHOLE by reading the tree from disk */ +static gboolean +_ostree_mutable_tree_make_whole (OstreeMutableTree *self, + GCancellable *cancellable, + GError **error) +{ + if (self->state == MTREE_STATE_WHOLE) + return TRUE; + + g_assert_cmpuint (self->state, ==, MTREE_STATE_LAZY); + g_assert_nonnull (self->repo); + g_assert_nonnull (self->contents_checksum); + g_assert_nonnull (self->metadata_checksum); + g_assert_cmpuint (g_hash_table_size (self->files), ==, 0); + g_assert_cmpuint (g_hash_table_size (self->subdirs), ==, 0); + + g_autoptr(GVariant) dirtree = NULL; + if (!ostree_repo_load_variant (self->repo, OSTREE_OBJECT_TYPE_DIR_TREE, + self->contents_checksum, &dirtree, error)) + return FALSE; + + { + g_autoptr(GVariant) dir_file_contents = g_variant_get_child_value (dirtree, 0); + GVariantIter viter; + g_variant_iter_init (&viter, dir_file_contents); + const char *fname; + GVariant *contents_csum_v = NULL; + while (g_variant_iter_loop (&viter, "(&s@ay)", &fname, &contents_csum_v)) + { + char tmp_checksum[OSTREE_SHA256_STRING_LEN + 1]; + _ostree_checksum_inplace_from_bytes_v (contents_csum_v, tmp_checksum); + g_hash_table_insert (self->files, g_strdup (fname), + g_strdup (tmp_checksum)); + } + } + + /* Process subdirectories */ + { + g_autoptr(GVariant) dir_subdirs = g_variant_get_child_value (dirtree, 1); + const char *dname; + GVariant *subdirtree_csum_v = NULL; + GVariant *subdirmeta_csum_v = NULL; + GVariantIter viter; + g_variant_iter_init (&viter, dir_subdirs); + while (g_variant_iter_loop (&viter, "(&s@ay@ay)", &dname, + &subdirtree_csum_v, &subdirmeta_csum_v)) + { + char subdirtree_checksum[OSTREE_SHA256_STRING_LEN+1]; + _ostree_checksum_inplace_from_bytes_v (subdirtree_csum_v, subdirtree_checksum); + char subdirmeta_checksum[OSTREE_SHA256_STRING_LEN+1]; + _ostree_checksum_inplace_from_bytes_v (subdirmeta_csum_v, subdirmeta_checksum); + insert_child_mtree (self, dname, ostree_mutable_tree_new_from_checksum ( + self->repo, subdirtree_checksum, subdirmeta_checksum)); + } + } + + g_clear_object (&self->repo); + self->state = MTREE_STATE_WHOLE; + return TRUE; +} + +/* _ostree_mutable_tree_make_whole can fail if state == MTREE_STATE_LAZY, but + * we have getters that preceed the existence of MTREE_STATE_LAZY which can't + * return errors. So instead this function will fail and print a warning. */ +static gboolean +_assert_ostree_mutable_tree_make_whole (OstreeMutableTree *self) +{ + if (self->cached_error) + return FALSE; + return _ostree_mutable_tree_make_whole (self, NULL, &self->cached_error); +} + void ostree_mutable_tree_set_metadata_checksum (OstreeMutableTree *self, const char *checksum) @@ -183,6 +278,8 @@ ostree_mutable_tree_set_contents_checksum (OstreeMutableTree *self, "already has a checksum set. Old checksum %s, new checksum %s", self->contents_checksum, checksum); + _assert_ostree_mutable_tree_make_whole (self); + g_free (self->contents_checksum); self->contents_checksum = g_strdup (checksum); } @@ -215,6 +312,9 @@ ostree_mutable_tree_replace_file (OstreeMutableTree *self, if (!ot_util_filename_validate (name, error)) goto out; + if (!_ostree_mutable_tree_make_whole (self, NULL, error)) + goto out; + if (g_hash_table_lookup (self->subdirs, name)) { g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, @@ -256,6 +356,9 @@ ostree_mutable_tree_ensure_dir (OstreeMutableTree *self, if (!ot_util_filename_validate (name, error)) goto out; + if (!_ostree_mutable_tree_make_whole (self, NULL, error)) + goto out; + if (g_hash_table_lookup (self->files, name)) { g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, @@ -267,6 +370,7 @@ ostree_mutable_tree_ensure_dir (OstreeMutableTree *self, if (!ret_dir) { ret_dir = ostree_mutable_tree_new (); + invalidate_contents_checksum (self); insert_child_mtree (self, name, g_object_ref (ret_dir)); } @@ -287,6 +391,9 @@ ostree_mutable_tree_lookup (OstreeMutableTree *self, g_autoptr(OstreeMutableTree) ret_subdir = NULL; g_autofree char *ret_file_checksum = NULL; + if (!_ostree_mutable_tree_make_whole (self, NULL, error)) + goto out; + ret_subdir = ot_gobject_refz (g_hash_table_lookup (self->subdirs, name)); if (!ret_subdir) { @@ -328,6 +435,9 @@ ostree_mutable_tree_ensure_parent_dirs (OstreeMutableTree *self, OstreeMutableTree *subdir = self; /* nofree */ g_autoptr(OstreeMutableTree) ret_parent = NULL; + if (!_ostree_mutable_tree_make_whole (self, NULL, error)) + goto out; + g_assert (metadata_checksum != NULL); if (!self->metadata_checksum) @@ -348,6 +458,7 @@ ostree_mutable_tree_ensure_parent_dirs (OstreeMutableTree *self, next = g_hash_table_lookup (subdir->subdirs, name); if (!next) { + invalidate_contents_checksum (subdir); next = ostree_mutable_tree_new (); ostree_mutable_tree_set_metadata_checksum (next, metadata_checksum); insert_child_mtree (subdir, g_strdup (name), next); @@ -364,6 +475,71 @@ ostree_mutable_tree_ensure_parent_dirs (OstreeMutableTree *self, return ret; } +const char empty_tree_csum[] = "6e340b9cffb37a989ca544e6bb780a2c78901d3fb33738768511a30617afa01d"; + +/** + * ostree_mutable_tree_fill_empty_from_dirtree: + * + * Merges @self with the tree given by @contents_checksum and + * @metadata_checksum, but only if it's possible without writing new objects to + * the @repo. We can do this if either @self is empty, the tree given by + * @contents_checksum is empty or if both trees already have the same + * @contents_checksum. + * + * Returns: @TRUE if merge was successful, @FALSE if it was not possible. + * + * This function enables optimisations when composing trees. The provided + * checksums are not loaded or checked when this function is called. Instead + * the contents will be loaded only when needed. + */ +gboolean +ostree_mutable_tree_fill_empty_from_dirtree (OstreeMutableTree *self, + OstreeRepo *repo, + const char *contents_checksum, + const char *metadata_checksum) +{ + g_return_val_if_fail (repo, FALSE); + g_return_val_if_fail (contents_checksum, FALSE); + g_return_val_if_fail (metadata_checksum, FALSE); + + switch (self->state) + { + case MTREE_STATE_LAZY: + { + if (g_strcmp0 (contents_checksum, self->contents_checksum) == 0 || + g_strcmp0 (empty_tree_csum, self->contents_checksum) == 0) + break; + + if (g_strcmp0 (empty_tree_csum, contents_checksum) == 0) + { + /* Adding an empty tree to a full one - stick with the old contents */ + contents_checksum = self->contents_checksum; + break; + } + else + return FALSE; + } + case MTREE_STATE_WHOLE: + if (g_hash_table_size (self->files) == 0 && + g_hash_table_size (self->subdirs) == 0) + break; + /* We're not empty - can't convert to a LAZY tree */ + return FALSE; + default: + g_assert_not_reached (); + } + + self->state = MTREE_STATE_LAZY; + g_set_object (&self->repo, repo); + ostree_mutable_tree_set_metadata_checksum (self, metadata_checksum); + if (g_strcmp0 (self->contents_checksum, contents_checksum) != 0) + { + invalidate_contents_checksum (self); + self->contents_checksum = g_strdup (contents_checksum); + } + return TRUE; +} + /** * ostree_mutable_tree_walk: * @self: Tree @@ -392,6 +568,8 @@ 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]); if (!subdir) @@ -410,6 +588,7 @@ ostree_mutable_tree_walk (OstreeMutableTree *self, GHashTable * ostree_mutable_tree_get_subdirs (OstreeMutableTree *self) { + _assert_ostree_mutable_tree_make_whole (self); return self->subdirs; } @@ -422,9 +601,35 @@ ostree_mutable_tree_get_subdirs (OstreeMutableTree *self) GHashTable * ostree_mutable_tree_get_files (OstreeMutableTree *self) { + _assert_ostree_mutable_tree_make_whole (self); return self->files; } +/** + * ostree_mutable_tree_check_error: + * @self: Tree + * + * In some cases, a tree may be in a "lazy" state that loads + * data in the background; if an error occurred during a non-throwing + * API call, it will have been cached. This function checks for a + * cached error. The tree remains in error state. + * + * Since: 2018.7 + * Returns: `TRUE` on success + */ +gboolean +ostree_mutable_tree_check_error (OstreeMutableTree *self, + GError **error) +{ + if (self->cached_error) + { + if (error) + *error = g_error_copy (self->cached_error); + return FALSE; + } + return TRUE; +} + /** * ostree_mutable_tree_new: * @@ -435,3 +640,27 @@ ostree_mutable_tree_new (void) { return (OstreeMutableTree*)g_object_new (OSTREE_TYPE_MUTABLE_TREE, NULL); } + +/** + * ostree_mutable_tree_new_from_checksum: + * @repo: The repo which contains the objects refered by the checksums. + * @contents_checksum: dirtree checksum + * @metadata_checksum: dirmeta checksum + * + * Creates a new OstreeMutableTree with the contents taken from the given repo + * and checksums. The data will be loaded from the repo lazily as needed. + * + * Returns: (transfer full): A new tree + */ +OstreeMutableTree * +ostree_mutable_tree_new_from_checksum (OstreeRepo *repo, + const char *contents_checksum, + const char *metadata_checksum) +{ + OstreeMutableTree* out = (OstreeMutableTree*)g_object_new (OSTREE_TYPE_MUTABLE_TREE, NULL); + out->state = MTREE_STATE_LAZY; + out->repo = g_object_ref (repo); + out->contents_checksum = g_strdup (contents_checksum); + out->metadata_checksum = g_strdup (metadata_checksum); + return out; +} diff --git a/src/libostree/ostree-mutable-tree.h b/src/libostree/ostree-mutable-tree.h index 2cb5e9a7..4b7f853e 100644 --- a/src/libostree/ostree-mutable-tree.h +++ b/src/libostree/ostree-mutable-tree.h @@ -52,6 +52,11 @@ GType ostree_mutable_tree_get_type (void) G_GNUC_CONST; _OSTREE_PUBLIC OstreeMutableTree *ostree_mutable_tree_new (void); +_OSTREE_PUBLIC +OstreeMutableTree * ostree_mutable_tree_new_from_checksum (OstreeRepo *repo, + const char *contents_checksum, + const char *metadata_checksum); + _OSTREE_PUBLIC void ostree_mutable_tree_set_metadata_checksum (OstreeMutableTree *self, const char *checksum); @@ -100,6 +105,17 @@ gboolean ostree_mutable_tree_walk (OstreeMutableTree *self, OstreeMutableTree **out_subdir, GError **error); +_OSTREE_PUBLIC +gboolean ostree_mutable_tree_fill_empty_from_dirtree (OstreeMutableTree *self, + OstreeRepo *repo, + const char *contents_checksum, + const char *metadata_checksum); + +_OSTREE_PUBLIC +gboolean +ostree_mutable_tree_check_error (OstreeMutableTree *self, + GError **error); + _OSTREE_PUBLIC GHashTable * ostree_mutable_tree_get_subdirs (OstreeMutableTree *self); _OSTREE_PUBLIC diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index 0b48323e..bbbe1961 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -3602,6 +3602,14 @@ write_directory_to_mtree_internal (OstreeRepo *self, if (!ostree_repo_file_ensure_resolved (repo_dir, error)) return FALSE; + /* ostree_mutable_tree_fill_from_dirtree returns FALSE if mtree isn't + * empty: in which case we're responsible for merging the trees. */ + if (ostree_mutable_tree_fill_empty_from_dirtree (mtree, + ostree_repo_file_get_repo (repo_dir), + ostree_repo_file_tree_get_contents_checksum (repo_dir), + ostree_repo_file_get_checksum (repo_dir))) + return TRUE; + ostree_mutable_tree_set_metadata_checksum (mtree, ostree_repo_file_tree_get_metadata_checksum (repo_dir)); filter_result = OSTREE_REPO_COMMIT_FILTER_ALLOW; @@ -3915,6 +3923,9 @@ ostree_repo_write_mtree (OstreeRepo *self, const char *contents_checksum, *metadata_checksum; g_autoptr(GFile) ret_file = NULL; + if (!ostree_mutable_tree_check_error (mtree, error)) + return glnx_prefix_error (error, "mtree"); + metadata_checksum = ostree_mutable_tree_get_metadata_checksum (mtree); if (!metadata_checksum) return glnx_throw (error, "Can't commit an empty tree"); From fef07889d38c55406d687c81b0b29e86532189dc Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 9 Jul 2018 12:22:10 -0400 Subject: [PATCH 41/47] deploy: Fix overriding kernel args for staged deployments This is the inverse of https://github.com/ostreedev/ostree/pull/1558 aka commits cadece6c4f398ca61d21e497bd6e3fbb549f9cf6 and 3358698c86d80821d81443c906621c92672f99fb Needed to fix `rpm-ostree kargs` test suite with default staging; skipping a test here for now as eventually what we'll do is turn on the rpm-ostree suite fully here. Closes: #1677 Approved by: jlebon --- src/libostree/ostree-sysroot-deploy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index a4387ab2..8198e191 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -2853,7 +2853,7 @@ _ostree_sysroot_finalize_staged (OstreeSysroot *self, if (!glnx_unlinkat (AT_FDCWD, _OSTREE_SYSROOT_RUNSTATE_STAGED, 0, error)) return FALSE; - if (!sysroot_finalize_deployment (self, self->staged_deployment, NULL, merge_deployment, + if (!sysroot_finalize_deployment (self, self->staged_deployment, kargs, merge_deployment, cancellable, error)) return FALSE; From be8bbc5f877109be0de43388b401b402d8e4bfb9 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 16 Jul 2018 14:43:17 -0400 Subject: [PATCH 42/47] build-sys: Link with gpg-error directly We use the API, and not linking breaks the build with e.g. `-fuse-ld=gold` in a Fedora 28 buildroot as gold doesn't do the "search indirect dependencies" thing. Closes: #1679 Approved by: jlebon --- configure.ac | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/configure.ac b/configure.ac index 896e7806..f0907a15 100644 --- a/configure.ac +++ b/configure.ac @@ -220,6 +220,10 @@ AS_IF([ test x$have_gpgme = xno ], [ AC_MSG_ERROR([Need GPGME_PTHREAD version $LIBGPGME_DEPENDENCY or later]) ]) OSTREE_FEATURES="$OSTREE_FEATURES gpgme" +dnl This apparently doesn't ship a pkg-config file either, and we need +dnl to link to it directly. +OT_DEP_GPGME_CFLAGS="${OT_DEP_GPGME_CFLAGS} $(gpg-error-config --cflags)" +OT_DEP_GPGME_LIBS="${OT_DEP_GPGME_LIBS} $(gpg-error-config --libs)" LIBARCHIVE_DEPENDENCY="libarchive >= 2.8.0" # What's in RHEL7.2. From ededa9de416033863e1280d2a068bd52211eae3c Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 16 Jul 2018 16:23:01 -0400 Subject: [PATCH 43/47] Update libglnx For `renameat2()` fix to build with latest glibc (e.g. Fedora rawhide). Update submodule: libglnx Closes: #1680 Approved by: jlebon --- libglnx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libglnx b/libglnx index e1a78cf2..470af876 160000 --- a/libglnx +++ b/libglnx @@ -1 +1 @@ -Subproject commit e1a78cf2f5351d5394ccfb79f3f5a7b4917f73f3 +Subproject commit 470af8763ff7b99bec950a6ae0a957c1dcfc8edd From 7306577e61563d70eb67240dc275e18006882e53 Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Tue, 17 Jul 2018 22:33:19 +0100 Subject: [PATCH 44/47] Add a check for gpg-error via pkg-config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some downstreams — namely, the Yocto Project — ship gpg-error with a pkg-config file, and modify gpg-error-config to error out when you try using it instead of pkg-config. We can check for gpg-error via pkg-config, and if it's not available, fall back to gpg-error-config. Signed-off-by: Emmanuele Bassi Closes: #1682 Approved by: cgwalters --- configure.ac | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index f0907a15..add72b8f 100644 --- a/configure.ac +++ b/configure.ac @@ -220,10 +220,15 @@ AS_IF([ test x$have_gpgme = xno ], [ AC_MSG_ERROR([Need GPGME_PTHREAD version $LIBGPGME_DEPENDENCY or later]) ]) OSTREE_FEATURES="$OSTREE_FEATURES gpgme" +PKG_CHECK_MODULES(OT_DEP_GPG_ERROR, [gpg-error], [], [ dnl This apparently doesn't ship a pkg-config file either, and we need dnl to link to it directly. -OT_DEP_GPGME_CFLAGS="${OT_DEP_GPGME_CFLAGS} $(gpg-error-config --cflags)" -OT_DEP_GPGME_LIBS="${OT_DEP_GPGME_LIBS} $(gpg-error-config --libs)" + AC_PATH_PROG(GPG_ERROR_CONFIG, [gpg-error-config], [AC_MSG_ERROR([Missing gpg-error-config])]) + OT_DEP_GPG_ERROR_CFLAGS="$( $GPG_ERROR_CONFIG --cflags )" + OT_DEP_GPG_ERROR_LIBS="$( $GPG_ERROR_CONFIG --libs )" +]) +OT_DEP_GPGME_CFLAGS="${OT_DEP_GPGME_CFLAGS} ${OT_DEP_GPG_ERROR_CFLAGS}" +OT_DEP_GPGME_LIBS="${OT_DEP_GPGME_LIBS} ${OT_DEP_GPG_ERROR_LIBS}" LIBARCHIVE_DEPENDENCY="libarchive >= 2.8.0" # What's in RHEL7.2. From 66079c7b65ec3524db0f1abdca5516c0f3837222 Mon Sep 17 00:00:00 2001 From: Umang Jain Date: Mon, 16 Jul 2018 17:57:20 +0530 Subject: [PATCH 45/47] lib/repo: Allow min-free-space-size and -percent to co-exist Previously, we would error out if both of the options were mentioned in the config file (even if one of them is disabled with 0). There were few suggestions that this behavior was not quite right. Therefore, instead of throwing error and exiting, it's preferred to warn the user. Hence, the solution that worked out is: * Allow both options to exist simulateneously * Check each config's value and decide: * If both are present and are non-zero, warn the user. Also, prefer to use min-free-space-size over the another. * If both are absent, then use -percent=3% as fallback * Every other case is valid hence, no warning https://phabricator.endlessm.com/T13698 (cherry picked from commit be68991cf80f0aa1da7d36ab6e1d2c4d6c7cd3fb) Signed-off-by: Robert McQueen Closes: #1685 Approved by: cgwalters --- man/ostree.repo-config.xml | 5 ++++- src/libostree/ostree-repo.c | 30 +++++++++++++++++++----------- 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/man/ostree.repo-config.xml b/man/ostree.repo-config.xml index 602d412e..4aedc138 100644 --- a/man/ostree.repo-config.xml +++ b/man/ostree.repo-config.xml @@ -121,7 +121,10 @@ Boston, MA 02111-1307, USA. min-free-space-percent Integer percentage value (0-99) that specifies a minimum percentage of total space (in blocks) in the underlying filesystem to - keep free. The default value is 3. + keep free. The default value is 3. + In case this option is co-existing with min-free-space-size (see below), + it would be ignored and min-free-space-size check would be enforced instead. + diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index cae38e6d..f4dcb703 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -2818,12 +2818,10 @@ reload_core_config (OstreeRepo *self, } { - if (g_key_file_has_key (self->config, "core", "min-free-space-size", NULL) && - g_key_file_has_key (self->config, "core", "min-free-space-percent", NULL)) - { - return glnx_throw (error, "min-free-space-percent and min-free-space-size are mutually exclusive"); - } - else if (g_key_file_has_key (self->config, "core", "min-free-space-size", NULL)) + /* Try to parse both min-free-space-* config options first. If both are absent, fallback on 3% free space. + * If both are present and are non-zero, use min-free-space-size unconditionally and display a warning. + */ + if (g_key_file_has_key (self->config, "core", "min-free-space-size", NULL)) { g_autofree char *min_free_space_size_str = NULL; @@ -2835,21 +2833,31 @@ reload_core_config (OstreeRepo *self, if (!min_free_space_size_validate_and_convert (self, min_free_space_size_str, error)) return glnx_prefix_error (error, "Invalid min-free-space-size '%s'", min_free_space_size_str); } - else + + if (g_key_file_has_key (self->config, "core", "min-free-space-percent", NULL)) { g_autofree char *min_free_space_percent_str = NULL; - /* If changing this, be sure to change the man page too */ - const char *default_min_free_space = "3"; if (!ot_keyfile_get_value_with_default (self->config, "core", "min-free-space-percent", - default_min_free_space, - &min_free_space_percent_str, error)) + NULL, &min_free_space_percent_str, error)) return FALSE; self->min_free_space_percent = g_ascii_strtoull (min_free_space_percent_str, NULL, 10); if (self->min_free_space_percent > 99) return glnx_throw (error, "Invalid min-free-space-percent '%s'", min_free_space_percent_str); } + else if (!g_key_file_has_key (self->config, "core", "min-free-space-size", NULL)) + { + /* Default fallback of 3% free space. If changing this, be sure to change the man page too */ + self->min_free_space_percent = 3; + self->min_free_space_mb = 0; + } + + if (self->min_free_space_percent != 0 && self->min_free_space_mb != 0) + { + self->min_free_space_percent = 0; + g_debug ("Both min-free-space-percent and -size are mentioned in config. Enforcing min-free-space-size check only."); + } } { From 988f4631ceee889982bbe780c24fa160b7b39124 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 20 Jul 2018 14:13:02 -0400 Subject: [PATCH 46/47] ci: Mark insttests as not required The reliablity has just not been what we need, and they haven't yet caught any real bugs. Until I can carve off some time to truly make this reliable let's just mark it as not required. I'd like to gather more statistics on the failure scenarios. Closes: #1686 Approved by: jlebon --- .papr-ex.yaml | 2 +- .papr.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.papr-ex.yaml b/.papr-ex.yaml index 4b12f7d3..8cba2ca3 100644 --- a/.papr-ex.yaml +++ b/.papr-ex.yaml @@ -5,7 +5,7 @@ branches: - try context: FAH28-insttests -required: true +required: false container: image: registry.fedoraproject.org/fedora:28 diff --git a/.papr.yml b/.papr.yml index ab2479d2..885d6a51 100644 --- a/.papr.yml +++ b/.papr.yml @@ -5,7 +5,7 @@ branches: - try context: FAH28-insttests -required: true +required: false # FIXME; temporary workaround # https://github.com/ostreedev/ostree/pull/1513#issuecomment-378784162 From 21318bbc1f6692fbced6d79f9fe3c3a9f9ab094d Mon Sep 17 00:00:00 2001 From: Umang Jain Date: Wed, 18 Jul 2018 11:03:29 +0530 Subject: [PATCH 47/47] Release 2018.7 Request via flatpak: mainly to port min-free-space-size Closes: #1683 Approved by: cgwalters --- configure.ac | 2 +- src/libostree/libostree-devel.sym | 6 ------ src/libostree/libostree-released.sym | 7 +++++++ tests/test-symbols.sh | 2 +- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/configure.ac b/configure.ac index add72b8f..4484e06a 100644 --- a/configure.ac +++ b/configure.ac @@ -7,7 +7,7 @@ m4_define([year_version], [2018]) m4_define([release_version], [7]) 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 49aa2b97..a4040ba6 100644 --- a/src/libostree/libostree-devel.sym +++ b/src/libostree/libostree-devel.sym @@ -18,12 +18,6 @@ ***/ /* Add new symbols here. Release commits should copy this section into -released.sym. */ -LIBOSTREE_2018.7 { -global: - ostree_mutable_tree_fill_empty_from_dirtree; - ostree_mutable_tree_new_from_checksum; - ostree_mutable_tree_check_error; -} LIBOSTREE_2018.6; /* Stub section for the stable release *after* this development one; don't * edit this other than to update the last number. This is just a copy/paste diff --git a/src/libostree/libostree-released.sym b/src/libostree/libostree-released.sym index c3c1d502..1e17e581 100644 --- a/src/libostree/libostree-released.sym +++ b/src/libostree/libostree-released.sym @@ -527,6 +527,13 @@ global: ostree_validate_collection_id; } LIBOSTREE_2018.5; +LIBOSTREE_2018.7 { +global: + ostree_mutable_tree_fill_empty_from_dirtree; + ostree_mutable_tree_new_from_checksum; + ostree_mutable_tree_check_error; +} LIBOSTREE_2018.6; + /* 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 57e5f6e1..faf58dcf 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 <