From 0d6ead1bffe8a38067444d527669f0a7b84997fd Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 14 Aug 2018 19:45:54 +0000 Subject: [PATCH 01/50] Post-release version bump Closes: #1705 Approved by: jlebon --- configure.ac | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 4ccd79c6..0cf2e532 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], [8]) +m4_define([release_version], [9]) m4_define([package_version], [year_version.release_version]) AC_INIT([libostree], [package_version], [walters@verbum.org]) -is_release_build=yes +is_release_build=no AC_CONFIG_HEADER([config.h]) AC_CONFIG_MACRO_DIR([buildutil]) AC_CONFIG_AUX_DIR([build-aux]) From 630b7864020163757fdcc68cb5ef66b3ad568d48 Mon Sep 17 00:00:00 2001 From: Laurent Bonnans Date: Fri, 31 Aug 2018 12:01:47 +0200 Subject: [PATCH 02/50] lib/fetcher: Fix some memory leaks in curl fetcher Closes: #1716 Approved by: cgwalters --- src/libostree/ostree-fetcher-curl.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libostree/ostree-fetcher-curl.c b/src/libostree/ostree-fetcher-curl.c index 2e090cfa..64e1c34e 100644 --- a/src/libostree/ostree-fetcher-curl.c +++ b/src/libostree/ostree-fetcher-curl.c @@ -172,6 +172,9 @@ _ostree_fetcher_finalize (GObject *object) curl_multi_cleanup (self->multi); g_free (self->remote_name); + g_free (self->tls_ca_db_path); + g_free (self->tls_client_cert_path); + g_free (self->tls_client_key_path); g_free (self->cookie_jar_path); g_free (self->proxy); g_assert_cmpint (g_hash_table_size (self->outstanding_requests), ==, 0); From 74bdf7e173f2849dd19e8f74ee8596d30b51a320 Mon Sep 17 00:00:00 2001 From: Felix Krull Date: Wed, 29 Aug 2018 20:23:03 +0200 Subject: [PATCH 03/50] lib/grub2: Support Debian-style grub.cfg path Debian and Debian-derived systems have their GRUB configuration file in /boot/grub/grub.cfg, rather than /boot/grub2/grub.cfg. Detecting this file is necessary to correctly generate GRUB boot configuration on Debian systems. Closes: #1714 Approved by: cgwalters --- src/libostree/ostree-bootloader-grub2.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/libostree/ostree-bootloader-grub2.c b/src/libostree/ostree-bootloader-grub2.c index f0d34809..6831382c 100644 --- a/src/libostree/ostree-bootloader-grub2.c +++ b/src/libostree/ostree-bootloader-grub2.c @@ -57,7 +57,8 @@ struct _OstreeBootloaderGrub2 GObject parent_instance; OstreeSysroot *sysroot; - GFile *config_path_bios; + GFile *config_path_bios_1; + GFile *config_path_bios_2; GFile *config_path_efi; gboolean is_efi; }; @@ -77,7 +78,8 @@ _ostree_bootloader_grub2_query (OstreeBootloader *bootloader, OstreeBootloaderGrub2 *self = OSTREE_BOOTLOADER_GRUB2 (bootloader); /* Look for the BIOS path first */ - if (g_file_query_exists (self->config_path_bios, NULL)) + if (g_file_query_exists (self->config_path_bios_1, NULL) || + g_file_query_exists (self->config_path_bios_2, NULL)) { /* If we found it, we're done */ *out_is_active = TRUE; @@ -97,7 +99,7 @@ _ostree_bootloader_grub2_query (OstreeBootloader *bootloader, cancellable, error); if (!direnum) return FALSE; - + while (TRUE) { GFileInfo *file_info; @@ -448,7 +450,7 @@ _ostree_bootloader_grub2_write_config (OstreeBootloader *bootloader, } static gboolean -_ostree_bootloader_grub2_is_atomic (OstreeBootloader *bootloader) +_ostree_bootloader_grub2_is_atomic (OstreeBootloader *bootloader) { OstreeBootloaderGrub2 *self = OSTREE_BOOTLOADER_GRUB2 (bootloader); return !self->is_efi; @@ -460,7 +462,8 @@ _ostree_bootloader_grub2_finalize (GObject *object) OstreeBootloaderGrub2 *self = OSTREE_BOOTLOADER_GRUB2 (object); g_clear_object (&self->sysroot); - g_clear_object (&self->config_path_bios); + g_clear_object (&self->config_path_bios_1); + g_clear_object (&self->config_path_bios_2); g_clear_object (&self->config_path_efi); G_OBJECT_CLASS (_ostree_bootloader_grub2_parent_class)->finalize (object); @@ -493,6 +496,9 @@ _ostree_bootloader_grub2_new (OstreeSysroot *sysroot) { OstreeBootloaderGrub2 *self = g_object_new (OSTREE_TYPE_BOOTLOADER_GRUB2, NULL); self->sysroot = g_object_ref (sysroot); - self->config_path_bios = g_file_resolve_relative_path (self->sysroot->path, "boot/grub2/grub.cfg"); + /* Used by (at least) Debian */ + self->config_path_bios_1 = g_file_resolve_relative_path (self->sysroot->path, "boot/grub/grub.cfg"); + /* Used by (at least) Fedora */ + self->config_path_bios_2 = g_file_resolve_relative_path (self->sysroot->path, "boot/grub2/grub.cfg"); return self; } From 3814d075cb7306f111bfa41f10b7d719d08b2679 Mon Sep 17 00:00:00 2001 From: Umang Jain Date: Fri, 31 Aug 2018 19:44:22 +0530 Subject: [PATCH 04/50] lib/repo: Ensure min-free-space* config value doesn't overflow when converted to bytes In a subsequent commit, we add a public API to read the value of min-free-space-* value in bytes. The value for free space check is enforced in terms of block size instead of bytes. Therefore, for consistency we check while preparing the transaction that the value doesn't overflow when converted to bytes. https://phabricator.endlessm.com/T23694 Closes: #1715 Approved by: cgwalters --- src/libostree/ostree-repo-commit.c | 28 +++++++++++++++++++--------- src/libostree/ostree-repo-private.h | 1 + 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index d464cd0a..1336d282 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -1536,23 +1536,28 @@ devino_cache_lookup (OstreeRepo *self, return dev_ino_val->checksum; } -static guint64 -min_free_space_calculate_reserved_blocks (OstreeRepo *self, struct statvfs *stvfsbuf) +static gboolean +min_free_space_calculate_reserved_blocks (OstreeRepo *self, struct statvfs *stvfsbuf, GError **error) { - guint64 reserved_blocks = 0; + self->reserved_blocks = 0; if (self->min_free_space_mb > 0) { - reserved_blocks = (self->min_free_space_mb << 20) / stvfsbuf->f_bsize; + if (self->min_free_space_mb > (G_MAXUINT64 >> 20) || + self->txn.blocksize > (1 << 20)) + return glnx_throw (error, "min-free-space value is greater than the maximum allowed value of %" G_GUINT64_FORMAT " bytes", + G_MAXUINT64 / stvfsbuf->f_bsize); + + self->reserved_blocks = (self->min_free_space_mb << 20) / self->txn.blocksize; } else if (self->min_free_space_percent > 0) { /* Convert fragment to blocks to compute the total */ guint64 total_blocks = (stvfsbuf->f_frsize * stvfsbuf->f_blocks) / stvfsbuf->f_bsize; - reserved_blocks = ((double)total_blocks) * (self->min_free_space_percent/100.0); + self->reserved_blocks = ((double)total_blocks) * (self->min_free_space_percent/100.0); } - return reserved_blocks; + return TRUE; } /** @@ -1650,11 +1655,16 @@ ostree_repo_prepare_transaction (OstreeRepo *self, g_mutex_lock (&self->txn_lock); self->txn.blocksize = stvfsbuf.f_bsize; - guint64 reserved_blocks = min_free_space_calculate_reserved_blocks (self, &stvfsbuf); + if (!min_free_space_calculate_reserved_blocks (self, &stvfsbuf, error)) + { + g_mutex_unlock (&self->txn_lock); + return FALSE; + } + /* 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; + if (bfree > self->reserved_blocks) + self->txn.max_blocks = bfree - self->reserved_blocks; else { self->cleanup_stagedir = TRUE; diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h index 99eaf494..3cc4aba0 100644 --- a/src/libostree/ostree-repo-private.h +++ b/src/libostree/ostree-repo-private.h @@ -153,6 +153,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 */ + guint64 reserved_blocks; gboolean cleanup_stagedir; guint test_error_flags; /* OstreeRepoTestErrorFlags */ From 68420f70bbb288cbd4424413f86f6cfecc06d7b1 Mon Sep 17 00:00:00 2001 From: Umang Jain Date: Fri, 31 Aug 2018 19:50:29 +0530 Subject: [PATCH 05/50] lib/repo: Add an API to get min-free-space-* reserved bytes https://phabricator.endlessm.com/T23694 Closes: #1715 Approved by: cgwalters --- apidoc/ostree-sections.txt | 1 + src/libostree/libostree-devel.sym | 1 + src/libostree/ostree-repo.c | 20 ++++++++++++++++++++ src/libostree/ostree-repo.h | 2 ++ 4 files changed, 24 insertions(+) diff --git a/apidoc/ostree-sections.txt b/apidoc/ostree-sections.txt index 2e174244..70f93bab 100644 --- a/apidoc/ostree-sections.txt +++ b/apidoc/ostree-sections.txt @@ -294,6 +294,7 @@ ostree_repo_create_at ostree_repo_create ostree_repo_get_path ostree_repo_get_mode +ostree_repo_get_min_free_space_bytes ostree_repo_get_config ostree_repo_get_dfd ostree_repo_hash diff --git a/src/libostree/libostree-devel.sym b/src/libostree/libostree-devel.sym index 24db9df2..0bdade75 100644 --- a/src/libostree/libostree-devel.sym +++ b/src/libostree/libostree-devel.sym @@ -19,6 +19,7 @@ /* Add new symbols here. Release commits should copy this section into -released.sym. */ LIBOSTREE_2018.9 { +ostree_repo_get_min_free_space_bytes; } LIBOSTREE_2018.7; /* Stub section for the stable release *after* this development one; don't diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 18736c24..edcb8af9 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -3292,6 +3292,26 @@ ostree_repo_get_mode (OstreeRepo *self) return self->mode; } +/** + * ostree_repo_get_min_free_space: + * @self: Repo + * + * It should be noted that this function should be used only if there + * is a transaction active. It is a programmer error to request it + * otherwise. + * + * Returns: Value (in bytes) of min-free-space-* config option + * Since: 2018.9 + */ +guint64 +ostree_repo_get_min_free_space_bytes (OstreeRepo *self) +{ + g_return_val_if_fail (OSTREE_IS_REPO (self), 0); + g_return_val_if_fail (self->in_transaction == TRUE, 0); + + return self->reserved_blocks * self->txn.blocksize; +} + /** * ostree_repo_get_parent: * @self: Repo diff --git a/src/libostree/ostree-repo.h b/src/libostree/ostree-repo.h index 60fa3d3e..11b7927e 100644 --- a/src/libostree/ostree-repo.h +++ b/src/libostree/ostree-repo.h @@ -127,6 +127,8 @@ gboolean ostree_repo_equal (OstreeRepo *a, _OSTREE_PUBLIC OstreeRepoMode ostree_repo_get_mode (OstreeRepo *self); +_OSTREE_PUBLIC +guint64 ostree_repo_get_min_free_space_bytes (OstreeRepo *self); _OSTREE_PUBLIC GKeyFile * ostree_repo_get_config (OstreeRepo *self); From a70d2f67310bd869c6a40c4013dc51a3e2f2d2a8 Mon Sep 17 00:00:00 2001 From: Umang Jain Date: Fri, 31 Aug 2018 20:20:23 +0530 Subject: [PATCH 06/50] Add tests for ostree_repo_get_min_free_space_bytes https://phabricator.endlessm.com/T23694 Closes: #1715 Approved by: cgwalters --- src/libostree/libostree-devel.sym | 2 +- tests/test-repo.c | 54 +++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/src/libostree/libostree-devel.sym b/src/libostree/libostree-devel.sym index 0bdade75..0ba9a215 100644 --- a/src/libostree/libostree-devel.sym +++ b/src/libostree/libostree-devel.sym @@ -19,7 +19,7 @@ /* Add new symbols here. Release commits should copy this section into -released.sym. */ LIBOSTREE_2018.9 { -ostree_repo_get_min_free_space_bytes; + ostree_repo_get_min_free_space_bytes; } LIBOSTREE_2018.7; /* Stub section for the stable release *after* this development one; don't diff --git a/tests/test-repo.c b/tests/test-repo.c index edccef59..c56f60c5 100644 --- a/tests/test-repo.c +++ b/tests/test-repo.c @@ -151,6 +151,57 @@ test_repo_equal (Fixture *fixture, g_assert_false (ostree_repo_equal (closed_repo, closed_repo)); } +static void +test_repo_get_min_free_space (Fixture *fixture, + gconstpointer test_data) +{ + g_autoptr (GKeyFile) config = NULL; + g_autoptr(GError) error = NULL; + typedef struct + { + const char *val; + gboolean should_succeed; + } min_free_space_value; + + g_autoptr(OstreeRepo) repo = ostree_repo_create_at (fixture->tmpdir.fd, ".", + OSTREE_REPO_MODE_ARCHIVE, + NULL, + NULL, &error); + g_assert_no_error (error); + + min_free_space_value values_to_test[] = { + {"500MB", TRUE }, + { "0MB", TRUE }, + { "17179869185GB", FALSE }, /* Overflow parameter: bytes > G_MAXUINT64 */ + { NULL, FALSE } + }; + + config = ostree_repo_copy_config (repo); + + for (guint i = 0; values_to_test[i].val != NULL; i++) + { + g_key_file_remove_key (config, "core", "min-free-space-size", NULL); + g_key_file_set_string (config, "core", "min-free-space-size", values_to_test[i].val); + + ostree_repo_write_config (repo, config, &error); + g_assert_no_error (error); + ostree_repo_reload_config (repo, NULL, &error); + g_assert_no_error (error); + + ostree_repo_prepare_transaction (repo, FALSE, NULL, &error); + if (values_to_test[i].should_succeed) + g_assert_no_error (error); + else + continue; + + ostree_repo_get_min_free_space_bytes (repo); + g_assert_no_error (error); + + ostree_repo_commit_transaction (repo, NULL, NULL, &error); + g_assert_no_error (error); + } +} + int main (int argc, char **argv) @@ -164,6 +215,9 @@ main (int argc, test_repo_hash_closed, teardown); g_test_add ("/repo/equal", Fixture, NULL, setup, test_repo_equal, teardown); + g_test_add ("/repo/get_min_free_space", Fixture, NULL, setup, + test_repo_get_min_free_space, teardown); + return g_test_run (); } From e4e6d85ea4e778c61b8f9a0301153dd0ee00b9b6 Mon Sep 17 00:00:00 2001 From: Matthew Leeds Date: Tue, 4 Sep 2018 21:49:23 -0700 Subject: [PATCH 07/50] avahi: Be robust to missing refs in peer summaries In the OstreeRepoFinderAvahi implementation, ostree_avahi_service_build_repo_finder_result() is where the DNS-SD records are processed and turned into OstreeRepoFinderResult objects. Each result object is supposed to have a hash table mapping refs to checksums, so this is accomplished by first adding a placeholder (a ref mapping to a NULL checksum) for each ref matched by the bloom filter, and later filling in the checksums using the remote's summary file, which happens in get_checksums(). The problem is that there's no guarantee all the checksums will be resolved (non-NULL), so the ostree_repo_finder_result_new() call then hits an assertion failure in is_valid_collection_ref_map() leading to a crash (in the case that one or more refs had NULL checksums). There are at least two situations where the ref checksum might not be found in the peer remote's summary file: 1) The bloom filter match was a false positive. This is going to happen sometimes by design. 2) The peer remote's summary is out of sync with its DNS-SD records. This shouldn't normally happen but it's still good to be robust to the possibility; in Endless OS nothing guarantees the atomicity of updating the summary and DNS-SD records. This commit changes libostree to be robust to the possibility of refs missing from the peer remote's summary, by removing any that still have a NULL checksum associated with them after the summary has been fetched and processed. The other OstreeRepoFinder implementations don't have this issue because they use summary files directly and therefore always have access to the checksum. Closes: #1717 Approved by: pwithnall --- src/libostree/ostree-repo-finder-avahi.c | 45 ++++++++++++++---------- 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/src/libostree/ostree-repo-finder-avahi.c b/src/libostree/ostree-repo-finder-avahi.c index bc383769..f8d1f878 100644 --- a/src/libostree/ostree-repo-finder-avahi.c +++ b/src/libostree/ostree-repo-finder-avahi.c @@ -509,6 +509,16 @@ fill_refs_and_checksums_from_summary (GVariant *summary, return TRUE; } +static gboolean +remove_null_checksum_refs_cb (gpointer key, + gpointer value, + gpointer user_data) +{ + const char *checksum = value; + + return (checksum == NULL); +} + /* Given a summary file (@summary_bytes), extract the refs it lists, and use that * to fill in the checksums in the @supported_ref_to_checksum map. This includes * the main refs list in the summary, and the map of collection IDs to further @@ -518,14 +528,13 @@ fill_refs_and_checksums_from_summary (GVariant *summary, * set and %FALSE will be returned. If the intersection of the summary file refs * and the keys in @supported_ref_to_checksum is empty, an error is set. */ static gboolean -get_refs_and_checksums_from_summary (GBytes *summary_bytes, - GHashTable *supported_ref_to_checksum /* (element-type OstreeCollectionRef utf8) */, - GError **error) +get_refs_and_checksums_from_summary (GBytes *summary_bytes, + GHashTable *supported_ref_to_checksum /* (element-type OstreeCollectionRef utf8) */, + OstreeRemote *remote, + GError **error) { g_autoptr(GVariant) summary = g_variant_ref_sink (g_variant_new_from_bytes (OSTREE_SUMMARY_GVARIANT_FORMAT, summary_bytes, FALSE)); - GHashTableIter iter; - const OstreeCollectionRef *ref; - const gchar *checksum; + guint removed_refs; if (!g_variant_is_normal_form (summary)) { @@ -544,20 +553,20 @@ get_refs_and_checksums_from_summary (GBytes *summary_bytes, if (!fill_refs_and_checksums_from_summary (summary, supported_ref_to_checksum, error)) return FALSE; - /* Check that at least one of the refs has a non-%NULL checksum set, otherwise - * we can discard this peer. */ - g_hash_table_iter_init (&iter, supported_ref_to_checksum); - while (g_hash_table_iter_next (&iter, - (gpointer *) &ref, - (gpointer *) &checksum)) + removed_refs = g_hash_table_foreach_remove (supported_ref_to_checksum, remove_null_checksum_refs_cb, NULL); + if (removed_refs > 0) + g_debug ("Removed %d refs from the list resolved from ‘%s’ (possibly bloom filter false positives)", + removed_refs, remote->name); + + /* If none of the refs had a non-%NULL checksum set, we can discard this peer. */ + if (g_hash_table_size (supported_ref_to_checksum) == 0) { - if (checksum != NULL) - return TRUE; + g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND, + "No matching refs were found in the summary file"); + return FALSE; } - g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND, - "No matching refs were found in the summary file"); - return FALSE; + return TRUE; } /* Download the summary file from @remote, and return the bytes of the file in @@ -661,7 +670,7 @@ get_checksums (OstreeRepoFinderAvahi *finder, return FALSE; } - return get_refs_and_checksums_from_summary (summary_bytes, supported_ref_to_checksum, error); + return get_refs_and_checksums_from_summary (summary_bytes, supported_ref_to_checksum, remote, error); } /* Build some #OstreeRepoFinderResults out of the given #OstreeAvahiService by From 3c46bcdb50ddb5ce3e9778d47dab8f531a5b7a84 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Thu, 20 Sep 2018 11:11:45 -0400 Subject: [PATCH 08/50] ci: Disable f28-rpmostree for now It started failing with: ``` ERROR: tests/check/test-ucontainer.sh - too few tests run (expected 2, got 0) tap-driver.sh: internal error getting exit status tap-driver.sh: fatal: I/O or internal error make[4]: *** [Makefile:4353: tests/check/test-ucontainer.sh.log] Error 1 make[4]: *** Waiting for unfinished jobs.... ``` And the artifacts are not being saved for some reason: ``` + cleanup + mv test-suite.log /var/tmp/checkout mv: cannot stat 'test-suite.log': No such file or directory + true + mv vmcheck /var/tmp/checkout mv: cannot stat 'vmcheck': No such file or directory + true ``` Let's just disable this for now so that some other pending patches can go in while we investigate. Closes: #1727 Approved by: cgwalters --- .papr.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.papr.yml b/.papr.yml index 885d6a51..f7044e5d 100644 --- a/.papr.yml +++ b/.papr.yml @@ -162,7 +162,8 @@ branches: - try context: f28-rpmostree -required: true +# XXX: some issues currently failing that need investigating. +required: false cluster: hosts: From 6b37fe831076f368f0d5ee62d6fc262e50f265cd Mon Sep 17 00:00:00 2001 From: Matthew Leeds Date: Tue, 18 Sep 2018 13:36:06 -0700 Subject: [PATCH 09/50] lib/repo: Clean up OstreeRepo docs This fixes typos and grammar in the docs for OstreeRepo, and copies the information about OSTREE_REPO_MODE_BARE_USER_ONLY from ostree-core.h Closes: #1725 Approved by: jlebon --- src/libostree/ostree-repo.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index edcb8af9..264a2e57 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -90,17 +90,19 @@ G_STATIC_ASSERT(sizeof(OstreeRepoPruneOptions) == * The #OstreeRepo is like git, a content-addressed object store. * Unlike git, it records uid, gid, and extended attributes. * - * There are three possible "modes" for an #OstreeRepo; - * %OSTREE_REPO_MODE_BARE is very simple - content files are - * represented exactly as they are, and checkouts are just hardlinks. - * %OSTREE_REPO_MODE_BARE_USER is similar, except the uid/gids are not - * set on the files, and checkouts as hardlinks hardlinks work only for user checkouts. - * A %OSTREE_REPO_MODE_ARCHIVE_Z2 repository in contrast stores - * content files zlib-compressed. It is suitable for non-root-owned + * There are four possible "modes" for an #OstreeRepo; %OSTREE_REPO_MODE_BARE + * is very simple - content files are represented exactly as they are, and + * checkouts are just hardlinks. %OSTREE_REPO_MODE_BARE_USER is similar, except + * the uid/gids are not set on the files, and checkouts as hardlinks work only + * for user checkouts. %OSTREE_REPO_MODE_BARE_USER_ONLY is the same as + * BARE_USER, but all metadata is not stored, so it can only be used for user + * checkouts. This mode does not require xattrs. A %OSTREE_REPO_MODE_ARCHIVE + * (also known as %OSTREE_REPO_MODE_ARCHIVE_Z2) repository in contrast stores + * content files zlib-compressed. It is suitable for non-root-owned * repositories that can be served via a static HTTP server. * * Creating an #OstreeRepo does not invoke any file I/O, and thus needs - * to be initialized, either from an existing contents or with a new + * to be initialized, either from existing contents or as a new * repository. If you have an existing repo, use ostree_repo_open() * to load it from disk and check its validity. To initialize a new * repository in the given filepath, use ostree_repo_create() instead. From b32c9e0df9ea402bcae1fb23f56467311aa8d66e Mon Sep 17 00:00:00 2001 From: Robert McQueen Date: Tue, 18 Sep 2018 15:46:24 +0100 Subject: [PATCH 10/50] OstreeMutableTree: add _remove method There is no API method to remove a file or subdirectory from a MutableTree besides directly manipulating the GHashTable returned by _get_files or _get_subdirs. This isn't possible from an introspection binding that transforms the returned GHashTable, and may also leave the tree checksum in an invalid state. Introduce a new method so that removing files or subdirectories is safe, and possible from bindings. Closes: #1724 Approved by: jlebon --- apidoc/ostree-sections.txt | 1 + src/libostree/libostree-devel.sym | 1 + src/libostree/ostree-mutable-tree.c | 45 +++++++++++++++++++++++++++++ src/libostree/ostree-mutable-tree.h | 6 ++++ 4 files changed, 53 insertions(+) diff --git a/apidoc/ostree-sections.txt b/apidoc/ostree-sections.txt index 70f93bab..0ec1cfd3 100644 --- a/apidoc/ostree-sections.txt +++ b/apidoc/ostree-sections.txt @@ -257,6 +257,7 @@ ostree_mutable_tree_get_metadata_checksum ostree_mutable_tree_set_contents_checksum ostree_mutable_tree_get_contents_checksum ostree_mutable_tree_replace_file +ostree_mutable_tree_remove ostree_mutable_tree_ensure_dir ostree_mutable_tree_lookup ostree_mutable_tree_ensure_parent_dirs diff --git a/src/libostree/libostree-devel.sym b/src/libostree/libostree-devel.sym index 0ba9a215..6d1fe934 100644 --- a/src/libostree/libostree-devel.sym +++ b/src/libostree/libostree-devel.sym @@ -19,6 +19,7 @@ /* Add new symbols here. Release commits should copy this section into -released.sym. */ LIBOSTREE_2018.9 { + ostree_mutable_tree_remove; ostree_repo_get_min_free_space_bytes; } LIBOSTREE_2018.7; diff --git a/src/libostree/ostree-mutable-tree.c b/src/libostree/ostree-mutable-tree.c index cd18927a..573f6260 100644 --- a/src/libostree/ostree-mutable-tree.c +++ b/src/libostree/ostree-mutable-tree.c @@ -332,6 +332,51 @@ ostree_mutable_tree_replace_file (OstreeMutableTree *self, return ret; } +/** + * ostree_mutable_tree_remove: + * @self: Tree + * @name: Name of file or subdirectory to remove + * @allow_noent: If @FALSE, an error will be thrown if @name does not exist in the tree + * @error: a #GError + * + * Remove the file or subdirectory named @name from the mutable tree @self. + */ +gboolean +ostree_mutable_tree_remove (OstreeMutableTree *self, + const char *name, + gboolean allow_noent, + GError **error) +{ + gboolean ret = FALSE; + + g_return_val_if_fail (name != NULL, FALSE); + + if (!ot_util_filename_validate (name, error)) + goto out; + + if (!_ostree_mutable_tree_make_whole (self, NULL, error)) + goto out; + + if (!g_hash_table_remove (self->files, name) && + !g_hash_table_remove (self->subdirs, name)) + { + if (allow_noent) + { + ret = TRUE; + goto out; + } + + set_error_noent (error, name); + goto out; + } + + invalidate_contents_checksum (self); + + ret = TRUE; + out: + return ret; +} + /** * ostree_mutable_tree_ensure_dir: * @self: Tree diff --git a/src/libostree/ostree-mutable-tree.h b/src/libostree/ostree-mutable-tree.h index 4b7f853e..753f96e7 100644 --- a/src/libostree/ostree-mutable-tree.h +++ b/src/libostree/ostree-mutable-tree.h @@ -77,6 +77,12 @@ gboolean ostree_mutable_tree_replace_file (OstreeMutableTree *self, const char *checksum, GError **error); +_OSTREE_PUBLIC +gboolean ostree_mutable_tree_remove (OstreeMutableTree *self, + const char *name, + gboolean allow_noent, + GError **error); + _OSTREE_PUBLIC gboolean ostree_mutable_tree_ensure_dir (OstreeMutableTree *self, const char *name, From fc84fb402c397213108710e17ddf88108f0ae2cb Mon Sep 17 00:00:00 2001 From: Matthew Leeds Date: Wed, 19 Sep 2018 14:59:47 -0700 Subject: [PATCH 11/50] lib/repo: Define a metadata key, ostree.deploy-collection-id This commit defines a metadata key that tells clients to update their remote config to add a collection ID. This functionality is currently implemented in Flatpak for the key "xa.collection-id", but there are two good reasons for moving the key to OSTree: 1) Servers such as Flathub shouldn't set xa.collection-id in their metadata now or in the medium term future, because many users are still using old versions of Flatpak and OSTree[1] which would hit various bugs[2][3][4] on the P2P code paths that are enabled by collection IDs. Defining a new key means that only clients running recent (as-yet-unreleased) versions of Flatpak and OSTree will pay attention to it and deploy the collection ID, leaving the users on old versions unaffected. 2) OSTree is as "invested" in collection IDs as Flatpak, so there's no reason the key should be defined in Flatpak rather than here. According to Philip Withnall, the reason the key was put in Flatpak originally was that at the time there was uncertainty about tying OSTree to collection IDs. [1] https://ahayzen.com/direct/flathub.html#downloadsbyflatpakstacked [2] https://github.com/ostreedev/ostree/commit/e4e6d85ea [3] https://github.com/flatpak/flatpak/commit/5813639f [4] https://github.com/flatpak/flatpak/commit/5b21a5b7 Closes: #1726 Approved by: pwithnall --- apidoc/ostree-sections.txt | 1 + src/libostree/ostree-repo.h | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/apidoc/ostree-sections.txt b/apidoc/ostree-sections.txt index 0ec1cfd3..440fe0c3 100644 --- a/apidoc/ostree-sections.txt +++ b/apidoc/ostree-sections.txt @@ -598,6 +598,7 @@ ostree_repo_pull_from_remotes_async ostree_repo_pull_from_remotes_finish ostree_repo_resolve_keyring_for_collection OSTREE_REPO_METADATA_REF +OSTREE_META_KEY_DEPLOY_COLLECTION_ID
diff --git a/src/libostree/ostree-repo.h b/src/libostree/ostree-repo.h index 11b7927e..6159fc24 100644 --- a/src/libostree/ostree-repo.h +++ b/src/libostree/ostree-repo.h @@ -1397,6 +1397,29 @@ gboolean ostree_repo_regenerate_summary (OstreeRepo *self, */ #define OSTREE_REPO_METADATA_REF "ostree-metadata" +/** + * OSTREE_META_KEY_DEPLOY_COLLECTION_ID: + * + * GVariant type `s`. This key can be used in the repo metadata which is stored + * in OSTREE_REPO_METADATA_REF as well as in the summary. The semantics of this + * are that the remote repository wants clients to update their remote config + * to add this collection ID (clients can't do P2P operations involving a + * remote without a collection ID configured on it, even if one is configured + * on the server side). Clients must never change or remove a collection ID + * already set in their remote config. + * + * Currently, OSTree does not implement changing a remote config based on this + * key, but it may do so in a later release, and until then clients such as + * Flatpak may implement it. + * + * This is a replacement for the similar metadata key implemented by flatpak, + * `xa.collection-id`, which is now deprecated as clients which supported it had + * bugs with their P2P implementations. + * + * Since: 2018.9 + */ +#define OSTREE_META_KEY_DEPLOY_COLLECTION_ID "ostree.deploy-collection-id" + G_END_DECLS From a0937b6cf03996416c338b996a87677d6b65da14 Mon Sep 17 00:00:00 2001 From: Umang Jain Date: Fri, 14 Sep 2018 01:07:32 +0530 Subject: [PATCH 12/50] lib/repo: Separate min-free-space-* calculation from transaction codepath Earlier, the actual reserved space (in blocks) were calculated inside the transaction codepath ostree_repo_prepare_transaction(). However, while reworking on ostree_repo_get_min_free_space_bytes() API, it was realized that this calculation can be done independently from the transaction's codepaths, hence enabling the usage for ostree_repo_get_min_free_space_bytes() API irrespective of whether there is an ongoing transaction or not. https://github.com/ostreedev/ostree/issues/1720 Closes: #1722 Approved by: pwithnall --- src/libostree/ostree-repo-commit.c | 28 ++-------------- src/libostree/ostree-repo.c | 53 +++++++++++++++++++++++++----- src/libostree/ostree-repo.h | 4 ++- 3 files changed, 50 insertions(+), 35 deletions(-) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index 1336d282..7c676f4c 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -1536,30 +1536,6 @@ devino_cache_lookup (OstreeRepo *self, return dev_ino_val->checksum; } -static gboolean -min_free_space_calculate_reserved_blocks (OstreeRepo *self, struct statvfs *stvfsbuf, GError **error) -{ - self->reserved_blocks = 0; - - if (self->min_free_space_mb > 0) - { - if (self->min_free_space_mb > (G_MAXUINT64 >> 20) || - self->txn.blocksize > (1 << 20)) - return glnx_throw (error, "min-free-space value is greater than the maximum allowed value of %" G_GUINT64_FORMAT " bytes", - G_MAXUINT64 / stvfsbuf->f_bsize); - - self->reserved_blocks = (self->min_free_space_mb << 20) / self->txn.blocksize; - } - else if (self->min_free_space_percent > 0) - { - /* Convert fragment to blocks to compute the total */ - guint64 total_blocks = (stvfsbuf->f_frsize * stvfsbuf->f_blocks) / stvfsbuf->f_bsize; - self->reserved_blocks = ((double)total_blocks) * (self->min_free_space_percent/100.0); - } - - return TRUE; -} - /** * ostree_repo_scan_hardlinks: * @self: An #OstreeRepo @@ -1631,6 +1607,7 @@ ostree_repo_prepare_transaction (OstreeRepo *self, GError **error) { g_autoptr(_OstreeRepoAutoTransaction) txn = NULL; + guint64 reserved_bytes = 0; g_return_val_if_fail (self->in_transaction == FALSE, FALSE); @@ -1655,11 +1632,12 @@ ostree_repo_prepare_transaction (OstreeRepo *self, g_mutex_lock (&self->txn_lock); self->txn.blocksize = stvfsbuf.f_bsize; - if (!min_free_space_calculate_reserved_blocks (self, &stvfsbuf, error)) + if (!ostree_repo_get_min_free_space_bytes (self, &reserved_bytes, error)) { g_mutex_unlock (&self->txn_lock); return FALSE; } + self->reserved_blocks = reserved_bytes / self->txn.blocksize; /* Use the appropriate free block count if we're unprivileged */ guint64 bfree = (getuid () != 0 ? stvfsbuf.f_bavail : stvfsbuf.f_bfree); diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 264a2e57..5c8f0b6f 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -2655,6 +2655,37 @@ get_remotes_d_dir (OstreeRepo *self, return g_file_resolve_relative_path (sysroot, SYSCONF_REMOTES); } +static gboolean +min_free_space_calculate_reserved_bytes (OstreeRepo *self, guint64 *bytes, GError **error) +{ + guint64 reserved_bytes = 0; + + struct statvfs stvfsbuf; + if (TEMP_FAILURE_RETRY (fstatvfs (self->repo_dir_fd, &stvfsbuf)) < 0) + return glnx_throw_errno_prefix (error, "fstatvfs"); + + if (self->min_free_space_mb > 0) + { + if (self->min_free_space_mb > (G_MAXUINT64 >> 20)) + return glnx_throw (error, "min-free-space value is greater than the maximum allowed value of %" G_GUINT64_FORMAT " bytes", + (G_MAXUINT64 >> 20)); + + reserved_bytes = self->min_free_space_mb << 20; + } + else if (self->min_free_space_percent > 0) + { + if (stvfsbuf.f_frsize > (G_MAXUINT64 / stvfsbuf.f_blocks)) + return glnx_throw (error, "Filesystem's size is greater than the maximum allowed value of %" G_GUINT64_FORMAT " bytes", + (G_MAXUINT64 / stvfsbuf.f_blocks)); + + guint64 total_bytes = (stvfsbuf.f_frsize * stvfsbuf.f_blocks); + reserved_bytes = ((double)total_bytes) * (self->min_free_space_percent/100.0); + } + + *bytes = reserved_bytes; + return TRUE; +} + static gboolean min_free_space_size_validate_and_convert (OstreeRepo *self, const char *min_free_space_size_str, @@ -3297,21 +3328,25 @@ ostree_repo_get_mode (OstreeRepo *self) /** * ostree_repo_get_min_free_space: * @self: Repo + * @out_reserved_bytes: (out): Location to store the result + * @error: Return location for a #GError * - * It should be noted that this function should be used only if there - * is a transaction active. It is a programmer error to request it - * otherwise. + * It can be used to query the value (in bytes) of min-free-space-* config option. * - * Returns: Value (in bytes) of min-free-space-* config option + * Returns: %TRUE on success, %FALSE otherwise. * Since: 2018.9 */ -guint64 -ostree_repo_get_min_free_space_bytes (OstreeRepo *self) +gboolean +ostree_repo_get_min_free_space_bytes (OstreeRepo *self, guint64 *out_reserved_bytes, GError **error) { - g_return_val_if_fail (OSTREE_IS_REPO (self), 0); - g_return_val_if_fail (self->in_transaction == TRUE, 0); + g_return_val_if_fail (OSTREE_IS_REPO (self), FALSE); + g_return_val_if_fail (out_reserved_bytes != NULL, FALSE); + g_return_val_if_fail (error == NULL || *error == NULL, FALSE); - return self->reserved_blocks * self->txn.blocksize; + if (!min_free_space_calculate_reserved_bytes (self, out_reserved_bytes, error)) + return glnx_prefix_error (error, "Error calculating min-free-space bytes"); + + return TRUE; } /** diff --git a/src/libostree/ostree-repo.h b/src/libostree/ostree-repo.h index 6159fc24..a6544761 100644 --- a/src/libostree/ostree-repo.h +++ b/src/libostree/ostree-repo.h @@ -128,7 +128,9 @@ _OSTREE_PUBLIC OstreeRepoMode ostree_repo_get_mode (OstreeRepo *self); _OSTREE_PUBLIC -guint64 ostree_repo_get_min_free_space_bytes (OstreeRepo *self); +gboolean ostree_repo_get_min_free_space_bytes (OstreeRepo *self, + guint64 *out_reserved_bytes, + GError **error); _OSTREE_PUBLIC GKeyFile * ostree_repo_get_config (OstreeRepo *self); From 5d2e62affb7eaf919411fff667f1097fb14885a1 Mon Sep 17 00:00:00 2001 From: Umang Jain Date: Fri, 14 Sep 2018 20:22:37 +0530 Subject: [PATCH 13/50] tests: Update tests for ostree_repo_get_min_free_space_bytes() https://github.com/ostreedev/ostree/issues/1720 Closes: #1722 Approved by: pwithnall --- tests/test-repo.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/tests/test-repo.c b/tests/test-repo.c index c56f60c5..9857228e 100644 --- a/tests/test-repo.c +++ b/tests/test-repo.c @@ -157,6 +157,7 @@ test_repo_get_min_free_space (Fixture *fixture, { g_autoptr (GKeyFile) config = NULL; g_autoptr(GError) error = NULL; + guint64 bytes = 0; typedef struct { const char *val; @@ -188,17 +189,11 @@ test_repo_get_min_free_space (Fixture *fixture, ostree_repo_reload_config (repo, NULL, &error); g_assert_no_error (error); - ostree_repo_prepare_transaction (repo, FALSE, NULL, &error); + ostree_repo_get_min_free_space_bytes (repo, &bytes, &error); if (values_to_test[i].should_succeed) g_assert_no_error (error); else continue; - - ostree_repo_get_min_free_space_bytes (repo); - g_assert_no_error (error); - - ostree_repo_commit_transaction (repo, NULL, NULL, &error); - g_assert_no_error (error); } } From 44d5f1cb8c1544b5df332a32149c61745fcfc43c Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 21 Sep 2018 09:47:36 -0400 Subject: [PATCH 14/50] deploy: Fix removing /var/.updated with separate /var mount There's some subtlety to this, we don't handle all cases. But the 99% cases are using `--sysroot deploy` to create an initial deployment, and then doing upgrades from inside a booted deployment. It was only the latter case that didn't work with a separate `/var`. Fixing all of them would probably require libostree to learn how to e.g. look at `/etc/fstab` (or worse, systemd mount units?) and handle the mounting. I don't think we want to do anything like that right now, since there are no active drivers for the use case. Closes: https://github.com/ostreedev/ostree/issues/1729 Closes: #1730 Approved by: akiernan --- src/libostree/ostree-sysroot-deploy.c | 46 ++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index 8198e191..246f1114 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -2509,6 +2509,47 @@ sysroot_initialize_deployment (OstreeSysroot *self, return TRUE; } +/* Get a directory fd for the /var of @deployment. + * Before we supported having /var be a separate mount point, + * this was easy. However, as https://github.com/ostreedev/ostree/issues/1729 + * raised, in the primary case where we're + * doing a new deployment for the booted stateroot, + * we need to use /var/. This code doesn't correctly + * handle the case of `ostree admin --sysroot upgrade`, + * nor (relatedly) the case of upgrading a separate stateroot. + */ +static gboolean +get_var_dfd (OstreeSysroot *self, + int osdeploy_dfd, + OstreeDeployment *deployment, + int *ret_fd, + GError **error) +{ + const char *booted_stateroot = + self->booted_deployment ? ostree_deployment_get_osname (self->booted_deployment) : NULL; + + int base_dfd; + const char *base_path; + /* The common case is when we're doing a new deployment for the same stateroot (osname). + * If we have a separate mounted /var, then we need to use it - the /var in the + * stateroot will probably just be an empty directory. + * + * If the stateroot doesn't match, just fall back to /var in the target's stateroot. + */ + if (g_strcmp0 (booted_stateroot, ostree_deployment_get_osname (deployment)) == 0) + { + base_dfd = AT_FDCWD; + base_path = "/var"; + } + else + { + base_dfd = osdeploy_dfd; + base_path = "var"; + } + + return glnx_opendirat (base_dfd, base_path, TRUE, ret_fd, error); +} + static gboolean sysroot_finalize_deployment (OstreeSysroot *self, OstreeDeployment *deployment, @@ -2547,6 +2588,9 @@ sysroot_finalize_deployment (OstreeSysroot *self, glnx_autofd int os_deploy_dfd = -1; if (!glnx_opendirat (self->sysroot_fd, osdeploypath, TRUE, &os_deploy_dfd, error)) return FALSE; + glnx_autofd int var_dfd = -1; + if (!get_var_dfd (self, os_deploy_dfd, deployment, &var_dfd, error)) + return FALSE; /* Ensure that the new deployment does not have /etc/.updated or * /var/.updated so that systemd ConditionNeedsUpdate=/etc|/var services run @@ -2554,7 +2598,7 @@ sysroot_finalize_deployment (OstreeSysroot *self, */ if (!ot_ensure_unlinked_at (deployment_dfd, "etc/.updated", error)) return FALSE; - if (!ot_ensure_unlinked_at (os_deploy_dfd, "var/.updated", error)) + if (!ot_ensure_unlinked_at (var_dfd, ".updated", error)) return FALSE; g_autoptr(OstreeSePolicy) sepolicy = ostree_sepolicy_new_at (deployment_dfd, cancellable, error); From 8b2940aa133de47632bd5a438d29f5dae8566705 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Fri, 21 Sep 2018 12:28:43 -0400 Subject: [PATCH 15/50] lib/fetcher-util: Mark journaled msgs as LOG_ERR E.g. for filtering, and so it shows up in red. Closes: #1732 Approved by: cgwalters --- src/libostree/ostree-fetcher-util.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libostree/ostree-fetcher-util.c b/src/libostree/ostree-fetcher-util.c index 6f759c86..35e8c3c3 100644 --- a/src/libostree/ostree-fetcher-util.c +++ b/src/libostree/ostree-fetcher-util.c @@ -172,6 +172,7 @@ _ostree_fetcher_journal_failure (const char *remote_name, "MESSAGE_ID=" SD_ID128_FORMAT_STR, SD_ID128_FORMAT_VAL(OSTREE_HTTP_FAILURE_ID), "OSTREE_REMOTE=%s", remote_name, "OSTREE_URL=%s", url, + "PRIORITY=%i", LOG_ERR, NULL); #endif } From a88032a09e4c7979dcb429bfda51494f01376fed Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Fri, 21 Sep 2018 12:30:35 -0400 Subject: [PATCH 16/50] lib/fetcher-curl: Drop unnecessary check `_ostree_fetcher_journal_failure()` already checks that we only log messages which have remotes. Closes: #1732 Approved by: cgwalters --- src/libostree/ostree-fetcher-curl.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/libostree/ostree-fetcher-curl.c b/src/libostree/ostree-fetcher-curl.c index 64e1c34e..2583143e 100644 --- a/src/libostree/ostree-fetcher-curl.c +++ b/src/libostree/ostree-fetcher-curl.c @@ -330,9 +330,8 @@ check_multi_info (OstreeFetcher *fetcher) g_task_return_new_error (task, G_IO_ERROR, G_IO_ERROR_FAILED, "[%u] %s", curlres, curl_easy_strerror (curlres)); - if (req->fetcher->remote_name) - _ostree_fetcher_journal_failure (req->fetcher->remote_name, - eff_url, curl_easy_strerror (curlres)); + _ostree_fetcher_journal_failure (req->fetcher->remote_name, + eff_url, curl_easy_strerror (curlres)); } } else From 4aadbe2159cd6096529d147dba8bd13470903039 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Fri, 21 Sep 2018 12:31:57 -0400 Subject: [PATCH 17/50] lib/fetcher-curl: Prefix fatal errors with full URL Just include the whole URL that failed if libcurl failed with something elementary like CURLE_COULDNT_CONNECT or CURLE_COULDNT_RESOLVE_HOST. Closes: #1731 Closes: #1732 Approved by: cgwalters --- src/libostree/ostree-fetcher-curl.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/libostree/ostree-fetcher-curl.c b/src/libostree/ostree-fetcher-curl.c index 2583143e..9738f980 100644 --- a/src/libostree/ostree-fetcher-curl.c +++ b/src/libostree/ostree-fetcher-curl.c @@ -322,13 +322,12 @@ check_multi_info (OstreeFetcher *fetcher) { /* Handle file not found */ g_task_return_new_error (task, G_IO_ERROR, G_IO_ERROR_NOT_FOUND, - "%s", - curl_easy_strerror (curlres)); + "%s", curl_easy_strerror (curlres)); } else { - g_task_return_new_error (task, G_IO_ERROR, G_IO_ERROR_FAILED, "[%u] %s", - curlres, + g_task_return_new_error (task, G_IO_ERROR, G_IO_ERROR_FAILED, + "While fetching %s: [%u] %s", eff_url, curlres, curl_easy_strerror (curlres)); _ostree_fetcher_journal_failure (req->fetcher->remote_name, eff_url, curl_easy_strerror (curlres)); From fc357adb7915c80037136b16dc519cf645e33e36 Mon Sep 17 00:00:00 2001 From: Matthew Leeds Date: Fri, 21 Sep 2018 15:35:50 -0700 Subject: [PATCH 18/50] create-usb: Always use archive mode Change the create-usb command so that it always creates the destination repository using the "archive" mode, rather than using archive mode when xattrs aren't supported and bare-user otherwise. This has a few advantages: 1. The archive mode works with FAT filesystems, which is what most USB drives are, and which doesn't support xattrs. 2. At least in some quick testing I did, archive mode is about twice as performant as bare-user mode, in terms of how long it takes for the create-usb command to complete. 3. This ensures that a tool can safely change the permissions on ".ostree/repo" and subdirectories after create-usb completes, which is important for Endless since otherwise you can't use `ostree create-usb` as root and then `flatpak create-usb` as a non-root user on the same USB drive (or in other words copy OS updates and apps to the same USB). Closes: #1733 Approved by: cgwalters --- src/ostree/ot-builtin-create-usb.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/ostree/ot-builtin-create-usb.c b/src/ostree/ot-builtin-create-usb.c index eee637c8..4230d7d1 100644 --- a/src/ostree/ot-builtin-create-usb.c +++ b/src/ostree/ot-builtin-create-usb.c @@ -102,21 +102,17 @@ ostree_builtin_create_usb (int argc, /* Open the destination repository on the USB stick or create it if it doesn’t exist. * Check it’s below @mount_root_path, and that it’s not the same as the source - * repository. - * - * If the destination file system supports xattrs (for example, ext4), we use - * a BARE_USER repository; if it doesn’t (for example, FAT), we use ARCHIVE. - * In either case, we want a lossless repository. */ + * repository. */ const char *dest_repo_path = (opt_destination_repo != NULL) ? opt_destination_repo : ".ostree/repo"; if (!glnx_shutil_mkdir_p_at (mount_root_dfd, dest_repo_path, 0755, cancellable, error)) return FALSE; - OstreeRepoMode mode = OSTREE_REPO_MODE_BARE_USER; - - if (TEMP_FAILURE_RETRY (fgetxattr (mount_root_dfd, "user.test", NULL, 0)) < 0 && - (errno == ENOTSUP || errno == EOPNOTSUPP)) - mode = OSTREE_REPO_MODE_ARCHIVE; + /* Always use the archive repo mode, which works on FAT file systems that + * don't support xattrs, compresses files to save space, doesn't store + * permission info directly in the file attributes, and is at least sometimes + * more performant than bare-user */ + OstreeRepoMode mode = OSTREE_REPO_MODE_ARCHIVE; g_debug ("%s: Creating repository in mode %u", G_STRFUNC, mode); From c141fe610b652f383a9c445169d3cc6f28f99bd1 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 24 Sep 2018 13:37:29 -0400 Subject: [PATCH 19/50] lib/commit: Don't copy xattrs for metadata objects Copying the xattrs on metadata objects is wrong in general, we don't "own" them. Notably this would fail in the situation of doing a pull from e.g. a `bare-user` source to a destination that was on a different mount point (so we couldn't hardlink), and the source had e.g. a `security.selinux` attribute. Closes: #1734 Closes: #1736 Approved by: jlebon --- src/libostree/ostree-repo-commit.c | 5 +++-- tests/installed/nondestructive/itest-pull.sh | 13 +++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index 7c676f4c..e883cedc 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -4293,11 +4293,12 @@ import_one_object_direct (OstreeRepo *dest_repo, } /* Don't want to copy xattrs for archive repos, nor for - * bare-user-only. + * bare-user-only. We also only do this for content + * objects. */ const gboolean src_is_bare_or_bare_user = G_IN_SET (src_repo->mode, OSTREE_REPO_MODE_BARE, OSTREE_REPO_MODE_BARE_USER); - if (src_is_bare_or_bare_user) + if (src_is_bare_or_bare_user && !OSTREE_OBJECT_TYPE_IS_META(objtype)) { g_autoptr(GVariant) xattrs = NULL; diff --git a/tests/installed/nondestructive/itest-pull.sh b/tests/installed/nondestructive/itest-pull.sh index fc2047ed..07056ea1 100755 --- a/tests/installed/nondestructive/itest-pull.sh +++ b/tests/installed/nondestructive/itest-pull.sh @@ -62,6 +62,19 @@ ostree --repo=repo init --mode=bare log_timestamps ostree --repo=repo pull-local /ostree/repo ${host_commit} log_timestamps ostree --repo=repo fsck cd .. + +# Also, we shouldn't copy xattrs on metadata objects +commit_path=objects/${host_commit:0:2}/${host_commit:2}.commit +ostree --repo=testarchive init --mode=archive +ostree --repo=testarchive pull-local --commit-metadata-only /ostree/repo ${host_commit} +setfattr -n user.ostreetesting -v hello testarchive/${commit_path} +ostree --repo=mnt/testarchive2 init --mode=archive +ostree --repo=mnt/testarchive2 pull-local --commit-metadata-only testarchive ${host_commit} +if getfattr -m user.ostreetesting mnt/testarchive2/${commit_path} 2>/dev/null; then + fatal "copied metadata xattr" +fi +echo "ok no metadata xattr copy" + umount mnt # Cleanup From 9a06c5409ec551a56fa1ad137efcd1f2d7044496 Mon Sep 17 00:00:00 2001 From: Matthew Leeds Date: Mon, 24 Sep 2018 13:57:42 -0700 Subject: [PATCH 20/50] create-usb: Add a --commit option Currently on Endless OS, the OSTree ref for the operating system is something like os/eos/amd64/eos3, so that's what gets passed to `ostree create-usb` when copying the OS to a USB drive (for offline updates). However, when eos-updater checks for updates it pulls the metadata for a candidate commit and in so doing updates that eos3 ref to point to the partial commit being examined as a potential update rather than the deployed commit. This causes `ostree create-usb` to fail with an error like "No such metadata object 7fb045cb2d1f1f3a81bfc157c6128ff443eb56350315b9536bdb56aee0659863.dirtree". OSTree creates deployment refs that look like "ostree/1/1/0" to maintain a pointer to the deployed commit, but create-usb can't use these because it shows up in the summary as just a ref, not a collection-ref. So this commit adds a --commit option to the create-usb command, so we can use the appropriate ref but copy the deployed commit rather than a (potentially partial) update commit. Closes: #1735 Approved by: cgwalters --- man/ostree-create-usb.xml | 9 +++++++++ src/ostree/ot-builtin-create-usb.c | 10 +++++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/man/ostree-create-usb.xml b/man/ostree-create-usb.xml index d250d915..c3dfbc68 100644 --- a/man/ostree-create-usb.xml +++ b/man/ostree-create-usb.xml @@ -112,6 +112,15 @@ Boston, MA 02111-1307, USA. + + =COMMIT + + + Pull COMMIT instead of whatever REF points to. This can only + be used if a single ref is specified. + + + diff --git a/src/ostree/ot-builtin-create-usb.c b/src/ostree/ot-builtin-create-usb.c index 4230d7d1..849684f6 100644 --- a/src/ostree/ot-builtin-create-usb.c +++ b/src/ostree/ot-builtin-create-usb.c @@ -33,11 +33,13 @@ static gboolean opt_disable_fsync = FALSE; static char *opt_destination_repo = NULL; +static char *opt_commit = 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", "DEST" }, + { "commit", 0, 0, G_OPTION_ARG_STRING, &opt_commit, "Pull a specific commit (only works when a single ref is specified)", "COMMIT" }, { NULL } }; @@ -78,6 +80,12 @@ ostree_builtin_create_usb (int argc, return FALSE; } + if (opt_commit && argc > 4) + { + ot_util_usage_error (context, "The --commit option can only be used when a single COLLECTION-ID REF pair is specified", error); + return FALSE; + } + /* Open the USB stick, which must exist. Allow automounting and following symlinks. */ const char *mount_root_path = argv[1]; struct stat mount_root_stbuf; @@ -154,7 +162,7 @@ ostree_builtin_create_usb (int argc, const OstreeCollectionRef *ref = g_ptr_array_index (refs, i); g_variant_builder_add (&refs_builder, "(sss)", - ref->collection_id, ref->ref_name, ""); + ref->collection_id, ref->ref_name, opt_commit ?: ""); } { From 0c8a6d64edcbc137d870f2e36ad40d2a8023c857 Mon Sep 17 00:00:00 2001 From: Matthew Leeds Date: Thu, 27 Sep 2018 18:07:51 -0700 Subject: [PATCH 21/50] lib/repo: Allow disabling lock timeout Currently the locking code checks if the value -1 was set for the config key "lock-timeout-secs" and if so, a thread trying to acquire a lock will block indefinitely. Positive values specify how long to attempt to acquire a lock in a non-blocking way (the attempt is made once every second). But when the value is read from the config file, g_ascii_strtoull() is used, which converts it to an unsigned integer. This commit makes libostree use g_ascii_strtoll() instead, so that it's possible to set that key to -1 as intended. Closes: #1737 Approved by: jlebon --- src/libostree/ostree-repo.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 5c8f0b6f..19f715aa 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -2832,7 +2832,7 @@ reload_core_config (OstreeRepo *self, &lock_timeout_seconds, error)) return FALSE; - self->lock_timeout_seconds = g_ascii_strtoull (lock_timeout_seconds, NULL, 10); + self->lock_timeout_seconds = g_ascii_strtoll (lock_timeout_seconds, NULL, 10); } } From 7892d35c0b192ad16d07dee93cec88d7294f9b63 Mon Sep 17 00:00:00 2001 From: Matthew Leeds Date: Thu, 27 Sep 2018 18:29:30 -0700 Subject: [PATCH 22/50] lib/repo: Fix minor mistake in locking docs The config option is "lock-timeout-secs" not "lock-timeout". Closes: #1737 Approved by: jlebon --- src/libostree/ostree-repo.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 19f715aa..73ee027c 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -454,9 +454,9 @@ pop_repo_lock (OstreeRepo *self, * state is not changed and the stack is simply updated. * * ostree_repo_lock_push() waits for the lock depending on the repository's - * lock-timeout configuration. When lock-timeout is -1, a blocking lock is + * lock-timeout-secs configuration. When lock-timeout-secs is -1, a blocking lock is * attempted. Otherwise, the lock is taken non-blocking and - * ostree_repo_lock_push() will sleep synchronously up to lock-timeout seconds + * ostree_repo_lock_push() will sleep synchronously up to lock-timeout-secs seconds * attempting to acquire the lock. If the lock cannot be acquired within the * timeout, a %G_IO_ERROR_WOULD_BLOCK error is returned. * @@ -544,9 +544,9 @@ _ostree_repo_lock_push (OstreeRepo *self, * lock. * * ostree_repo_lock_pop() waits for the lock depending on the repository's - * lock-timeout configuration. When lock-timeout is -1, a blocking lock is + * lock-timeout-secs configuration. When lock-timeout-secs is -1, a blocking lock is * attempted. Otherwise, the lock is removed non-blocking and - * ostree_repo_lock_pop() will sleep synchronously up to lock-timeout seconds + * ostree_repo_lock_pop() will sleep synchronously up to lock-timeout-secs seconds * attempting to remove the lock. If the lock cannot be removed within the * timeout, a %G_IO_ERROR_WOULD_BLOCK error is returned. * From 1a7536c0f92063ccb36b43850c6ef8c75c9f84df Mon Sep 17 00:00:00 2001 From: Matthew Leeds Date: Thu, 27 Sep 2018 18:30:10 -0700 Subject: [PATCH 23/50] man/ostree.repo-config: Document locking options This commit documents the "locking" and "lock-timeout-secs" options which have been around for a few releases. Closes: #1737 Approved by: jlebon --- man/ostree.repo-config.xml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/man/ostree.repo-config.xml b/man/ostree.repo-config.xml index dc126d65..8942a00b 100644 --- a/man/ostree.repo-config.xml +++ b/man/ostree.repo-config.xml @@ -198,6 +198,23 @@ Boston, MA 02111-1307, USA. + + locking + Boolean value controlling whether or not OSTree does + repository locking internally. This uses file locks and is + hence for multiple process exclusion (e.g. Flatpak and OSTree + writing to the same repository separately). This is enabled by + default since 2018.5. + + + + + lock-timeout-secs + Integer value controlling the number of seconds to + block while attempting to acquire a lock (see above). A value + of -1 means block indefinitely. The default value is 30. + + From 6e3d4f9054b7a9ee571c7c679ab3adb346f973ee Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Wed, 26 Sep 2018 20:43:02 -0400 Subject: [PATCH 24/50] ci: Bump rpm-ostree tag to 2018.8 2018.7 started failing `test-ucontainer.sh`. I don't have the cycles to look more deeply into what was going on there, but bumping to 2018.8 fixes it at least. (And of course, it's passing in rpm-ostree too.) Closes: #1728 Approved by: cgwalters --- ci/rpmostree.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/rpmostree.sh b/ci/rpmostree.sh index 0d8a5e41..421aca38 100755 --- a/ci/rpmostree.sh +++ b/ci/rpmostree.sh @@ -6,7 +6,7 @@ set -xeuo pipefail # Frozen to a tag for now to help predictability; it's # also useful to test building *older* versions since # that must work. -RPMOSTREE_TAG=v2018.7 +RPMOSTREE_TAG=v2018.8 dn=$(dirname $0) . ${dn}/libpaprci/libbuild.sh From 899b0bfad29643c3b215aa922a6d2e5ae94edd8c Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 28 Sep 2018 16:34:53 -0400 Subject: [PATCH 25/50] lib/progress: Fix leak of GSource Closes: https://github.com/ostreedev/ostree/issues/1738 Closes: #1741 Approved by: jlebon --- src/libostree/ostree-async-progress.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libostree/ostree-async-progress.c b/src/libostree/ostree-async-progress.c index 9b105886..a8e629ee 100644 --- a/src/libostree/ostree-async-progress.c +++ b/src/libostree/ostree-async-progress.c @@ -228,7 +228,7 @@ idle_invoke_async_progress (gpointer user_data) OstreeAsyncProgress *self = user_data; g_mutex_lock (&self->lock); - self->idle_source = NULL; + g_clear_pointer (&self->idle_source, g_source_unref); g_mutex_unlock (&self->lock); g_signal_emit (self, signals[CHANGED], 0); From 39d5db7e1ef99b7c01a21dab3b189ccf660f9d48 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Fri, 28 Sep 2018 17:36:45 -0400 Subject: [PATCH 26/50] lib/mutable-tree: Port to new style Some therapeutic style conversion to finish off the week. Pretty straightforward overall. Closes: #1742 Approved by: cgwalters --- src/libostree/ostree-mutable-tree.c | 118 +++++++++------------------- 1 file changed, 38 insertions(+), 80 deletions(-) diff --git a/src/libostree/ostree-mutable-tree.c b/src/libostree/ostree-mutable-tree.c index 573f6260..f6577210 100644 --- a/src/libostree/ostree-mutable-tree.c +++ b/src/libostree/ostree-mutable-tree.c @@ -305,31 +305,22 @@ ostree_mutable_tree_replace_file (OstreeMutableTree *self, const char *checksum, GError **error) { - gboolean ret = FALSE; - g_return_val_if_fail (name != NULL, FALSE); if (!ot_util_filename_validate (name, error)) - goto out; + return FALSE; if (!_ostree_mutable_tree_make_whole (self, NULL, error)) - goto out; + return FALSE; if (g_hash_table_lookup (self->subdirs, name)) - { - g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, - "Can't replace directory with file: %s", name); - goto out; - } + return glnx_throw (error, "Can't replace directory with file: %s", name); invalidate_contents_checksum (self); g_hash_table_replace (self->files, g_strdup (name), g_strdup (checksum)); - - ret = TRUE; - out: - return ret; + return TRUE; } /** @@ -347,34 +338,24 @@ ostree_mutable_tree_remove (OstreeMutableTree *self, gboolean allow_noent, GError **error) { - gboolean ret = FALSE; - g_return_val_if_fail (name != NULL, FALSE); if (!ot_util_filename_validate (name, error)) - goto out; + return FALSE; if (!_ostree_mutable_tree_make_whole (self, NULL, error)) - goto out; + return FALSE; if (!g_hash_table_remove (self->files, name) && !g_hash_table_remove (self->subdirs, name)) { if (allow_noent) - { - ret = TRUE; - goto out; - } - - set_error_noent (error, name); - goto out; + return TRUE; /* NB: early return */ + return set_error_noent (error, name); } invalidate_contents_checksum (self); - - ret = TRUE; - out: - return ret; + return TRUE; } /** @@ -393,36 +374,29 @@ ostree_mutable_tree_ensure_dir (OstreeMutableTree *self, OstreeMutableTree **out_subdir, GError **error) { - gboolean ret = FALSE; - g_autoptr(OstreeMutableTree) ret_dir = NULL; - g_return_val_if_fail (name != NULL, FALSE); if (!ot_util_filename_validate (name, error)) - goto out; + return FALSE; if (!_ostree_mutable_tree_make_whole (self, NULL, error)) - goto out; + return FALSE; if (g_hash_table_lookup (self->files, name)) - { - g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, - "Can't replace file with directory: %s", name); - goto out; - } + return glnx_throw (error, "Can't replace file with directory: %s", name); - ret_dir = ot_gobject_refz (g_hash_table_lookup (self->subdirs, name)); + g_autoptr(OstreeMutableTree) ret_dir = + ot_gobject_refz (g_hash_table_lookup (self->subdirs, name)); if (!ret_dir) { ret_dir = ostree_mutable_tree_new (); invalidate_contents_checksum (self); insert_child_mtree (self, name, g_object_ref (ret_dir)); } - - ret = TRUE; - ot_transfer_out_value (out_subdir, &ret_dir); - out: - return ret; + + if (out_subdir) + *out_subdir = g_steal_pointer (&ret_dir); + return TRUE; } gboolean @@ -432,29 +406,24 @@ ostree_mutable_tree_lookup (OstreeMutableTree *self, OstreeMutableTree **out_subdir, GError **error) { - gboolean ret = FALSE; - g_autoptr(OstreeMutableTree) ret_subdir = NULL; - g_autofree char *ret_file_checksum = NULL; - if (!_ostree_mutable_tree_make_whole (self, NULL, error)) - goto out; + return FALSE; - ret_subdir = ot_gobject_refz (g_hash_table_lookup (self->subdirs, name)); + g_autofree char *ret_file_checksum = NULL; + g_autoptr(OstreeMutableTree) ret_subdir = + ot_gobject_refz (g_hash_table_lookup (self->subdirs, name)); if (!ret_subdir) { ret_file_checksum = g_strdup (g_hash_table_lookup (self->files, name)); if (!ret_file_checksum) - { - set_error_noent (error, name); - goto out; - } + return set_error_noent (error, name); } - ret = TRUE; - ot_transfer_out_value (out_file_checksum, &ret_file_checksum); - ot_transfer_out_value (out_subdir, &ret_subdir); - out: - return ret; + if (out_file_checksum) + *out_file_checksum = g_steal_pointer (&ret_file_checksum); + if (out_subdir) + *out_subdir = g_steal_pointer (&ret_subdir); + return TRUE; } /** @@ -475,49 +444,38 @@ ostree_mutable_tree_ensure_parent_dirs (OstreeMutableTree *self, OstreeMutableTree **out_parent, GError **error) { - gboolean ret = FALSE; - int i; - OstreeMutableTree *subdir = self; /* nofree */ - g_autoptr(OstreeMutableTree) ret_parent = NULL; + g_assert (metadata_checksum != NULL); if (!_ostree_mutable_tree_make_whole (self, NULL, error)) - goto out; - - g_assert (metadata_checksum != NULL); + return FALSE; if (!self->metadata_checksum) ostree_mutable_tree_set_metadata_checksum (self, metadata_checksum); - for (i = 0; i+1 < split_path->len; i++) + OstreeMutableTree *subdir = self; /* nofree */ + for (guint i = 0; i+1 < split_path->len; i++) { OstreeMutableTree *next; const char *name = split_path->pdata[i]; if (g_hash_table_lookup (subdir->files, name)) - { - g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, - "Can't replace file with directory: %s", name); - goto out; - } + return glnx_throw (error, "Can't replace file with directory: %s", name); next = g_hash_table_lookup (subdir->subdirs, name); - if (!next) + 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); } - + subdir = next; } - ret_parent = g_object_ref (subdir); - - ret = TRUE; - ot_transfer_out_value (out_parent, &ret_parent); - out: - return ret; + if (out_parent) + *out_parent = g_object_ref (subdir); + return TRUE; } const char empty_tree_csum[] = "6e340b9cffb37a989ca544e6bb780a2c78901d3fb33738768511a30617afa01d"; From 5cada0f051403255059e6eeeb27665f87311a4e5 Mon Sep 17 00:00:00 2001 From: Matthew Leeds Date: Sun, 30 Sep 2018 22:36:42 -0700 Subject: [PATCH 27/50] bash-completion: Fix --repo autocomplete This commit fixes the bash tab completion handling of the "--repo" argument. Before this commit, the completion only works if "--repo" comes after the main command. After this commit, you can use "--repo" directly after "ostree" in the command line, as is natural. Closes: #1745 Approved by: jlebon --- bash/ostree | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/bash/ostree b/bash/ostree index 677aee7c..52b111ec 100644 --- a/bash/ostree +++ b/bash/ostree @@ -24,12 +24,6 @@ # - Structured option arguments (e.g. --foo KEY=VALUE) are not parsed. # # - Static deltas could likely be completed. (e.g. ostree static-delta delete [TAB]) -# -# - The "--repo PATH" option needs to come after the subcommand or it -# won't be picked up for completion of subsequent options. -# i.e. ostree commit --repo PATH ... <-- works -# ostree --repo PATH commit ... <-- does not work -# (Possibly an easy fix.) # Finds the position of the first non-flag word. @@ -183,9 +177,16 @@ __ostree_subcommands() { # This handles "ostree [TAB]" (without a subcommand). _ostree_ostree() { + case "$prev" in + --repo) + __ostree_compreply_dirs_only + return 0 + ;; + esac + case "$cur" in -*) - COMPREPLY=( $( compgen -W "$main_boolean_options" -- "$cur" ) ) + COMPREPLY=( $( compgen -W "$main_options" -- "$cur" ) ) ;; *) COMPREPLY=( $( compgen -W "$commands" -- "$cur" ) ) @@ -1753,6 +1754,14 @@ _ostree() { --verbose -v --version " + local main_options_with_args=" + --repo + " + local main_options_with_args_glob=$( __ostree_to_extglob "$main_options_with_args" ) + local main_options=" + $main_boolean_options + $main_options_with_args + " COMPREPLY=() local cur prev words cword From 2c55bc699778a69c77d66b4448a9dfa284764057 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sun, 30 Sep 2018 20:10:14 -0400 Subject: [PATCH 28/50] Only verify OSTREE_MAX_METADATA_SIZE for HTTP fetches There are use cases for libostree as a local content store for content derived or delivered via other mechanisms (e.g. OCI images, RPMs, etc.). rpm-ostree today imports RPMs into OSTree branches, and puts the RPM header value as commit metadata. Some of these can be quite large because the header includes permissions for each file. Similarly, some OCI metadata is large. Since there's no security issues with this, support committing such content. We still by default limit the size of metadata fetches, although for good measure we make this configurable too via a new `max-metadata-size` value. Closes: https://github.com/ostreedev/ostree/issues/1721 Closes: #1744 Approved by: jlebon --- src/libostree/ostree-core.h | 14 ++++----- src/libostree/ostree-repo-commit.c | 37 ---------------------- src/libostree/ostree-repo-pull.c | 8 ++++- tests/test-basic-c.c | 49 ++++++++++++++++++++++++++++++ 4 files changed, 63 insertions(+), 45 deletions(-) diff --git a/src/libostree/ostree-core.h b/src/libostree/ostree-core.h index 08b7d451..69477a75 100644 --- a/src/libostree/ostree-core.h +++ b/src/libostree/ostree-core.h @@ -31,18 +31,18 @@ G_BEGIN_DECLS /** * OSTREE_MAX_METADATA_SIZE: - * - * Maximum permitted size in bytes of metadata objects. This is an - * arbitrary number, but really, no one should be putting humongous - * data in metadata. + * + * Default limit for maximum permitted size in bytes of metadata objects fetched + * over HTTP (including repo/config files, refs, and commit/dirtree/dirmeta + * objects). This is an arbitrary number intended to mitigate disk space + * exhaustion attacks. */ #define OSTREE_MAX_METADATA_SIZE (10 * 1024 * 1024) /** * OSTREE_MAX_METADATA_WARN_SIZE: - * - * Objects committed above this size will be allowed, but a warning - * will be emitted. + * + * This variable is no longer meaningful, it is kept only for compatibility. */ #define OSTREE_MAX_METADATA_WARN_SIZE (7 * 1024 * 1024) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index e883cedc..9521dc6c 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -1336,18 +1336,6 @@ write_metadata_object (OstreeRepo *self, gsize len; const guint8 *bufp = g_bytes_get_data (buf, &len); - /* Do the size warning here, to avoid warning for already extant metadata */ - if (G_UNLIKELY (len > OSTREE_MAX_METADATA_WARN_SIZE)) - { - g_autofree char *metasize = g_format_size (len); - g_autofree char *warnsize = g_format_size (OSTREE_MAX_METADATA_WARN_SIZE); - g_autofree char *maxsize = g_format_size (OSTREE_MAX_METADATA_SIZE); - g_warning ("metadata object %s is %s, which is larger than the warning threshold of %s." \ - " The hard limit on metadata size is %s. Put large content in the tree itself, not in metadata.", - actual_checksum, - metasize, warnsize, maxsize); - } - /* Write the metadata to a temporary file */ g_auto(GLnxTmpfile) tmpf = { 0, }; if (!glnx_open_tmpfile_linkable_at (commit_tmp_dfd (self), ".", O_WRONLY|O_CLOEXEC, @@ -2278,25 +2266,6 @@ ostree_repo_abort_transaction (OstreeRepo *self, return TRUE; } -/* These limits were introduced since in some cases we may be processing - * malicious metadata, and we want to make disk space exhaustion attacks harder. - */ -static gboolean -metadata_size_valid (OstreeObjectType objtype, - gsize len, - GError **error) -{ - if (G_UNLIKELY (len > OSTREE_MAX_METADATA_SIZE)) - { - g_autofree char *input_bytes = g_format_size (len); - g_autofree char *max_bytes = g_format_size (OSTREE_MAX_METADATA_SIZE); - return glnx_throw (error, "Metadata object of type '%s' is %s; maximum metadata size is %s", - ostree_object_type_to_string (objtype), input_bytes, max_bytes); - } - - return TRUE; -} - /** * ostree_repo_write_metadata: * @self: Repo @@ -2349,9 +2318,6 @@ ostree_repo_write_metadata (OstreeRepo *self, normalized = g_variant_get_normal_form (object); } - if (!metadata_size_valid (objtype, g_variant_get_size (normalized), error)) - return FALSE; - /* For untrusted objects, verify their structure here */ if (expected_checksum) { @@ -2389,9 +2355,6 @@ ostree_repo_write_metadata_stream_trusted (OstreeRepo *self, GCancellable *cancellable, GError **error) { - if (length > 0 && !metadata_size_valid (objtype, length, error)) - return FALSE; - /* This is all pretty ridiculous, but we're keeping this API for backwards * compatibility, it doesn't really need to be fast. */ diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index c6b70ffb..f5c52e4a 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -150,6 +150,7 @@ typedef struct { gboolean timestamp_check; /* Verify commit timestamps */ int maxdepth; + guint64 max_metadata_size; guint64 start_time; gboolean is_mirror; @@ -2193,7 +2194,7 @@ start_fetch (OtPullData *pull_data, if (expected_max_size_p) expected_max_size = *expected_max_size_p; else if (OSTREE_OBJECT_TYPE_IS_META (objtype)) - expected_max_size = OSTREE_MAX_METADATA_SIZE; + expected_max_size = pull_data->max_metadata_size; else expected_max_size = 0; @@ -3488,6 +3489,7 @@ initiate_request (OtPullData *pull_data, * * require-static-deltas (b): Require static deltas * * override-commit-ids (as): Array of specific commit IDs to fetch for refs * * timestamp-check (b): Verify commit timestamps are newer than current (when pulling via ref); Since: 2017.11 + * * metadata-size-restriction (t): Restrict metadata objects to a maximum number of bytes; 0 to disable. Since: 2018.9 * * dry-run (b): Only print information on what will be downloaded (requires static deltas) * * override-url (s): Fetch objects from this URL if remote specifies no metalink in options * * inherit-transaction (b): Don't initiate, finish or abort a transaction, useful to do multiple pulls in one transaction. @@ -3543,6 +3545,9 @@ ostree_repo_pull_with_options (OstreeRepo *self, */ const char *the_ref_to_fetch = NULL; + /* Default */ + pull_data->max_metadata_size = OSTREE_MAX_METADATA_SIZE; + if (options) { int flags_i = OSTREE_REPO_PULL_FLAGS_NONE; @@ -3570,6 +3575,7 @@ ostree_repo_pull_with_options (OstreeRepo *self, (void) g_variant_lookup (options, "update-frequency", "u", &update_frequency); (void) g_variant_lookup (options, "localcache-repos", "^a&s", &opt_localcache_repos); (void) g_variant_lookup (options, "timestamp-check", "b", &pull_data->timestamp_check); + (void) g_variant_lookup (options, "max-metadata-size", "t", &pull_data->max_metadata_size); (void) g_variant_lookup (options, "append-user-agent", "s", &pull_data->append_user_agent); opt_n_network_retries_set = g_variant_lookup (options, "n-network-retries", "u", &pull_data->n_network_retries); diff --git a/tests/test-basic-c.c b/tests/test-basic-c.c index 8515b129..ff6d353c 100644 --- a/tests/test-basic-c.c +++ b/tests/test-basic-c.c @@ -430,6 +430,54 @@ test_devino_cache_xattrs (void) g_assert_cmpint (stats.content_objects_written, ==, 1); } +/* https://github.com/ostreedev/ostree/issues/1721 + * We should be able to commit large metadata objects now. + */ +static void +test_big_metadata (void) +{ + g_autoptr(GError) error = NULL; + gboolean ret = FALSE; + + g_autoptr(GFile) repo_path = g_file_new_for_path ("repo"); + + /* init as bare-user-only so we run everywhere */ + ret = ot_test_run_libtest ("setup_test_repository bare-user-only", &error); + g_assert_no_error (error); + g_assert (ret); + + g_autoptr(OstreeRepo) repo = ostree_repo_new (repo_path); + ret = ostree_repo_open (repo, NULL, &error); + g_assert_no_error (error); + g_assert (ret); + + g_autoptr(GFile) object_to_commit = NULL; + ret = ostree_repo_read_commit (repo, "test2", &object_to_commit, NULL, NULL, &error); + g_assert_no_error (error); + g_assert (ret); + + g_autoptr(OstreeMutableTree) mtree = ostree_mutable_tree_new (); + ret = ostree_repo_write_directory_to_mtree (repo, object_to_commit, mtree, NULL, + NULL, &error); + g_assert_no_error (error); + g_assert (ret); + + const size_t len = 20 * 1024 * 1024; + g_assert_cmpint (len, >, OSTREE_MAX_METADATA_SIZE); + g_autofree char *large_buf = g_malloc (len); + memset (large_buf, 0x42, len); + g_autoptr(GVariantBuilder) builder = + g_variant_builder_new (G_VARIANT_TYPE ("a{sv}")); + g_autofree char *commit_checksum = NULL; + g_variant_builder_add (builder, "{sv}", "large-value", + g_variant_new_fixed_array ((GVariantType*)"y", + large_buf, len, sizeof (char))); + ret = ostree_repo_write_commit (repo, NULL, NULL, NULL, g_variant_builder_end (builder), + OSTREE_REPO_FILE (object_to_commit), &commit_checksum, NULL, &error); + g_assert_no_error (error); + g_assert (ret); +} + int main (int argc, char **argv) { g_autoptr(GError) error = NULL; @@ -447,6 +495,7 @@ int main (int argc, char **argv) g_test_add_func ("/xattrs-devino-cache", test_devino_cache_xattrs); g_test_add_func ("/break-hardlink", test_break_hardlink); g_test_add_func ("/remotename", test_validate_remotename); + g_test_add_func ("/big-metadata", test_big_metadata); return g_test_run(); out: From 039bbe5682a6d7136df714bde8606f181592b5dd Mon Sep 17 00:00:00 2001 From: Matthew Leeds Date: Mon, 1 Oct 2018 21:22:40 -0700 Subject: [PATCH 29/50] man/create-usb: Don't recommend summary updates This commit removes the recommendation in the create-usb man page for the user to update the summary in the source repo before using the create-usb command. I'm not sure where I got the idea that create-usb depends on a summary in the source repo. I went back to the first commit that introduced the create-usb command and even using that a summary isn't required, so it seems unlikely that this changed recently. This is good news because the exclusive lock that's taken for summary updates has been causing problems on Endless (due to other processes having a lock for the duration of the 30 second acquire time out period). Closes: #1746 Approved by: cgwalters --- man/ostree-create-usb.xml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/man/ostree-create-usb.xml b/man/ostree-create-usb.xml index c3dfbc68..90b2fc5b 100644 --- a/man/ostree-create-usb.xml +++ b/man/ostree-create-usb.xml @@ -83,11 +83,6 @@ Boston, MA 02111-1307, USA. USB mounts use signed per-repo and per-commit metadata instead of summary signatures. - - This command relies on the summary file in the source repo, so you - may want to run ostree summary -u before running - this command. - From 367be40a8995fb4fb3434690369ad36561688d86 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Fri, 28 Sep 2018 14:21:39 -0400 Subject: [PATCH 30/50] boot: Remove [Install] from ostree-finalize-staged Let's just make this service not installable anymore. It should only be activated manually. Closes: #1750 Approved by: cgwalters --- src/boot/ostree-finalize-staged.service | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/boot/ostree-finalize-staged.service b/src/boot/ostree-finalize-staged.service index 570138cd..bc24a612 100644 --- a/src/boot/ostree-finalize-staged.service +++ b/src/boot/ostree-finalize-staged.service @@ -31,6 +31,3 @@ Conflicts=final.target Type=oneshot RemainAfterExit=yes ExecStop=/usr/bin/ostree admin finalize-staged - -[Install] -WantedBy=multi-user.target From 9161eb8c326442cd2d9a79c14ce8dd09733ed023 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Fri, 28 Sep 2018 14:15:22 -0400 Subject: [PATCH 31/50] boot: Add Documentation= lines to services It's a neat way to point folks to the documentation (of course, better would be to have man pages for each of those services). Also consistently use Title Case everywhere. Closes: #1750 Approved by: cgwalters --- src/boot/ostree-finalize-staged.service | 1 + src/boot/ostree-prepare-root.service | 1 + src/boot/ostree-remount.service | 3 ++- 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/boot/ostree-finalize-staged.service b/src/boot/ostree-finalize-staged.service index bc24a612..2ae64918 100644 --- a/src/boot/ostree-finalize-staged.service +++ b/src/boot/ostree-finalize-staged.service @@ -19,6 +19,7 @@ # https://lists.freedesktop.org/archives/systemd-devel/2018-March/040557.html [Unit] Description=OSTree Finalize Staged Deployment +Documentation=man:ostree(1) ConditionPathExists=/run/ostree-booted DefaultDependencies=no diff --git a/src/boot/ostree-prepare-root.service b/src/boot/ostree-prepare-root.service index 52cf8863..5467bcfd 100644 --- a/src/boot/ostree-prepare-root.service +++ b/src/boot/ostree-prepare-root.service @@ -17,6 +17,7 @@ [Unit] Description=OSTree Prepare OS/ +Documentation=man:ostree(1) DefaultDependencies=no ConditionKernelCommandLine=ostree ConditionPathExists=/etc/initrd-release diff --git a/src/boot/ostree-remount.service b/src/boot/ostree-remount.service index 47e1387a..b98110c2 100644 --- a/src/boot/ostree-remount.service +++ b/src/boot/ostree-remount.service @@ -16,7 +16,8 @@ # Boston, MA 02111-1307, USA. [Unit] -Description=OSTree Remount OS/ bind mounts +Description=OSTree Remount OS/ Bind Mounts +Documentation=man:ostree(1) DefaultDependencies=no ConditionKernelCommandLine=ostree OnFailure=emergency.target From 05e99da7a7395e80353210a576db52ba5062f1e7 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Fri, 5 Oct 2018 17:06:21 -0400 Subject: [PATCH 32/50] lib/sysroot-deploy: Write to journal when finalizing Write to the journal when starting to finalize a staged deployment. Combined with the "Transaction completed" message we already emit, this makes it easy later on to determine whether the operation was successful by inspecting the journal. This will be used by `rpm-ostree status`. Closes: #1750 Approved by: cgwalters --- src/libostree/ostree-sysroot-deploy.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index 246f1114..b424d5e9 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -49,9 +49,10 @@ #include "libglnx.h" #ifdef HAVE_LIBSYSTEMD -#define OSTREE_VARRELABEL_ID SD_ID128_MAKE(da,67,9b,08,ac,d3,45,04,b7,89,d9,6f,81,8e,a7,81) -#define OSTREE_CONFIGMERGE_ID SD_ID128_MAKE(d3,86,3b,ae,c1,3e,44,49,ab,03,84,68,4a,8a,f3,a7) -#define OSTREE_DEPLOYMENT_COMPLETE_ID SD_ID128_MAKE(dd,44,0e,3e,54,90,83,b6,3d,0e,fc,7d,c1,52,55,f1) +#define OSTREE_VARRELABEL_ID SD_ID128_MAKE(da,67,9b,08,ac,d3,45,04,b7,89,d9,6f,81,8e,a7,81) +#define OSTREE_CONFIGMERGE_ID SD_ID128_MAKE(d3,86,3b,ae,c1,3e,44,49,ab,03,84,68,4a,8a,f3,a7) +#define OSTREE_DEPLOYMENT_COMPLETE_ID SD_ID128_MAKE(dd,44,0e,3e,54,90,83,b6,3d,0e,fc,7d,c1,52,55,f1) +#define OSTREE_DEPLOYMENT_FINALIZING_ID SD_ID128_MAKE(e8,64,6c,d6,3d,ff,46,25,b7,79,09,a8,e7,a4,09,94) #endif /* @@ -2862,6 +2863,21 @@ _ostree_sysroot_finalize_staged (OstreeSysroot *self, if (!self->staged_deployment) return TRUE; + /* Notice we send this *after* the trivial `return TRUE` above; this msg implies we've + * committed to finalizing the deployment. */ +#ifdef HAVE_LIBSYSTEMD + sd_journal_send ("MESSAGE_ID=" SD_ID128_FORMAT_STR, + SD_ID128_FORMAT_VAL(OSTREE_DEPLOYMENT_FINALIZING_ID), + "MESSAGE=Finalizing staged deployment", + "OSTREE_OSNAME=%s", + ostree_deployment_get_osname (self->staged_deployment), + "OSTREE_CHECKSUM=%s", + ostree_deployment_get_csum (self->staged_deployment), + "OSTREE_DEPLOYSERIAL=%u", + ostree_deployment_get_deployserial (self->staged_deployment), + NULL); +#endif + g_assert (self->staged_deployment_data); g_autoptr(OstreeDeployment) merge_deployment = NULL; From c70526841e86bf72d0714db84305cd35ac62f527 Mon Sep 17 00:00:00 2001 From: Sinny Kumari Date: Thu, 4 Oct 2018 19:18:05 +0530 Subject: [PATCH 33/50] src/ostree: Don't delete refs having aliases Deleting a ref with aliases makes them dangling. In such cases, display an error message to the user. Fixes #1597 Signed-off-by: Sinny Kumari Closes: #1749 Approved by: cgwalters --- src/ostree/ot-builtin-refs.c | 17 +++++++++++++++-- tests/test-refs.sh | 5 +++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/ostree/ot-builtin-refs.c b/src/ostree/ot-builtin-refs.c index 5c2214cc..f88d08a6 100644 --- a/src/ostree/ot-builtin-refs.c +++ b/src/ostree/ot-builtin-refs.c @@ -146,8 +146,10 @@ static gboolean do_ref (OstreeRepo *repo, const char *refspec_prefix, GCancellab /* If we're doing aliasing, we need the full list of aliases mostly to allow * replacing existing aliases. + * If we are deleting a ref, we want to make sure that it doesn't have + * any corresponding aliases. */ - if (opt_alias) + if (opt_alias || opt_delete) { if (!ostree_repo_list_refs_ext (repo, NULL, &ref_aliases, OSTREE_REPO_LIST_REFS_EXT_ALIASES, @@ -245,7 +247,18 @@ static gboolean do_ref (OstreeRepo *repo, const char *refspec_prefix, GCancellab if (!ostree_parse_refspec (refspec, &remote, &ref, error)) goto out; - + + /* Look for alias if it exists for a ref we want to delete */ + GLNX_HASH_TABLE_FOREACH_KV (ref_aliases, const char *, + ref_alias, const char *, value) + { + if (!strcmp (ref, value)) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, + "Ref '%s' has an active alias: '%s'", ref, ref_alias); + goto out; + } + } if (!ostree_repo_set_ref_immediate (repo, remote, ref, NULL, cancellable, error)) goto out; diff --git a/tests/test-refs.sh b/tests/test-refs.sh index f4fe1833..1730423d 100755 --- a/tests/test-refs.sh +++ b/tests/test-refs.sh @@ -192,6 +192,11 @@ done ${CMD_PREFIX} ostree --repo=repo refs -A > refs.txt assert_file_has_content_literal refs.txt 'exampleos/x86_64/stable/server -> exampleos/x86_64/27/server' +# Test that we don't delete a ref having aliases +if ${CMD_PREFIX} ostree --repo=repo refs --delete exampleos/x86_64/27/server; then + assert_not_reached "refs --delete unexpectedly succeeded in deleting a ref containing alias!" +fi + ${CMD_PREFIX} ostree --repo=repo summary -u echo "ok ref symlink" From 673cacd633f9d6b653cdea530657d3e780a41bbd Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 11 Oct 2018 09:22:16 -0400 Subject: [PATCH 34/50] repo: Add a checkout option to not hardlink zero-sized files In rpm-ostree we've hit a few cases where hardlinking zero-sized files causes us problems. The most prominent is lock files in `/usr/etc`, such as `/usr/etc/selinux/semanage.LOCK`. If there are two zero-sized lock files to grab, but they're hardlinked, then locking will fail. Another case here is if one is using ostree inside a container and don't have access to FUSE (i.e. `rofiles-fuse`), then the ostree hardlinking can cause files that aren't ordinarily hardlinked to become so, and mutation of one mutates all. An example where this is concerning is Python `__init__.py` files. Now, these lock files should clearly not be in the tree to begin with, but - we're not gaining a huge amount by hardlinking these files either, so let's add an option to disable it. Closes: #1752 Approved by: jlebon --- src/libostree/ostree-repo-checkout.c | 8 +++++++- src/libostree/ostree-repo.h | 3 ++- src/ostree/ot-builtin-checkout.c | 6 +++++- tests/basic-test.sh | 17 ++++++++++++++++- 4 files changed, 30 insertions(+), 4 deletions(-) diff --git a/src/libostree/ostree-repo-checkout.c b/src/libostree/ostree-repo-checkout.c index 6317c9e8..fcae6a7f 100644 --- a/src/libostree/ostree-repo-checkout.c +++ b/src/libostree/ostree-repo-checkout.c @@ -586,6 +586,7 @@ checkout_one_file_at (OstreeRepo *repo, const gboolean is_symlink = (g_file_info_get_file_type (source_info) == G_FILE_TYPE_SYMBOLIC_LINK); const gboolean is_whiteout = (!is_symlink && options->process_whiteouts && g_str_has_prefix (destination_name, WHITEOUT_PREFIX)); + const gboolean is_reg_zerosized = (!is_symlink && g_file_info_get_size (source_info) == 0); /* First, see if it's a Docker whiteout, * https://github.com/docker/docker/blob/1a714e76a2cb9008cd19609059e9988ff1660b78/pkg/archive/whiteouts.go @@ -604,6 +605,10 @@ checkout_one_file_at (OstreeRepo *repo, need_copy = FALSE; } + else if (options->force_copy_zerosized && is_reg_zerosized) + { + need_copy = TRUE; + } else if (!options->force_copy) { HardlinkResult hardlink_res = HARDLINK_RESULT_NOT_SUPPORTED; @@ -699,6 +704,7 @@ checkout_one_file_at (OstreeRepo *repo, if (can_cache && !is_whiteout && !is_symlink + && !is_reg_zerosized && need_copy && repo->mode == OSTREE_REPO_MODE_ARCHIVE && options->mode == OSTREE_REPO_CHECKOUT_MODE_USER) @@ -762,7 +768,7 @@ checkout_one_file_at (OstreeRepo *repo, * succeeded at hardlinking above. */ if (options->no_copy_fallback) - g_assert (is_bare_user_symlink); + g_assert (is_bare_user_symlink || is_reg_zerosized); if (!ostree_repo_load_file (repo, checksum, &input, NULL, &xattrs, cancellable, error)) return FALSE; diff --git a/src/libostree/ostree-repo.h b/src/libostree/ostree-repo.h index a6544761..eddbbf87 100644 --- a/src/libostree/ostree-repo.h +++ b/src/libostree/ostree-repo.h @@ -933,7 +933,8 @@ typedef struct { gboolean no_copy_fallback; gboolean force_copy; /* Since: 2017.6 */ gboolean bareuseronly_dirs; /* Since: 2017.7 */ - gboolean unused_bools[5]; + gboolean force_copy_zerosized; /* Since: 2018.9 */ + gboolean unused_bools[4]; /* 4 byte hole on 64 bit */ const char *subpath; diff --git a/src/ostree/ot-builtin-checkout.c b/src/ostree/ot-builtin-checkout.c index e7d6a634..1519e34e 100644 --- a/src/ostree/ot-builtin-checkout.c +++ b/src/ostree/ot-builtin-checkout.c @@ -45,6 +45,7 @@ static char *opt_from_file; static gboolean opt_disable_fsync; static gboolean opt_require_hardlinks; static gboolean opt_force_copy; +static gboolean opt_force_copy_zerosized; static gboolean opt_bareuseronly_dirs; static char *opt_skiplist_file; static char *opt_selinux_policy; @@ -84,6 +85,7 @@ static GOptionEntry options[] = { { "from-file", 0, 0, G_OPTION_ARG_STRING, &opt_from_file, "Process many checkouts from input file", "FILE" }, { "fsync", 0, 0, G_OPTION_ARG_CALLBACK, parse_fsync_cb, "Specify how to invoke fsync()", "POLICY" }, { "require-hardlinks", 'H', 0, G_OPTION_ARG_NONE, &opt_require_hardlinks, "Do not fall back to full copies if hardlinking fails", NULL }, + { "force-copy-zerosized", 'z', 0, G_OPTION_ARG_NONE, &opt_force_copy_zerosized, "Do not hardlink zero-sized files", NULL }, { "force-copy", 'C', 0, G_OPTION_ARG_NONE, &opt_force_copy, "Never hardlink (but may reflink if available)", NULL }, { "bareuseronly-dirs", 'M', 0, G_OPTION_ARG_NONE, &opt_bareuseronly_dirs, "Suppress mode bits outside of 0775 for directories (suid, world writable, etc.)", NULL }, { "skip-list", 0, 0, G_OPTION_ARG_FILENAME, &opt_skiplist_file, "File containing list of files to skip", "PATH" }, @@ -130,7 +132,8 @@ process_one_checkout (OstreeRepo *repo, * convenient infrastructure for testing C APIs with data. */ if (opt_disable_cache || opt_whiteouts || opt_require_hardlinks || - opt_union_add || opt_force_copy || opt_bareuseronly_dirs || opt_union_identical || + opt_union_add || opt_force_copy || opt_force_copy_zerosized || + opt_bareuseronly_dirs || opt_union_identical || opt_skiplist_file || opt_selinux_policy || opt_selinux_prefix) { OstreeRepoCheckoutAtOptions options = { 0, }; @@ -218,6 +221,7 @@ process_one_checkout (OstreeRepo *repo, options.no_copy_fallback = opt_require_hardlinks; options.force_copy = opt_force_copy; + options.force_copy_zerosized = opt_force_copy_zerosized; options.bareuseronly_dirs = opt_bareuseronly_dirs; if (!ostree_repo_checkout_at (repo, &options, diff --git a/tests/basic-test.sh b/tests/basic-test.sh index a0c2f1f7..a817b9d1 100644 --- a/tests/basic-test.sh +++ b/tests/basic-test.sh @@ -21,7 +21,7 @@ set -euo pipefail -echo "1..$((83 + ${extra_basic_tests:-0}))" +echo "1..$((84 + ${extra_basic_tests:-0}))" CHECKOUT_U_ARG="" CHECKOUT_H_ARGS="-H" @@ -694,6 +694,21 @@ for v in bin link; do done echo "ok checkout union identical conflicts" +cd ${test_tmpdir} +rm files -rf && mkdir files +touch files/anemptyfile +touch files/anotheremptyfile +$CMD_PREFIX ostree --repo=repo commit --consume -b tree-with-empty-files --tree=dir=files +$CMD_PREFIX ostree --repo=repo checkout ${CHECKOUT_H_ARGS} -z tree-with-empty-files tree-with-empty-files +if files_are_hardlinked tree-with-empty-files/an{,other}emptyfile; then + fatal "--force-copy-zerosized failed" +fi +rm tree-with-empty-files -rf +$CMD_PREFIX ostree --repo=repo checkout ${CHECKOUT_H_ARGS} tree-with-empty-files tree-with-empty-files +assert_files_hardlinked tree-with-empty-files/an{,other}emptyfile +rm tree-with-empty-files -rf +echo "ok checkout --force-copy-zerosized" + cd ${test_tmpdir} rm files -rf && mkdir files mkdir files/worldwritable-dir From 9367a1befe045663a69265e10e0b4593068649e4 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 11 Oct 2018 14:35:23 -0400 Subject: [PATCH 35/50] checkout: Support --union-identical and --force-copy{,--zerosized} Actually testing the patch to add `--force-copy-zerosized` to rpm-ostree tripped over the fact that it uses `--union-identical`, and we just hit an assertion failure with that combination. Fix this by copying over the logic we have for the hardlink case. Closes: #1753 Approved by: jlebon --- src/libostree/ostree-repo-checkout.c | 34 +++++++++++++++++++++++++--- tests/basic-test.sh | 16 ++++++++++++- 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/src/libostree/ostree-repo-checkout.c b/src/libostree/ostree-repo-checkout.c index fcae6a7f..5ae79923 100644 --- a/src/libostree/ostree-repo-checkout.c +++ b/src/libostree/ostree-repo-checkout.c @@ -196,6 +196,7 @@ static gboolean create_file_copy_from_input_at (OstreeRepo *repo, OstreeRepoCheckoutAtOptions *options, CheckoutState *state, + const char *checksum, GFileInfo *file_info, GVariant *xattrs, GInputStream *input, @@ -358,8 +359,35 @@ create_file_copy_from_input_at (OstreeRepo *repo, replace_mode = GLNX_LINK_TMPFILE_NOREPLACE_IGNORE_EXIST; break; case OSTREE_REPO_CHECKOUT_OVERWRITE_UNION_IDENTICAL: - /* We don't support copying in union identical */ - g_assert_not_reached (); + { + replace_mode = GLNX_LINK_TMPFILE_NOREPLACE; + struct stat dest_stbuf; + if (!glnx_fstatat_allow_noent (destination_dfd, destination_name, &dest_stbuf, + AT_SYMLINK_NOFOLLOW, error)) + return FALSE; + if (errno == 0) + { + /* We do a checksum comparison; see also equivalent code in + * checkout_file_hardlink(). + */ + OstreeChecksumFlags flags = 0; + if (repo->disable_xattrs) + flags |= OSTREE_CHECKSUM_FLAGS_IGNORE_XATTRS; + + g_autofree char *actual_checksum = NULL; + if (!ostree_checksum_file_at (destination_dfd, destination_name, + &dest_stbuf, OSTREE_OBJECT_TYPE_FILE, + flags, &actual_checksum, cancellable, error)) + return FALSE; + + if (g_str_equal (checksum, actual_checksum)) + return TRUE; + + /* Otherwise, fall through and do the link, we should + * get EEXIST. + */ + } + } break; } @@ -773,7 +801,7 @@ checkout_one_file_at (OstreeRepo *repo, cancellable, error)) return FALSE; - if (!create_file_copy_from_input_at (repo, options, state, source_info, xattrs, input, + if (!create_file_copy_from_input_at (repo, options, state, checksum, source_info, xattrs, input, destination_dfd, destination_name, cancellable, error)) return glnx_prefix_error (error, "Copy checkout of %s to %s", checksum, destination_name); diff --git a/tests/basic-test.sh b/tests/basic-test.sh index a817b9d1..3c4823d7 100644 --- a/tests/basic-test.sh +++ b/tests/basic-test.sh @@ -21,7 +21,7 @@ set -euo pipefail -echo "1..$((84 + ${extra_basic_tests:-0}))" +echo "1..$((85 + ${extra_basic_tests:-0}))" CHECKOUT_U_ARG="" CHECKOUT_H_ARGS="-H" @@ -709,6 +709,20 @@ assert_files_hardlinked tree-with-empty-files/an{,other}emptyfile rm tree-with-empty-files -rf echo "ok checkout --force-copy-zerosized" +# These should merge, they're identical +$CMD_PREFIX ostree --repo=repo checkout ${CHECKOUT_H_ARGS} --union-identical -z tree-with-empty-files tree-with-empty-files +$CMD_PREFIX ostree --repo=repo checkout ${CHECKOUT_H_ARGS} --union-identical -z tree-with-empty-files tree-with-empty-files +echo notempty > tree-with-empty-files/anemptyfile.new && mv tree-with-empty-files/anemptyfile{.new,} +$CMD_PREFIX ostree --repo=repo commit --consume -b tree-with-conflicting-empty-files --tree=dir=tree-with-empty-files +# Reset back to base +rm tree-with-empty-files -rf +$CMD_PREFIX ostree --repo=repo checkout ${CHECKOUT_H_ARGS} --union-identical -z tree-with-empty-files tree-with-empty-files +if $CMD_PREFIX ostree --repo=repo checkout ${CHECKOUT_H_ARGS} --union-identical -z tree-with-conflicting-empty-files tree-with-empty-files 2>err.txt; then + fatal "--union-identical --force-copy-zerosized unexpectedly succeeded with non-identical files" +fi +assert_file_has_content err.txt 'error:.*File exists' +echo "ok checkout --union-identical --force-copy-zerosized" + cd ${test_tmpdir} rm files -rf && mkdir files mkdir files/worldwritable-dir From 43d9cac4fc4f38c18b8aa164fbbf77609e2b5b10 Mon Sep 17 00:00:00 2001 From: Dan Nicholson Date: Fri, 12 Oct 2018 12:18:36 +0000 Subject: [PATCH 36/50] lib/commit: Don't chown objects to repo target owner The idea is that if the process is running as root, it can change ownership of newly written files to match the owner of the repo. Unfortunately, it currently applies in the other direction, too - a non-root user writing to a root owned repository. If the repo is writable by the user but owned by root, it can still create files and directories there, but it can't change ownership of them. This feature comes from https://bugzilla.gnome.org/show_bug.cgi?id=738954. As it turns out, this feature was never completed. It only works on content objects and not metadata objects, refs, deltas, summaries, etc. Rather than try to fix all of those, remove the feature until someone has interest in completing it. Closes: #1754 Approved by: cgwalters --- src/libostree/ostree-repo-commit.c | 11 +---------- src/libostree/ostree-repo-private.h | 2 -- src/libostree/ostree-repo.c | 10 ---------- 3 files changed, 1 insertion(+), 22 deletions(-) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index 9521dc6c..134024b8 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -245,16 +245,7 @@ commit_loose_regfile_object (OstreeRepo *self, GCancellable *cancellable, GError **error) { - /* We may be writing as root to a non-root-owned repository; if so, - * automatically inherit the non-root ownership. - */ - if (self->mode == OSTREE_REPO_MODE_ARCHIVE - && self->target_owner_uid != -1) - { - if (fchown (tmpf->fd, self->target_owner_uid, self->target_owner_gid) < 0) - return glnx_throw_errno_prefix (error, "fchown"); - } - else if (self->mode == OSTREE_REPO_MODE_BARE) + if (self->mode == OSTREE_REPO_MODE_BARE) { if (TEMP_FAILURE_RETRY (fchown (tmpf->fd, uid, gid)) < 0) return glnx_throw_errno_prefix (error, "fchown"); diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h index 3cc4aba0..ad1269b3 100644 --- a/src/libostree/ostree-repo-private.h +++ b/src/libostree/ostree-repo-private.h @@ -149,8 +149,6 @@ struct OstreeRepo { dev_t device; ino_t inode; uid_t owner_uid; /* Cache of repo's owner uid */ - uid_t target_owner_uid; /* Ensure files are chowned to this uid/gid */ - 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 */ guint64 reserved_blocks; diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 73ee027c..857b2d4c 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -3080,16 +3080,6 @@ ostree_repo_open (OstreeRepo *self, return FALSE; self->owner_uid = stbuf.st_uid; - if (stbuf.st_uid != getuid () || stbuf.st_gid != getgid ()) - { - self->target_owner_uid = stbuf.st_uid; - self->target_owner_gid = stbuf.st_gid; - } - else - { - self->target_owner_uid = self->target_owner_gid = -1; - } - if (self->writable) { /* Always try to recreate the tmpdir to be nice to people From 04aff9c1c022e70ec46597bbb48c4ecfd103b50c Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 2 Oct 2018 11:47:48 -0400 Subject: [PATCH 37/50] rofiles-fuse: Improve error message for failure to open root I was debugging some rpm-ostree work and saw: `openat: No such file or directory` and it wasn't immediately obvious it was stderr from `rofiles-fuse`. Use the `err` API which is better in many ways; in this case it automatically prefixes with `argv0`. Closes: #1747 Approved by: jlebon --- src/rofiles-fuse/main.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/rofiles-fuse/main.c b/src/rofiles-fuse/main.c index fd16e29b..4033caa4 100644 --- a/src/rofiles-fuse/main.c +++ b/src/rofiles-fuse/main.c @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -628,10 +629,7 @@ rofs_parse_opt (void *data, const char *arg, int key, { basefd = openat (AT_FDCWD, arg, O_RDONLY | O_NONBLOCK | O_DIRECTORY | O_CLOEXEC | O_NOCTTY); if (basefd == -1) - { - perror ("openat"); - exit (EXIT_FAILURE); - } + err (1, "opening rootfs %s", arg); return 0; } else From e242033fe722eb54776c428c124ff300075ff7d0 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 12 Oct 2018 11:37:49 -0400 Subject: [PATCH 38/50] finalize-staged: Bump timeout to 5 minutes See https://github.com/projectatomic/rpm-ostree/issues/1568 Basically for people on e.g. rotational media, the default 90 second timeout can be too small. We're in a tough situation here, because delaying shutdown can be problematic too if the user is trying to shut down their laptop to put in a backpack, etc. There's potential optimizations here to make; I think we could pre-copy the kernel/initramfs for example. I suspect for some people the grub2 os-prober is a factor here too, if that tries to e.g. inspect attached USB rotational hard drives. But hopefully we'll get rid of that soon. Closes: #1755 Approved by: jlebon --- src/boot/ostree-finalize-staged.service | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/boot/ostree-finalize-staged.service b/src/boot/ostree-finalize-staged.service index 2ae64918..10e551e4 100644 --- a/src/boot/ostree-finalize-staged.service +++ b/src/boot/ostree-finalize-staged.service @@ -32,3 +32,7 @@ Conflicts=final.target Type=oneshot RemainAfterExit=yes ExecStop=/usr/bin/ostree admin finalize-staged +# This is a quite long timeout intentionally; the failure mode +# here is that people don't get an upgrade. We need to handle +# cases with slow rotational media, etc. +TimeoutStopSec=5m From 5183c8f35e5b730dd6d6e9673b05eba355c737b6 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 17 Oct 2018 16:06:50 +0000 Subject: [PATCH 39/50] sysroot: Update some code to use fstatat_allow_noent API It's much easier to read and use correctly. Making this change since I saw an unprefixed error in an issue. Closes: #1757 Approved by: jlebon --- src/libostree/ostree-sysroot.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/libostree/ostree-sysroot.c b/src/libostree/ostree-sysroot.c index 4b2c654c..8a916289 100644 --- a/src/libostree/ostree-sysroot.c +++ b/src/libostree/ostree-sysroot.c @@ -376,12 +376,11 @@ _ostree_sysroot_read_current_subbootversion (OstreeSysroot *self, g_autofree char *ostree_bootdir_name = g_strdup_printf ("ostree/boot.%d", bootversion); struct stat stbuf; - if (fstatat (self->sysroot_fd, ostree_bootdir_name, &stbuf, AT_SYMLINK_NOFOLLOW) != 0) + if (!glnx_fstatat_allow_noent (self->sysroot_fd, ostree_bootdir_name, &stbuf, AT_SYMLINK_NOFOLLOW, error)) + return FALSE; + if (errno == ENOENT) { - if (errno == ENOENT) - *out_subbootversion = 0; - else - return glnx_throw_errno (error); + *out_subbootversion = 0; } else { @@ -499,10 +498,10 @@ read_current_bootversion (OstreeSysroot *self, int ret_bootversion; struct stat stbuf; - if (fstatat (self->sysroot_fd, "boot/loader", &stbuf, AT_SYMLINK_NOFOLLOW) != 0) + if (!glnx_fstatat_allow_noent (self->sysroot_fd, "boot/loader", &stbuf, AT_SYMLINK_NOFOLLOW, error)) + return FALSE; + if (errno == ENOENT) { - if (errno != ENOENT) - return glnx_throw_errno (error); ret_bootversion = 0; } else @@ -976,11 +975,12 @@ ostree_sysroot_load_if_changed (OstreeSysroot *self, /* Otherwise - check for /sysroot which should only exist in a deployment, * not in ${sysroot} (a metavariable for the real physical root). */ - else if (fstatat (self->sysroot_fd, "sysroot", &stbuf, 0) < 0) + else { - if (errno != ENOENT) - return glnx_throw_errno_prefix (error, "fstatat"); - self->is_physical = TRUE; + if (!glnx_fstatat_allow_noent (self->sysroot_fd, "sysroot", &stbuf, 0, error)) + return FALSE; + if (errno == ENOENT) + self->is_physical = TRUE; } /* Otherwise, the default is FALSE */ } From 1db0db3d7aa98e3db98ba93f7fc2e17305f95007 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 17 Oct 2018 16:10:40 +0000 Subject: [PATCH 40/50] sysroot: Add error prefixing to deployment parsing I think this is where the bare `readlinkat` came from in https://github.com/ostreedev/ostree/issues/1459 `Error setting up sysroot: readlinkat: No such file or directory` Closes: #1757 Approved by: jlebon --- src/libostree/ostree-sysroot.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libostree/ostree-sysroot.c b/src/libostree/ostree-sysroot.c index 8a916289..8485edec 100644 --- a/src/libostree/ostree-sysroot.c +++ b/src/libostree/ostree-sysroot.c @@ -612,6 +612,10 @@ parse_deployment (OstreeSysroot *self, error)) return FALSE; + g_autofree char *errprefix = + g_strdup_printf ("Parsing deployment %i in stateroot '%s'", treebootserial, osname); + GLNX_AUTO_PREFIX_ERROR(errprefix, error); + const char *relative_boot_link = boot_link; if (*relative_boot_link == '/') relative_boot_link++; From ae99b9ccdc0ba0b75f3877461d5cc2b3b49fd9be Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Thu, 18 Oct 2018 16:40:03 -0400 Subject: [PATCH 41/50] ostree-prepare-root.service: Use RemainAfterExit=yes For the same reasons as #1697. This is especially important in services that are likely to be used as an `After/Before=` target in other units. `ostree-prepare-root.service` is one such service. Closes: #1759 Approved by: cgwalters --- src/boot/ostree-prepare-root.service | 1 + 1 file changed, 1 insertion(+) diff --git a/src/boot/ostree-prepare-root.service b/src/boot/ostree-prepare-root.service index 5467bcfd..455afc3e 100644 --- a/src/boot/ostree-prepare-root.service +++ b/src/boot/ostree-prepare-root.service @@ -32,3 +32,4 @@ ExecStart=/usr/lib/ostree/ostree-prepare-root /sysroot StandardInput=null StandardOutput=syslog StandardError=syslog+console +RemainAfterExit=yes From a4a49724d6f898fd5e76bd6de49d36f7ed8d237e Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Thu, 18 Oct 2018 16:44:05 -0400 Subject: [PATCH 42/50] ostree-prepare-root.service: Run earlier in initrd Previously, we were preparing the root very late in the boot process; right before we switch root. The issue with that is that most services in the initrd that run `After=initrd-root-fs.target` expect that `/sysroot` already points to the rootfs we'll be pivoting to. Running this late violates that assumption. This patch fixes this by making `ostree-prepare-root.service` instead run right after `sysroot.mount` (the physical sysroot mounted by systemd) but still before `initrd-root-fs.target` (which is the target signalling that `/sysroot` is now valid and ready). This should make it easier to integrate OSTree with other initrd services such as Ignition. Related: https://github.com/dustymabe/ignition-dracut/issues/20 Closes: #1759 Approved by: cgwalters --- src/boot/dracut/module-setup.sh | 4 ++-- src/boot/ostree-prepare-root.service | 5 ++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/boot/dracut/module-setup.sh b/src/boot/dracut/module-setup.sh index 70364c4b..4d12fae6 100755 --- a/src/boot/dracut/module-setup.sh +++ b/src/boot/dracut/module-setup.sh @@ -36,7 +36,7 @@ depends() { install() { dracut_install /usr/lib/ostree/ostree-prepare-root inst_simple "${systemdsystemunitdir}/ostree-prepare-root.service" - mkdir -p "${initdir}${systemdsystemconfdir}/initrd-switch-root.target.wants" + mkdir -p "${initdir}${systemdsystemconfdir}/initrd-root-fs.target.wants" ln_r "${systemdsystemunitdir}/ostree-prepare-root.service" \ - "${systemdsystemconfdir}/initrd-switch-root.target.wants/ostree-prepare-root.service" + "${systemdsystemconfdir}/initrd-root-fs.target.wants/ostree-prepare-root.service" } diff --git a/src/boot/ostree-prepare-root.service b/src/boot/ostree-prepare-root.service index 455afc3e..63357581 100644 --- a/src/boot/ostree-prepare-root.service +++ b/src/boot/ostree-prepare-root.service @@ -22,9 +22,8 @@ DefaultDependencies=no ConditionKernelCommandLine=ostree ConditionPathExists=/etc/initrd-release OnFailure=emergency.target -After=initrd-switch-root.target -Before=initrd-switch-root.service -Before=plymouth-switch-root.service +After=sysroot.mount +Before=initrd-root-fs.target [Service] Type=oneshot From 3956fc885b39e0a0755547f07067b206606142eb Mon Sep 17 00:00:00 2001 From: Matthew Leeds Date: Wed, 17 Oct 2018 12:55:38 -0700 Subject: [PATCH 43/50] Allow disabling pulling from LAN/USB/Internet Currently libostree essentially has two modes when it's pulling refs: the "legacy" code paths pull only from the Internet, and the code paths that are aware of collection IDs try to pull from the Internet, the local network, and mounted filesystems (such as USB drives). The problem is that while we eventually want to migrate everyone to using collection IDs, we don't want to force checking LAN and USB sources if the user just wants to pull from the Internet, since the LAN/USB code paths can have privacy[1], security[2], and performance[3] implications. So this commit implements a new repo config option called "repo-finders" which can be configured to, for example, "config;lan;mount;" to check all three sources or "config;mount;" to disable searching the LAN. The set of values mirror those used for the --finders option of the find-remotes command. This configuration affects pulls in three places: 1. the ostree_repo_find_remotes_async() API, regardless of whether or not the user of the API provided a list of OstreeRepoFinders 2. the ostree_repo_finder_resolve_async() / ostree_repo_finder_resolve_all_async() API 3. the find-remotes command This feature is especially important right now since we soon want to have Flathub publish a metadata key which will have Flatpak clients update the remote config to add a collection ID.[4] This effectively fixes https://github.com/flatpak/flatpak/issues/1863 but I'll patch Flatpak too, so it doesn't pass finders to libostree only to then have them be removed. [1] https://github.com/flatpak/flatpak/issues/1863#issuecomment-404128824 [2] https://github.com/ostreedev/ostree/issues/1527 [3] Based on how long the "ostree find-remotes" command takes to complete, having the LAN finder enabled slows down that step of the pull process by about 40%. See also https://github.com/flatpak/flatpak/issues/1862 [4] https://github.com/flathub/flathub/issues/676 Closes: #1758 Approved by: cgwalters --- apidoc/ostree-sections.txt | 1 + man/ostree.repo-config.xml | 14 +++++++ src/libostree/libostree-devel.sym | 1 + src/libostree/ostree-repo-private.h | 1 + src/libostree/ostree-repo-pull.c | 62 +++++++++++++++++------------ src/libostree/ostree-repo.c | 54 +++++++++++++++++++++++++ src/libostree/ostree-repo.h | 3 ++ 7 files changed, 110 insertions(+), 26 deletions(-) diff --git a/apidoc/ostree-sections.txt b/apidoc/ostree-sections.txt index 440fe0c3..a91663ed 100644 --- a/apidoc/ostree-sections.txt +++ b/apidoc/ostree-sections.txt @@ -298,6 +298,7 @@ ostree_repo_get_mode ostree_repo_get_min_free_space_bytes ostree_repo_get_config ostree_repo_get_dfd +ostree_repo_get_repo_finders ostree_repo_hash ostree_repo_equal ostree_repo_copy_config diff --git a/man/ostree.repo-config.xml b/man/ostree.repo-config.xml index 8942a00b..49044b46 100644 --- a/man/ostree.repo-config.xml +++ b/man/ostree.repo-config.xml @@ -215,6 +215,20 @@ Boston, MA 02111-1307, USA. of -1 means block indefinitely. The default value is 30. + + + repo-finders + Semicolon separated default list of finders (sources + for refs) to use when pulling. This can be used to disable + pulling from mounted filesystems, peers on the local network, + or the Internet. However note that it only applies when a set + of finders isn't explicitly specified, either by a consumer of + libostree API or on the command line. Possible values: + config, lan, and + mount (or any combination thereof). If + unset, this defaults to config;lan;mount;. + + diff --git a/src/libostree/libostree-devel.sym b/src/libostree/libostree-devel.sym index 6d1fe934..80b04fd5 100644 --- a/src/libostree/libostree-devel.sym +++ b/src/libostree/libostree-devel.sym @@ -21,6 +21,7 @@ LIBOSTREE_2018.9 { ostree_mutable_tree_remove; ostree_repo_get_min_free_space_bytes; + ostree_repo_get_repo_finders; } LIBOSTREE_2018.7; /* Stub section for the stable release *after* this development one; don't diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h index ad1269b3..40be7263 100644 --- a/src/libostree/ostree-repo-private.h +++ b/src/libostree/ostree-repo-private.h @@ -168,6 +168,7 @@ struct OstreeRepo { gint lock_timeout_seconds; guint64 payload_link_threshold; gint fs_support_reflink; /* The underlying filesystem has support for ioctl (FICLONE..) */ + gchar **repo_finders; OstreeRepo *parent_repo; }; diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index f5c52e4a..4a3adf2a 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -4999,45 +4999,55 @@ ostree_repo_find_remotes_async (OstreeRepo *self, /* Are we using #OstreeRepoFinders provided by the user, or the defaults? */ if (finders == NULL) { + guint finder_index = 0; #ifdef HAVE_AVAHI + guint avahi_index; GMainContext *context = g_main_context_get_thread_default (); g_autoptr(GError) local_error = NULL; #endif /* HAVE_AVAHI */ - finder_config = OSTREE_REPO_FINDER (ostree_repo_finder_config_new ()); - finder_mount = OSTREE_REPO_FINDER (ostree_repo_finder_mount_new (NULL)); + if (g_strv_contains ((const char * const *)self->repo_finders, "config")) + default_finders[finder_index++] = finder_config = OSTREE_REPO_FINDER (ostree_repo_finder_config_new ()); + + if (g_strv_contains ((const char * const *)self->repo_finders, "mount")) + default_finders[finder_index++] = finder_mount = OSTREE_REPO_FINDER (ostree_repo_finder_mount_new (NULL)); + #ifdef HAVE_AVAHI - finder_avahi = OSTREE_REPO_FINDER (ostree_repo_finder_avahi_new (context)); + if (g_strv_contains ((const char * const *)self->repo_finders, "lan")) + { + avahi_index = finder_index; + default_finders[finder_index++] = finder_avahi = OSTREE_REPO_FINDER (ostree_repo_finder_avahi_new (context)); + } #endif /* HAVE_AVAHI */ - default_finders[0] = finder_config; - default_finders[1] = finder_mount; - default_finders[2] = finder_avahi; - + g_assert (default_finders != NULL); finders = default_finders; #ifdef HAVE_AVAHI - ostree_repo_finder_avahi_start (OSTREE_REPO_FINDER_AVAHI (finder_avahi), - &local_error); - - if (local_error != NULL) + if (finder_avahi != NULL) { - /* See ostree-repo-finder-avahi.c:ostree_repo_finder_avahi_start, we - * intentionally throw this so as to distinguish between the Avahi - * finder failing because the Avahi daemon wasn't running and - * the Avahi finder failing because of some actual error. - * - * We need to distinguish between g_debug and g_warning here because - * unit tests that use this code may set G_DEBUG=fatal-warnings which - * would cause client code to abort if a warning were emitted. - */ - if (g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) - g_debug ("Avahi finder failed under normal operation; removing it: %s", local_error->message); - else - g_warning ("Avahi finder failed abnormally; removing it: %s", local_error->message); + ostree_repo_finder_avahi_start (OSTREE_REPO_FINDER_AVAHI (finder_avahi), + &local_error); - default_finders[2] = NULL; - g_clear_object (&finder_avahi); + if (local_error != NULL) + { + /* See ostree-repo-finder-avahi.c:ostree_repo_finder_avahi_start, we + * intentionally throw this so as to distinguish between the Avahi + * finder failing because the Avahi daemon wasn't running and + * the Avahi finder failing because of some actual error. + * + * We need to distinguish between g_debug and g_warning here because + * unit tests that use this code may set G_DEBUG=fatal-warnings which + * would cause client code to abort if a warning were emitted. + */ + if (g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) + g_debug ("Avahi finder failed under normal operation; removing it: %s", local_error->message); + else + g_warning ("Avahi finder failed abnormally; removing it: %s", local_error->message); + + default_finders[avahi_index] = NULL; + g_clear_object (&finder_avahi); + } } #endif /* HAVE_AVAHI */ } diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 857b2d4c..18fb7733 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -1035,6 +1035,7 @@ ostree_repo_finalize (GObject *object) g_mutex_clear (&self->cache_lock); g_mutex_clear (&self->txn_lock); g_free (self->collection_id); + g_strfreev (self->repo_finders); g_clear_pointer (&self->remotes, g_hash_table_destroy); g_mutex_clear (&self->remotes_lock); @@ -2938,6 +2939,40 @@ reload_core_config (OstreeRepo *self, self->payload_link_threshold = g_ascii_strtoull (payload_threshold, NULL, 10); } + { g_auto(GStrv) configured_finders = NULL; + g_autoptr(GError) local_error = NULL; + + configured_finders = g_key_file_get_string_list (self->config, "core", "repo-finders", + NULL, &local_error); + if (g_error_matches (local_error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_KEY_NOT_FOUND)) + g_clear_error (&local_error); + else if (local_error != NULL) + { + g_propagate_error (error, g_steal_pointer (&local_error)); + return FALSE; + } + + if (configured_finders != NULL && *configured_finders == NULL) + return glnx_throw (error, "Invalid empty repo-finders configuration"); + + for (char **iter = configured_finders; iter && *iter; iter++) + { + const char *repo_finder = *iter; + + if (strcmp (repo_finder, "config") != 0 && + strcmp (repo_finder, "lan") != 0 && + strcmp (repo_finder, "mount") != 0) + return glnx_throw (error, "Invalid configured repo-finder '%s'", repo_finder); + } + + /* Fall back to a default set of finders */ + if (configured_finders == NULL) + configured_finders = g_strsplit ("config;lan;mount", ";", -1); + + g_clear_pointer (&self->repo_finders, g_strfreev); + self->repo_finders = g_steal_pointer (&configured_finders); + } + return TRUE; } @@ -5915,3 +5950,22 @@ ostree_repo_set_collection_id (OstreeRepo *self, return TRUE; } + +/** + * ostree_repo_get_repo_finders: + * @self: an #OstreeRepo + * + * Get the set of repo finders configured. See the documentation for + * the "core.repo-finders" config key. + * + * Returns: (array zero-terminated=1) (element-type utf8): + * %NULL-terminated array of strings. + * Since: 2018.9 + */ +const gchar * const * +ostree_repo_get_repo_finders (OstreeRepo *self) +{ + g_return_val_if_fail (OSTREE_IS_REPO (self), NULL); + + return (const gchar * const *)self->repo_finders; +} diff --git a/src/libostree/ostree-repo.h b/src/libostree/ostree-repo.h index eddbbf87..2f7abf6d 100644 --- a/src/libostree/ostree-repo.h +++ b/src/libostree/ostree-repo.h @@ -112,6 +112,9 @@ gboolean ostree_repo_set_collection_id (OstreeRepo *self, const gchar *collection_id, GError **error); +_OSTREE_PUBLIC +const gchar * const * ostree_repo_get_repo_finders (OstreeRepo *self); + _OSTREE_PUBLIC GFile * ostree_repo_get_path (OstreeRepo *self); From 1d6347fe971761978244413b578552f7082e3299 Mon Sep 17 00:00:00 2001 From: Matthew Leeds Date: Wed, 17 Oct 2018 17:02:27 -0700 Subject: [PATCH 44/50] lib/repo-pull: Disable LAN updates by default This commit disables searching on the local network for refs, unless explicitly requested by the user either by changing the value of the "core.repo-finders" config option, or by passing an OstreeRepoFinderAvahi to ostree_repo_find_remotes_async() / ostree_repo_finder_resolve_async(), or by specifying "lan" in the --finders option of the find-remotes command. The primary reason for this is that ostree_repo_find_remotes_async() takes about 40% longer to complete with the LAN finder enabled, and that API is used widely (e.g. in every flatpak operation). It's also probable that some users don't want ostree doing potentially unexpected traffic on the local network, even though everything pulled from a peer is GPG verified. Flathub will soon deploy collection IDs to everyone[1] so these code paths will soon see a lot more use and that's why this change is being made now. Endless is the only potential user of the LAN updates feature, and we can revert this patch on our fork of ostree. For it to be used outside Endless OS we will need to upstream eos-updater-avahi and eos-update-server into ostree. [1] https://github.com/flathub/flathub/issues/676 Closes: #1758 Approved by: cgwalters --- man/ostree.repo-config.xml | 5 +++-- src/libostree/ostree-repo.c | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/man/ostree.repo-config.xml b/man/ostree.repo-config.xml index 49044b46..3ea24486 100644 --- a/man/ostree.repo-config.xml +++ b/man/ostree.repo-config.xml @@ -225,8 +225,9 @@ Boston, MA 02111-1307, USA. of finders isn't explicitly specified, either by a consumer of libostree API or on the command line. Possible values: config, lan, and - mount (or any combination thereof). If - unset, this defaults to config;lan;mount;. + mount (or any combination thereof). If unset, this + defaults to config;mount; (since the LAN finder is + costly). diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 18fb7733..fe095057 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -2967,7 +2967,7 @@ reload_core_config (OstreeRepo *self, /* Fall back to a default set of finders */ if (configured_finders == NULL) - configured_finders = g_strsplit ("config;lan;mount", ";", -1); + configured_finders = g_strsplit ("config;mount", ";", -1); g_clear_pointer (&self->repo_finders, g_strfreev); self->repo_finders = g_steal_pointer (&configured_finders); From 1e16aec357c5c6f02d358c81ce8c81ce68fe0818 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sun, 21 Oct 2018 12:53:27 -0400 Subject: [PATCH 45/50] remount: Refactor to helper function instead of loop Prep for further work. It was silly to use a loop on a static array of two elements. Closes: #1760 Approved by: jlebon --- src/switchroot/ostree-remount.c | 67 ++++++++++++++++----------------- 1 file changed, 33 insertions(+), 34 deletions(-) diff --git a/src/switchroot/ostree-remount.c b/src/switchroot/ostree-remount.c index 71b7b395..5e6d23d3 100644 --- a/src/switchroot/ostree-remount.c +++ b/src/switchroot/ostree-remount.c @@ -39,13 +39,40 @@ #include "ostree-mount-util.h" +static void +do_remount (const char *target) +{ + struct stat stbuf; + if (lstat (target, &stbuf) < 0) + return; + /* Silently ignore symbolic links; we expect these to point to + * /sysroot, and thus there isn't a bind mount there. + */ + if (S_ISLNK (stbuf.st_mode)) + return; + /* If not a mountpoint, skip it */ + struct statvfs stvfsbuf; + if (statvfs (target, &stvfsbuf) == -1) + return; + /* If no read-only flag, skip it */ + if ((stvfsbuf.f_flag & ST_RDONLY) == 0) + return; + /* It's a mounted, read-only fs; remount it */ + if (mount (target, target, NULL, MS_REMOUNT | MS_SILENT, NULL) < 0) + { + /* Also ignore EINVAL - if the target isn't a mountpoint + * already, then assume things are OK. + */ + if (errno != EINVAL) + err (EXIT_FAILURE, "failed to remount %s", target); + } + else + printf ("Remounted: %s\n", target); +} + int main(int argc, char *argv[]) { - const char *remounts[] = { "/sysroot", "/var", NULL }; - struct stat stbuf; - int i; - /* When systemd is in use this is normally created via the generator, but * we ensure it's created here as well for redundancy. */ @@ -65,39 +92,11 @@ main(int argc, char *argv[]) /* If / isn't writable, don't do any remounts; we don't want * to clear the readonly flag in that case. */ - exit (EXIT_SUCCESS); } - for (i = 0; remounts[i] != NULL; i++) - { - const char *target = remounts[i]; - if (lstat (target, &stbuf) < 0) - continue; - /* Silently ignore symbolic links; we expect these to point to - * /sysroot, and thus there isn't a bind mount there. - */ - if (S_ISLNK (stbuf.st_mode)) - continue; - /* If not a mountpoint, skip it */ - struct statvfs stvfsbuf; - if (statvfs (target, &stvfsbuf) == -1) - continue; - /* If no read-only flag, skip it */ - if ((stvfsbuf.f_flag & ST_RDONLY) == 0) - continue; - /* It's a mounted, read-only fs; remount it */ - if (mount (target, target, NULL, MS_REMOUNT | MS_SILENT, NULL) < 0) - { - /* Also ignore EINVAL - if the target isn't a mountpoint - * already, then assume things are OK. - */ - if (errno != EINVAL) - err (EXIT_FAILURE, "failed to remount %s", target); - } - else - printf ("Remounted: %s\n", target); - } + do_remount ("/sysroot"); + do_remount ("/var"); exit (EXIT_SUCCESS); } From ac1a919ffd4fe944d06c4f4510604baa73d1bf8e Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Fri, 28 Sep 2018 14:30:38 -0400 Subject: [PATCH 46/50] boot: Add ostree-finalize-staged.path Rather than manually starting the `ostree-finalize-staged.service` unit, we can leverage systemd's path units for this. It fits quite nicely too, given that we already have a path we drop iif we have a staged deployment. To give some time for the preset to make it to systems, we don't yet drop the explicit call to `systemctl start`. Though we do make it conditional based on a DEBUG env var so that we can actually test it in CI for now. Once we're sure this has propagated, we can drop the `systemctl start` path and the env var together. Closes: #1740 Approved by: cgwalters --- Makefile-boot.am | 5 +++- src/boot/ostree-finalize-staged.path | 28 +++++++++++++++++++ src/libostree/ostree-sysroot-deploy.c | 9 ++++++ src/libostree/ostree-sysroot-private.h | 3 ++ src/libostree/ostree-sysroot.c | 1 + tests/installed/destructive/staged-deploy.yml | 17 ++++++++++- 6 files changed, 61 insertions(+), 2 deletions(-) create mode 100644 src/boot/ostree-finalize-staged.path diff --git a/Makefile-boot.am b/Makefile-boot.am index 5b512b6c..d18351af 100644 --- a/Makefile-boot.am +++ b/Makefile-boot.am @@ -39,7 +39,10 @@ endif if BUILDOPT_SYSTEMD systemdsystemunit_DATA = src/boot/ostree-prepare-root.service \ - src/boot/ostree-remount.service src/boot/ostree-finalize-staged.service + src/boot/ostree-remount.service \ + src/boot/ostree-finalize-staged.service \ + src/boot/ostree-finalize-staged.path \ + $(NULL) systemdtmpfilesdir = $(prefix)/lib/tmpfiles.d dist_systemdtmpfiles_DATA = src/boot/ostree-tmpfiles.conf diff --git a/src/boot/ostree-finalize-staged.path b/src/boot/ostree-finalize-staged.path new file mode 100644 index 00000000..f0f76151 --- /dev/null +++ b/src/boot/ostree-finalize-staged.path @@ -0,0 +1,28 @@ +# Copyright (C) 2018 Red Hat, Inc. +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library; if not, write to the +# Free Software Foundation, Inc., 59 Temple Place - Suite 330, +# Boston, MA 02111-1307, USA. + +# For some implementation discussion, see: +# https://lists.freedesktop.org/archives/systemd-devel/2018-March/040557.html +[Unit] +Description=OSTree Monitor Staged Deployment +Documentation=man:ostree(1) + +[Path] +PathExists=/run/ostree/staged-deployment + +[Install] +WantedBy=multi-user.target diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index b424d5e9..b16f65b3 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -2761,6 +2761,10 @@ ostree_sysroot_stage_tree (OstreeSysroot *self, if (booted_deployment == NULL) return glnx_throw (error, "Cannot stage a deployment when not currently booted into an OSTree system"); + /* This is used by the testsuite to exercise the path unit, until it becomes the default + * (which is pending on the preset making it everywhere). */ + if ((self->debug_flags & OSTREE_SYSROOT_DEBUG_TEST_STAGED_PATH) == 0) + { /* This is a bit of a hack. When adding a new service we have to end up getting * into the presets for downstream distros; see e.g. https://src.fedoraproject.org/rpms/ostree/pull-request/7 * @@ -2773,6 +2777,11 @@ ostree_sysroot_stage_tree (OstreeSysroot *self, return FALSE; if (!g_spawn_check_exit_status (estatus, error)) return FALSE; + } + else + { + g_print ("test-staged-path: Not running `systemctl start`\n"); + } /* OSTREE_SYSROOT_DEBUG_TEST_STAGED_PATH */ g_autoptr(OstreeDeployment) deployment = NULL; if (!sysroot_initialize_deployment (self, osname, revision, origin, override_kernel_argv, diff --git a/src/libostree/ostree-sysroot-private.h b/src/libostree/ostree-sysroot-private.h index 16cca5a1..9da6d4c9 100644 --- a/src/libostree/ostree-sysroot-private.h +++ b/src/libostree/ostree-sysroot-private.h @@ -36,6 +36,9 @@ typedef enum { OSTREE_SYSROOT_DEBUG_NO_XATTRS = 1 << 1, /* https://github.com/ostreedev/ostree/pull/1049 */ OSTREE_SYSROOT_DEBUG_TEST_FIFREEZE = 1 << 2, + /* This is a temporary flag until we fully drop the explicit `systemctl start + * ostree-finalize-staged.service` so that tests can exercise the new path unit. */ + OSTREE_SYSROOT_DEBUG_TEST_STAGED_PATH = 1 << 3, } OstreeSysrootDebugFlags; /** diff --git a/src/libostree/ostree-sysroot.c b/src/libostree/ostree-sysroot.c index 8485edec..84c12301 100644 --- a/src/libostree/ostree-sysroot.c +++ b/src/libostree/ostree-sysroot.c @@ -189,6 +189,7 @@ ostree_sysroot_init (OstreeSysroot *self) { "mutable-deployments", OSTREE_SYSROOT_DEBUG_MUTABLE_DEPLOYMENTS }, { "test-fifreeze", OSTREE_SYSROOT_DEBUG_TEST_FIFREEZE }, { "no-xattrs", OSTREE_SYSROOT_DEBUG_NO_XATTRS }, + { "test-staged-path", OSTREE_SYSROOT_DEBUG_TEST_STAGED_PATH }, }; self->debug_flags = g_parse_debug_string (g_getenv ("OSTREE_SYSROOT_DEBUG"), diff --git a/tests/installed/destructive/staged-deploy.yml b/tests/installed/destructive/staged-deploy.yml index bc53d06d..cfd9165b 100644 --- a/tests/installed/destructive/staged-deploy.yml +++ b/tests/installed/destructive/staged-deploy.yml @@ -1,16 +1,31 @@ # Test the deploy --stage functionality; first, we stage a deployment # reboot, and validate that it worked. +# for now, until the preset propagates down +- name: Start up path unit + shell: | + set -xeuo pipefail + systemctl enable --now ostree-finalize-staged.path - name: Write staged-deploy commit shell: | set -xeuo pipefail + export OSTREE_SYSROOT_DEBUG="test-staged-path" cd /ostree/repo/tmp # https://github.com/ostreedev/ostree/issues/1569 ostree checkout -H ${commit} t ostree commit --no-bindings --parent="${commit}" -b staged-deploy -I --consume t newcommit=$(ostree rev-parse staged-deploy) orig_mtime=$(stat -c '%.Y' /sysroot/ostree/deploy) - ostree admin deploy --stage staged-deploy + systemctl show -p SubState ostree-finalize-staged.path | grep -q waiting + systemctl show -p ActiveState ostree-finalize-staged.service | grep -q inactive + systemctl show -p TriggeredBy ostree-finalize-staged.service | grep -q path + ostree admin deploy --stage staged-deploy | tee out.txt + if ! grep -q 'test-staged-path: Not running' out.txt; then + cat out.txt + exit 1 + fi + systemctl show -p SubState ostree-finalize-staged.path | grep running + systemctl show -p ActiveState ostree-finalize-staged.service | grep active new_mtime=$(stat -c '%.Y' /sysroot/ostree/deploy) test "${orig_mtime}" != "${new_mtime}" test -f /run/ostree/staged-deployment From c9a9e6c3811002f18de9baa72d32aff5463f19d4 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 22 Oct 2018 10:58:44 -0400 Subject: [PATCH 47/50] README: Add bindings section Since rust-libostree now exists too, let's make sure people know about it. Closes: #1762 Approved by: jlebon --- README.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/README.md b/README.md index 36bcfc24..6ea343ee 100644 --- a/README.md +++ b/README.md @@ -74,6 +74,22 @@ The [BuildStream](https://gitlab.com/BuildStream/buildstream) build and integration tool uses libostree as a caching system to store and share built artifacts. +Language bindings +---- + +libostree is accessible via [GObject Introspection](https://gi.readthedocs.io/en/latest/); +any language which has implemented the GI binding model should work. +For example, Both [pygobject](https://pygobject.readthedocs.io/en/latest/) +and [gjs](https://gitlab.gnome.org/GNOME/gjs) are known to work +and further are actually used in libostree's test suite today. + +Some bindings take the approach of using GI as a lower level and +write higher level manual bindings on top; this is more common +for statically compiled languages. Here's a list of such bindings: + + - [ostree-go](https://github.com/ostreedev/ostree-go/) + - [rust-libostree](https://gitlab.com/fkrull/rust-libostree/) + Building -------- From 3fc46f37f7de3edd4d1c57f2554a6d57f2dc51c9 Mon Sep 17 00:00:00 2001 From: Matthew Leeds Date: Mon, 22 Oct 2018 15:11:14 -0700 Subject: [PATCH 48/50] lib/repo-pull: Add an explanatory comment Closes: #1763 Approved by: pwithnall --- src/libostree/ostree-repo-pull.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 4a3adf2a..1efb38b3 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -5020,6 +5020,7 @@ ostree_repo_find_remotes_async (OstreeRepo *self, } #endif /* HAVE_AVAHI */ + /* self->repo_finders is guaranteed to be non-empty */ g_assert (default_finders != NULL); finders = default_finders; From ed41822b45ff8b00c10528e1d918a69a4389fa21 Mon Sep 17 00:00:00 2001 From: Matthew Leeds Date: Mon, 22 Oct 2018 15:11:39 -0700 Subject: [PATCH 49/50] Rename core.repo-finders to core.default-repo-finders This renames a config key to make its semantics more obvious. Despite what the commit message says, it only applies when a set of repo finders is not specified (either on the command line or in a library API call). This also renames the corresponding ostree_repo_get function. We can do this since it hasn't been released yet. Closes: #1763 Approved by: pwithnall --- apidoc/ostree-sections.txt | 2 +- man/ostree.repo-config.xml | 2 +- src/libostree/libostree-devel.sym | 2 +- src/libostree/ostree-repo.c | 12 ++++++------ src/libostree/ostree-repo.h | 2 +- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/apidoc/ostree-sections.txt b/apidoc/ostree-sections.txt index a91663ed..5dbafc5f 100644 --- a/apidoc/ostree-sections.txt +++ b/apidoc/ostree-sections.txt @@ -298,7 +298,7 @@ ostree_repo_get_mode ostree_repo_get_min_free_space_bytes ostree_repo_get_config ostree_repo_get_dfd -ostree_repo_get_repo_finders +ostree_repo_get_default_repo_finders ostree_repo_hash ostree_repo_equal ostree_repo_copy_config diff --git a/man/ostree.repo-config.xml b/man/ostree.repo-config.xml index 3ea24486..acaa3be5 100644 --- a/man/ostree.repo-config.xml +++ b/man/ostree.repo-config.xml @@ -217,7 +217,7 @@ Boston, MA 02111-1307, USA. - repo-finders + default-repo-finders Semicolon separated default list of finders (sources for refs) to use when pulling. This can be used to disable pulling from mounted filesystems, peers on the local network, diff --git a/src/libostree/libostree-devel.sym b/src/libostree/libostree-devel.sym index 80b04fd5..38f71486 100644 --- a/src/libostree/libostree-devel.sym +++ b/src/libostree/libostree-devel.sym @@ -21,7 +21,7 @@ LIBOSTREE_2018.9 { ostree_mutable_tree_remove; ostree_repo_get_min_free_space_bytes; - ostree_repo_get_repo_finders; + ostree_repo_get_default_repo_finders; } LIBOSTREE_2018.7; /* Stub section for the stable release *after* this development one; don't diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index fe095057..fa3c2a94 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -2942,7 +2942,7 @@ reload_core_config (OstreeRepo *self, { g_auto(GStrv) configured_finders = NULL; g_autoptr(GError) local_error = NULL; - configured_finders = g_key_file_get_string_list (self->config, "core", "repo-finders", + configured_finders = g_key_file_get_string_list (self->config, "core", "default-repo-finders", NULL, &local_error); if (g_error_matches (local_error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_KEY_NOT_FOUND)) g_clear_error (&local_error); @@ -2953,7 +2953,7 @@ reload_core_config (OstreeRepo *self, } if (configured_finders != NULL && *configured_finders == NULL) - return glnx_throw (error, "Invalid empty repo-finders configuration"); + return glnx_throw (error, "Invalid empty default-repo-finders configuration"); for (char **iter = configured_finders; iter && *iter; iter++) { @@ -5952,18 +5952,18 @@ ostree_repo_set_collection_id (OstreeRepo *self, } /** - * ostree_repo_get_repo_finders: + * ostree_repo_get_default_repo_finders: * @self: an #OstreeRepo * - * Get the set of repo finders configured. See the documentation for - * the "core.repo-finders" config key. + * Get the set of default repo finders configured. See the documentation for + * the "core.default-repo-finders" config key. * * Returns: (array zero-terminated=1) (element-type utf8): * %NULL-terminated array of strings. * Since: 2018.9 */ const gchar * const * -ostree_repo_get_repo_finders (OstreeRepo *self) +ostree_repo_get_default_repo_finders (OstreeRepo *self) { g_return_val_if_fail (OSTREE_IS_REPO (self), NULL); diff --git a/src/libostree/ostree-repo.h b/src/libostree/ostree-repo.h index 2f7abf6d..829164ba 100644 --- a/src/libostree/ostree-repo.h +++ b/src/libostree/ostree-repo.h @@ -113,7 +113,7 @@ gboolean ostree_repo_set_collection_id (OstreeRepo *self, GError **error); _OSTREE_PUBLIC -const gchar * const * ostree_repo_get_repo_finders (OstreeRepo *self); +const gchar * const * ostree_repo_get_default_repo_finders (OstreeRepo *self); _OSTREE_PUBLIC GFile * ostree_repo_get_path (OstreeRepo *self); From f3eba6bcec39c163eb831c02c148ffa483292906 Mon Sep 17 00:00:00 2001 From: Umang Jain Date: Tue, 23 Oct 2018 23:03:31 +0530 Subject: [PATCH 50/50] Release 2018.9 Closes: #1761 Approved by: cgwalters --- configure.ac | 2 +- src/libostree/libostree-devel.sym | 5 ----- src/libostree/libostree-released.sym | 6 ++++++ tests/test-symbols.sh | 2 +- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/configure.ac b/configure.ac index 0cf2e532..65e0e68c 100644 --- a/configure.ac +++ b/configure.ac @@ -7,7 +7,7 @@ m4_define([year_version], [2018]) m4_define([release_version], [9]) 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 38f71486..a4040ba6 100644 --- a/src/libostree/libostree-devel.sym +++ b/src/libostree/libostree-devel.sym @@ -18,11 +18,6 @@ ***/ /* Add new symbols here. Release commits should copy this section into -released.sym. */ -LIBOSTREE_2018.9 { - ostree_mutable_tree_remove; - ostree_repo_get_min_free_space_bytes; - ostree_repo_get_default_repo_finders; -} LIBOSTREE_2018.7; /* 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 ae3bb5ed..90b363ca 100644 --- a/src/libostree/libostree-released.sym +++ b/src/libostree/libostree-released.sym @@ -536,6 +536,12 @@ global: /* No new symbols in 2018.8 */ +LIBOSTREE_2018.9 { + ostree_mutable_tree_remove; + ostree_repo_get_min_free_space_bytes; + ostree_repo_get_default_repo_finders; +} LIBOSTREE_2018.7; + /* 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 df7f8648..47f3ad73 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 <