From c5dad0d396cb01a6460a7cf80a43e68d4470355c Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 4 Dec 2017 13:02:06 -0500 Subject: [PATCH 01/43] build-sys: Post-release version bump Closes: #1361 Approved by: jlebon --- configure.ac | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 629c923b..3827470b 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], [2017]) -m4_define([release_version], [14]) +m4_define([release_version], [15]) 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 8d6b30c3819e962324a2e836ed9b576215373195 Mon Sep 17 00:00:00 2001 From: Sam Thursfield Date: Mon, 4 Dec 2017 17:38:51 +0000 Subject: [PATCH 02/43] README.md: Link to BuildStream This is an example of a tool using libostree to cache and share build results. Closes: #1360 Approved by: cgwalters --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index 3e72c061..5bf8b7b0 100644 --- a/README.md +++ b/README.md @@ -70,6 +70,10 @@ projects. where OSTree was born - as a high performance continuous delivery/testing system for GNOME. +The [BuildStream](https://gitlab.com/BuildStream/buildstream) build and +integration tool uses libostree as a caching system to store and share +built artifacts. + Building -------- From 89a57bb6d82afdccd07f7ff98b498c4a24bd4820 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 1 Dec 2017 15:18:37 -0500 Subject: [PATCH 03/43] lib/repo: Add MT support for transaction_set_ref(), clarify MT rules For rpm-ostree I'd like to do importing in parallel with threads; the code is *almost* ready for that except today it calls `ostree_repo_transaction_set_ref()`. Looking at the code, there's really a "transaction" struct here, not just stats. Let's lift that struct out, and move the refs into it under the existing lock. Clarify the documentation around multithreading for various functions. Closes: #1358 Approved by: jlebon --- src/libostree/ostree-repo-commit.c | 151 +++++++++++++++++----------- src/libostree/ostree-repo-private.h | 18 ++-- src/libostree/ostree-repo.c | 8 +- 3 files changed, 108 insertions(+), 69 deletions(-) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index a286e7ad..4f0a9239 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -658,19 +658,19 @@ write_content_object (OstreeRepo *self, /* Free space check; only applies during transactions */ if (self->min_free_space_percent > 0 && self->in_transaction) { - g_mutex_lock (&self->txn_stats_lock); - g_assert_cmpint (self->txn_blocksize, >, 0); - const fsblkcnt_t object_blocks = (size / self->txn_blocksize) + 1; - if (object_blocks > self->max_txn_blocks) + g_mutex_lock (&self->txn_lock); + g_assert_cmpint (self->txn.blocksize, >, 0); + const fsblkcnt_t object_blocks = (size / self->txn.blocksize) + 1; + if (object_blocks > self->txn.max_blocks) { - g_mutex_unlock (&self->txn_stats_lock); - g_autofree char *formatted_required = g_format_size ((guint64)object_blocks * self->txn_blocksize); + g_mutex_unlock (&self->txn_lock); + g_autofree char *formatted_required = g_format_size ((guint64)object_blocks * self->txn.blocksize); return glnx_throw (error, "min-free-space-percent '%u%%' would be exceeded, %s more required", self->min_free_space_percent, formatted_required); } /* This is the main bit that needs mutex protection */ - self->max_txn_blocks -= object_blocks; - g_mutex_unlock (&self->txn_stats_lock); + self->txn.max_blocks -= object_blocks; + g_mutex_unlock (&self->txn_lock); } /* For regular files, we create them with default mode, and only @@ -777,9 +777,9 @@ write_content_object (OstreeRepo *self, /* If we already have it, just update the stats. */ if (have_obj) { - g_mutex_lock (&self->txn_stats_lock); - self->txn_stats.content_objects_total++; - g_mutex_unlock (&self->txn_stats_lock); + g_mutex_lock (&self->txn_lock); + self->txn.stats.content_objects_total++; + g_mutex_unlock (&self->txn_lock); if (out_csum) *out_csum = ostree_checksum_to_bytes (actual_checksum); /* Note early return */ @@ -849,11 +849,11 @@ write_content_object (OstreeRepo *self, } /* Update statistics */ - g_mutex_lock (&self->txn_stats_lock); - self->txn_stats.content_objects_written++; - self->txn_stats.content_bytes_written += file_object_length; - self->txn_stats.content_objects_total++; - g_mutex_unlock (&self->txn_stats_lock); + g_mutex_lock (&self->txn_lock); + self->txn.stats.content_objects_written++; + self->txn.stats.content_bytes_written += file_object_length; + self->txn.stats.content_objects_total++; + g_mutex_unlock (&self->txn_lock); if (out_csum) { @@ -1018,9 +1018,9 @@ write_metadata_object (OstreeRepo *self, */ if (have_obj) { - g_mutex_lock (&self->txn_stats_lock); - self->txn_stats.metadata_objects_total++; - g_mutex_unlock (&self->txn_stats_lock); + g_mutex_lock (&self->txn_lock); + self->txn.stats.metadata_objects_total++; + g_mutex_unlock (&self->txn_lock); if (out_csum) *out_csum = ostree_checksum_to_bytes (actual_checksum); @@ -1090,10 +1090,10 @@ write_metadata_object (OstreeRepo *self, } /* Update the stats, note we both wrote one and add to total */ - g_mutex_lock (&self->txn_stats_lock); - self->txn_stats.metadata_objects_written++; - self->txn_stats.metadata_objects_total++; - g_mutex_unlock (&self->txn_stats_lock); + g_mutex_lock (&self->txn_lock); + self->txn.stats.metadata_objects_written++; + self->txn.stats.metadata_objects_total++; + g_mutex_unlock (&self->txn_lock); if (out_csum) *out_csum = ostree_checksum_to_bytes (actual_checksum); @@ -1259,6 +1259,8 @@ devino_cache_lookup (OstreeRepo *self, * existing ostree objects, then this will speed up considerably, so call it * before you call ostree_write_directory_to_mtree() or similar. However, * ostree_repo_devino_cache_new() is better as it avoids scanning all objects. + * + * Multithreading: This function is *not* MT safe. */ gboolean ostree_repo_scan_hardlinks (OstreeRepo *self, @@ -1287,8 +1289,17 @@ ostree_repo_scan_hardlinks (OstreeRepo *self, * ostree_repo_commit_transaction(), or abort the transaction with * ostree_repo_abort_transaction(). * - * Currently, transactions are not atomic, and aborting a transaction - * will not erase any data you write during the transaction. + * Currently, transactions may result in partial commits or data in the target + * repository if interrupted during ostree_repo_commit_transaction(), and + * further writing refs is also not currently atomic. + * + * There can be at most one transaction active on a repo at a time per instance + * of `OstreeRepo`; however, it is safe to have multiple threads writing objects + * on a single `OstreeRepo` instance as long as their lifetime is bounded by the + * transaction. + * + * Multithreading: This function is *not* MT safe; only one transaction can be + * active at a time. */ gboolean ostree_repo_prepare_transaction (OstreeRepo *self, @@ -1299,7 +1310,7 @@ ostree_repo_prepare_transaction (OstreeRepo *self, g_return_val_if_fail (self->in_transaction == FALSE, FALSE); - memset (&self->txn_stats, 0, sizeof (OstreeRepoTransactionStats)); + memset (&self->txn.stats, 0, sizeof (OstreeRepoTransactionStats)); self->in_transaction = TRUE; if (self->min_free_space_percent > 0) @@ -1307,23 +1318,23 @@ ostree_repo_prepare_transaction (OstreeRepo *self, struct statvfs stvfsbuf; if (TEMP_FAILURE_RETRY (fstatvfs (self->repo_dir_fd, &stvfsbuf)) < 0) return glnx_throw_errno_prefix (error, "fstatvfs"); - g_mutex_lock (&self->txn_stats_lock); - self->txn_blocksize = stvfsbuf.f_bsize; + g_mutex_lock (&self->txn_lock); + self->txn.blocksize = stvfsbuf.f_bsize; /* Convert fragment to blocks to compute the total */ guint64 total_blocks = (stvfsbuf.f_frsize * stvfsbuf.f_blocks) / stvfsbuf.f_bsize; /* Use the appropriate free block count if we're unprivileged */ guint64 bfree = (getuid () != 0 ? stvfsbuf.f_bavail : stvfsbuf.f_bfree); guint64 reserved_blocks = ((double)total_blocks) * (self->min_free_space_percent/100.0); if (bfree > reserved_blocks) - self->max_txn_blocks = bfree - reserved_blocks; + self->txn.max_blocks = bfree - reserved_blocks; else { - g_mutex_unlock (&self->txn_stats_lock); - g_autofree char *formatted_free = g_format_size (bfree * self->txn_blocksize); + g_mutex_unlock (&self->txn_lock); + g_autofree char *formatted_free = g_format_size (bfree * self->txn.blocksize); return glnx_throw (error, "min-free-space-percent '%u%%' would be exceeded, %s available", self->min_free_space_percent, formatted_free); } - g_mutex_unlock (&self->txn_stats_lock); + g_mutex_unlock (&self->txn_lock); } gboolean ret_transaction_resume = FALSE; @@ -1547,10 +1558,10 @@ cleanup_tmpdir (OstreeRepo *self, static void ensure_txn_refs (OstreeRepo *self) { - if (self->txn_refs == NULL) - self->txn_refs = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free); - if (self->txn_collection_refs == NULL) - self->txn_collection_refs = g_hash_table_new_full (ostree_collection_ref_hash, + if (self->txn.refs == NULL) + self->txn.refs = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free); + if (self->txn.collection_refs == NULL) + self->txn.collection_refs = g_hash_table_new_full (ostree_collection_ref_hash, ostree_collection_ref_equal, (GDestroyNotify) ostree_collection_ref_free, g_free); @@ -1565,6 +1576,8 @@ ensure_txn_refs (OstreeRepo *self) * Like ostree_repo_transaction_set_ref(), but takes concatenated * @refspec format as input instead of separate remote and name * arguments. + * + * Multithreading: Since v2017.15 this function is MT safe. */ void ostree_repo_transaction_set_refspec (OstreeRepo *self, @@ -1573,9 +1586,10 @@ ostree_repo_transaction_set_refspec (OstreeRepo *self, { g_return_if_fail (self->in_transaction == TRUE); + g_mutex_lock (&self->txn_lock); ensure_txn_refs (self); - - g_hash_table_replace (self->txn_refs, g_strdup (refspec), g_strdup (checksum)); + g_hash_table_replace (self->txn.refs, g_strdup (refspec), g_strdup (checksum)); + g_mutex_unlock (&self->txn_lock); } /** @@ -1592,10 +1606,20 @@ ostree_repo_transaction_set_refspec (OstreeRepo *self, * Otherwise, if @checksum is %NULL, then record that the ref should * be deleted. * - * The change will not be written out immediately, but when the transaction - * is completed with ostree_repo_commit_transaction(). If the transaction - * is instead aborted with ostree_repo_abort_transaction(), no changes will - * be made to the repository. + * The change will be written when the transaction is completed with + * ostree_repo_commit_transaction(); that function takes care of writing all of + * the objects (such as the commit referred to by @checksum) before updating the + * refs. If the transaction is instead aborted with + * ostree_repo_abort_transaction(), no changes to the ref will be made to the + * repository. + * + * Note however that currently writing *multiple* refs is not truly atomic; if + * the process or system is terminated during + * ostree_repo_commit_transaction(), it is possible that just some of the refs + * will have been updated. Your application should take care to handle this + * case. + * + * Multithreading: Since v2017.15 this function is MT safe. */ void ostree_repo_transaction_set_ref (OstreeRepo *self, @@ -1603,18 +1627,18 @@ ostree_repo_transaction_set_ref (OstreeRepo *self, const char *ref, const char *checksum) { - char *refspec; - g_return_if_fail (self->in_transaction == TRUE); - ensure_txn_refs (self); - + char *refspec; if (remote) refspec = g_strdup_printf ("%s:%s", remote, ref); else refspec = g_strdup (ref); - g_hash_table_replace (self->txn_refs, refspec, g_strdup (checksum)); + g_mutex_lock (&self->txn_lock); + ensure_txn_refs (self); + g_hash_table_replace (self->txn.refs, refspec, g_strdup (checksum)); + g_mutex_unlock (&self->txn_lock); } /** @@ -1634,6 +1658,8 @@ ostree_repo_transaction_set_ref (OstreeRepo *self, * is instead aborted with ostree_repo_abort_transaction(), no changes will * be made to the repository. * + * Multithreading: Since v2017.15 this function is MT safe. + * * Since: 2017.8 */ void @@ -1646,10 +1672,11 @@ ostree_repo_transaction_set_collection_ref (OstreeRepo *self, g_return_if_fail (ref != NULL); g_return_if_fail (checksum == NULL || ostree_validate_checksum_string (checksum, NULL)); + g_mutex_lock (&self->txn_lock); ensure_txn_refs (self); - - g_hash_table_replace (self->txn_collection_refs, + g_hash_table_replace (self->txn.collection_refs, ostree_collection_ref_dup (ref), g_strdup (checksum)); + g_mutex_unlock (&self->txn_lock); } /** @@ -1664,6 +1691,8 @@ ostree_repo_transaction_set_collection_ref (OstreeRepo *self, * This is like ostree_repo_transaction_set_ref(), except it may be * invoked outside of a transaction. This is presently safe for the * case where we're creating or overwriting an existing ref. + * + * Multithreading: This function is MT safe. */ gboolean ostree_repo_set_ref_immediate (OstreeRepo *self, @@ -1745,6 +1774,12 @@ ostree_repo_set_collection_ref_immediate (OstreeRepo *self, * Complete the transaction. Any refs set with * ostree_repo_transaction_set_ref() or * ostree_repo_transaction_set_refspec() will be written out. + * + * Note that if multiple threads are performing writes, all such threads must + * have terminated before this function is invoked. + * + * Multithreading: This function is *not* MT safe; only one transaction can be + * active at a time. */ gboolean ostree_repo_commit_transaction (OstreeRepo *self, @@ -1782,15 +1817,15 @@ ostree_repo_commit_transaction (OstreeRepo *self, if (self->loose_object_devino_hash) g_hash_table_remove_all (self->loose_object_devino_hash); - if (self->txn_refs) - if (!_ostree_repo_update_refs (self, self->txn_refs, cancellable, error)) + if (self->txn.refs) + if (!_ostree_repo_update_refs (self, self->txn.refs, cancellable, error)) return FALSE; - g_clear_pointer (&self->txn_refs, g_hash_table_destroy); + g_clear_pointer (&self->txn.refs, g_hash_table_destroy); - if (self->txn_collection_refs) - if (!_ostree_repo_update_collection_refs (self, self->txn_collection_refs, cancellable, error)) + if (self->txn.collection_refs) + if (!_ostree_repo_update_collection_refs (self, self->txn.collection_refs, cancellable, error)) return FALSE; - g_clear_pointer (&self->txn_collection_refs, g_hash_table_destroy); + g_clear_pointer (&self->txn.collection_refs, g_hash_table_destroy); self->in_transaction = FALSE; @@ -1798,7 +1833,7 @@ ostree_repo_commit_transaction (OstreeRepo *self, return FALSE; if (out_stats) - *out_stats = self->txn_stats; + *out_stats = self->txn.stats; return TRUE; } @@ -1829,8 +1864,8 @@ ostree_repo_abort_transaction (OstreeRepo *self, if (self->loose_object_devino_hash) g_hash_table_remove_all (self->loose_object_devino_hash); - g_clear_pointer (&self->txn_refs, g_hash_table_destroy); - g_clear_pointer (&self->txn_collection_refs, g_hash_table_destroy); + g_clear_pointer (&self->txn.refs, g_hash_table_destroy); + g_clear_pointer (&self->txn.collection_refs, g_hash_table_destroy); glnx_tmpdir_unset (&self->commit_stagedir); glnx_release_lock_file (&self->commit_stagedir_lock); diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h index 58c104b9..418181cd 100644 --- a/src/libostree/ostree-repo-private.h +++ b/src/libostree/ostree-repo-private.h @@ -82,6 +82,15 @@ typedef enum { OSTREE_REPO_SYSROOT_KIND_IS_SYSROOT_OSTREE, /* We match /ostree/repo */ } OstreeRepoSysrootKind; +typedef struct { + GHashTable *refs; /* (element-type utf8 utf8) */ + GHashTable *collection_refs; /* (element-type OstreeCollectionRef utf8) */ + OstreeRepoTransactionStats stats; + /* Implementation of min-free-space-percent */ + gulong blocksize; + fsblkcnt_t max_blocks; +} OstreeRepoTxn; + /** * OstreeRepo: * @@ -109,13 +118,8 @@ struct OstreeRepo { GWeakRef sysroot; /* Weak to avoid a circular ref; see also `is_system` */ char *remotes_config_dir; - GHashTable *txn_refs; /* (element-type utf8 utf8) */ - GHashTable *txn_collection_refs; /* (element-type OstreeCollectionRef utf8) */ - GMutex txn_stats_lock; - OstreeRepoTransactionStats txn_stats; - /* Implementation of min-free-space-percent */ - gulong txn_blocksize; - fsblkcnt_t max_txn_blocks; + GMutex txn_lock; + OstreeRepoTxn txn; GMutex cache_lock; guint dirmeta_cache_refcount; diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index afd56e4c..69cea4f0 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -505,13 +505,13 @@ ostree_repo_finalize (GObject *object) g_hash_table_destroy (self->updated_uncompressed_dirs); if (self->config) g_key_file_free (self->config); - g_clear_pointer (&self->txn_refs, g_hash_table_destroy); - g_clear_pointer (&self->txn_collection_refs, g_hash_table_destroy); + g_clear_pointer (&self->txn.refs, g_hash_table_destroy); + g_clear_pointer (&self->txn.collection_refs, g_hash_table_destroy); g_clear_error (&self->writable_error); g_clear_pointer (&self->object_sizes, (GDestroyNotify) g_hash_table_unref); g_clear_pointer (&self->dirmeta_cache, (GDestroyNotify) g_hash_table_unref); g_mutex_clear (&self->cache_lock); - g_mutex_clear (&self->txn_stats_lock); + g_mutex_clear (&self->txn_lock); g_free (self->collection_id); g_clear_pointer (&self->remotes, g_hash_table_destroy); @@ -672,7 +672,7 @@ ostree_repo_init (OstreeRepo *self) test_error_keys, G_N_ELEMENTS (test_error_keys)); g_mutex_init (&self->cache_lock); - g_mutex_init (&self->txn_stats_lock); + g_mutex_init (&self->txn_lock); self->remotes = g_hash_table_new_full (g_str_hash, g_str_equal, (GDestroyNotify) NULL, From e48262c659722ecac4d46ad6a1e1d8e82cd565cd Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 28 Nov 2017 12:02:03 -0500 Subject: [PATCH 04/43] lib/repo: Add some error prefixing in commit, repo create I was getting a bare `error: Creating temp file: No such file or directory` when debugging `test-concurrency.py`; with this I get `error: Writing content object: Creating temp file: No such file or directory` which helps me pin it down. Closes: #1343 Approved by: cgwalters --- src/libostree/ostree-repo-commit.c | 8 ++++++-- src/libostree/ostree-repo.c | 1 + 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index 4f0a9239..b64ef1e0 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -602,6 +602,7 @@ write_content_object (OstreeRepo *self, GCancellable *cancellable, GError **error) { + GLNX_AUTO_PREFIX_ERROR ("Writing content object", error); g_return_val_if_fail (expected_checksum || out_csum, FALSE); if (g_cancellable_set_error_if_cancelled (cancellable, error)) @@ -844,8 +845,7 @@ write_content_object (OstreeRepo *self, uid, gid, mode, xattrs, cancellable, error)) - return glnx_prefix_error (error, "Writing object %s.%s", actual_checksum, - ostree_object_type_to_string (OSTREE_OBJECT_TYPE_FILE)); + return FALSE; } /* Update statistics */ @@ -886,6 +886,8 @@ adopt_and_commit_regfile (OstreeRepo *self, GCancellable *cancellable, GError **error) { + GLNX_AUTO_PREFIX_ERROR ("Commit regfile (adopt)", error); + g_assert (G_IN_SET (self->mode, OSTREE_REPO_MODE_BARE, OSTREE_REPO_MODE_BARE_USER_ONLY)); g_autoptr(GBytes) header = _ostree_file_header_new (finfo, xattrs); @@ -981,6 +983,8 @@ write_metadata_object (OstreeRepo *self, GCancellable *cancellable, GError **error) { + GLNX_AUTO_PREFIX_ERROR ("Writing metadata object", error); + g_return_val_if_fail (expected_checksum || out_csum, FALSE); if (g_cancellable_set_error_if_cancelled (cancellable, error)) diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 69cea4f0..21d96cc6 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -1797,6 +1797,7 @@ repo_create_at_internal (int dfd, GCancellable *cancellable, GError **error) { + GLNX_AUTO_PREFIX_ERROR ("Creating repo", error); struct stat stbuf; /* We do objects/ last - if it exists we do nothing and exit successfully */ const char *state_dirs[] = { "tmp", "extensions", "state", From 4e78ddd2da4c0a2ced9527ef20a30f8e3b8567b8 Mon Sep 17 00:00:00 2001 From: Dan Nicholson Date: Thu, 5 Oct 2017 15:25:11 -0500 Subject: [PATCH 05/43] lib/repo: Add repo locking mechanism Currently ostree has no method of guarding against concurrent pruning. When there are multiple repo writers, it's possible to have a pull or commit race against a prune and end up with missing objects. This adds a file based repo locking mechanism. The intention is to take a shared lock when writing objects and an exclusive lock when deleting them. In order to make use of the locking throughout the library in a fine grained fashion, the lock acts recursively with a stack of lock states. If the lock becomes exclusive, it will stay in that state until the stack is unwound past the initial exclusive push. The file locking is similar to GLnxLockFile in that it uses open file descriptor locks but falls back to flock when needed. The lock also attempts to be thread safe by storing the lock state in thread local storage with GPrivate. This means that each thread will have an independent lock for each repository it opens. There are some drawbacks to that, but it seemed impossible to manage the lock state coherently in the face of multithreaded access. The API is a push/pop interface in accordance with the recursive nature of the locking. The push interface uses an enum that's translated to LOCK_SH or LOCK_EX as needed. Both interfaces use an internal timeout field to decide whether to manage the lock in a blocking or non-blocking fashion. The intention is to allow ostree applications as well as administrators to control this timeout. For now, the default is a 30 second timeout. Note that the timeout is handled synchronously in thread since the lock is maintained in thread local storage. I.e., the thread that acquires the lock needs to be the same thread that runs the operation. There may be a way to offer an asynchronous version, but it's not clear exactly how that would work since it would likely involve a separate thread that invokes a callback when the locking operation completes. https://bugzilla.gnome.org/show_bug.cgi?id=759442 Closes: #1343 Approved by: cgwalters --- apidoc/ostree-experimental-sections.txt | 3 + src/libostree/libostree-experimental.sym | 2 + src/libostree/ostree-repo-private.h | 20 + src/libostree/ostree-repo.c | 465 +++++++++++++++++++++++ src/libostree/ostree-repo.h | 24 ++ 5 files changed, 514 insertions(+) diff --git a/apidoc/ostree-experimental-sections.txt b/apidoc/ostree-experimental-sections.txt index 60daaca5..61f43f28 100644 --- a/apidoc/ostree-experimental-sections.txt +++ b/apidoc/ostree-experimental-sections.txt @@ -90,6 +90,9 @@ ostree_repo_finder_override_get_type
ostree-misc-experimental +OstreeRepoLockType +ostree_repo_lock_push +ostree_repo_lock_pop ostree_repo_get_collection_id ostree_repo_set_collection_id ostree_validate_collection_id diff --git a/src/libostree/libostree-experimental.sym b/src/libostree/libostree-experimental.sym index b83ad1b0..15730546 100644 --- a/src/libostree/libostree-experimental.sym +++ b/src/libostree/libostree-experimental.sym @@ -94,4 +94,6 @@ LIBOSTREE_2017.14_EXPERIMENTAL { global: ostree_remote_get_type; ostree_remote_get_url; + ostree_repo_lock_pop; + ostree_repo_lock_push; } LIBOSTREE_2017.13_EXPERIMENTAL; diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h index 418181cd..452b2b35 100644 --- a/src/libostree/ostree-repo-private.h +++ b/src/libostree/ostree-repo-private.h @@ -37,6 +37,8 @@ G_BEGIN_DECLS #define _OSTREE_MAX_OUTSTANDING_FETCHER_REQUESTS 8 #define _OSTREE_MAX_OUTSTANDING_DELTAPART_REQUESTS 2 +#define _OSTREE_DEFAULT_LOCK_TIMEOUT_SECONDS 30 + /* In most cases, writing to disk should be much faster than * fetching from the network, so we shouldn't actually hit * this. But if using pipelining and e.g. pulling over LAN @@ -157,6 +159,7 @@ struct OstreeRepo { guint64 tmp_expiry_seconds; gchar *collection_id; gboolean add_remotes_config_dir; /* Add new remotes in remotes.d dir */ + gint lock_timeout_seconds; OstreeRepo *parent_repo; }; @@ -436,6 +439,23 @@ _ostree_repo_get_remote_inherited (OstreeRepo *self, #ifndef OSTREE_ENABLE_EXPERIMENTAL_API +/* All the locking APIs below are duplicated in ostree-repo.h. Remove the ones + * here once it's no longer experimental. + */ + +typedef enum { + OSTREE_REPO_LOCK_SHARED, + OSTREE_REPO_LOCK_EXCLUSIVE +} OstreeRepoLockType; + +gboolean ostree_repo_lock_push (OstreeRepo *self, + OstreeRepoLockType lock_type, + GCancellable *cancellable, + GError **error); +gboolean ostree_repo_lock_pop (OstreeRepo *self, + GCancellable *cancellable, + GError **error); + const gchar * ostree_repo_get_collection_id (OstreeRepo *self); gboolean ostree_repo_set_collection_id (OstreeRepo *self, const gchar *collection_id, diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 21d96cc6..9c2d5fb5 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -156,6 +156,462 @@ G_DEFINE_TYPE (OstreeRepo, ostree_repo, G_TYPE_OBJECT) #define SYSCONF_REMOTES SHORTENED_SYSCONFDIR "/ostree/remotes.d" +/* Repository locking + * + * To guard against objects being deleted (e.g., prune) while they're in + * use by another operation is accessing them (e.g., commit), the + * repository must be locked by concurrent writers. + * + * The locking is implemented by maintaining a thread local table of + * lock stacks per repository. This allows thread safe locking since + * each thread maintains its own lock stack. See the OstreeRepoLock type + * below. + * + * The actual locking is done using either open file descriptor locks or + * flock locks. This allows the locking to work with concurrent + * processes. The lock file is held on the ".lock" file within the + * repository. + * + * The intended usage is to take a shared lock when writing objects or + * reading objects in critical sections. Exclusive locks are taken when + * deleting objects. + * + * To allow fine grained locking within libostree, the lock is + * maintained as a stack. The core APIs then push or pop from the stack. + * When pushing or popping a lock state identical to the existing or + * next state, the stack is simply updated. Only when upgrading or + * downgrading the lock (changing to/from unlocked, pushing exclusive on + * shared or popping exclusive to shared) are actual locking operations + * performed. + */ + +static void +free_repo_lock_table (gpointer data) +{ + GHashTable *lock_table = data; + + if (lock_table != NULL) + { + g_debug ("Free lock table"); + g_hash_table_destroy (lock_table); + } +} + +static GPrivate repo_lock_table = G_PRIVATE_INIT (free_repo_lock_table); + +typedef struct { + int fd; + GQueue stack; +} OstreeRepoLock; + +typedef struct { + guint len; + int state; + const char *name; +} OstreeRepoLockInfo; + +static void +repo_lock_info (OstreeRepoLock *lock, OstreeRepoLockInfo *out_info) +{ + g_assert (lock != NULL); + g_assert (out_info != NULL); + + OstreeRepoLockInfo info; + info.len = g_queue_get_length (&lock->stack); + if (info.len == 0) + { + info.state = LOCK_UN; + info.name = "unlocked"; + } + else + { + info.state = GPOINTER_TO_INT (g_queue_peek_head (&lock->stack)); + info.name = (info.state == LOCK_EX) ? "exclusive" : "shared"; + } + + *out_info = info; +} + +static void +free_repo_lock (gpointer data) +{ + OstreeRepoLock *lock = data; + + if (lock != NULL) + { + OstreeRepoLockInfo info; + repo_lock_info (lock, &info); + + g_debug ("Free lock: state=%s, depth=%u", info.name, info.len); + g_queue_clear (&lock->stack); + if (lock->fd >= 0) + { + g_debug ("Closing repo lock file"); + (void) close (lock->fd); + } + g_free (lock); + } +} + +/* Wrapper to handle flock vs OFD locking based on GLnxLockFile */ +static gboolean +do_repo_lock (int fd, + int flags) +{ + int res; + +#ifdef F_OFD_SETLK + struct flock fl = { + .l_type = (flags & ~LOCK_NB) == LOCK_EX ? F_WRLCK : F_RDLCK, + .l_whence = SEEK_SET, + .l_start = 0, + .l_len = 0, + }; + + res = TEMP_FAILURE_RETRY (fcntl (fd, (flags & LOCK_NB) ? F_OFD_SETLK : F_OFD_SETLKW, &fl)); +#else + res = -1; + errno = EINVAL; +#endif + + /* Fallback to flock when OFD locks not available */ + if (res < 0) + { + if (errno == EINVAL) + res = TEMP_FAILURE_RETRY (flock (fd, flags)); + if (res < 0) + return FALSE; + } + + return TRUE; +} + +/* Wrapper to handle flock vs OFD unlocking based on GLnxLockFile */ +static gboolean +do_repo_unlock (int fd, + int flags) +{ + int res; + +#ifdef F_OFD_SETLK + struct flock fl = { + .l_type = F_UNLCK, + .l_whence = SEEK_SET, + .l_start = 0, + .l_len = 0, + }; + + res = TEMP_FAILURE_RETRY (fcntl (fd, (flags & LOCK_NB) ? F_OFD_SETLK : F_OFD_SETLKW, &fl)); +#else + res = -1; + errno = EINVAL; +#endif + + /* Fallback to flock when OFD locks not available */ + if (res < 0) + { + if (errno == EINVAL) + res = TEMP_FAILURE_RETRY (flock (fd, LOCK_UN | flags)); + if (res < 0) + return FALSE; + } + + return TRUE; +} + +static gboolean +push_repo_lock (OstreeRepo *self, + OstreeRepoLockType lock_type, + gboolean blocking, + GError **error) +{ + int flags = (lock_type == OSTREE_REPO_LOCK_EXCLUSIVE) ? LOCK_EX : LOCK_SH; + if (!blocking) + flags |= LOCK_NB; + + GHashTable *lock_table = g_private_get (&repo_lock_table); + if (lock_table == NULL) + { + g_debug ("Creating repo lock table"); + lock_table = g_hash_table_new_full (NULL, NULL, NULL, + (GDestroyNotify)free_repo_lock); + g_private_set (&repo_lock_table, lock_table); + } + + OstreeRepoLock *lock = g_hash_table_lookup (lock_table, self); + if (lock == NULL) + { + lock = g_new0 (OstreeRepoLock, 1); + g_queue_init (&lock->stack); + g_debug ("Opening repo lock file"); + lock->fd = TEMP_FAILURE_RETRY (openat (self->repo_dir_fd, ".lock", + O_CREAT | O_RDWR | O_CLOEXEC, + 0600)); + if (lock->fd < 0) + { + free_repo_lock (lock); + return glnx_throw_errno_prefix (error, + "Opening lock file %s/.lock failed", + gs_file_get_path_cached (self->repodir)); + } + g_hash_table_insert (lock_table, self, lock); + } + + OstreeRepoLockInfo info; + repo_lock_info (lock, &info); + g_debug ("Push lock: state=%s, depth=%u", info.name, info.len); + + if (info.state == LOCK_EX) + { + g_debug ("Repo already locked exclusively, extending stack"); + g_queue_push_head (&lock->stack, GINT_TO_POINTER (LOCK_EX)); + } + else + { + int next_state = (flags & LOCK_EX) ? LOCK_EX : LOCK_SH; + const char *next_state_name = (flags & LOCK_EX) ? "exclusive" : "shared"; + + g_debug ("Locking repo %s", next_state_name); + if (!do_repo_lock (lock->fd, flags)) + return glnx_throw_errno_prefix (error, "Locking repo %s failed", + next_state_name); + + g_queue_push_head (&lock->stack, GINT_TO_POINTER (next_state)); + } + + return TRUE; +} + +static gboolean +pop_repo_lock (OstreeRepo *self, + gboolean blocking, + GError **error) +{ + int flags = blocking ? 0 : LOCK_NB; + + GHashTable *lock_table = g_private_get (&repo_lock_table); + g_return_val_if_fail (lock_table != NULL, FALSE); + + OstreeRepoLock *lock = g_hash_table_lookup (lock_table, self); + g_return_val_if_fail (lock != NULL, FALSE); + g_return_val_if_fail (lock->fd != -1, FALSE); + + OstreeRepoLockInfo info; + repo_lock_info (lock, &info); + g_return_val_if_fail (info.len > 0, FALSE); + + g_debug ("Pop lock: state=%s, depth=%u", info.name, info.len); + if (info.len > 1) + { + int next_state = GPOINTER_TO_INT (g_queue_peek_nth (&lock->stack, 1)); + + /* Drop back to the previous lock state if it differs */ + if (next_state != info.state) + { + /* We should never drop from shared to exclusive */ + g_return_val_if_fail (next_state == LOCK_SH, FALSE); + g_debug ("Returning lock state to shared"); + if (!do_repo_lock (lock->fd, next_state | flags)) + return glnx_throw_errno_prefix (error, + "Setting repo lock to shared failed"); + } + else + g_debug ("Maintaining lock state as %s", info.name); + } + else + { + /* Lock stack will be empty, unlock */ + g_debug ("Unlocking repo"); + if (!do_repo_unlock (lock->fd, flags)) + return glnx_throw_errno_prefix (error, "Unlocking repo failed"); + } + + g_queue_pop_head (&lock->stack); + + return TRUE; +} + +/** + * ostree_repo_lock_push: + * @self: a #OstreeRepo + * @lock_type: the type of lock to acquire + * @cancellable: a #GCancellable + * @error: a #GError + * + * Takes a lock on the repository and adds it to the lock stack. If @lock_type + * is %OSTREE_REPO_LOCK_SHARED, a shared lock is taken. If @lock_type is + * %OSTREE_REPO_LOCK_EXCLUSIVE, an exclusive lock is taken. The actual lock + * state is only changed when locking a previously unlocked repository or + * upgrading the lock from shared to exclusive. If the requested lock state is + * unchanged or would represent a downgrade (exclusive to shared), the lock + * 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 + * attempted. Otherwise, the lock is taken non-blocking and + * ostree_repo_lock_push() will sleep synchronously up to lock-timeout seconds + * attempting to acquire the lock. If the lock cannot be acquired within the + * timeout, a %G_IO_ERROR_WOULD_BLOCK error is returned. + * + * If @self is not writable by the user, then no locking is attempted and + * %TRUE is returned. + * + * Returns: %TRUE on success, otherwise %FALSE with @error set + * Since: 2017.14 + */ +gboolean +ostree_repo_lock_push (OstreeRepo *self, + OstreeRepoLockType lock_type, + GCancellable *cancellable, + GError **error) +{ + g_return_val_if_fail (self != NULL, FALSE); + g_return_val_if_fail (OSTREE_IS_REPO (self), FALSE); + g_return_val_if_fail (self->inited, FALSE); + g_return_val_if_fail (cancellable == NULL || G_IS_CANCELLABLE (cancellable), FALSE); + g_return_val_if_fail (error == NULL || *error == NULL, FALSE); + + if (!self->writable) + return TRUE; + + g_assert (self->lock_timeout_seconds >= -1); + if (self->lock_timeout_seconds == -1) + { + g_debug ("Pushing lock blocking"); + return push_repo_lock (self, lock_type, TRUE, error); + } + else + { + /* Convert to unsigned to guard against negative values */ + guint lock_timeout_seconds = self->lock_timeout_seconds; + guint waited = 0; + g_debug ("Pushing lock non-blocking with timeout %u", + lock_timeout_seconds); + for (;;) + { + if (g_cancellable_set_error_if_cancelled (cancellable, error)) + return FALSE; + + g_autoptr(GError) local_error = NULL; + if (push_repo_lock (self, lock_type, FALSE, &local_error)) + return TRUE; + + if (!g_error_matches (local_error, G_IO_ERROR, + G_IO_ERROR_WOULD_BLOCK)) + { + g_propagate_error (error, g_steal_pointer (&local_error)); + return FALSE; + } + + if (waited >= lock_timeout_seconds) + { + g_debug ("Push lock: Could not acquire lock within %u seconds", + lock_timeout_seconds); + g_propagate_error (error, g_steal_pointer (&local_error)); + return FALSE; + } + + /* Sleep 1 second and try again */ + if (waited % 60 == 0) + { + guint remaining = lock_timeout_seconds - waited; + g_debug ("Push lock: Waiting %u more second%s to acquire lock", + remaining, (remaining == 1) ? "" : "s"); + } + waited++; + sleep (1); + } + } +} + +/** + * ostree_repo_lock_pop: + * @self: a #OstreeRepo + * @cancellable: a #GCancellable + * @error: a #GError + * + * Remove the current repository lock state from the lock stack. If the lock + * stack becomes empty, the repository is unlocked. Otherwise, the lock state + * only changes when transitioning from an exclusive lock back to a shared + * 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 + * attempted. Otherwise, the lock is removed non-blocking and + * ostree_repo_lock_pop() will sleep synchronously up to lock-timeout seconds + * attempting to remove the lock. If the lock cannot be removed within the + * timeout, a %G_IO_ERROR_WOULD_BLOCK error is returned. + * + * If @self is not writable by the user, then no unlocking is attempted and + * %TRUE is returned. + * + * Returns: %TRUE on success, otherwise %FALSE with @error set + * Since: 2017.14 + */ +gboolean +ostree_repo_lock_pop (OstreeRepo *self, + GCancellable *cancellable, + GError **error) +{ + g_return_val_if_fail (self != NULL, FALSE); + g_return_val_if_fail (OSTREE_IS_REPO (self), FALSE); + g_return_val_if_fail (self->inited, FALSE); + g_return_val_if_fail (cancellable == NULL || G_IS_CANCELLABLE (cancellable), FALSE); + g_return_val_if_fail (error == NULL || *error == NULL, FALSE); + + if (!self->writable) + return TRUE; + + g_assert (self->lock_timeout_seconds >= -1); + if (self->lock_timeout_seconds == -1) + { + g_debug ("Popping lock blocking"); + return pop_repo_lock (self, TRUE, error); + } + else + { + /* Convert to unsigned to guard against negative values */ + guint lock_timeout_seconds = self->lock_timeout_seconds; + guint waited = 0; + g_debug ("Popping lock non-blocking with timeout %u", + lock_timeout_seconds); + for (;;) + { + if (g_cancellable_set_error_if_cancelled (cancellable, error)) + return FALSE; + + g_autoptr(GError) local_error = NULL; + if (pop_repo_lock (self, FALSE, &local_error)) + return TRUE; + + if (!g_error_matches (local_error, G_IO_ERROR, + G_IO_ERROR_WOULD_BLOCK)) + { + g_propagate_error (error, g_steal_pointer (&local_error)); + return FALSE; + } + + if (waited >= lock_timeout_seconds) + { + g_debug ("Pop lock: Could not remove lock within %u seconds", + lock_timeout_seconds); + g_propagate_error (error, g_steal_pointer (&local_error)); + return FALSE; + } + + /* Sleep 1 second and try again */ + if (waited % 60 == 0) + { + guint remaining = lock_timeout_seconds - waited; + g_debug ("Pop lock: Waiting %u more second%s to remove lock", + remaining, (remaining == 1) ? "" : "s"); + } + waited++; + sleep (1); + } + } +} + static GFile * get_remotes_d_dir (OstreeRepo *self, GFile *sysroot); @@ -517,6 +973,14 @@ ostree_repo_finalize (GObject *object) g_clear_pointer (&self->remotes, g_hash_table_destroy); g_mutex_clear (&self->remotes_lock); + GHashTable *lock_table = g_private_get (&repo_lock_table); + if (lock_table) + { + g_hash_table_remove (lock_table, self); + if (g_hash_table_size (lock_table) == 0) + g_private_replace (&repo_lock_table, NULL); + } + G_OBJECT_CLASS (ostree_repo_parent_class)->finalize (object); } @@ -685,6 +1149,7 @@ ostree_repo_init (OstreeRepo *self) self->objects_dir_fd = -1; self->uncompressed_objects_dir_fd = -1; self->sysroot_kind = OSTREE_REPO_SYSROOT_KIND_UNKNOWN; + self->lock_timeout_seconds = _OSTREE_DEFAULT_LOCK_TIMEOUT_SECONDS; } /** diff --git a/src/libostree/ostree-repo.h b/src/libostree/ostree-repo.h index db54f022..13074582 100644 --- a/src/libostree/ostree-repo.h +++ b/src/libostree/ostree-repo.h @@ -107,6 +107,30 @@ OstreeRepo * ostree_repo_create_at (int dfd, #ifdef OSTREE_ENABLE_EXPERIMENTAL_API +/** + * OstreeRepoLockType: + * @OSTREE_REPO_LOCK_SHARED: A shared lock + * @OSTREE_REPO_LOCK_EXCLUSIVE: An exclusive lock + * + * The type of repository lock to acquire. + * + * Since: 2017.14 + */ +typedef enum { + OSTREE_REPO_LOCK_SHARED, + OSTREE_REPO_LOCK_EXCLUSIVE +} OstreeRepoLockType; + +_OSTREE_PUBLIC +gboolean ostree_repo_lock_push (OstreeRepo *self, + OstreeRepoLockType lock_type, + GCancellable *cancellable, + GError **error); +_OSTREE_PUBLIC +gboolean ostree_repo_lock_pop (OstreeRepo *self, + GCancellable *cancellable, + GError **error); + _OSTREE_PUBLIC const gchar * ostree_repo_get_collection_id (OstreeRepo *self); _OSTREE_PUBLIC From 7d863ed9e4c15725c8439b389b5657872db1bd85 Mon Sep 17 00:00:00 2001 From: Dan Nicholson Date: Fri, 13 Oct 2017 19:31:35 +0000 Subject: [PATCH 06/43] lib/repo: Add locking auto cleanup handler Define an auto cleanup handler for use with repo locking. This is based on the existing auto transaction cleanup. A wrapper for ostree_repo_lock_push() is added with it. The intended usage is like so: g_autoptr(OstreeRepoAutoLock) lock = NULL; lock = ostree_repo_auto_lock_push (repo, lock_type, cancellable, error); if (!lock) return FALSE; The functions and type are marked to be skipped by introspection since I can't see them being usable from bindings. Closes: #1343 Approved by: cgwalters --- apidoc/ostree-experimental-sections.txt | 3 ++ src/libostree/libostree-experimental.sym | 2 + src/libostree/ostree-autocleanups.h | 1 + src/libostree/ostree-repo-private.h | 9 ++++ src/libostree/ostree-repo.c | 60 ++++++++++++++++++++++++ src/libostree/ostree-repo.h | 18 +++++++ 6 files changed, 93 insertions(+) diff --git a/apidoc/ostree-experimental-sections.txt b/apidoc/ostree-experimental-sections.txt index 61f43f28..0d168406 100644 --- a/apidoc/ostree-experimental-sections.txt +++ b/apidoc/ostree-experimental-sections.txt @@ -93,6 +93,9 @@ ostree_repo_finder_override_get_type OstreeRepoLockType ostree_repo_lock_push ostree_repo_lock_pop +OstreeRepoAutoLock +ostree_repo_auto_lock_push +ostree_repo_auto_lock_cleanup ostree_repo_get_collection_id ostree_repo_set_collection_id ostree_validate_collection_id diff --git a/src/libostree/libostree-experimental.sym b/src/libostree/libostree-experimental.sym index 15730546..3f3454f3 100644 --- a/src/libostree/libostree-experimental.sym +++ b/src/libostree/libostree-experimental.sym @@ -94,6 +94,8 @@ LIBOSTREE_2017.14_EXPERIMENTAL { global: ostree_remote_get_type; ostree_remote_get_url; + ostree_repo_auto_lock_cleanup; + ostree_repo_auto_lock_push; ostree_repo_lock_pop; ostree_repo_lock_push; } LIBOSTREE_2017.13_EXPERIMENTAL; diff --git a/src/libostree/ostree-autocleanups.h b/src/libostree/ostree-autocleanups.h index c8e8a857..b3974317 100644 --- a/src/libostree/ostree-autocleanups.h +++ b/src/libostree/ostree-autocleanups.h @@ -59,6 +59,7 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC (OstreeSysrootUpgrader, g_object_unref) G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC (OstreeRepoCommitTraverseIter, ostree_repo_commit_traverse_iter_clear) #ifdef OSTREE_ENABLE_EXPERIMENTAL_API +G_DEFINE_AUTOPTR_CLEANUP_FUNC (OstreeRepoAutoLock, ostree_repo_auto_lock_cleanup) G_DEFINE_AUTOPTR_CLEANUP_FUNC (OstreeCollectionRef, ostree_collection_ref_free) G_DEFINE_AUTO_CLEANUP_FREE_FUNC (OstreeCollectionRefv, ostree_collection_ref_freev, NULL) G_DEFINE_AUTOPTR_CLEANUP_FUNC (OstreeRemote, ostree_remote_unref) diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h index 452b2b35..ae51cea3 100644 --- a/src/libostree/ostree-repo-private.h +++ b/src/libostree/ostree-repo-private.h @@ -456,6 +456,15 @@ gboolean ostree_repo_lock_pop (OstreeRepo *self, GCancellable *cancellable, GError **error); +typedef OstreeRepo OstreeRepoAutoLock; + +OstreeRepoAutoLock * ostree_repo_auto_lock_push (OstreeRepo *self, + OstreeRepoLockType lock_type, + GCancellable *cancellable, + GError **error); +void ostree_repo_auto_lock_cleanup (OstreeRepoAutoLock *lock); +G_DEFINE_AUTOPTR_CLEANUP_FUNC (OstreeRepoAutoLock, ostree_repo_auto_lock_cleanup) + const gchar * ostree_repo_get_collection_id (OstreeRepo *self); gboolean ostree_repo_set_collection_id (OstreeRepo *self, const gchar *collection_id, diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 9c2d5fb5..039f437b 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -612,6 +612,66 @@ ostree_repo_lock_pop (OstreeRepo *self, } } +/** + * ostree_repo_auto_lock_push: (skip) + * @self: a #OstreeRepo + * @lock_type: the type of lock to acquire + * @cancellable: a #GCancellable + * @error: a #GError + * + * Like ostree_repo_lock_push(), but for usage with #OstreeRepoAutoLock. + * The intended usage is to declare the #OstreeRepoAutoLock with + * g_autoptr() so that ostree_repo_auto_lock_cleanup() is called when it + * goes out of scope. This will automatically pop the lock status off + * the stack if it was acquired successfully. + * + * |[ + * g_autoptr(OstreeRepoAutoLock) lock = NULL; + * lock = ostree_repo_auto_lock_push (repo, lock_type, cancellable, error); + * if (!lock) + * return FALSE; + * ]| + * + * Returns: @self on success, otherwise %NULL with @error set + * Since: 2017.14 + */ +OstreeRepoAutoLock * +ostree_repo_auto_lock_push (OstreeRepo *self, + OstreeRepoLockType lock_type, + GCancellable *cancellable, + GError **error) +{ + if (!ostree_repo_lock_push (self, lock_type, cancellable, error)) + return NULL; + return (OstreeRepoAutoLock *)self; +} + +/** + * ostree_repo_auto_lock_cleanup: (skip) + * @lock: a #OstreeRepoAutoLock + * + * A cleanup handler for use with ostree_repo_auto_lock_push(). If @lock is + * not %NULL, ostree_repo_lock_pop() will be called on it. If + * ostree_repo_lock_pop() fails, a critical warning will be emitted. + * + * Since: 2017.14 + */ +void +ostree_repo_auto_lock_cleanup (OstreeRepoAutoLock *lock) +{ + OstreeRepo *repo = lock; + if (repo) + { + g_autoptr(GError) error = NULL; + int errsv = errno; + + if (!ostree_repo_lock_pop (repo, NULL, &error)) + g_critical ("Cleanup repo lock failed: %s", error->message); + + errno = errsv; + } +} + static GFile * get_remotes_d_dir (OstreeRepo *self, GFile *sysroot); diff --git a/src/libostree/ostree-repo.h b/src/libostree/ostree-repo.h index 13074582..7ce47a56 100644 --- a/src/libostree/ostree-repo.h +++ b/src/libostree/ostree-repo.h @@ -131,6 +131,24 @@ gboolean ostree_repo_lock_pop (OstreeRepo *self, GCancellable *cancellable, GError **error); +/** + * OstreeRepoAutoLock: (skip) + * + * This is simply an alias to #OstreeRepo used for automatic lock cleanup. + * See ostree_repo_auto_lock_push() for its intended usage. + * + * Since: 2017.14 + */ +typedef OstreeRepo OstreeRepoAutoLock; + +_OSTREE_PUBLIC +OstreeRepoAutoLock * ostree_repo_auto_lock_push (OstreeRepo *self, + OstreeRepoLockType lock_type, + GCancellable *cancellable, + GError **error); +_OSTREE_PUBLIC +void ostree_repo_auto_lock_cleanup (OstreeRepoAutoLock *lock); + _OSTREE_PUBLIC const gchar * ostree_repo_get_collection_id (OstreeRepo *self); _OSTREE_PUBLIC From 6d978893f12f745db529bc1859418e393c6fdc09 Mon Sep 17 00:00:00 2001 From: Dan Nicholson Date: Fri, 6 Oct 2017 10:56:09 +0000 Subject: [PATCH 07/43] lib/commit: Add repository locking during transactions Take a shared repo lock during a transaction to ensure that another process doesn't delete objects. Closes: #1343 Approved by: cgwalters --- src/libostree/ostree-repo-commit.c | 21 +++++++++++++++++++++ src/libostree/ostree-repo-private.h | 1 + 2 files changed, 22 insertions(+) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index b64ef1e0..79162890 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -1304,6 +1304,8 @@ ostree_repo_scan_hardlinks (OstreeRepo *self, * * Multithreading: This function is *not* MT safe; only one transaction can be * active at a time. + * + * This function takes a shared lock on the @self repository. */ gboolean ostree_repo_prepare_transaction (OstreeRepo *self, @@ -1316,6 +1318,11 @@ ostree_repo_prepare_transaction (OstreeRepo *self, memset (&self->txn.stats, 0, sizeof (OstreeRepoTransactionStats)); + self->txn_locked = ostree_repo_lock_push (self, OSTREE_REPO_LOCK_SHARED, + cancellable, error); + if (!self->txn_locked) + return FALSE; + self->in_transaction = TRUE; if (self->min_free_space_percent > 0) { @@ -1836,6 +1843,13 @@ ostree_repo_commit_transaction (OstreeRepo *self, if (!ot_ensure_unlinked_at (self->repo_dir_fd, "transaction", 0)) return FALSE; + if (self->txn_locked) + { + if (!ostree_repo_lock_pop (self, cancellable, error)) + return FALSE; + self->txn_locked = FALSE; + } + if (out_stats) *out_stats = self->txn.stats; @@ -1876,6 +1890,13 @@ ostree_repo_abort_transaction (OstreeRepo *self, self->in_transaction = FALSE; + if (self->txn_locked) + { + if (!ostree_repo_lock_pop (self, cancellable, error)) + return FALSE; + self->txn_locked = FALSE; + } + return TRUE; } diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h index ae51cea3..764540a2 100644 --- a/src/libostree/ostree-repo-private.h +++ b/src/libostree/ostree-repo-private.h @@ -122,6 +122,7 @@ struct OstreeRepo { GMutex txn_lock; OstreeRepoTxn txn; + gboolean txn_locked; GMutex cache_lock; guint dirmeta_cache_refcount; From df7f33e498627c91c6ac21ca1c3ffb1ddd8fa643 Mon Sep 17 00:00:00 2001 From: Dan Nicholson Date: Fri, 6 Oct 2017 11:04:22 +0000 Subject: [PATCH 08/43] lib/prune: Take exclusive repository lock Add exclusive repository locking to all the pruning entry points. This ensures that objects and deltas will not be removed while another process is writing to the repository. Closes: #1343 Approved by: cgwalters --- src/libostree/ostree-repo-prune.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/libostree/ostree-repo-prune.c b/src/libostree/ostree-repo-prune.c index 88c52abf..9eec4ebe 100644 --- a/src/libostree/ostree-repo-prune.c +++ b/src/libostree/ostree-repo-prune.c @@ -23,6 +23,7 @@ #include "ostree-core-private.h" #include "ostree-repo-private.h" +#include "ostree-autocleanups.h" #include "otutil.h" typedef struct { @@ -160,12 +161,20 @@ _ostree_repo_prune_tmp (OstreeRepo *self, * Prune static deltas, if COMMIT is specified then delete static delta files only * targeting that commit; otherwise any static delta of non existing commits are * deleted. + * + * This function takes an exclusive lock on the @self repository. */ gboolean ostree_repo_prune_static_deltas (OstreeRepo *self, const char *commit, GCancellable *cancellable, GError **error) { + g_autoptr(OstreeRepoAutoLock) lock = + ostree_repo_auto_lock_push (self, OSTREE_REPO_LOCK_EXCLUSIVE, cancellable, + error); + if (!lock) + return FALSE; + g_autoptr(GPtrArray) deltas = NULL; if (!ostree_repo_list_static_delta_names (self, &deltas, cancellable, error)) @@ -286,6 +295,8 @@ repo_prune_internal (OstreeRepo *self, * Use the %OSTREE_REPO_PRUNE_FLAGS_NO_PRUNE to just determine * statistics on objects that would be deleted, without actually * deleting them. + * + * This function takes an exclusive lock on the @self repository. */ gboolean ostree_repo_prune (OstreeRepo *self, @@ -297,6 +308,12 @@ ostree_repo_prune (OstreeRepo *self, GCancellable *cancellable, GError **error) { + g_autoptr(OstreeRepoAutoLock) lock = + ostree_repo_auto_lock_push (self, OSTREE_REPO_LOCK_EXCLUSIVE, cancellable, + error); + if (!lock) + return FALSE; + g_autoptr(GHashTable) objects = NULL; gboolean refs_only = flags & OSTREE_REPO_PRUNE_FLAGS_REFS_ONLY; @@ -391,6 +408,8 @@ ostree_repo_prune (OstreeRepo *self, * * The %OSTREE_REPO_PRUNE_FLAGS_NO_PRUNE flag may be specified to just determine * statistics on objects that would be deleted, without actually deleting them. + * + * This function takes an exclusive lock on the @self repository. */ gboolean ostree_repo_prune_from_reachable (OstreeRepo *self, @@ -401,6 +420,12 @@ ostree_repo_prune_from_reachable (OstreeRepo *self, GCancellable *cancellable, GError **error) { + g_autoptr(OstreeRepoAutoLock) lock = + ostree_repo_auto_lock_push (self, OSTREE_REPO_LOCK_EXCLUSIVE, cancellable, + error); + if (!lock) + return FALSE; + g_autoptr(GHashTable) objects = NULL; if (!ostree_repo_list_objects (self, OSTREE_REPO_LIST_OBJECTS_ALL | OSTREE_REPO_LIST_OBJECTS_NO_PARENTS, From 3f4506f088b17fa0db7929fc4818f11f9ad1396b Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 13 Oct 2017 19:50:28 -0500 Subject: [PATCH 09/43] tests: Test concurrent operations Test that concurrent commits and prunes can succeed. Mostly this is a check that the new locking works correctly and the concurrent processes will properly wait until they've acquired the appropriate repository lock. Closes: #1343 Approved by: cgwalters --- Makefile-tests.am | 1 + tests/test-concurrency.py | 107 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+) create mode 100755 tests/test-concurrency.py diff --git a/Makefile-tests.am b/Makefile-tests.am index 6d0e0865..2b335556 100644 --- a/Makefile-tests.am +++ b/Makefile-tests.am @@ -108,6 +108,7 @@ _installed_or_uninstalled_test_scripts = \ tests/test-xattrs.sh \ tests/test-auto-summary.sh \ tests/test-prune.sh \ + tests/test-concurrency.py \ tests/test-refs.sh \ tests/test-demo-buildsystem.sh \ tests/test-switchroot.sh \ diff --git a/tests/test-concurrency.py b/tests/test-concurrency.py new file mode 100755 index 00000000..6aa8f269 --- /dev/null +++ b/tests/test-concurrency.py @@ -0,0 +1,107 @@ +#!/usr/bin/env python +# +# Copyright (C) 2017 Colin Walters +# +# 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. + +from __future__ import print_function +import os +import sys +import shutil +import subprocess +from multiprocessing import cpu_count + +def fatal(msg): + sys.stderr.write(msg) + sys.stderr.write('\n') + sys.exit(1) + +# Create 20 files with content based on @dname + a serial, basically to have +# different files with different checksums. +def mktree(dname, serial=0): + print('Creating tree', dname, file=sys.stderr) + os.mkdir(dname, 0755) + for v in xrange(20): + with open('{}/{}'.format(dname, v), 'w') as f: + f.write('{} {} {}\n'.format(dname, serial, v)) + +subprocess.check_call(['ostree', '--repo=repo', 'init', '--mode=bare']) +# like the bit in libtest, but let's do it unconditionally since it's simpler, +# and we don't need xattr coverage for this +with open('repo/config', 'a') as f: + f.write('disable-xattrs=true\n') + +def commit(v): + tdir='tree{}'.format(v) + cmd = ['ostree', '--repo=repo', 'commit', '--fsync=0', '-b', tdir, '--tree=dir='+tdir] + proc = subprocess.Popen(cmd) + print('PID {}'.format(proc.pid), *cmd, file=sys.stderr) + return proc +def prune(): + cmd = ['ostree', '--repo=repo', 'prune', '--refs-only'] + proc = subprocess.Popen(cmd) + print('PID {}:'.format(proc.pid), *cmd, file=sys.stderr) + return proc + +def wait_check(proc): + pid = proc.pid + proc.wait() + if proc.returncode != 0: + sys.stderr.write("process {} exited with code {}\n".format(proc.pid, proc.returncode)) + return False + else: + sys.stderr.write('PID {} exited OK\n'.format(pid)) + return True + +print("1..2") + +def run(n_committers, n_pruners): + # The number of committers needs to be even since we only create half as + # many trees + n_committers += n_committers % 2 + + committers = set() + pruners = set() + + print('n_committers', n_committers, 'n_pruners', n_pruners, file=sys.stderr) + n_trees = n_committers / 2 + for v in xrange(n_trees): + mktree('tree{}'.format(v)) + + for v in xrange(n_committers): + committers.add(commit(v / 2)) + for v in xrange(n_pruners): + pruners.add(prune()) + + failed = False + for committer in committers: + if not wait_check(committer): + failed = True + for pruner in pruners: + if not wait_check(pruner): + failed = True + if failed: + fatal('A child process exited abnormally') + + for v in xrange(n_trees): + shutil.rmtree('tree{}'.format(v)) + +# No concurrent pruning +run(cpu_count()/2 + 2, 0) +print("ok no concurrent prunes") + +run(cpu_count()/2 + 4, 3) +print("ok concurrent prunes") From 7173ac76bc1930eaa592417ea52dce308842d2a7 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 6 Dec 2017 17:07:48 -0500 Subject: [PATCH 10/43] pull: Add http2=false remote config option This seems to work around https://github.com/ostreedev/ostree/issues/1362 Though I'm not entirely sure why yet. But at least with this it'll be easier for people to work around things locally. Closes: #1368 Approved by: jlebon --- man/ostree.repo-config.xml | 8 ++++++++ src/libostree/ostree-fetcher-curl.c | 11 +++++++---- src/libostree/ostree-fetcher.h | 3 ++- src/libostree/ostree-repo-pull.c | 9 +++++++++ 4 files changed, 26 insertions(+), 5 deletions(-) diff --git a/man/ostree.repo-config.xml b/man/ostree.repo-config.xml index 388b9e62..9378a6b9 100644 --- a/man/ostree.repo-config.xml +++ b/man/ostree.repo-config.xml @@ -222,6 +222,14 @@ Boston, MA 02111-1307, USA. Path to file containing trusted anchors instead of the system CA database. + + http2 + A boolean value, defaults to true. By + default, libostree will use HTTP2; setting this to false + will disable it. May be useful to work around broken servers. + + + unconfigured-state If set, pulls from this remote will fail with the configured text. This is intended for OS vendors which have a subscription process to access content. diff --git a/src/libostree/ostree-fetcher-curl.c b/src/libostree/ostree-fetcher-curl.c index 58835529..a233a191 100644 --- a/src/libostree/ostree-fetcher-curl.c +++ b/src/libostree/ostree-fetcher-curl.c @@ -771,14 +771,17 @@ initiate_next_curl_request (FetcherRequest *req, * there are numerous HTTP/2 fixes since the original version in * libcurl 7.43.0. */ + if (!(self->config_flags & OSTREE_FETCHER_FLAGS_DISABLE_HTTP2)) + { #if CURL_AT_LEAST_VERSION(7, 51, 0) - curl_easy_setopt (req->easy, CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_2_0); + curl_easy_setopt (req->easy, CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_2_0); #endif - /* https://github.com/curl/curl/blob/curl-7_53_0/docs/examples/http2-download.c */ + /* https://github.com/curl/curl/blob/curl-7_53_0/docs/examples/http2-download.c */ #if (CURLPIPE_MULTIPLEX > 0) - /* wait for pipe connection to confirm */ - curl_easy_setopt (req->easy, CURLOPT_PIPEWAIT, 1L); + /* wait for pipe connection to confirm */ + curl_easy_setopt (req->easy, CURLOPT_PIPEWAIT, 1L); #endif + } curl_easy_setopt (req->easy, CURLOPT_WRITEFUNCTION, write_cb); if (g_getenv ("OSTREE_DEBUG_HTTP")) curl_easy_setopt (req->easy, CURLOPT_VERBOSE, 1L); diff --git a/src/libostree/ostree-fetcher.h b/src/libostree/ostree-fetcher.h index 18aaec40..9cd2f008 100644 --- a/src/libostree/ostree-fetcher.h +++ b/src/libostree/ostree-fetcher.h @@ -50,7 +50,8 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(OstreeFetcher, g_object_unref) typedef enum { OSTREE_FETCHER_FLAGS_NONE = 0, OSTREE_FETCHER_FLAGS_TLS_PERMISSIVE = (1 << 0), - OSTREE_FETCHER_FLAGS_TRANSFER_GZIP = (1 << 1) + OSTREE_FETCHER_FLAGS_TRANSFER_GZIP = (1 << 1), + OSTREE_FETCHER_FLAGS_DISABLE_HTTP2 = (1 << 2), } OstreeFetcherConfigFlags; typedef enum { diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index c6fe7625..8ac48506 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -2690,6 +2690,15 @@ _ostree_repo_remote_new_fetcher (OstreeRepo *self, if (gzip) fetcher_flags |= OSTREE_FETCHER_FLAGS_TRANSFER_GZIP; + { gboolean http2 = TRUE; + if (!ostree_repo_get_remote_boolean_option (self, remote_name, + "http2", TRUE, + &http2, error)) + goto out; + if (!http2) + fetcher_flags |= OSTREE_FETCHER_FLAGS_DISABLE_HTTP2; + } + fetcher = _ostree_fetcher_new (self->tmp_dir_fd, remote_name, fetcher_flags); { From e108bef816de0c3cecf0186eb557f20ce0db3580 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 7 Dec 2017 05:47:31 -0500 Subject: [PATCH 11/43] docs/related: Add Balena It's quite related. Closes: #1369 Approved by: jlebon --- docs/manual/related-projects.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/manual/related-projects.md b/docs/manual/related-projects.md index c4f558da..a44f28a4 100644 --- a/docs/manual/related-projects.md +++ b/docs/manual/related-projects.md @@ -347,3 +347,9 @@ and furthermore the tar format is flexible, with multiple ways to represent data making it hard to impossible to reassemble and verify from on-disk state. The [tarsum](https://github.com/docker/docker/blob/master/pkg/tarsum/tarsum_spec.md) effort was intended to address this, but it was not adopted in the end for v2. + +## Docker-related: Balena + +The [Balena](https://github.com/resin-os/balena) project forks Docker and aims +to even use Docker/OCI format for the root filesystem, and adds wire deltas +using librsync. See also [discussion on libostree-list](https://mail.gnome.org/archives/ostree-list/2017-December/msg00002.html). From 9bb59511ae70b325d6d693f64bda976aed19e2e1 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 5 Dec 2017 16:33:31 -0500 Subject: [PATCH 12/43] lib/commit: Refactor file commits to separate subdir from content One major thing we can do to speed up local commits is multithreading. In preparation for that, split up the recursion function so that the subdirectory case is separate from the content (regfile/symlink) case. Then for non-subdirs, we can easily peel off worker threads and gather the final checksums and update the mtree from the main thread. The diff here looks large but it's pretty straightforward; amazingly this change compiled the very first time I tried it! Closes: #1365 Approved by: jlebon --- src/libostree/ostree-repo-commit.c | 437 +++++++++++++++++------------ 1 file changed, 258 insertions(+), 179 deletions(-) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index 79162890..107f2ecd 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -2833,12 +2833,112 @@ typedef enum { WRITE_DIR_CONTENT_FLAGS_CAN_ADOPT = 1, } WriteDirContentFlags; -/* Given either a dir_enum or a dfd_iter, writes the directory entry to the mtree. For - * subdirs, we go back through either write_dfd_iter_to_mtree_internal (dfd_iter case) or - * write_directory_to_mtree_internal (dir_enum case) which will do the actual dirmeta + - * dirent iteration. */ +/* Given either a dir_enum or a dfd_iter, writes the directory entry (which is + * itself a directory) to the mtree. For subdirs, we go back through either + * write_dfd_iter_to_mtree_internal (dfd_iter case) or + * write_directory_to_mtree_internal (dir_enum case) which will do the actual + * dirmeta + dirent iteration. */ static gboolean -write_directory_content_to_mtree_internal (OstreeRepo *self, +write_dir_entry_to_mtree_internal (OstreeRepo *self, + OstreeRepoFile *repo_dir, + GFileEnumerator *dir_enum, + GLnxDirFdIterator *dfd_iter, + WriteDirContentFlags writeflags, + GFileInfo *child_info, + OstreeMutableTree *mtree, + OstreeRepoCommitModifier *modifier, + GPtrArray *path, + GCancellable *cancellable, + GError **error) +{ + g_assert (dir_enum != NULL || dfd_iter != NULL); + g_assert (g_file_info_get_file_type (child_info) == G_FILE_TYPE_DIRECTORY); + + const char *name = g_file_info_get_name (child_info); + + /* We currently only honor the CONSUME flag in the dfd_iter case to avoid even + * more complexity in this function, and it'd mostly only be useful when + * operating on local filesystems anyways. + */ + const gboolean delete_after_commit = dfd_iter && modifier && + (modifier->flags & OSTREE_REPO_COMMIT_MODIFIER_FLAGS_CONSUME); + + /* Build the full path which we need for callbacks */ + g_ptr_array_add (path, (char*)name); + g_autofree char *child_relpath = ptrarray_path_join (path); + + /* Call the filter */ + g_autoptr(GFileInfo) modified_info = NULL; + OstreeRepoCommitFilterResult filter_result = + _ostree_repo_commit_modifier_apply (self, modifier, child_relpath, child_info, &modified_info); + + if (filter_result != OSTREE_REPO_COMMIT_FILTER_ALLOW) + { + g_ptr_array_remove_index (path, path->len - 1); + if (delete_after_commit) + { + g_assert (dfd_iter); + if (!glnx_shutil_rm_rf_at (dfd_iter->fd, name, cancellable, error)) + return FALSE; + } + /* Note: early return */ + return TRUE; + } + + g_autoptr(GFile) child = NULL; + if (dir_enum != NULL) + child = g_file_enumerator_get_child (dir_enum, child_info); + + g_autoptr(OstreeMutableTree) child_mtree = NULL; + if (!ostree_mutable_tree_ensure_dir (mtree, name, &child_mtree, error)) + return FALSE; + + /* Finally, recurse on the dir */ + if (dir_enum != NULL) + { + if (!write_directory_to_mtree_internal (self, child, child_mtree, + modifier, path, + cancellable, error)) + return FALSE; + } + else if (repo_dir) + { + g_assert (dir_enum != NULL); + g_debug ("Adding: %s", gs_file_get_path_cached (child)); + if (!ostree_mutable_tree_replace_file (mtree, name, + ostree_repo_file_get_checksum ((OstreeRepoFile*) child), + error)) + return FALSE; + } + else + { + g_auto(GLnxDirFdIterator) child_dfd_iter = { 0, }; + + if (!glnx_dirfd_iterator_init_at (dfd_iter->fd, name, FALSE, &child_dfd_iter, error)) + return FALSE; + + if (!write_dfd_iter_to_mtree_internal (self, &child_dfd_iter, child_mtree, + modifier, path, + cancellable, error)) + return FALSE; + + if (delete_after_commit) + { + if (!glnx_unlinkat (dfd_iter->fd, name, AT_REMOVEDIR, error)) + return FALSE; + } + } + + g_ptr_array_remove_index (path, path->len - 1); + + return TRUE; +} + +/* Given either a dir_enum or a dfd_iter, writes a non-dir (regfile/symlink) to + * the mtree. + */ +static gboolean +write_content_to_mtree_internal (OstreeRepo *self, OstreeRepoFile *repo_dir, GFileEnumerator *dir_enum, GLnxDirFdIterator *dfd_iter, @@ -2871,7 +2971,7 @@ write_directory_content_to_mtree_internal (OstreeRepo *self, /* See if we have a devino hit; this is used below in a few places. */ const char *loose_checksum = NULL; - if (dfd_iter != NULL && (file_type != G_FILE_TYPE_DIRECTORY)) + if (dfd_iter != NULL) { guint32 dev = g_file_info_get_attribute_uint32 (child_info, "unix::device"); guint64 inode = g_file_info_get_attribute_uint64 (child_info, "unix::inode"); @@ -2933,7 +3033,6 @@ write_directory_content_to_mtree_internal (OstreeRepo *self, switch (file_type) { - case G_FILE_TYPE_DIRECTORY: case G_FILE_TYPE_SYMBOLIC_LINK: case G_FILE_TYPE_REGULAR: break; @@ -2945,182 +3044,139 @@ write_directory_content_to_mtree_internal (OstreeRepo *self, if (dir_enum != NULL) child = g_file_enumerator_get_child (dir_enum, child_info); - if (file_type == G_FILE_TYPE_DIRECTORY) + /* Our filters have passed, etc.; now we prepare to write the content object */ + glnx_autofd int file_input_fd = -1; + + /* Open the file now, since it's better for reading xattrs + * rather than using the /proc/self/fd links. + * + * TODO: Do this lazily, since for e.g. bare-user-only repos + * we don't have xattrs and don't need to open every file + * for things that have devino cache hits. + */ + if (file_type == G_FILE_TYPE_REGULAR && dfd_iter != NULL) { - g_autoptr(OstreeMutableTree) child_mtree = NULL; - if (!ostree_mutable_tree_ensure_dir (mtree, name, &child_mtree, error)) + if (!glnx_openat_rdonly (dfd_iter->fd, name, FALSE, &file_input_fd, error)) return FALSE; - - if (dir_enum != NULL) - { - if (!write_directory_to_mtree_internal (self, child, child_mtree, - modifier, path, - cancellable, error)) - return FALSE; - } - else - { - g_auto(GLnxDirFdIterator) child_dfd_iter = { 0, }; - - if (!glnx_dirfd_iterator_init_at (dfd_iter->fd, name, FALSE, &child_dfd_iter, error)) - return FALSE; - - if (!write_dfd_iter_to_mtree_internal (self, &child_dfd_iter, child_mtree, - modifier, path, - cancellable, error)) - return FALSE; - - if (delete_after_commit) - { - if (!glnx_unlinkat (dfd_iter->fd, name, AT_REMOVEDIR, error)) - return FALSE; - } - } } - else if (repo_dir) + + g_autoptr(GVariant) xattrs = NULL; + gboolean xattrs_were_modified; + if (dir_enum != NULL) { - g_assert (dir_enum != NULL); - g_debug ("Adding: %s", gs_file_get_path_cached (child)); - if (!ostree_mutable_tree_replace_file (mtree, name, - ostree_repo_file_get_checksum ((OstreeRepoFile*) child), - error)) + if (!get_final_xattrs (self, modifier, child_relpath, child_info, child, + -1, name, source_xattrs, &xattrs, &xattrs_were_modified, + cancellable, error)) return FALSE; } else { - glnx_autofd int file_input_fd = -1; - - /* Open the file now, since it's better for reading xattrs - * rather than using the /proc/self/fd links. - * - * TODO: Do this lazily, since for e.g. bare-user-only repos - * we don't have xattrs and don't need to open every file - * for things that have devino cache hits. + /* These contortions are basically so we use glnx_fd_get_all_xattrs() + * for regfiles, and glnx_dfd_name_get_all_xattrs() for symlinks. */ - if (file_type == G_FILE_TYPE_REGULAR && dfd_iter != NULL) - { - if (!glnx_openat_rdonly (dfd_iter->fd, name, FALSE, &file_input_fd, error)) - return FALSE; - } + int xattr_fd_arg = (file_input_fd != -1) ? file_input_fd : dfd_iter->fd; + const char *xattr_path_arg = (file_input_fd != -1) ? NULL : name; + if (!get_final_xattrs (self, modifier, child_relpath, child_info, child, + xattr_fd_arg, xattr_path_arg, source_xattrs, + &xattrs, &xattrs_were_modified, + cancellable, error)) + return FALSE; + } - g_autoptr(GVariant) xattrs = NULL; - gboolean xattrs_were_modified; - if (dir_enum != NULL) - { - if (!get_final_xattrs (self, modifier, child_relpath, child_info, child, - -1, name, source_xattrs, &xattrs, &xattrs_were_modified, - cancellable, error)) - return FALSE; - } - else - { - /* These contortions are basically so we use glnx_fd_get_all_xattrs() - * for regfiles, and glnx_dfd_name_get_all_xattrs() for symlinks. - */ - int xattr_fd_arg = (file_input_fd != -1) ? file_input_fd : dfd_iter->fd; - const char *xattr_path_arg = (file_input_fd != -1) ? NULL : name; - if (!get_final_xattrs (self, modifier, child_relpath, child_info, child, - xattr_fd_arg, xattr_path_arg, source_xattrs, - &xattrs, &xattrs_were_modified, - cancellable, error)) - return FALSE; - } + /* Used below to see whether we can do a fast path commit */ + const gboolean modified_file_meta = child_info_was_modified || xattrs_were_modified; - /* Used below to see whether we can do a fast path commit */ - const gboolean modified_file_meta = child_info_was_modified || xattrs_were_modified; - - /* A big prerequisite list of conditions for whether or not we can - * "adopt", i.e. just checksum and rename() into place + /* A big prerequisite list of conditions for whether or not we can + * "adopt", i.e. just checksum and rename() into place + */ + const gboolean can_adopt_basic = + file_type == G_FILE_TYPE_REGULAR + && dfd_iter != NULL + && delete_after_commit + && ((writeflags & WRITE_DIR_CONTENT_FLAGS_CAN_ADOPT) > 0); + gboolean can_adopt = can_adopt_basic; + /* If basic prerquisites are met, check repo mode specific ones */ + if (can_adopt) + { + /* For bare repos, we could actually chown/reset the xattrs, but let's + * do the basic optimizations here first. */ - const gboolean can_adopt_basic = - file_type == G_FILE_TYPE_REGULAR - && dfd_iter != NULL - && delete_after_commit - && ((writeflags & WRITE_DIR_CONTENT_FLAGS_CAN_ADOPT) > 0); - gboolean can_adopt = can_adopt_basic; - /* If basic prerquisites are met, check repo mode specific ones */ - if (can_adopt) - { - /* For bare repos, we could actually chown/reset the xattrs, but let's - * do the basic optimizations here first. - */ - if (self->mode == OSTREE_REPO_MODE_BARE) - can_adopt = !modified_file_meta; - else if (self->mode == OSTREE_REPO_MODE_BARE_USER_ONLY) - can_adopt = canonical_permissions; - else - /* This covers bare-user and archive. See comments in adopt_and_commit_regfile() - * for notes on adding bare-user later here. - */ - can_adopt = FALSE; - } - gboolean did_adopt = FALSE; - - /* The very fast path - we have a devino cache hit, nothing to write */ - if (loose_checksum && !modified_file_meta) - { - if (!ostree_mutable_tree_replace_file (mtree, name, loose_checksum, - error)) - return FALSE; - } - /* Next fast path - we can "adopt" the file */ - else if (can_adopt) - { - char checksum[OSTREE_SHA256_STRING_LEN+1]; - if (!adopt_and_commit_regfile (self, dfd_iter->fd, name, modified_info, xattrs, - checksum, cancellable, error)) - return FALSE; - if (!ostree_mutable_tree_replace_file (mtree, name, checksum, error)) - return FALSE; - did_adopt = TRUE; - } + if (self->mode == OSTREE_REPO_MODE_BARE) + can_adopt = !modified_file_meta; + else if (self->mode == OSTREE_REPO_MODE_BARE_USER_ONLY) + can_adopt = canonical_permissions; else - { - g_autoptr(GInputStream) file_input = NULL; + /* This covers bare-user and archive. See comments in adopt_and_commit_regfile() + * for notes on adding bare-user later here. + */ + can_adopt = FALSE; + } + gboolean did_adopt = FALSE; - if (file_type == G_FILE_TYPE_REGULAR) + /* The very fast path - we have a devino cache hit, nothing to write */ + if (loose_checksum && !modified_file_meta) + { + if (!ostree_mutable_tree_replace_file (mtree, name, loose_checksum, + error)) + return FALSE; + } + /* Next fast path - we can "adopt" the file */ + else if (can_adopt) + { + char checksum[OSTREE_SHA256_STRING_LEN+1]; + if (!adopt_and_commit_regfile (self, dfd_iter->fd, name, modified_info, xattrs, + checksum, cancellable, error)) + return FALSE; + if (!ostree_mutable_tree_replace_file (mtree, name, checksum, error)) + return FALSE; + did_adopt = TRUE; + } + else + { + g_autoptr(GInputStream) file_input = NULL; + + if (file_type == G_FILE_TYPE_REGULAR) + { + if (dir_enum != NULL) { - if (dir_enum != NULL) - { - g_assert (child != NULL); - file_input = (GInputStream*)g_file_read (child, cancellable, error); - if (!file_input) - return FALSE; - } - else - { - /* We already opened the fd above */ - file_input = g_unix_input_stream_new (file_input_fd, FALSE); - } + g_assert (child != NULL); + file_input = (GInputStream*)g_file_read (child, cancellable, error); + if (!file_input) + return FALSE; + } + else + { + /* We already opened the fd above */ + file_input = g_unix_input_stream_new (file_input_fd, FALSE); } - - g_autoptr(GInputStream) file_object_input = NULL; - guint64 file_obj_length; - if (!ostree_raw_file_to_content_stream (file_input, - modified_info, xattrs, - &file_object_input, &file_obj_length, - cancellable, error)) - return FALSE; - g_autofree guchar *child_file_csum = NULL; - if (!ostree_repo_write_content (self, NULL, file_object_input, file_obj_length, - &child_file_csum, cancellable, error)) - return FALSE; - - char tmp_checksum[OSTREE_SHA256_STRING_LEN+1]; - ostree_checksum_inplace_from_bytes (child_file_csum, tmp_checksum); - if (!ostree_mutable_tree_replace_file (mtree, name, tmp_checksum, - error)) - return FALSE; } - /* Process delete_after_commit. In the adoption case though, we already - * took ownership of the file above, usually via a renameat(). - */ - if (delete_after_commit && !did_adopt) - { - if (!glnx_unlinkat (dfd_iter->fd, name, 0, error)) - return FALSE; - } + g_autoptr(GInputStream) file_object_input = NULL; + guint64 file_obj_length; + if (!ostree_raw_file_to_content_stream (file_input, + modified_info, xattrs, + &file_object_input, &file_obj_length, + cancellable, error)) + return FALSE; + g_autofree guchar *child_file_csum = NULL; + if (!ostree_repo_write_content (self, NULL, file_object_input, file_obj_length, + &child_file_csum, cancellable, error)) + return FALSE; + + char tmp_checksum[OSTREE_SHA256_STRING_LEN+1]; + ostree_checksum_inplace_from_bytes (child_file_csum, tmp_checksum); + if (!ostree_mutable_tree_replace_file (mtree, name, tmp_checksum, + error)) + return FALSE; + } + + /* Process delete_after_commit. In the adoption case though, we already + * took ownership of the file above, usually via a renameat(). + */ + if (delete_after_commit && !did_adopt) + { + if (!glnx_unlinkat (dfd_iter->fd, name, 0, error)) + return FALSE; } g_ptr_array_remove_index (path, path->len - 1); @@ -3129,7 +3185,7 @@ write_directory_content_to_mtree_internal (OstreeRepo *self, } /* Handles the dirmeta for the given GFile dir and then calls - * write_directory_content_to_mtree_internal() for each directory entry. */ + * write_{dir_entry,content}_to_mtree_internal() for each directory entry. */ static gboolean write_directory_to_mtree_internal (OstreeRepo *self, GFile *dir, @@ -3214,12 +3270,24 @@ write_directory_to_mtree_internal (OstreeRepo *self, if (child_info == NULL) break; - if (!write_directory_content_to_mtree_internal (self, repo_dir, dir_enum, NULL, - WRITE_DIR_CONTENT_FLAGS_NONE, - child_info, - mtree, modifier, path, - cancellable, error)) - return FALSE; + if (g_file_info_get_file_type (child_info) == G_FILE_TYPE_DIRECTORY) + { + if (!write_dir_entry_to_mtree_internal (self, repo_dir, dir_enum, NULL, + WRITE_DIR_CONTENT_FLAGS_NONE, + child_info, + mtree, modifier, path, + cancellable, error)) + return FALSE; + } + else + { + if (!write_content_to_mtree_internal (self, repo_dir, dir_enum, NULL, + WRITE_DIR_CONTENT_FLAGS_NONE, + child_info, + mtree, modifier, path, + cancellable, error)) + return FALSE; + } } } @@ -3227,7 +3295,7 @@ write_directory_to_mtree_internal (OstreeRepo *self, } /* Handles the dirmeta for the dir described by src_dfd_iter and then calls - * write_directory_content_to_mtree_internal() for each directory entry. */ + * write_{dir_entry,content}_to_mtree_internal() for each directory entry. */ static gboolean write_dfd_iter_to_mtree_internal (OstreeRepo *self, GLnxDirFdIterator *src_dfd_iter, @@ -3304,6 +3372,18 @@ write_dfd_iter_to_mtree_internal (OstreeRepo *self, g_autoptr(GFileInfo) child_info = _ostree_stbuf_to_gfileinfo (&stbuf); g_file_info_set_name (child_info, dent->d_name); + if (S_ISDIR (stbuf.st_mode)) + { + if (!write_dir_entry_to_mtree_internal (self, NULL, NULL, src_dfd_iter, + flags, child_info, + mtree, modifier, path, + cancellable, error)) + return FALSE; + + /* We handled the dir, move onto the next */ + continue; + } + if (S_ISREG (stbuf.st_mode)) ; else if (S_ISLNK (stbuf.st_mode)) @@ -3312,18 +3392,17 @@ write_dfd_iter_to_mtree_internal (OstreeRepo *self, child_info, cancellable, error)) return FALSE; } - else if (S_ISDIR (stbuf.st_mode)) - ; else { return glnx_throw (error, "Not a regular file or symlink: %s", dent->d_name); } - if (!write_directory_content_to_mtree_internal (self, NULL, NULL, src_dfd_iter, - flags, child_info, - mtree, modifier, path, - cancellable, error)) + /* Write a content object, we handled directories above */ + if (!write_content_to_mtree_internal (self, NULL, NULL, src_dfd_iter, + flags, child_info, + mtree, modifier, path, + cancellable, error)) return FALSE; } From 9917887a3f6b1a9a6e9741a992c009352c0504ac Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 5 Dec 2017 10:32:07 -0500 Subject: [PATCH 13/43] lib/repo-file: Add casts to appease GLib g_object_ref cast PR This fixes the build with https://bugzilla.gnome.org/show_bug.cgi?id=790697 Closes: #1363 Approved by: jlebon --- src/libostree/ostree-repo-file.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libostree/ostree-repo-file.c b/src/libostree/ostree-repo-file.c index 7d6ac01e..6e19bff6 100644 --- a/src/libostree/ostree-repo-file.c +++ b/src/libostree/ostree-repo-file.c @@ -508,7 +508,7 @@ ostree_repo_file_get_parent (GFile *file) { OstreeRepoFile *self = OSTREE_REPO_FILE (file); - return g_object_ref (self->parent); + return (GFile*)g_object_ref (self->parent); } static GFile * @@ -621,7 +621,7 @@ ostree_repo_file_resolve_relative_path (GFile *file, g_assert (*relative_path == '/'); if (strcmp (relative_path, "/") == 0) - return g_object_ref (ostree_repo_file_get_root (self)); + return (GFile*)g_object_ref (ostree_repo_file_get_root (self)); if (self->parent) return ostree_repo_file_resolve_relative_path ((GFile*)ostree_repo_file_get_root (self), From 102f30f6cc601aeafd92481ff788bdd35e3f052d Mon Sep 17 00:00:00 2001 From: Matthew Leeds Date: Tue, 5 Dec 2017 12:57:46 -0800 Subject: [PATCH 14/43] lib/repo: Properly list remotes of parent repos This commit fixes an infinite loop that happens if you try to list the remotes of a repo that has a parent repo set. It also adds a unit test to ensure the right behavior, which is that both the child remotes and parent remotes are listed. Closes: #1366 Approved by: cgwalters --- src/libostree/ostree-repo.c | 2 +- tests/test-remote-add.sh | 14 +++++++++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 039f437b..9591d01c 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -1791,7 +1791,7 @@ _ostree_repo_remote_list (OstreeRepo *self, g_mutex_unlock (&self->remotes_lock); if (self->parent_repo) - _ostree_repo_remote_list (self, out); + _ostree_repo_remote_list (self->parent_repo, out); } /** diff --git a/tests/test-remote-add.sh b/tests/test-remote-add.sh index badf1495..01864b6a 100755 --- a/tests/test-remote-add.sh +++ b/tests/test-remote-add.sh @@ -21,7 +21,7 @@ set -euo pipefail . $(dirname $0)/libtest.sh -echo '1..13' +echo '1..14' setup_test_repository "bare" $OSTREE remote add origin http://example.com/ostree/gnome @@ -63,6 +63,18 @@ assert_file_has_content list.txt "http://another.com/repo" assert_file_has_content list.txt "http://another-noexist.example.com/anotherrepo" echo "ok remote list with urls" +cd ${test_tmpdir} +rm -rf parent-repo +ostree_repo_init parent-repo +$OSTREE config set core.parent ${test_tmpdir}/parent-repo +${CMD_PREFIX} ostree --repo=parent-repo remote add --no-gpg-verify parent-remote http://parent-remote.example.com/parent-remote +$OSTREE remote list > list.txt +assert_file_has_content list.txt "origin" +assert_file_has_content list.txt "another" +assert_file_has_content list.txt "another-noexist" +assert_file_has_content list.txt "parent-remote" +echo "ok remote list with parent repo remotes" + $OSTREE remote delete another echo "ok remote delete" From 73d910e82eb9076c3551eb9323d55d0a9c768e22 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 5 Dec 2017 14:27:15 -0500 Subject: [PATCH 15/43] Add public API for fsck, use it before loading metadata A while ago I did `truncate -s 0 /path/to/repo/00/123.commit`, and expected a checksum error, but I actually got a validation error due to us loading the commit into a variant and trying to parse out the parent checksum, etc. I first started by changing the `load_and_fsck_one_object()` function to checksum before loading, but the problem is that we do a traverse of all objects first. Fixing this is going to require an `OSTREE_REPO_COMMIT_TRAVER_FLAG_FSCK` or something. In the meantime at least though, let's add a public API to fsck a single object which *does* checksum cleanly before parsing the object, and change the `fsck` command to use it. We then change the fsck binary to do this while iterating over the refs and finding the commit object. This way we'll at least get a checksum first for commit objects, even if not dirtree/dirmeta. Closes: #1364 Approved by: jlebon --- apidoc/ostree-sections.txt | 1 + src/libostree/libostree-devel.sym | 1 + src/libostree/ostree-repo.c | 109 ++++++++++++++++++++++++++ src/libostree/ostree-repo.h | 9 ++- src/ostree/ot-builtin-fsck.c | 126 ++++++------------------------ tests/pull-test.sh | 2 +- tests/test-corruption.sh | 16 +++- tests/test-pull-corruption.sh | 2 +- 8 files changed, 160 insertions(+), 106 deletions(-) diff --git a/apidoc/ostree-sections.txt b/apidoc/ostree-sections.txt index b37c8914..4e474d8d 100644 --- a/apidoc/ostree-sections.txt +++ b/apidoc/ostree-sections.txt @@ -348,6 +348,7 @@ ostree_repo_import_object_from_with_trust ostree_repo_import_archive_to_mtree ostree_repo_export_tree_to_archive ostree_repo_delete_object +ostree_repo_fsck_object OstreeRepoCommitFilterResult OstreeRepoCommitFilter OstreeRepoCommitModifier diff --git a/src/libostree/libostree-devel.sym b/src/libostree/libostree-devel.sym index df18b9ab..36926ce1 100644 --- a/src/libostree/libostree-devel.sym +++ b/src/libostree/libostree-devel.sym @@ -20,6 +20,7 @@ /* Add new symbols here. Release commits should copy this section into -released.sym. */ LIBOSTREE_2017.15 { + ostree_repo_fsck_object; } LIBOSTREE_2017.14; /* 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 9591d01c..08a6276b 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -3914,6 +3914,115 @@ ostree_repo_delete_object (OstreeRepo *self, return TRUE; } +static gboolean +fsck_metadata_object (OstreeRepo *self, + OstreeObjectType objtype, + const char *sha256, + GCancellable *cancellable, + GError **error) +{ + const char *errmsg = glnx_strjoina ("fsck ", sha256, ".", ostree_object_type_to_string (objtype)); + GLNX_AUTO_PREFIX_ERROR (errmsg, error); + g_autoptr(GVariant) metadata = NULL; + if (!load_metadata_internal (self, objtype, sha256, TRUE, + &metadata, NULL, NULL, NULL, + cancellable, error)) + return FALSE; + + g_auto(OtChecksum) hasher = { 0, }; + ot_checksum_init (&hasher); + ot_checksum_update (&hasher, g_variant_get_data (metadata), g_variant_get_size (metadata)); + + char actual_checksum[OSTREE_SHA256_STRING_LEN+1]; + ot_checksum_get_hexdigest (&hasher, actual_checksum, sizeof (actual_checksum)); + if (!_ostree_compare_object_checksum (objtype, sha256, actual_checksum, error)) + return FALSE; + + switch (objtype) + { + case OSTREE_OBJECT_TYPE_COMMIT: + if (!ostree_validate_structureof_commit (metadata, error)) + return FALSE; + break; + case OSTREE_OBJECT_TYPE_DIR_TREE: + if (!ostree_validate_structureof_dirtree (metadata, error)) + return FALSE; + break; + case OSTREE_OBJECT_TYPE_DIR_META: + if (!ostree_validate_structureof_dirmeta (metadata, error)) + return FALSE; + break; + case OSTREE_OBJECT_TYPE_TOMBSTONE_COMMIT: + case OSTREE_OBJECT_TYPE_COMMIT_META: + /* TODO */ + break; + case OSTREE_OBJECT_TYPE_FILE: + g_assert_not_reached (); + break; + } + + return TRUE; +} + +static gboolean +fsck_content_object (OstreeRepo *self, + const char *sha256, + GCancellable *cancellable, + GError **error) +{ + const char *errmsg = glnx_strjoina ("fsck content object ", sha256); + GLNX_AUTO_PREFIX_ERROR (errmsg, error); + g_autoptr(GInputStream) input = NULL; + g_autoptr(GFileInfo) file_info = NULL; + g_autoptr(GVariant) xattrs = NULL; + + if (!ostree_repo_load_file (self, sha256, &input, &file_info, &xattrs, + cancellable, error)) + return FALSE; + + /* TODO more consistency checks here */ + const guint32 mode = g_file_info_get_attribute_uint32 (file_info, "unix::mode"); + if (!ostree_validate_structureof_file_mode (mode, error)) + return FALSE; + + g_autofree guchar *computed_csum = NULL; + if (!ostree_checksum_file_from_input (file_info, xattrs, input, + OSTREE_OBJECT_TYPE_FILE, &computed_csum, + cancellable, error)) + return FALSE; + + char actual_checksum[OSTREE_SHA256_STRING_LEN+1]; + ostree_checksum_inplace_from_bytes (computed_csum, actual_checksum); + return _ostree_compare_object_checksum (OSTREE_OBJECT_TYPE_FILE, sha256, actual_checksum, error); +} + +/** + * ostree_repo_fsck_object: + * @self: Repo + * @objtype: Object type + * @sha256: Checksum + * @cancellable: Cancellable + * @error: Error + * + * Verify consistency of the object; this performs checks only relevant to the + * immediate object itself, such as checksumming. This API call will not itself + * traverse metadata objects for example. + * + * Since: 2017.15 + */ +gboolean +ostree_repo_fsck_object (OstreeRepo *self, + OstreeObjectType objtype, + const char *sha256, + GCancellable *cancellable, + GError **error) +{ + if (OSTREE_OBJECT_TYPE_IS_META (objtype)) + return fsck_metadata_object (self, objtype, sha256, cancellable, error); + else + return fsck_content_object (self, sha256, cancellable, error); +} + /** * ostree_repo_import_object_from: * @self: Destination repo diff --git a/src/libostree/ostree-repo.h b/src/libostree/ostree-repo.h index 7ce47a56..5ef12bb9 100644 --- a/src/libostree/ostree-repo.h +++ b/src/libostree/ostree-repo.h @@ -650,10 +650,17 @@ gboolean ostree_repo_import_object_from_with_trust (OstreeRepo *s _OSTREE_PUBLIC gboolean ostree_repo_delete_object (OstreeRepo *self, OstreeObjectType objtype, - const char *sha256, + const char *sha256, GCancellable *cancellable, GError **error); +_OSTREE_PUBLIC +gboolean ostree_repo_fsck_object (OstreeRepo *self, + OstreeObjectType objtype, + const char *sha256, + GCancellable *cancellable, + GError **error); + /** * OstreeRepoCommitFilterResult: * @OSTREE_REPO_COMMIT_FILTER_ALLOW: Do commit this object diff --git a/src/ostree/ot-builtin-fsck.c b/src/ostree/ot-builtin-fsck.c index 116fdc6b..eeaa23a8 100644 --- a/src/ostree/ot-builtin-fsck.c +++ b/src/ostree/ot-builtin-fsck.c @@ -44,118 +44,36 @@ static GOptionEntry options[] = { }; static gboolean -load_and_fsck_one_object (OstreeRepo *repo, - const char *checksum, - OstreeObjectType objtype, - gboolean *out_found_corruption, - GCancellable *cancellable, - GError **error) +fsck_one_object (OstreeRepo *repo, + const char *checksum, + OstreeObjectType objtype, + gboolean *out_found_corruption, + GCancellable *cancellable, + GError **error) { - gboolean missing = FALSE; - g_autoptr(GVariant) metadata = NULL; - g_autoptr(GInputStream) input = NULL; - g_autoptr(GFileInfo) file_info = NULL; - g_autoptr(GVariant) xattrs = NULL; g_autoptr(GError) temp_error = NULL; - - if (OSTREE_OBJECT_TYPE_IS_META (objtype)) + if (!ostree_repo_fsck_object (repo, objtype, checksum, cancellable, &temp_error)) { - if (!ostree_repo_load_variant (repo, objtype, - checksum, &metadata, &temp_error)) + if (g_error_matches (temp_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) { - if (g_error_matches (temp_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) - { - g_clear_error (&temp_error); - g_printerr ("Object missing: %s.%s\n", checksum, - ostree_object_type_to_string (objtype)); - missing = TRUE; - } - else - { - g_propagate_error (error, g_steal_pointer (&temp_error)); - return glnx_prefix_error (error, "Loading metadata object %s", checksum); - } + g_clear_error (&temp_error); + g_printerr ("Object missing: %s.%s\n", checksum, + ostree_object_type_to_string (objtype)); + *out_found_corruption = TRUE; } else { - if (objtype == OSTREE_OBJECT_TYPE_COMMIT) - { - if (!ostree_validate_structureof_commit (metadata, error)) - return glnx_prefix_error (error, "While validating commit metadata '%s'", checksum); - } - else if (objtype == OSTREE_OBJECT_TYPE_DIR_TREE) - { - if (!ostree_validate_structureof_dirtree (metadata, error)) - return glnx_prefix_error (error, "While validating directory tree '%s'", checksum); - } - else if (objtype == OSTREE_OBJECT_TYPE_DIR_META) - { - if (!ostree_validate_structureof_dirmeta (metadata, error)) - return glnx_prefix_error (error, "While validating directory metadata '%s'", checksum); - } - - input = g_memory_input_stream_new_from_data (g_variant_get_data (metadata), - g_variant_get_size (metadata), - NULL); - - } - } - else - { - guint32 mode; - g_assert (objtype == OSTREE_OBJECT_TYPE_FILE); - if (!ostree_repo_load_file (repo, checksum, &input, &file_info, - &xattrs, cancellable, &temp_error)) - { - if (g_error_matches (temp_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) - { - g_clear_error (&temp_error); - g_printerr ("Object missing: %s.%s\n", checksum, - ostree_object_type_to_string (objtype)); - missing = TRUE; - } - else - { - g_propagate_error (error, g_steal_pointer (&temp_error)); - return glnx_prefix_error (error, "Loading file object %s", checksum); - } - } - else - { - mode = g_file_info_get_attribute_uint32 (file_info, "unix::mode"); - if (!ostree_validate_structureof_file_mode (mode, error)) - return glnx_prefix_error (error, "While validating file '%s'", checksum); - } - } - - if (missing) - { - *out_found_corruption = TRUE; - } - else - { - g_autofree guchar *computed_csum = NULL; - g_autofree char *tmp_checksum = NULL; - - if (!ostree_checksum_file_from_input (file_info, xattrs, input, - objtype, &computed_csum, - cancellable, error)) - return FALSE; - - tmp_checksum = ostree_checksum_from_bytes (computed_csum); - if (strcmp (checksum, tmp_checksum) != 0) - { - g_autofree char *msg = g_strdup_printf ("corrupted object %s.%s; actual checksum: %s", - checksum, ostree_object_type_to_string (objtype), - tmp_checksum); if (opt_delete) { - g_printerr ("%s\n", msg); + g_printerr ("%s\n", temp_error->message); (void) ostree_repo_delete_object (repo, objtype, checksum, cancellable, NULL); *out_found_corruption = TRUE; } else - return glnx_throw (error, "%s", msg); + { + g_propagate_error (error, g_steal_pointer (&temp_error)); + return FALSE; + } } } @@ -201,8 +119,8 @@ fsck_reachable_objects_from_commits (OstreeRepo *repo, ostree_object_name_deserialize (serialized_key, &checksum, &objtype); - if (!load_and_fsck_one_object (repo, checksum, objtype, out_found_corruption, - cancellable, error)) + if (!fsck_one_object (repo, checksum, objtype, out_found_corruption, + cancellable, error)) return FALSE; if (mod == 0 || (i % mod == 0)) @@ -239,6 +157,12 @@ ostree_builtin_fsck (int argc, char **argv, OstreeCommandInvocation *invocation, { const char *refname = key; const char *checksum = value; + + if (!fsck_one_object (repo, checksum, OSTREE_OBJECT_TYPE_COMMIT, + &found_corruption, + cancellable, error)) + return FALSE; + g_autoptr(GVariant) commit = NULL; if (!ostree_repo_load_variant (repo, OSTREE_OBJECT_TYPE_COMMIT, checksum, &commit, error)) diff --git a/tests/pull-test.sh b/tests/pull-test.sh index c09feb30..e6317fbf 100644 --- a/tests/pull-test.sh +++ b/tests/pull-test.sh @@ -183,7 +183,7 @@ if ! skip_one_without_user_xattrs; then if ${CMD_PREFIX} ostree --repo=cacherepo fsck 2>err.txt; then fatal "corrupt repo fsck?" fi - assert_file_has_content err.txt "corrupted.*${checksum}" + assert_file_has_content err.txt "Corrupted.*${checksum}" rm ostree-srv/corruptrepo -rf ostree_repo_init ostree-srv/corruptrepo --mode=archive ${CMD_PREFIX} ostree --repo=ostree-srv/corruptrepo pull-local cacherepo main diff --git a/tests/test-corruption.sh b/tests/test-corruption.sh index 8e2aba56..52a8189a 100755 --- a/tests/test-corruption.sh +++ b/tests/test-corruption.sh @@ -1,6 +1,6 @@ #!/bin/bash # -# Copyright (C) 2011 Colin Walters +# Copyright (C) 2011,2017 Colin Walters # # This library is free software; you can redistribute it and/or # modify it under the terms of the GNU Lesser General Public @@ -19,7 +19,7 @@ set -euo pipefail -echo "1..3" +echo "1..4" . $(dirname $0)/libtest.sh @@ -35,6 +35,18 @@ $OSTREE fsck -q echo "ok chmod" +cd ${test_tmpdir} +rm repo files -rf +setup_test_repository "bare" +rev=$($OSTREE rev-parse test2) +echo -n > repo/objects/${rev:0:2}/${rev:2}.commit +if $OSTREE fsck -q 2>err.txt; then + fatal "fsck unexpectedly succeeded" +fi +assert_file_has_content_literal err.txt "Corrupted commit object; checksum expected" + +echo "ok metadata checksum" + cd ${test_tmpdir} rm repo files -rf setup_test_repository "bare" diff --git a/tests/test-pull-corruption.sh b/tests/test-pull-corruption.sh index ea29a87c..3c8d4c9a 100755 --- a/tests/test-pull-corruption.sh +++ b/tests/test-pull-corruption.sh @@ -76,7 +76,7 @@ if ! skip_one_without_user_xattrs; then if ${CMD_PREFIX} ostree --repo=ostree-srv/gnomerepo fsck 2>err.txt; then assert_not_reached "fsck with corrupted commit worked?" fi - assert_file_has_content err.txt "corrupted object ${corruptrev}\.commit" + assert_file_has_content_literal err.txt "Corrupted commit object; checksum expected='${corruptrev}' actual='${rev}'" # Do a pull-local; this should succeed since we don't verify checksums # for local repos by default. From d102cd7db00fca88af27cba73fe5b7a4fd8f8484 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 5 Dec 2017 20:55:13 -0500 Subject: [PATCH 16/43] tests: Change test-corruption to use fatal() It's clearer. Closes: #1364 Approved by: jlebon --- tests/test-corruption.sh | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/test-corruption.sh b/tests/test-corruption.sh index 52a8189a..cb5e9c09 100755 --- a/tests/test-corruption.sh +++ b/tests/test-corruption.sh @@ -29,7 +29,9 @@ setup_test_repository "bare" $OSTREE checkout test2 checkout-test2 cd checkout-test2 chmod o+x firstfile -$OSTREE fsck -q && (echo 1>&2 "fsck unexpectedly succeeded"; exit 1) +if $OSTREE fsck -q; then + fatal "fsck unexpectedly succeeded" +fi chmod o-x firstfile $OSTREE fsck -q @@ -54,7 +56,9 @@ rm checkout-test2 -rf $OSTREE checkout test2 checkout-test2 cd checkout-test2 chmod o+x firstfile -$OSTREE fsck -q --delete && (echo 1>&2 "fsck unexpectedly succeeded"; exit 1) +if $OSTREE fsck -q --delete; then + fatal "fsck unexpectedly succeeded" +fi echo "ok chmod" From 6d8aaf629c904d6b0e1c2a06641829590eeebf45 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 8 Dec 2017 14:39:01 -0500 Subject: [PATCH 17/43] lib/commit: Fix memleak in bare-user devino hit path I noticed this while chasing an entirely different issue: https://github.com/projectatomic/rpm-ostree/pull/1139 Closes: #1370 Approved by: jlebon --- src/libostree/ostree-repo-commit.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index 107f2ecd..ca384fb8 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -3004,12 +3004,13 @@ write_content_to_mtree_internal (OstreeRepo *self, * there. */ g_autoptr(GVariant) source_xattrs = NULL; + g_autoptr(GFileInfo) source_child_info = NULL; if (loose_checksum && self->mode == OSTREE_REPO_MODE_BARE_USER) { - child_info = NULL; - if (!ostree_repo_load_file (self, loose_checksum, NULL, &child_info, &source_xattrs, + if (!ostree_repo_load_file (self, loose_checksum, NULL, &source_child_info, &source_xattrs, cancellable, error)) return FALSE; + child_info = source_child_info; } /* Call the filter */ From f81e3c6f03f6636f1e4e4d1f13a79983820a9d29 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 8 Dec 2017 13:55:39 -0500 Subject: [PATCH 18/43] lib/commit: Use more direct path for regfile commits In the non-`CONSUME` path for regfiles (which happens currently for `bare-user`), we go to a lot of contortions to make an "object stream", only to immediately parse it again. Fixing this will also enable the `G_IS_FILE_DESCRIPTOR_BASED()` fast path in commit, since the input stream will actually reference the file descriptor and not be an `_OstreeChainInputStream`. There's a slight concern here in that we're no longer checksumming *literally* the object stream passed in for the stream case, but I mention in the comment, the data should be the same, and if it's not somehow we're not adding risk, since the checksum is still covering the data we actually care about. Prep for further changes to break up the `write_content_object()` path into separate paths for archive, as well as regfile vs symlink in non-archive. Closes: #1371 Approved by: jlebon --- src/libostree/ostree-repo-commit.c | 63 +++++++++++++++++----------- src/libotutil/ot-checksum-instream.c | 12 ++++++ src/libotutil/ot-checksum-instream.h | 2 + 3 files changed, 52 insertions(+), 25 deletions(-) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index ca384fb8..50ffeac5 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -597,7 +597,8 @@ static gboolean write_content_object (OstreeRepo *self, const char *expected_checksum, GInputStream *input, - guint64 file_object_length, + GFileInfo *file_info, + GVariant *xattrs, guchar **out_csum, GCancellable *cancellable, GError **error) @@ -610,18 +611,30 @@ write_content_object (OstreeRepo *self, OstreeRepoMode repo_mode = ostree_repo_get_mode (self); + GInputStream *file_input; /* Unowned alias */ + g_autoptr(GInputStream) file_input_owned = NULL; /* We need a temporary for bare-user symlinks */ glnx_unref_object OtChecksumInstream *checksum_input = NULL; if (out_csum) - checksum_input = ot_checksum_instream_new (input, G_CHECKSUM_SHA256); - - g_autoptr(GInputStream) file_input = NULL; - g_autoptr(GVariant) xattrs = NULL; - g_autoptr(GFileInfo) file_info = NULL; - if (!ostree_content_stream_parse (FALSE, checksum_input ? (GInputStream*)checksum_input : input, - file_object_length, FALSE, - &file_input, &file_info, &xattrs, - cancellable, error)) - return FALSE; + { + /* Previously we checksummed the input verbatim; now + * ostree_repo_write_content() parses without checksumming, then we + * re-synthesize a header here. The data should be identical; if somehow + * it's not that's not a serious problem because we're still computing a + * checksum over the data we actually use. + */ + g_autoptr(GBytes) header = _ostree_file_header_new (file_info, xattrs); + size_t len; + const guint8 *buf = g_bytes_get_data (header, &len); + /* Give a null input if there's no content */ + g_autoptr(GInputStream) null_input = NULL; + if (!input) + null_input = input = g_memory_input_stream_new_from_data ("", 0, NULL); + checksum_input = ot_checksum_instream_new_with_start (input, G_CHECKSUM_SHA256, + buf, len); + file_input = (GInputStream*)checksum_input; + } + else + file_input = input; gboolean phys_object_is_symlink = FALSE; const GFileType object_file_type = g_file_info_get_file_type (file_info); @@ -645,10 +658,8 @@ write_content_object (OstreeRepo *self, const char *target_str = g_file_info_get_symlink_target (file_info); g_autoptr(GBytes) target = g_bytes_new (target_str, strlen (target_str) + 1); - if (file_input != NULL) - g_object_unref (file_input); /* Include the terminating zero so we can e.g. mmap this file */ - file_input = g_memory_input_stream_new_from_bytes (target); + file_input = file_input_owned = g_memory_input_stream_new_from_bytes (target); size = g_bytes_get_size (target); } else if (!phys_object_is_symlink) @@ -851,7 +862,7 @@ write_content_object (OstreeRepo *self, /* Update statistics */ g_mutex_lock (&self->txn_lock); self->txn.stats.content_objects_written++; - self->txn.stats.content_bytes_written += file_object_length; + self->txn.stats.content_bytes_written += g_file_info_get_size (file_info); self->txn.stats.content_objects_total++; g_mutex_unlock (&self->txn_lock); @@ -2246,8 +2257,17 @@ ostree_repo_write_content (OstreeRepo *self, } } + /* Parse the stream */ + g_autoptr(GInputStream) file_input = NULL; + g_autoptr(GVariant) xattrs = NULL; + g_autoptr(GFileInfo) file_info = NULL; + if (!ostree_content_stream_parse (FALSE, object_input, length, FALSE, + &file_input, &file_info, &xattrs, + cancellable, error)) + return FALSE; + return write_content_object (self, expected_checksum, - object_input, length, out_csum, + file_input, file_info, xattrs, out_csum, cancellable, error); } @@ -3152,16 +3172,9 @@ write_content_to_mtree_internal (OstreeRepo *self, } } - g_autoptr(GInputStream) file_object_input = NULL; - guint64 file_obj_length; - if (!ostree_raw_file_to_content_stream (file_input, - modified_info, xattrs, - &file_object_input, &file_obj_length, - cancellable, error)) - return FALSE; g_autofree guchar *child_file_csum = NULL; - if (!ostree_repo_write_content (self, NULL, file_object_input, file_obj_length, - &child_file_csum, cancellable, error)) + if (!write_content_object (self, NULL, file_input, modified_info, xattrs, + &child_file_csum, cancellable, error)) return FALSE; char tmp_checksum[OSTREE_SHA256_STRING_LEN+1]; diff --git a/src/libotutil/ot-checksum-instream.c b/src/libotutil/ot-checksum-instream.c index 342b14b4..a536d7bb 100644 --- a/src/libotutil/ot-checksum-instream.c +++ b/src/libotutil/ot-checksum-instream.c @@ -65,6 +65,16 @@ ot_checksum_instream_init (OtChecksumInstream *self) OtChecksumInstream * ot_checksum_instream_new (GInputStream *base, GChecksumType checksum_type) +{ + return ot_checksum_instream_new_with_start (base, checksum_type, NULL, 0); +} + +/* Initialize a checksum stream with starting state from data */ +OtChecksumInstream * +ot_checksum_instream_new_with_start (GInputStream *base, + GChecksumType checksum_type, + const guint8 *buf, + size_t len) { OtChecksumInstream *stream; @@ -77,6 +87,8 @@ ot_checksum_instream_new (GInputStream *base, /* For now */ g_assert (checksum_type == G_CHECKSUM_SHA256); ot_checksum_init (&stream->priv->checksum); + if (buf) + ot_checksum_update (&stream->priv->checksum, buf, len); return (OtChecksumInstream*) (stream); } diff --git a/src/libotutil/ot-checksum-instream.h b/src/libotutil/ot-checksum-instream.h index 410047a9..c13c0898 100644 --- a/src/libotutil/ot-checksum-instream.h +++ b/src/libotutil/ot-checksum-instream.h @@ -51,6 +51,8 @@ struct _OtChecksumInstreamClass GType ot_checksum_instream_get_type (void) G_GNUC_CONST; OtChecksumInstream * ot_checksum_instream_new (GInputStream *stream, GChecksumType checksum); +OtChecksumInstream * ot_checksum_instream_new_with_start (GInputStream *stream, GChecksumType checksum, + const guint8 *buf, size_t len); char * ot_checksum_instream_get_string (OtChecksumInstream *stream); From ac092895b10be506eada983a15c8d128158b7ad7 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 11 Dec 2017 17:43:57 -0500 Subject: [PATCH 19/43] bin/commit: Add --add-metadata that accepts g_variant_print() format Mostly adding this for use in test cases; it allows us to add e.g. integers, and we need to deal with byteswapping those. Someone mind also find it useful to add fully structured metadata, although most of those users should be using a real language and not shell script. Closes: #1372 Approved by: jlebon --- src/ostree/ot-builtin-commit.c | 48 +++++++++++++++++++++++++--------- tests/basic-test.sh | 6 ++++- 2 files changed, 40 insertions(+), 14 deletions(-) diff --git a/src/ostree/ot-builtin-commit.c b/src/ostree/ot-builtin-commit.c index c24e06c7..7c81712a 100644 --- a/src/ostree/ot-builtin-commit.c +++ b/src/ostree/ot-builtin-commit.c @@ -42,6 +42,7 @@ static char *opt_branch; static char *opt_statoverride_file; static char *opt_skiplist_file; static char **opt_metadata_strings; +static char **opt_metadata_variants; static char **opt_detached_metadata_strings; static gboolean opt_link_checkout_speedup; static gboolean opt_skip_if_unchanged; @@ -92,6 +93,7 @@ static GOptionEntry options[] = { { "bind-ref", 0, 0, G_OPTION_ARG_STRING_ARRAY, &opt_bind_refs, "Add a ref to ref binding commit metadata", "BRANCH" }, { "tree", 0, 0, G_OPTION_ARG_STRING_ARRAY, &opt_trees, "Overlay the given argument as a tree", "dir=PATH or tar=TARFILE or ref=COMMIT" }, { "add-metadata-string", 0, 0, G_OPTION_ARG_STRING_ARRAY, &opt_metadata_strings, "Add a key/value pair to metadata", "KEY=VALUE" }, + { "add-metadata", 0, 0, G_OPTION_ARG_STRING_ARRAY, &opt_metadata_variants, "Add a key/value pair to metadata, where the KEY is a string, an VALUE is g_variant_parse() formatted", "KEY=VALUE" }, { "add-detached-metadata-string", 0, 0, G_OPTION_ARG_STRING_ARRAY, &opt_detached_metadata_strings, "Add a key/value pair to detached metadata", "KEY=VALUE" }, { "owner-uid", 0, 0, G_OPTION_ARG_INT, &opt_owner_uid, "Set file ownership user id", "UID" }, { "owner-gid", 0, 0, G_OPTION_ARG_INT, &opt_owner_gid, "Set file ownership group id", "GID" }, @@ -321,13 +323,11 @@ commit_editor (OstreeRepo *repo, } static gboolean -parse_keyvalue_strings (char **strings, - GVariant **out_metadata, +parse_keyvalue_strings (GVariantBuilder *builder, + char **strings, + gboolean is_gvariant_print, GError **error) { - g_autoptr(GVariantBuilder) builder = - g_variant_builder_new (G_VARIANT_TYPE ("a{sv}")); - for (char ** iter = strings; *iter; iter++) { const char *s = *iter; @@ -335,11 +335,19 @@ parse_keyvalue_strings (char **strings, if (!eq) return glnx_throw (error, "Missing '=' in KEY=VALUE metadata '%s'", s); g_autofree char *key = g_strndup (s, eq - s); - g_variant_builder_add (builder, "{sv}", key, - g_variant_new_string (eq + 1)); + if (is_gvariant_print) + { + g_autoptr(GVariant) value = g_variant_parse (NULL, eq + 1, NULL, NULL, error); + if (!value) + return glnx_prefix_error (error, "Parsing %s", s); + + g_variant_builder_add (builder, "{sv}", key, value); + } + else + g_variant_builder_add (builder, "{sv}", key, + g_variant_new_string (eq + 1)); } - *out_metadata = g_variant_ref_sink (g_variant_builder_end (builder)); return TRUE; } @@ -458,17 +466,31 @@ ostree_builtin_commit (int argc, char **argv, OstreeCommandInvocation *invocatio goto out; } - if (opt_metadata_strings) + if (opt_metadata_strings || opt_metadata_variants) { - if (!parse_keyvalue_strings (opt_metadata_strings, - &metadata, error)) + g_autoptr(GVariantBuilder) builder = + g_variant_builder_new (G_VARIANT_TYPE ("a{sv}")); + + if (opt_metadata_strings && + !parse_keyvalue_strings (builder, opt_metadata_strings, FALSE, error)) + goto out; + + if (opt_metadata_variants && + !parse_keyvalue_strings (builder, opt_metadata_variants, TRUE, error)) goto out; + + metadata = g_variant_ref_sink (g_variant_builder_end (builder)); } + if (opt_detached_metadata_strings) { - if (!parse_keyvalue_strings (opt_detached_metadata_strings, - &detached_metadata, error)) + g_autoptr(GVariantBuilder) builder = + g_variant_builder_new (G_VARIANT_TYPE ("a{sv}")); + + if (!parse_keyvalue_strings (builder, opt_detached_metadata_strings, FALSE, error)) goto out; + + detached_metadata = g_variant_ref_sink (g_variant_builder_end (builder)); } if (!(opt_branch || opt_orphan)) diff --git a/tests/basic-test.sh b/tests/basic-test.sh index d7c5425c..e1af66cc 100644 --- a/tests/basic-test.sh +++ b/tests/basic-test.sh @@ -751,12 +751,16 @@ $OSTREE commit ${COMMIT_ARGS} -s sometest -b test2 checkout-test2 echo "ok commit with directory filename" cd $test_tmpdir/checkout-test2 -$OSTREE commit ${COMMIT_ARGS} -b test2 -s "Metadata string" --add-metadata-string=FOO=BAR --add-metadata-string=KITTENS=CUTE --add-detached-metadata-string=SIGNATURE=HANCOCK --tree=ref=test2 +$OSTREE commit ${COMMIT_ARGS} -b test2 -s "Metadata string" --add-metadata-string=FOO=BAR \ + --add-metadata-string=KITTENS=CUTE --add-detached-metadata-string=SIGNATURE=HANCOCK \ + --add-metadata=SOMENUM='uint64 42' --tree=ref=test2 cd ${test_tmpdir} $OSTREE show --print-metadata-key=FOO test2 > test2-meta assert_file_has_content test2-meta "BAR" $OSTREE show --print-metadata-key=KITTENS test2 > test2-meta assert_file_has_content test2-meta "CUTE" +$OSTREE show --print-metadata-key=SOMENUM test2 > test2-meta +assert_file_has_content test2-meta "uint64 3026418949592973312" $OSTREE show --print-detached-metadata-key=SIGNATURE test2 > test2-meta assert_file_has_content test2-meta "HANCOCK" echo "ok metadata commit with strings" From 7b8a6d0c6525ea984edf577f91bf1c42ca2796fb Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sun, 10 Dec 2017 15:01:44 -0500 Subject: [PATCH 20/43] bin/show: Add --no-byteswap rpm-ostree writes host-endian data when importing packages, so let's add support for not byteswapping. Closes: #1372 Approved by: jlebon --- src/ostree/ot-builtin-show.c | 12 +++++++++++- src/ostree/ot-dump.c | 7 ++++++- src/ostree/ot-dump.h | 5 +++-- tests/basic-test.sh | 2 ++ 4 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/ostree/ot-builtin-show.c b/src/ostree/ot-builtin-show.c index 2eec7f35..73ef492c 100644 --- a/src/ostree/ot-builtin-show.c +++ b/src/ostree/ot-builtin-show.c @@ -32,6 +32,7 @@ static char* opt_print_variant_type; static char* opt_print_metadata_key; static char* opt_print_detached_metadata_key; static gboolean opt_raw; +static gboolean opt_no_byteswap; static char *opt_gpg_homedir; static char *opt_gpg_verify_remote; @@ -46,6 +47,7 @@ static GOptionEntry options[] = { { "print-metadata-key", 0, 0, G_OPTION_ARG_STRING, &opt_print_metadata_key, "Print string value of metadata key", "KEY" }, { "print-detached-metadata-key", 0, 0, G_OPTION_ARG_STRING, &opt_print_detached_metadata_key, "Print string value of detached metadata key", "KEY" }, { "raw", 0, 0, G_OPTION_ARG_NONE, &opt_raw, "Show raw variant data" }, + { "no-byteswap", 'B', 0, G_OPTION_ARG_NONE, &opt_no_byteswap, "Do not automatically convert variant data from big endian" }, { "gpg-homedir", 0, 0, G_OPTION_ARG_FILENAME, &opt_gpg_homedir, "GPG Homedir to use when looking for keyrings", "HOMEDIR"}, { "gpg-verify-remote", 0, 0, G_OPTION_ARG_STRING, &opt_gpg_verify_remote, "Use REMOTE name for GPG configuration", "REMOTE"}, { NULL } @@ -132,7 +134,13 @@ do_print_metadata_key (OstreeRepo *repo, return FALSE; } - ot_dump_variant (value); + if (opt_no_byteswap) + { + g_autofree char *formatted = g_variant_print (value, TRUE); + g_print ("%s\n", formatted); + } + else + ot_dump_variant (value); return TRUE; } @@ -150,6 +158,8 @@ print_object (OstreeRepo *repo, return FALSE; if (opt_raw) flags |= OSTREE_DUMP_RAW; + if (opt_no_byteswap) + flags |= OSTREE_DUMP_UNSWAPPED; ot_dump_object (objtype, checksum, variant, flags); if (objtype == OSTREE_OBJECT_TYPE_COMMIT) diff --git a/src/ostree/ot-dump.c b/src/ostree/ot-dump.c index 7f7f8b6b..071530f8 100644 --- a/src/ostree/ot-dump.c +++ b/src/ostree/ot-dump.c @@ -157,7 +157,12 @@ ot_dump_object (OstreeObjectType objtype, { g_print ("%s %s\n", ostree_object_type_to_string (objtype), checksum); - if (flags & OSTREE_DUMP_RAW) + if (flags & OSTREE_DUMP_UNSWAPPED) + { + g_autofree char *formatted = g_variant_print (variant, TRUE); + g_print ("%s\n", formatted); + } + else if (flags & OSTREE_DUMP_RAW) { ot_dump_variant (variant); return; diff --git a/src/ostree/ot-dump.h b/src/ostree/ot-dump.h index 010449c3..0839b57c 100644 --- a/src/ostree/ot-dump.h +++ b/src/ostree/ot-dump.h @@ -26,8 +26,9 @@ #include "ostree-core.h" typedef enum { - OSTREE_DUMP_NONE = 0, - OSTREE_DUMP_RAW = 1, + OSTREE_DUMP_NONE = (1 << 0), + OSTREE_DUMP_RAW = (1 << 1), + OSTREE_DUMP_UNSWAPPED = (1 << 2), } OstreeDumpFlags; void ot_dump_variant (GVariant *variant); diff --git a/tests/basic-test.sh b/tests/basic-test.sh index e1af66cc..06092c12 100644 --- a/tests/basic-test.sh +++ b/tests/basic-test.sh @@ -761,6 +761,8 @@ $OSTREE show --print-metadata-key=KITTENS test2 > test2-meta assert_file_has_content test2-meta "CUTE" $OSTREE show --print-metadata-key=SOMENUM test2 > test2-meta assert_file_has_content test2-meta "uint64 3026418949592973312" +$OSTREE show -B --print-metadata-key=SOMENUM test2 > test2-meta +assert_file_has_content test2-meta "uint64 42" $OSTREE show --print-detached-metadata-key=SIGNATURE test2 > test2-meta assert_file_has_content test2-meta "HANCOCK" echo "ok metadata commit with strings" From 8ae4869c9b859ff38cc407bfeb6a06badf4c61ce Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 12 Dec 2017 16:04:11 -0500 Subject: [PATCH 21/43] build-sys: Add --disable-http2 I may punt and use this in Fedora at least for now until we have time to debug the issues. Closes: #1373 Approved by: jlebon --- configure.ac | 10 ++++++++++ src/libostree/ostree-fetcher-curl.c | 2 ++ 2 files changed, 12 insertions(+) diff --git a/configure.ac b/configure.ac index 3827470b..e2c27822 100644 --- a/configure.ac +++ b/configure.ac @@ -135,6 +135,15 @@ AS_IF([test x$with_curl != xno ], [ ], [with_soup_default=check]) AM_CONDITIONAL(USE_CURL, test x$with_curl != xno) if test x$with_curl = xyes; then OSTREE_FEATURES="$OSTREE_FEATURES libcurl"; fi +AC_ARG_ENABLE(http2, +AS_HELP_STRING([--disable-http2], + [Disable use of http2 (default: no)]),, + [enable_http2=yes]) +AS_IF([test x$enable_http2 != xno ], [ + AC_DEFINE([BUILDOPT_HTTP2], 1, [Define if we enable http2]) +], [ + OSTREE_FEATURES="$OSTREE_FEATURES no-http2" +]) dnl When bumping the libsoup-2.4 dependency, remember to bump dnl SOUP_VERSION_MIN_REQUIRED and SOUP_VERSION_MAX_ALLOWED in @@ -556,6 +565,7 @@ AC_OUTPUT echo " libostree $VERSION ($release_build_type) + features: $OSTREE_FEATURES =============== diff --git a/src/libostree/ostree-fetcher-curl.c b/src/libostree/ostree-fetcher-curl.c index a233a191..272d609a 100644 --- a/src/libostree/ostree-fetcher-curl.c +++ b/src/libostree/ostree-fetcher-curl.c @@ -771,6 +771,7 @@ initiate_next_curl_request (FetcherRequest *req, * there are numerous HTTP/2 fixes since the original version in * libcurl 7.43.0. */ +#ifdef BUILDOPT_HTTP2 if (!(self->config_flags & OSTREE_FETCHER_FLAGS_DISABLE_HTTP2)) { #if CURL_AT_LEAST_VERSION(7, 51, 0) @@ -782,6 +783,7 @@ initiate_next_curl_request (FetcherRequest *req, curl_easy_setopt (req->easy, CURLOPT_PIPEWAIT, 1L); #endif } +#endif curl_easy_setopt (req->easy, CURLOPT_WRITEFUNCTION, write_cb); if (g_getenv ("OSTREE_DEBUG_HTTP")) curl_easy_setopt (req->easy, CURLOPT_VERBOSE, 1L); From 1160d3a110f706dcea986c0c833a769f39147372 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Mon, 20 Nov 2017 11:58:10 +0000 Subject: [PATCH 22/43] ostree/fsck: Factor out common commit checking code This will make upcoming commits a bit cleaner. Signed-off-by: Philip Withnall Closes: #1347 Approved by: cgwalters --- src/ostree/ot-builtin-fsck.c | 57 +++++++++++++++++++++++------------- 1 file changed, 37 insertions(+), 20 deletions(-) diff --git a/src/ostree/ot-builtin-fsck.c b/src/ostree/ot-builtin-fsck.c index eeaa23a8..097af054 100644 --- a/src/ostree/ot-builtin-fsck.c +++ b/src/ostree/ot-builtin-fsck.c @@ -131,6 +131,37 @@ fsck_reachable_objects_from_commits (OstreeRepo *repo, return TRUE; } +/* Check that a given commit object is valid for the ref it was looked up via. + * @collection_id will be %NULL for normal refs, and non-%NULL for collection–refs. */ +static gboolean +fsck_commit_for_ref (OstreeRepo *repo, + const char *checksum, + const char *collection_id, + const char *ref_name, + gboolean *found_corruption, + GCancellable *cancellable, + GError **error) +{ + if (!fsck_one_object (repo, checksum, OSTREE_OBJECT_TYPE_COMMIT, + found_corruption, + cancellable, error)) + return FALSE; + + /* Check the commit exists. */ + g_autoptr(GVariant) commit = NULL; + if (!ostree_repo_load_variant (repo, OSTREE_OBJECT_TYPE_COMMIT, + checksum, &commit, error)) + { + if (collection_id != NULL) + return glnx_prefix_error (error, "Loading commit for ref (%s, %s)", + collection_id, ref_name); + else + return glnx_prefix_error (error, "Loading commit for ref %s", ref_name); + } + + return TRUE; +} + gboolean ostree_builtin_fsck (int argc, char **argv, OstreeCommandInvocation *invocation, GCancellable *cancellable, GError **error) { @@ -154,20 +185,9 @@ ostree_builtin_fsck (int argc, char **argv, OstreeCommandInvocation *invocation, gpointer key, value; g_hash_table_iter_init (&hash_iter, all_refs); while (g_hash_table_iter_next (&hash_iter, &key, &value)) - { - const char *refname = key; - const char *checksum = value; - - if (!fsck_one_object (repo, checksum, OSTREE_OBJECT_TYPE_COMMIT, - &found_corruption, - cancellable, error)) - return FALSE; - - g_autoptr(GVariant) commit = NULL; - if (!ostree_repo_load_variant (repo, OSTREE_OBJECT_TYPE_COMMIT, - checksum, &commit, error)) - return glnx_prefix_error (error, "Loading commit for ref %s", refname); - } + if (!fsck_commit_for_ref (repo, value, NULL, key, + &found_corruption, cancellable, error)) + return FALSE; #ifdef OSTREE_ENABLE_EXPERIMENTAL_API if (!opt_quiet) @@ -183,12 +203,9 @@ ostree_builtin_fsck (int argc, char **argv, OstreeCommandInvocation *invocation, while (g_hash_table_iter_next (&hash_iter, &key, &value)) { const OstreeCollectionRef *ref = key; - const char *checksum = value; - g_autoptr(GVariant) commit = NULL; - if (!ostree_repo_load_variant (repo, OSTREE_OBJECT_TYPE_COMMIT, - checksum, &commit, error)) - return glnx_prefix_error (error, "Loading commit for ref (%s, %s)", - ref->collection_id, ref->ref_name); + if (!fsck_commit_for_ref (repo, value, ref->collection_id, ref->ref_name, + &found_corruption, cancellable, error)) + return FALSE; } #endif /* OSTREE_ENABLE_EXPERIMENTAL_API */ From 1b7d83114e8e98b079a27d3689c3006a06bc5b93 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Mon, 20 Nov 2017 12:37:24 +0000 Subject: [PATCH 23/43] lib/pull: Split verify_bindings() out into a cmdprivate method It will be used by the fsck utility in future. We could expose it publicly in future too, if needed. Signed-off-by: Philip Withnall Closes: #1347 Approved by: cgwalters --- Makefile-libostree.am | 1 + apidoc/Makefile.am | 1 + src/libostree/ostree-cmdprivate.c | 4 +- src/libostree/ostree-cmdprivate.h | 1 + src/libostree/ostree-repo-pull-private.h | 32 +++++++++++ src/libostree/ostree-repo-pull.c | 69 +++++++++++++++--------- 6 files changed, 82 insertions(+), 26 deletions(-) create mode 100644 src/libostree/ostree-repo-pull-private.h diff --git a/Makefile-libostree.am b/Makefile-libostree.am index 39dc0d14..0a4de6de 100644 --- a/Makefile-libostree.am +++ b/Makefile-libostree.am @@ -101,6 +101,7 @@ libostree_1_la_SOURCES = \ src/libostree/ostree-repo-checkout.c \ src/libostree/ostree-repo-commit.c \ src/libostree/ostree-repo-pull.c \ + src/libostree/ostree-repo-pull-private.h \ src/libostree/ostree-repo-libarchive.c \ src/libostree/ostree-repo-prune.c \ src/libostree/ostree-repo-refs.c \ diff --git a/apidoc/Makefile.am b/apidoc/Makefile.am index f3405fb0..d46eac78 100644 --- a/apidoc/Makefile.am +++ b/apidoc/Makefile.am @@ -83,6 +83,7 @@ IGNORE_HFILES= \ ostree-metalink.h \ ostree-repo-file-enumerator.h \ ostree-repo-private.h \ + ostree-repo-pull-private.h \ ostree-repo-static-delta-private.h \ ostree-sysroot-private.h \ ostree-tls-cert-interaction.h \ diff --git a/src/libostree/ostree-cmdprivate.c b/src/libostree/ostree-cmdprivate.c index 3e60b125..5637e98b 100644 --- a/src/libostree/ostree-cmdprivate.c +++ b/src/libostree/ostree-cmdprivate.c @@ -22,6 +22,7 @@ #include "ostree-cmdprivate.h" #include "ostree-repo-private.h" #include "ostree-core-private.h" +#include "ostree-repo-pull-private.h" #include "ostree-repo-static-delta-private.h" #include "ostree-sysroot.h" #include "ostree-bootloader-grub2.h" @@ -48,7 +49,8 @@ ostree_cmd__private__ (void) impl_ostree_generate_grub2_config, _ostree_repo_static_delta_dump, _ostree_repo_static_delta_query_exists, - _ostree_repo_static_delta_delete + _ostree_repo_static_delta_delete, + _ostree_repo_verify_bindings }; return &table; diff --git a/src/libostree/ostree-cmdprivate.h b/src/libostree/ostree-cmdprivate.h index f636ab15..2ba535ec 100644 --- a/src/libostree/ostree-cmdprivate.h +++ b/src/libostree/ostree-cmdprivate.h @@ -31,6 +31,7 @@ typedef struct { gboolean (* ostree_static_delta_dump) (OstreeRepo *repo, const char *delta_id, GCancellable *cancellable, GError **error); gboolean (* ostree_static_delta_query_exists) (OstreeRepo *repo, const char *delta_id, gboolean *out_exists, GCancellable *cancellable, GError **error); gboolean (* ostree_static_delta_delete) (OstreeRepo *repo, const char *delta_id, GCancellable *cancellable, GError **error); + gboolean (* ostree_repo_verify_bindings) (const char *collection_id, const char *ref_name, GVariant *commit, GError **error); } OstreeCmdPrivateVTable; /* Note this not really "public", we just export the symbol, but not the header */ diff --git a/src/libostree/ostree-repo-pull-private.h b/src/libostree/ostree-repo-pull-private.h new file mode 100644 index 00000000..ba30b153 --- /dev/null +++ b/src/libostree/ostree-repo-pull-private.h @@ -0,0 +1,32 @@ +/* + * Copyright © 2017 Endless Mobile, 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. + */ + +#pragma once + +#include "ostree-core.h" + +G_BEGIN_DECLS + +gboolean +_ostree_repo_verify_bindings (const char *collection_id, + const char *ref_name, + GVariant *commit, + GError **error); + +G_END_DECLS diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 8ac48506..42d802b2 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -32,6 +32,7 @@ #include "ostree-core-private.h" #include "ostree-repo-private.h" +#include "ostree-repo-pull-private.h" #include "ostree-repo-static-delta-private.h" #include "ostree-metalink.h" #include "ostree-fetcher-util.h" @@ -1475,30 +1476,40 @@ get_remote_repo_collection_id (OtPullData *pull_data) } #endif /* OSTREE_ENABLE_EXPERIMENTAL_API */ -/* Verify the ref and collection bindings. +#endif /* HAVE_LIBCURL_OR_LIBSOUP */ + +/** + * _ostree_repo_verify_bindings: + * @collection_id: (nullable): Locally specified collection ID for the remote + * the @commit was retrieved from, or %NULL if none is configured + * @ref_name: (nullable): Ref name the commit was retrieved using, or %NULL if + * the commit was retrieved by checksum + * @commit: Commit data to check + * @error: Return location for a #GError, or %NULL + * + * Verify the ref and collection bindings. * * The ref binding is verified only if it exists. But if we have the - * collection ID specified in the remote configuration then the ref - * binding must exist, otherwise the verification will fail. Parts of - * the verification can be skipped by passing NULL to the requested_ref - * parameter (in case we requested a checksum directly, without looking it up - * from a ref). + * collection ID specified in the remote configuration (@collection_id is + * non-%NULL) then the ref binding must exist, otherwise the verification will + * fail. Parts of the verification can be skipped by passing %NULL to the + * @ref_name parameter (in case we requested a checksum directly, without + * looking it up from a ref). * * The collection binding is verified only when we have collection ID * specified in the remote configuration. If it is specified, then the * binding must exist and must be equal to the remote repository * collection ID. + * + * Returns: %TRUE if bindings are correct, %FALSE otherwise + * Since: 2017.14 */ -static gboolean -verify_bindings (OtPullData *pull_data, - GVariant *commit, - const OstreeCollectionRef *requested_ref, - GError **error) +gboolean +_ostree_repo_verify_bindings (const char *collection_id, + const char *ref_name, + GVariant *commit, + GError **error) { - g_autofree char *remote_collection_id = NULL; -#ifdef OSTREE_ENABLE_EXPERIMENTAL_API - remote_collection_id = get_remote_repo_collection_id (pull_data); -#endif /* OSTREE_ENABLE_EXPERIMENTAL_API */ g_autoptr(GVariant) metadata = g_variant_get_child_value (commit, 0); g_autofree const char **refs = NULL; if (!g_variant_lookup (metadata, @@ -1510,7 +1521,7 @@ verify_bindings (OtPullData *pull_data, * we certainly will not verify the collection binding in the * commit. */ - if (remote_collection_id == NULL) + if (collection_id == NULL) return TRUE; return glnx_throw (error, @@ -1518,9 +1529,9 @@ verify_bindings (OtPullData *pull_data, "binding information, found none"); } - if (requested_ref != NULL) + if (ref_name != NULL) { - if (!g_strv_contains ((const char *const *) refs, requested_ref->ref_name)) + if (!g_strv_contains ((const char *const *) refs, ref_name)) { g_autoptr(GString) refs_dump = g_string_new (NULL); const char *refs_str; @@ -1545,33 +1556,35 @@ verify_bindings (OtPullData *pull_data, return glnx_throw (error, "commit has no requested ref ‘%s’ " "in ref binding metadata (%s)", - requested_ref->ref_name, refs_str); + ref_name, refs_str); } } - if (remote_collection_id != NULL) + if (collection_id != NULL) { #ifdef OSTREE_ENABLE_EXPERIMENTAL_API - const char *collection_id; + const char *collection_id_binding; if (!g_variant_lookup (metadata, OSTREE_COMMIT_META_KEY_COLLECTION_BINDING, "&s", - &collection_id)) + &collection_id_binding)) return glnx_throw (error, "expected commit metadata to have collection ID " "binding information, found none"); - if (!g_str_equal (collection_id, remote_collection_id)) + if (!g_str_equal (collection_id_binding, collection_id)) return glnx_throw (error, "commit has collection ID ‘%s’ in collection binding " "metadata, while the remote it came from has " "collection ID ‘%s’", - collection_id, remote_collection_id); + collection_id_binding, collection_id); #endif } return TRUE; } +#ifdef HAVE_LIBCURL_OR_LIBSOUP + /* Look at a commit object, and determine whether there are * more things to fetch. */ @@ -1626,7 +1639,13 @@ scan_commit_object (OtPullData *pull_data, /* If ref is non-NULL then the commit we fetched was requested through the * branch, otherwise we requested a commit checksum without specifying a branch. */ - if (!verify_bindings (pull_data, commit, ref, error)) + g_autofree char *remote_collection_id = NULL; +#ifdef OSTREE_ENABLE_EXPERIMENTAL_API + remote_collection_id = get_remote_repo_collection_id (pull_data); +#endif /* OSTREE_ENABLE_EXPERIMENTAL_API */ + if (!_ostree_repo_verify_bindings (remote_collection_id, + (ref != NULL) ? ref->ref_name : NULL, + commit, error)) return glnx_prefix_error (error, "Commit %s", checksum); if (pull_data->timestamp_check) From 609bd4748ea266735392627b2463e9134510abeb Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Mon, 20 Nov 2017 12:50:16 +0000 Subject: [PATCH 24/43] lib/pull: Fix capitalisation in binding verification error messages Make them suitable for output from fsck. Signed-off-by: Philip Withnall Closes: #1347 Approved by: cgwalters --- src/libostree/ostree-repo-pull.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 42d802b2..4de2e8f3 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -1525,7 +1525,7 @@ _ostree_repo_verify_bindings (const char *collection_id, return TRUE; return glnx_throw (error, - "expected commit metadata to have ref " + "Expected commit metadata to have ref " "binding information, found none"); } @@ -1554,7 +1554,7 @@ _ostree_repo_verify_bindings (const char *collection_id, refs_str = "no refs"; } - return glnx_throw (error, "commit has no requested ref ‘%s’ " + return glnx_throw (error, "Commit has no requested ref ‘%s’ " "in ref binding metadata (%s)", ref_name, refs_str); } @@ -1569,11 +1569,11 @@ _ostree_repo_verify_bindings (const char *collection_id, "&s", &collection_id_binding)) return glnx_throw (error, - "expected commit metadata to have collection ID " + "Expected commit metadata to have collection ID " "binding information, found none"); if (!g_str_equal (collection_id_binding, collection_id)) return glnx_throw (error, - "commit has collection ID ‘%s’ in collection binding " + "Commit has collection ID ‘%s’ in collection binding " "metadata, while the remote it came from has " "collection ID ‘%s’", collection_id_binding, collection_id); From 931cbe6fc9d65205b35de0bbe90c90742c9979b3 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Mon, 20 Nov 2017 12:50:44 +0000 Subject: [PATCH 25/43] lib/static-delta: Drop duplicated declaration from private header Signed-off-by: Philip Withnall Closes: #1347 Approved by: cgwalters --- src/libostree/ostree-repo-static-delta-private.h | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/libostree/ostree-repo-static-delta-private.h b/src/libostree/ostree-repo-static-delta-private.h index 30d336b4..1389492a 100644 --- a/src/libostree/ostree-repo-static-delta-private.h +++ b/src/libostree/ostree-repo-static-delta-private.h @@ -128,11 +128,6 @@ _ostree_static_delta_part_open (GInputStream *part_in, GCancellable *cancellable, GError **error); -gboolean _ostree_static_delta_dump (OstreeRepo *repo, - const char *delta_id, - GCancellable *cancellable, - GError **error); - typedef struct { guint n_ops_executed[OSTREE_STATIC_DELTA_N_OPS]; } OstreeDeltaExecuteStats; From 97bdb3b2714c07ba71ce25aadddee4182bc08118 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Mon, 20 Nov 2017 12:51:28 +0000 Subject: [PATCH 26/43] ostree/fsck: Verify commit bindings for each ref MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since an OSTree client will refuse to pull from a remote which it has locally configured with a collection ID, if the commit on that remote has incorrect or missing bindings, we’d better verify them as part of fsck. Signed-off-by: Philip Withnall Closes: #1347 Approved by: cgwalters --- src/ostree/ot-builtin-fsck.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/ostree/ot-builtin-fsck.c b/src/ostree/ot-builtin-fsck.c index 097af054..6ebb3c83 100644 --- a/src/ostree/ot-builtin-fsck.c +++ b/src/ostree/ot-builtin-fsck.c @@ -159,6 +159,10 @@ fsck_commit_for_ref (OstreeRepo *repo, return glnx_prefix_error (error, "Loading commit for ref %s", ref_name); } + /* Check its bindings. */ + if (!ostree_cmd__private__ ()->ostree_repo_verify_bindings (collection_id, ref_name, commit, error)) + return glnx_prefix_error (error, "Commit %s", checksum); + return TRUE; } From b0e7b26921a7d6a6d5aa0295f91cfb9725d66b4c Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 13 Dec 2017 12:51:59 +0000 Subject: [PATCH 27/43] ostree/fsck: Handle refspecs from ostree_repo_list_refs() It seems ostree_repo_list_refs() can return refspecs as hash table keys, as well as just ref names. Handle that by parsing them before trying to use them as ref names. Signed-off-by: Philip Withnall Closes: #1347 Approved by: cgwalters --- src/ostree/ot-builtin-fsck.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/ostree/ot-builtin-fsck.c b/src/ostree/ot-builtin-fsck.c index 6ebb3c83..6cd9b457 100644 --- a/src/ostree/ot-builtin-fsck.c +++ b/src/ostree/ot-builtin-fsck.c @@ -189,9 +189,16 @@ ostree_builtin_fsck (int argc, char **argv, OstreeCommandInvocation *invocation, gpointer key, value; g_hash_table_iter_init (&hash_iter, all_refs); while (g_hash_table_iter_next (&hash_iter, &key, &value)) - if (!fsck_commit_for_ref (repo, value, NULL, key, - &found_corruption, cancellable, error)) - return FALSE; + { + const char *refspec = key; + const char *checksum = value; + g_autofree char *ref_name = NULL; + if (!ostree_parse_refspec (refspec, NULL, &ref_name, error)) + return FALSE; + if (!fsck_commit_for_ref (repo, checksum, NULL, ref_name, + &found_corruption, cancellable, error)) + return FALSE; + } #ifdef OSTREE_ENABLE_EXPERIMENTAL_API if (!opt_quiet) From 38152d71aa0fc1167313723ec190b65421324472 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 6 Dec 2017 12:53:10 +0000 Subject: [PATCH 28/43] lib/repo: Clarify documentation for ostree_repo_list_refs{,_ext}() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Try and clarify what happens with the prefixes, and that they always return refspecs. I’m still not 100% sure this is right. Signed-off-by: Philip Withnall Closes: #1347 Approved by: cgwalters --- src/libostree/ostree-repo-refs.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/libostree/ostree-repo-refs.c b/src/libostree/ostree-repo-refs.c index ed496253..97206cfb 100644 --- a/src/libostree/ostree-repo-refs.c +++ b/src/libostree/ostree-repo-refs.c @@ -726,13 +726,17 @@ _ostree_repo_list_refs_internal (OstreeRepo *self, * @self: Repo * @refspec_prefix: (allow-none): Only list refs which match this prefix * @out_all_refs: (out) (element-type utf8 utf8) (transfer container): - * Mapping from ref to checksum + * Mapping from refspec to checksum * @cancellable: Cancellable * @error: Error * * If @refspec_prefix is %NULL, list all local and remote refspecs, * with their current values in @out_all_refs. Otherwise, only list * refspecs which have @refspec_prefix as a prefix. + * + * @out_all_refs will be returned as a mapping from refspecs (including the + * remote name) to checksums. If @refspec_prefix is non-%NULL, it will be + * removed as a prefix from the hash table keys. */ gboolean ostree_repo_list_refs (OstreeRepo *self, @@ -752,16 +756,18 @@ ostree_repo_list_refs (OstreeRepo *self, * @self: Repo * @refspec_prefix: (allow-none): Only list refs which match this prefix * @out_all_refs: (out) (element-type utf8 utf8) (transfer container): - * Mapping from ref to checksum + * Mapping from refspec to checksum * @flags: Options controlling listing behavior * @cancellable: Cancellable * @error: Error * * If @refspec_prefix is %NULL, list all local and remote refspecs, * with their current values in @out_all_refs. Otherwise, only list - * refspecs which have @refspec_prefix as a prefix. Differently from - * ostree_repo_list_refs(), the prefix will not be removed from the ref - * name. + * refspecs which have @refspec_prefix as a prefix. + * + * @out_all_refs will be returned as a mapping from refspecs (including the + * remote name) to checksums. Differently from ostree_repo_list_refs(), the + * @refspec_prefix will not be removed from the refspecs in the hash table. */ gboolean ostree_repo_list_refs_ext (OstreeRepo *self, From fb7692bd448ef0a395485a79a050193e50e417ac Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Mon, 20 Nov 2017 13:16:35 +0000 Subject: [PATCH 29/43] ostree/fsck: Add --verify-back-refs option MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This new option verifies that the refs listed in the ref-bindings for each commit all point to that commit (i.e. there aren’t multiple commits listing the same ref in their ref-bindings, and there aren’t any commits with non-empty ref-bindings which aren’t pointed at by a ref). This is useful when generating a new repository from scratch, but not useful when adding new commits to an existing repository (since the old commits will still, correctly, have ref-bindings from when the refs pointed at them). That’s why it has to be enabled explicitly using --verify-back-refs, rather than being on by default. Signed-off-by: Philip Withnall Closes: #1347 Approved by: cgwalters --- bash/ostree | 1 + man/ostree-fsck.xml | 11 ++++++ src/ostree/ot-builtin-fsck.c | 73 ++++++++++++++++++++++++++++++++++++ 3 files changed, 85 insertions(+) diff --git a/bash/ostree b/bash/ostree index c132a43f..2a67d1aa 100644 --- a/bash/ostree +++ b/bash/ostree @@ -963,6 +963,7 @@ _ostree_fsck() { --add-tombstones --delete --quiet -q + --verify-back-refs " local options_with_args=" diff --git a/man/ostree-fsck.xml b/man/ostree-fsck.xml index 2cae92cf..f634933b 100644 --- a/man/ostree-fsck.xml +++ b/man/ostree-fsck.xml @@ -85,6 +85,17 @@ Boston, MA 02111-1307, USA. Add tombstone commit for referenced but missing commits. + + + + + Verify that all the refs listed in a commit’s ref-bindings + point to that commit. This cannot be used in repositories + where the target of refs is changed over time as new commits + are added, but can be used in repositories which are + regenerated from scratch for each commit. + + diff --git a/src/ostree/ot-builtin-fsck.c b/src/ostree/ot-builtin-fsck.c index 6cd9b457..3505f7b3 100644 --- a/src/ostree/ot-builtin-fsck.c +++ b/src/ostree/ot-builtin-fsck.c @@ -30,6 +30,7 @@ static gboolean opt_quiet; static gboolean opt_delete; static gboolean opt_add_tombstones; +static gboolean opt_verify_back_refs; /* ATTENTION: * Please remember to update the bash-completion script (bash/ostree) and @@ -40,6 +41,7 @@ static GOptionEntry options[] = { { "add-tombstones", 0, 0, G_OPTION_ARG_NONE, &opt_add_tombstones, "Add tombstones for missing commits", NULL }, { "quiet", 'q', 0, G_OPTION_ARG_NONE, &opt_quiet, "Only print error messages", NULL }, { "delete", 0, 0, G_OPTION_ARG_NONE, &opt_delete, "Remove corrupted objects", NULL }, + { "verify-back-refs", 0, 0, G_OPTION_ARG_NONE, &opt_verify_back_refs, "Verify back-references", NULL }, { NULL } }; @@ -253,6 +255,77 @@ ostree_builtin_fsck (int argc, char **argv, OstreeCommandInvocation *invocation, if (!ostree_repo_load_commit (repo, checksum, &commit, &commitstate, error)) return FALSE; + /* If requested, check that all the refs listed in the ref-bindings + * for this commit resolve back to this commit. */ + if (opt_verify_back_refs) + { + g_autoptr(GVariant) metadata = g_variant_get_child_value (commit, 0); + + const char *collection_id = NULL; +#ifdef OSTREE_ENABLE_EXPERIMENTAL_API + if (!g_variant_lookup (metadata, + OSTREE_COMMIT_META_KEY_COLLECTION_BINDING, + "&s", + &collection_id)) + collection_id = NULL; +#endif /* OSTREE_ENABLE_EXPERIMENTAL_API */ + + g_autofree const char **refs = NULL; + if (g_variant_lookup (metadata, + OSTREE_COMMIT_META_KEY_REF_BINDING, + "^a&s", + &refs)) + { + for (const char **iter = refs; *iter != NULL; ++iter) + { + g_autofree char *checksum_for_ref = NULL; + +#ifdef OSTREE_ENABLE_EXPERIMENTAL_API + if (collection_id != NULL) + { + const OstreeCollectionRef collection_ref = { (char *) collection_id, (char *) *iter }; + if (!ostree_repo_resolve_collection_ref (repo, &collection_ref, + TRUE, + OSTREE_REPO_RESOLVE_REV_EXT_NONE, + &checksum_for_ref, + cancellable, + error)) + return FALSE; + } + else +#endif /* OSTREE_ENABLE_EXPERIMENTAL_API */ + { + if (!ostree_repo_resolve_rev (repo, *iter, TRUE, + &checksum_for_ref, error)) + return FALSE; + } + + if (checksum_for_ref == NULL) + { + if (collection_id != NULL) + return glnx_throw (error, + "Collection–ref (%s, %s) in bindings for commit %s does not exist", + collection_id, *iter, checksum); + else + return glnx_throw (error, + "Ref ‘%s’ in bindings for commit %s does not exist", + *iter, checksum); + } + else if (g_strcmp0 (checksum_for_ref, checksum) != 0) + { + if (collection_id != NULL) + return glnx_throw (error, + "Collection–ref (%s, %s) in bindings for commit %s does not resolve to that commit", + collection_id, *iter, checksum); + else + return glnx_throw (error, + "Ref ‘%s’ in bindings for commit %s does not resolve to that commit", + *iter, checksum); + } + } + } + } + if (opt_add_tombstones) { GError *local_error = NULL; From 5d1753f59bace948f0763e5de533c22e6ed5304b Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Mon, 20 Nov 2017 14:48:04 +0000 Subject: [PATCH 30/43] ostree/commit: Allow --orphan and --bind-ref to be specified together MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Typically you’d use --branch and --bind-ref together to add additional bindings as well as creating a main --branch for the commit. However, you might also want to occasionally use --orphan --bind-ref to create a commit with bindings for one or more refs, but not actually create any of those refs pointing to the commit (you might create them as a later step). Signed-off-by: Philip Withnall Closes: #1347 Approved by: cgwalters --- src/ostree/ot-builtin-commit.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/ostree/ot-builtin-commit.c b/src/ostree/ot-builtin-commit.c index 7c81712a..e53d7309 100644 --- a/src/ostree/ot-builtin-commit.c +++ b/src/ostree/ot-builtin-commit.c @@ -378,13 +378,11 @@ compare_strings (gconstpointer a, gconstpointer b) static void add_ref_binding (GVariantBuilder *metadata_builder) { - if (opt_orphan) - return; - - g_assert_nonnull (opt_branch); + g_assert (opt_branch != NULL || opt_orphan); g_autoptr(GPtrArray) refs = g_ptr_array_new (); - g_ptr_array_add (refs, opt_branch); + if (opt_branch != NULL) + g_ptr_array_add (refs, opt_branch); for (char **iter = opt_bind_refs; iter != NULL && *iter != NULL; ++iter) g_ptr_array_add (refs, *iter); g_ptr_array_sort (refs, compare_strings); From e48a1bcfe7e879479b3b608bd8607cad0dd14149 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Mon, 20 Nov 2017 14:51:04 +0000 Subject: [PATCH 31/43] tests: Fix LC_ALL for systems which use .utf8 suffixes libtest-core.sh tries to clear the locale to a UTF-8 supporting C locale, either by setting it to C.UTF-8 (preferred) or just C. Some systems, like Fedora 26, use the locale name C.utf8, rather than C.UTF-8. Support that too. Signed-off-by: Philip Withnall Closes: #1347 Approved by: cgwalters --- tests/libtest-core.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/libtest-core.sh b/tests/libtest-core.sh index ce0e4bb1..2144e1ac 100644 --- a/tests/libtest-core.sh +++ b/tests/libtest-core.sh @@ -37,6 +37,8 @@ assert_not_reached () { # (https://sourceware.org/glibc/wiki/Proposals/C.UTF-8) if locale -a | grep C.UTF-8 >/dev/null; then export LC_ALL=C.UTF-8 +elif locale -a | grep C.utf8 >/dev/null; then + export LC_ALL=C.utf8 else export LC_ALL=C fi From bc3d80550b96086c1e537dc4644678eae99230a6 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Mon, 20 Nov 2017 14:47:31 +0000 Subject: [PATCH 32/43] tests: Expand fsck unit tests to cover checks on bindings Signed-off-by: Philip Withnall Closes: #1347 Approved by: cgwalters --- tests/test-fsck-collections.sh | 153 +++++++++++++++++++++++++++++++-- 1 file changed, 144 insertions(+), 9 deletions(-) diff --git a/tests/test-fsck-collections.sh b/tests/test-fsck-collections.sh index 5de17bcf..22e8c1d2 100755 --- a/tests/test-fsck-collections.sh +++ b/tests/test-fsck-collections.sh @@ -21,12 +21,28 @@ set -euo pipefail . $(dirname $0)/libtest.sh -echo '1..2' +echo '1..11' cd ${test_tmpdir} -# Check that fsck detects errors with refs which have collection IDs (i.e. refs in refs/mirrors). -set_up_repo() { +# Create a new repository with one ref with the repository’s collection ID, and +# one ref with a different collection ID (which should be in refs/mirrors). +set_up_repo_with_collection_id() { + rm -rf repo files + + mkdir repo + ostree_repo_init repo --collection-id org.example.Collection + + mkdir files + pushd files + ${CMD_PREFIX} ostree --repo=../repo commit -s "Commit 1" -b ref1 > ../ref1-checksum + ${CMD_PREFIX} ostree --repo=../repo commit -s "Commit 2" --orphan --bind-ref ref2 --add-metadata-string=ostree.collection-binding=org.example.Collection2 > ../ref2-checksum + ${CMD_PREFIX} ostree --repo=../repo refs --collections --create=org.example.Collection2:ref2 $(cat ../ref2-checksum) + popd +} + +# Create a new repository with one ref and no collection IDs. +set_up_repo_without_collection_id() { rm -rf repo files mkdir repo @@ -34,12 +50,12 @@ set_up_repo() { mkdir files pushd files - ${CMD_PREFIX} ostree --repo=../repo commit -s "Commit 1" -b original-ref > ../original-ref-checksum + ${CMD_PREFIX} ostree --repo=../repo commit -s "Commit 3" -b ref3 --bind-ref ref4 > ../ref3-checksum + ${CMD_PREFIX} ostree --repo=../repo refs --create=ref4 $(cat ../ref3-checksum) popd - ${CMD_PREFIX} ostree --repo=repo refs --collections --create=org.example.Collection:some-ref $(cat original-ref-checksum) } -set_up_repo +set_up_repo_with_collection_id # fsck at this point should succeed ${CMD_PREFIX} ostree fsck --repo=repo > fsck @@ -47,9 +63,9 @@ assert_file_has_content fsck "^Validating refs in collections...$" # Drop the commit the ref points to, and drop the original ref so that fsck doesn’t prematurely fail on that. find repo/objects -name '*.commit' -delete -print | wc -l > commitcount -assert_file_has_content commitcount "^1$" +assert_file_has_content commitcount "^2$" -rm repo/refs/heads/original-ref +rm repo/refs/heads/ref1 # fsck should now fail if ${CMD_PREFIX} ostree fsck --repo=repo > fsck; then @@ -62,7 +78,7 @@ echo "ok 1 fsck-collections" # Try fsck in an old repository where refs/mirrors doesn’t exist to begin with. # It should succeed. -set_up_repo +set_up_repo_with_collection_id rm -rf repo/refs/mirrors ${CMD_PREFIX} ostree fsck --repo=repo > fsck @@ -70,3 +86,122 @@ assert_file_has_content fsck "^Validating refs...$" assert_file_has_content fsck "^Validating refs in collections...$" echo "ok 2 fsck-collections in old repository" + +# Test that fsck detects commits which are pointed to by refs, but which don’t +# list those refs in their ref-bindings. +set_up_repo_with_collection_id +${CMD_PREFIX} ostree --repo=repo refs --create=new-ref $(cat ref1-checksum) + +# fsck should now fail +if ${CMD_PREFIX} ostree fsck --repo=repo > fsck 2> fsck-error; then + assert_not_reached "fsck unexpectedly succeeded after adding unbound ref!" +fi +assert_file_has_content fsck-error "Commit has no requested ref ‘new-ref’ in ref binding metadata (‘ref1’)" +assert_file_has_content fsck "^Validating refs...$" + +echo "ok 3 fsck detects missing ref bindings" + +# And the same where the ref is a collection–ref. +set_up_repo_with_collection_id +${CMD_PREFIX} ostree --repo=repo refs --collections --create=org.example.Collection2:new-ref $(cat ref1-checksum) + +# fsck should now fail +if ${CMD_PREFIX} ostree fsck --repo=repo > fsck 2> fsck-error; then + assert_not_reached "fsck unexpectedly succeeded after adding unbound ref!" +fi +assert_file_has_content fsck-error "Commit has no requested ref ‘new-ref’ in ref binding metadata (‘ref1’)" +assert_file_has_content fsck "^Validating refs...$" +assert_file_has_content fsck "^Validating refs in collections...$" + +echo "ok 4 fsck detects missing collection–ref bindings" + +# Check that a ref with a different collection ID but the same ref name is caught. +set_up_repo_with_collection_id +${CMD_PREFIX} ostree --repo=repo refs --collections --create=org.example.Collection2:ref1 $(cat ref1-checksum) + +# fsck should now fail +if ${CMD_PREFIX} ostree fsck --repo=repo > fsck 2> fsck-error; then + assert_not_reached "fsck unexpectedly succeeded after adding unbound ref!" +fi +assert_file_has_content fsck-error "Commit has collection ID ‘org.example.Collection’ in collection binding metadata, while the remote it came from has collection ID ‘org.example.Collection2’" +assert_file_has_content fsck "^Validating refs...$" +assert_file_has_content fsck "^Validating refs in collections...$" + +echo "ok 5 fsck detects missing collection–ref bindings" + +# Check that a commit with ref bindings which aren’t pointed to by refs is OK. +set_up_repo_with_collection_id +${CMD_PREFIX} ostree --repo=repo refs --delete ref1 + +# fsck at this point should succeed +${CMD_PREFIX} ostree fsck --repo=repo > fsck +assert_file_has_content fsck "^Validating refs in collections...$" + +echo "ok 6 fsck ignores unreferenced ref bindings" + +# …but it’s not OK if we pass --verify-back-refs to fsck. +if ${CMD_PREFIX} ostree fsck --repo=repo --verify-back-refs > fsck 2> fsck-error; then + assert_not_reached "fsck unexpectedly succeeded after adding unbound ref!" +fi +assert_file_has_content fsck-error "Collection–ref (org.example.Collection, ref1) in bindings for commit .* does not exist" +assert_file_has_content fsck "^Validating refs...$" +assert_file_has_content fsck "^Validating refs in collections...$" + +echo "ok 7 fsck ignores unreferenced ref bindings" + +# +# Now repeat most of the above tests with a repository without collection IDs. +# + +set_up_repo_without_collection_id + +# fsck at this point should succeed +${CMD_PREFIX} ostree fsck --repo=repo > fsck +assert_file_has_content fsck "^Validating refs in collections...$" + +# Drop the commit the ref points to, and drop the original ref so that fsck doesn’t prematurely fail on that. +find repo/objects -name '*.commit' -delete -print | wc -l > commitcount +assert_file_has_content commitcount "^1$" + +rm repo/refs/heads/ref3 + +# fsck should now fail +if ${CMD_PREFIX} ostree fsck --repo=repo > fsck; then + assert_not_reached "fsck unexpectedly succeeded after deleting commit!" +fi +assert_file_has_content fsck "^Validating refs...$" + +echo "ok 8 fsck-collections" + +# Test that fsck detects commits which are pointed to by refs, but which don’t +# list those refs in their ref-bindings. +set_up_repo_without_collection_id +${CMD_PREFIX} ostree --repo=repo refs --create=new-ref $(cat ref3-checksum) + +# fsck should now fail +if ${CMD_PREFIX} ostree fsck --repo=repo > fsck 2> fsck-error; then + assert_not_reached "fsck unexpectedly succeeded after adding unbound ref!" +fi +assert_file_has_content fsck-error "Commit has no requested ref ‘new-ref’ in ref binding metadata (‘ref3’, ‘ref4’)" +assert_file_has_content fsck "^Validating refs...$" + +echo "ok 9 fsck detects missing ref bindings" + +# Check that a commit with ref bindings which aren’t pointed to by refs is OK. +set_up_repo_without_collection_id +${CMD_PREFIX} ostree --repo=repo refs --delete ref3 + +# fsck at this point should succeed +${CMD_PREFIX} ostree fsck --repo=repo > fsck +assert_file_has_content fsck "^Validating refs...$" + +echo "ok 10 fsck ignores unreferenced ref bindings" + +# …but it’s not OK if we pass --verify-back-refs to fsck. +if ${CMD_PREFIX} ostree fsck --repo=repo --verify-back-refs > fsck 2> fsck-error; then + assert_not_reached "fsck unexpectedly succeeded after adding unbound ref!" +fi +assert_file_has_content fsck-error "Ref ‘ref3’ in bindings for commit .* does not exist" +assert_file_has_content fsck "^Validating refs...$" + +echo "ok 11 fsck ignores unreferenced ref bindings" From a9a9445582471d8f80927f347422eb197d1691e5 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 13 Dec 2017 14:20:30 -0500 Subject: [PATCH 33/43] lib/repo: Make locking timeout configurable I want to make locking fully configurable (and probably off by default for now). This is a prep commit for that. Closes: #1375 Approved by: jlebon --- src/libostree/ostree-repo-private.h | 2 -- src/libostree/ostree-repo.c | 10 +++++++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h index 764540a2..4bbf6a06 100644 --- a/src/libostree/ostree-repo-private.h +++ b/src/libostree/ostree-repo-private.h @@ -37,8 +37,6 @@ G_BEGIN_DECLS #define _OSTREE_MAX_OUTSTANDING_FETCHER_REQUESTS 8 #define _OSTREE_MAX_OUTSTANDING_DELTAPART_REQUESTS 2 -#define _OSTREE_DEFAULT_LOCK_TIMEOUT_SECONDS 30 - /* In most cases, writing to disk should be much faster than * fetching from the network, so we shouldn't actually hit * this. But if using pipelining and e.g. pulling over LAN diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 08a6276b..e5ffba91 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -1209,7 +1209,6 @@ ostree_repo_init (OstreeRepo *self) self->objects_dir_fd = -1; self->uncompressed_objects_dir_fd = -1; self->sysroot_kind = OSTREE_REPO_SYSROOT_KIND_UNKNOWN; - self->lock_timeout_seconds = _OSTREE_DEFAULT_LOCK_TIMEOUT_SECONDS; } /** @@ -2734,6 +2733,15 @@ reload_core_config (OstreeRepo *self, self->tmp_expiry_seconds = g_ascii_strtoull (tmp_expiry_seconds, NULL, 10); } + { g_autofree char *lock_timeout_seconds = NULL; + + if (!ot_keyfile_get_value_with_default (self->config, "core", "lock-timeout-secs", "30", + &lock_timeout_seconds, error)) + return FALSE; + + self->lock_timeout_seconds = g_ascii_strtoull (lock_timeout_seconds, NULL, 10); + } + { g_autofree char *compression_level_str = NULL; /* gzip defaults to 6 */ From ad814d1c8abcf8ed27863586301ba47329e273b6 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 13 Dec 2017 14:27:19 -0500 Subject: [PATCH 34/43] lib/repo: Disable locking by default, add locking=true boolean I want some time to play with this more with different callers and work through test scenarios. Let's disable the locking by default for now, but make it easy to enable. Closes: #1375 Approved by: jlebon --- src/libostree/ostree-repo.c | 37 ++++++++++++++++++++++++++++--------- tests/test-concurrency.py | 1 + 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index e5ffba91..ec509e9d 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -46,6 +46,9 @@ #include #include +#define REPO_LOCK_DISABLED (-2) +#define REPO_LOCK_BLOCKING (-1) + /* ABI Size checks for ostree-repo.h, only for LP64 systems; * https://en.wikipedia.org/wiki/64-bit_computing#64-bit_data_models * @@ -474,8 +477,10 @@ ostree_repo_lock_push (OstreeRepo *self, if (!self->writable) return TRUE; - g_assert (self->lock_timeout_seconds >= -1); - if (self->lock_timeout_seconds == -1) + g_assert (self->lock_timeout_seconds >= REPO_LOCK_DISABLED); + if (self->lock_timeout_seconds == REPO_LOCK_DISABLED) + return TRUE; /* No locking */ + else if (self->lock_timeout_seconds == REPO_LOCK_BLOCKING) { g_debug ("Pushing lock blocking"); return push_repo_lock (self, lock_type, TRUE, error); @@ -562,8 +567,10 @@ ostree_repo_lock_pop (OstreeRepo *self, if (!self->writable) return TRUE; - g_assert (self->lock_timeout_seconds >= -1); - if (self->lock_timeout_seconds == -1) + g_assert (self->lock_timeout_seconds >= REPO_LOCK_DISABLED); + if (self->lock_timeout_seconds == REPO_LOCK_DISABLED) + return TRUE; + else if (self->lock_timeout_seconds == REPO_LOCK_BLOCKING) { g_debug ("Popping lock blocking"); return pop_repo_lock (self, TRUE, error); @@ -2733,13 +2740,25 @@ reload_core_config (OstreeRepo *self, self->tmp_expiry_seconds = g_ascii_strtoull (tmp_expiry_seconds, NULL, 10); } - { g_autofree char *lock_timeout_seconds = NULL; - - if (!ot_keyfile_get_value_with_default (self->config, "core", "lock-timeout-secs", "30", - &lock_timeout_seconds, error)) + /* Disable locking by default for now */ + { gboolean locking; + if (!ot_keyfile_get_boolean_with_default (self->config, "core", "locking", + FALSE, &locking, error)) return FALSE; + if (!locking) + { + self->lock_timeout_seconds = REPO_LOCK_DISABLED; + } + else + { + g_autofree char *lock_timeout_seconds = NULL; - self->lock_timeout_seconds = g_ascii_strtoull (lock_timeout_seconds, NULL, 10); + if (!ot_keyfile_get_value_with_default (self->config, "core", "lock-timeout-secs", "30", + &lock_timeout_seconds, error)) + return FALSE; + + self->lock_timeout_seconds = g_ascii_strtoull (lock_timeout_seconds, NULL, 10); + } } { g_autofree char *compression_level_str = NULL; diff --git a/tests/test-concurrency.py b/tests/test-concurrency.py index 6aa8f269..bdcc1d91 100755 --- a/tests/test-concurrency.py +++ b/tests/test-concurrency.py @@ -43,6 +43,7 @@ subprocess.check_call(['ostree', '--repo=repo', 'init', '--mode=bare']) # and we don't need xattr coverage for this with open('repo/config', 'a') as f: f.write('disable-xattrs=true\n') + f.write('locking=true\n') def commit(v): tdir='tree{}'.format(v) From 7935b881bfa1d57c9cad4ae0522fab90bcf46ab5 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 14 Dec 2017 09:48:26 -0500 Subject: [PATCH 35/43] lib/repo: Add an API to mark a commit as partial MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For the [rpm-ostree jigdo ♲📦](https://github.com/projectatomic/rpm-ostree/issues/1081) work. We're basically doing "pull" via a non-libostree mechanism, and this should be fully supported. As I mentioned earlier we should try to have `ostree-repo-pull.c` only use public APIs; this gets us closer to that. Closes: #1376 Approved by: jlebon --- apidoc/ostree-sections.txt | 1 + src/libostree/libostree-devel.sym | 1 + src/libostree/ostree-repo-commit.c | 42 ++++++++++++++++++++++++++++++ src/libostree/ostree-repo-prune.c | 12 +-------- src/libostree/ostree-repo-pull.c | 27 ++++--------------- src/libostree/ostree-repo.h | 6 +++++ 6 files changed, 56 insertions(+), 33 deletions(-) diff --git a/apidoc/ostree-sections.txt b/apidoc/ostree-sections.txt index 4e474d8d..d60e8f2a 100644 --- a/apidoc/ostree-sections.txt +++ b/apidoc/ostree-sections.txt @@ -320,6 +320,7 @@ ostree_repo_set_alias_ref_immediate ostree_repo_set_cache_dir ostree_repo_sign_delta ostree_repo_has_object +ostree_repo_mark_commit_partial ostree_repo_write_metadata ostree_repo_write_metadata_async ostree_repo_write_metadata_finish diff --git a/src/libostree/libostree-devel.sym b/src/libostree/libostree-devel.sym index 36926ce1..ca3afa87 100644 --- a/src/libostree/libostree-devel.sym +++ b/src/libostree/libostree-devel.sym @@ -21,6 +21,7 @@ LIBOSTREE_2017.15 { ostree_repo_fsck_object; + ostree_repo_mark_commit_partial; } LIBOSTREE_2017.14; /* Stub section for the stable release *after* this development one; don't diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index 50ffeac5..4392f700 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -1589,6 +1589,48 @@ ensure_txn_refs (OstreeRepo *self) g_free); } +/** + * ostree_repo_mark_commit_partial: + * @self: Repo + * @checksum: Commit SHA-256 + * @is_partial: Whether or not this commit is partial + * @error: Error + * + * Commits in "partial" state do not have all their child objects written. This + * occurs in various situations, such as during a pull, but also if a "subpath" + * pull is used, as well as "commit only" pulls. + * + * This function is used by ostree_repo_pull_with_options(); you + * should use this if you are implementing a different type of transport. + * + * Since: 2017.15 + */ +gboolean +ostree_repo_mark_commit_partial (OstreeRepo *self, + const char *checksum, + gboolean is_partial, + GError **error) +{ + g_autofree char *commitpartial_path = _ostree_get_commitpartial_path (checksum); + if (is_partial) + { + glnx_autofd int fd = openat (self->repo_dir_fd, commitpartial_path, + O_EXCL | O_CREAT | O_WRONLY | O_CLOEXEC | O_NOCTTY, 0644); + if (fd == -1) + { + if (errno != EEXIST) + return glnx_throw_errno_prefix (error, "open(%s)", commitpartial_path); + } + } + else + { + if (!ot_ensure_unlinked_at (self->repo_dir_fd, commitpartial_path, 0)) + return FALSE; + } + + return TRUE; +} + /** * ostree_repo_transaction_set_refspec: * @self: An #OstreeRepo diff --git a/src/libostree/ostree-repo-prune.c b/src/libostree/ostree-repo-prune.c index 9eec4ebe..1b65ae1c 100644 --- a/src/libostree/ostree-repo-prune.c +++ b/src/libostree/ostree-repo-prune.c @@ -36,16 +36,6 @@ typedef struct { guint64 freed_bytes; } OtPruneData; -static gboolean -prune_commitpartial_file (OstreeRepo *repo, - const char *checksum, - GCancellable *cancellable, - GError **error) -{ - g_autofree char *path = _ostree_get_commitpartial_path (checksum); - return ot_ensure_unlinked_at (repo->repo_dir_fd, path, error); -} - static gboolean maybe_prune_loose_object (OtPruneData *data, OstreeRepoPruneFlags flags, @@ -68,7 +58,7 @@ maybe_prune_loose_object (OtPruneData *data, if (objtype == OSTREE_OBJECT_TYPE_COMMIT) { - if (!prune_commitpartial_file (data->repo, checksum, cancellable, error)) + if (!ostree_repo_mark_commit_partial (data->repo, checksum, FALSE, error)) return FALSE; } diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 4de2e8f3..2e6e308c 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -558,21 +558,6 @@ fetch_uri_contents_utf8_sync (OstreeFetcher *fetcher, cancellable, error); } -static gboolean -write_commitpartial_for (OtPullData *pull_data, - const char *checksum, - GError **error) -{ - g_autofree char *commitpartial_path = _ostree_get_commitpartial_path (checksum); - glnx_autofd int fd = openat (pull_data->repo->repo_dir_fd, commitpartial_path, O_EXCL | O_CREAT | O_WRONLY | O_CLOEXEC | O_NOCTTY, 0644); - if (fd == -1) - { - if (errno != EEXIST) - return glnx_throw_errno_prefix (error, "open(%s)", commitpartial_path); - } - return TRUE; -} - static void enqueue_one_object_request (OtPullData *pull_data, const char *checksum, @@ -1267,7 +1252,7 @@ meta_fetch_on_complete (GObject *object, pull_data->cancellable, error)) goto out; - if (!write_commitpartial_for (pull_data, checksum, error)) + if (!ostree_repo_mark_commit_partial (pull_data->repo, checksum, TRUE, error)) goto out; } @@ -1821,7 +1806,7 @@ scan_one_metadata_object (OtPullData *pull_data, if (objtype == OSTREE_OBJECT_TYPE_COMMIT) { /* mark as partial to ensure we scan the commit below */ - if (!write_commitpartial_for (pull_data, checksum, error)) + if (!ostree_repo_mark_commit_partial (pull_data->repo, checksum, TRUE, error)) return FALSE; } @@ -1854,7 +1839,7 @@ scan_one_metadata_object (OtPullData *pull_data, if (objtype == OSTREE_OBJECT_TYPE_COMMIT) { /* mark as partial to ensure we scan the commit below */ - if (!write_commitpartial_for (pull_data, checksum, error)) + if (!ostree_repo_mark_commit_partial (pull_data->repo, checksum, TRUE, error)) return FALSE; } if (!_ostree_repo_import_object (pull_data->repo, refd_repo, @@ -4340,15 +4325,13 @@ ostree_repo_pull_with_options (OstreeRepo *self, { GLNX_HASH_TABLE_FOREACH_V (requested_refs_to_fetch, const char*, checksum) { - g_autofree char *commitpartial_path = _ostree_get_commitpartial_path (checksum); - if (!ot_ensure_unlinked_at (pull_data->repo->repo_dir_fd, commitpartial_path, 0)) + if (!ostree_repo_mark_commit_partial (pull_data->repo, checksum, FALSE, error)) goto out; } GLNX_HASH_TABLE_FOREACH_V (commits_to_fetch, const char*, commit) { - g_autofree char *commitpartial_path = _ostree_get_commitpartial_path (commit); - if (!ot_ensure_unlinked_at (pull_data->repo->repo_dir_fd, commitpartial_path, 0)) + if (!ostree_repo_mark_commit_partial (pull_data->repo, commit, FALSE, error)) goto out; } } diff --git a/src/libostree/ostree-repo.h b/src/libostree/ostree-repo.h index 5ef12bb9..e2608d84 100644 --- a/src/libostree/ostree-repo.h +++ b/src/libostree/ostree-repo.h @@ -362,6 +362,12 @@ gboolean ostree_repo_abort_transaction (OstreeRepo *self, GCancellable *cancellable, GError **error); +_OSTREE_PUBLIC +gboolean ostree_repo_mark_commit_partial (OstreeRepo *self, + const char *checksum, + gboolean is_partial, + GError **error); + _OSTREE_PUBLIC void ostree_repo_transaction_set_refspec (OstreeRepo *self, const char *refspec, From d340fe406079c4b3c2b62af2a1c10cd9afd44b09 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 14 Dec 2017 12:36:48 -0500 Subject: [PATCH 36/43] bin/fsck: Make ref binding verification optional Today the rpm-ostree test suite uses `refs --create` to save commits. I think this is a legitimate use case, and other people may be doing something similar. On the other hand, I think we should probably be changing the rpm-ostree test suite to create "unbound" commits. But let's be maximially compatible here since we hit a real-world case where something needed to change. Closes: #1379 Approved by: pwithnall --- bash/ostree | 1 + man/ostree-fsck.xml | 20 +++++++++++++++----- src/ostree/ot-builtin-fsck.c | 14 +++++++++++--- tests/test-fsck-collections.sh | 14 ++++++++------ 4 files changed, 35 insertions(+), 14 deletions(-) diff --git a/bash/ostree b/bash/ostree index 2a67d1aa..0ba135e7 100644 --- a/bash/ostree +++ b/bash/ostree @@ -963,6 +963,7 @@ _ostree_fsck() { --add-tombstones --delete --quiet -q + --verify-bindings --verify-back-refs " diff --git a/man/ostree-fsck.xml b/man/ostree-fsck.xml index f634933b..2432506b 100644 --- a/man/ostree-fsck.xml +++ b/man/ostree-fsck.xml @@ -86,14 +86,24 @@ Boston, MA 02111-1307, USA. + + + + Verify that the commits pointed to by each ref have that + ref in the binding set. You should usually add this + option; it only defaults to off for backwards compatibility. + + + - Verify that all the refs listed in a commit’s ref-bindings - point to that commit. This cannot be used in repositories - where the target of refs is changed over time as new commits - are added, but can be used in repositories which are - regenerated from scratch for each commit. + Verify that all the refs listed in a commit’s ref-bindings + point to that commit. This cannot be used in repositories + where the target of refs is changed over time as new commits + are added, but can be used in repositories which are + regenerated from scratch for each commit. + Implies --verify-bindings as well. diff --git a/src/ostree/ot-builtin-fsck.c b/src/ostree/ot-builtin-fsck.c index 3505f7b3..79fd9e21 100644 --- a/src/ostree/ot-builtin-fsck.c +++ b/src/ostree/ot-builtin-fsck.c @@ -30,6 +30,7 @@ static gboolean opt_quiet; static gboolean opt_delete; static gboolean opt_add_tombstones; +static gboolean opt_verify_bindings; static gboolean opt_verify_back_refs; /* ATTENTION: @@ -41,7 +42,8 @@ static GOptionEntry options[] = { { "add-tombstones", 0, 0, G_OPTION_ARG_NONE, &opt_add_tombstones, "Add tombstones for missing commits", NULL }, { "quiet", 'q', 0, G_OPTION_ARG_NONE, &opt_quiet, "Only print error messages", NULL }, { "delete", 0, 0, G_OPTION_ARG_NONE, &opt_delete, "Remove corrupted objects", NULL }, - { "verify-back-refs", 0, 0, G_OPTION_ARG_NONE, &opt_verify_back_refs, "Verify back-references", NULL }, + { "verify-bindings", 0, 0, G_OPTION_ARG_NONE, &opt_verify_bindings, "Verify ref bindings", NULL }, + { "verify-back-refs", 0, 0, G_OPTION_ARG_NONE, &opt_verify_back_refs, "Verify back-references (implies --verify-bindings)", NULL }, { NULL } }; @@ -162,8 +164,11 @@ fsck_commit_for_ref (OstreeRepo *repo, } /* Check its bindings. */ - if (!ostree_cmd__private__ ()->ostree_repo_verify_bindings (collection_id, ref_name, commit, error)) - return glnx_prefix_error (error, "Commit %s", checksum); + if (opt_verify_bindings) + { + if (!ostree_cmd__private__ ()->ostree_repo_verify_bindings (collection_id, ref_name, commit, error)) + return glnx_prefix_error (error, "Commit %s", checksum); + } return TRUE; } @@ -238,6 +243,9 @@ ostree_builtin_fsck (int argc, char **argv, OstreeCommandInvocation *invocation, if (opt_add_tombstones) tombstones = g_ptr_array_new_with_free_func (g_free); + if (opt_verify_back_refs) + opt_verify_bindings = TRUE; + guint n_partial = 0; g_hash_table_iter_init (&hash_iter, objects); while (g_hash_table_iter_next (&hash_iter, &key, &value)) diff --git a/tests/test-fsck-collections.sh b/tests/test-fsck-collections.sh index 22e8c1d2..a8a323c0 100755 --- a/tests/test-fsck-collections.sh +++ b/tests/test-fsck-collections.sh @@ -92,8 +92,10 @@ echo "ok 2 fsck-collections in old repository" set_up_repo_with_collection_id ${CMD_PREFIX} ostree --repo=repo refs --create=new-ref $(cat ref1-checksum) +# For compatibility we don't check for this by default +${CMD_PREFIX} ostree fsck --repo=repo # fsck should now fail -if ${CMD_PREFIX} ostree fsck --repo=repo > fsck 2> fsck-error; then +if ${CMD_PREFIX} ostree fsck --repo=repo --verify-bindings > fsck 2> fsck-error; then assert_not_reached "fsck unexpectedly succeeded after adding unbound ref!" fi assert_file_has_content fsck-error "Commit has no requested ref ‘new-ref’ in ref binding metadata (‘ref1’)" @@ -106,7 +108,7 @@ set_up_repo_with_collection_id ${CMD_PREFIX} ostree --repo=repo refs --collections --create=org.example.Collection2:new-ref $(cat ref1-checksum) # fsck should now fail -if ${CMD_PREFIX} ostree fsck --repo=repo > fsck 2> fsck-error; then +if ${CMD_PREFIX} ostree fsck --repo=repo --verify-bindings > fsck 2> fsck-error; then assert_not_reached "fsck unexpectedly succeeded after adding unbound ref!" fi assert_file_has_content fsck-error "Commit has no requested ref ‘new-ref’ in ref binding metadata (‘ref1’)" @@ -120,7 +122,7 @@ set_up_repo_with_collection_id ${CMD_PREFIX} ostree --repo=repo refs --collections --create=org.example.Collection2:ref1 $(cat ref1-checksum) # fsck should now fail -if ${CMD_PREFIX} ostree fsck --repo=repo > fsck 2> fsck-error; then +if ${CMD_PREFIX} ostree fsck --repo=repo --verify-bindings > fsck 2> fsck-error; then assert_not_reached "fsck unexpectedly succeeded after adding unbound ref!" fi assert_file_has_content fsck-error "Commit has collection ID ‘org.example.Collection’ in collection binding metadata, while the remote it came from has collection ID ‘org.example.Collection2’" @@ -156,7 +158,7 @@ echo "ok 7 fsck ignores unreferenced ref bindings" set_up_repo_without_collection_id # fsck at this point should succeed -${CMD_PREFIX} ostree fsck --repo=repo > fsck +${CMD_PREFIX} ostree fsck --repo=repo --verify-bindings > fsck assert_file_has_content fsck "^Validating refs in collections...$" # Drop the commit the ref points to, and drop the original ref so that fsck doesn’t prematurely fail on that. @@ -166,7 +168,7 @@ assert_file_has_content commitcount "^1$" rm repo/refs/heads/ref3 # fsck should now fail -if ${CMD_PREFIX} ostree fsck --repo=repo > fsck; then +if ${CMD_PREFIX} ostree fsck --repo=repo --verify-bindings > fsck; then assert_not_reached "fsck unexpectedly succeeded after deleting commit!" fi assert_file_has_content fsck "^Validating refs...$" @@ -179,7 +181,7 @@ set_up_repo_without_collection_id ${CMD_PREFIX} ostree --repo=repo refs --create=new-ref $(cat ref3-checksum) # fsck should now fail -if ${CMD_PREFIX} ostree fsck --repo=repo > fsck 2> fsck-error; then +if ${CMD_PREFIX} ostree fsck --repo=repo --verify-bindings > fsck 2> fsck-error; then assert_not_reached "fsck unexpectedly succeeded after adding unbound ref!" fi assert_file_has_content fsck-error "Commit has no requested ref ‘new-ref’ in ref binding metadata (‘ref3’, ‘ref4’)" From 4a2e08148de1ada60ba854b21489159229ed08e3 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 14 Dec 2017 11:10:35 -0500 Subject: [PATCH 37/43] lib/core: Add a "break hardlink" API This imports the code from rpm-ostree: https://github.com/projectatomic/rpm-ostree/blob/9ff9f6c997d914cb7d97d6b59d8045ba64a1882c/src/libpriv/rpmostree-util.c#L742 I plan to use this for rofiles-fuse to implement copyup: https://github.com/ostreedev/ostree/issues/1377 But it's just obviously generally useful for projects using libostree I think. Closes: #1378 Approved by: jlebon --- apidoc/ostree-sections.txt | 1 + src/libostree/libostree-devel.sym | 1 + src/libostree/ostree-core.c | 80 +++++++++++++++++++++++++++++++ src/libostree/ostree-core.h | 7 +++ tests/test-basic-c.c | 68 ++++++++++++++++++++++++++ 5 files changed, 157 insertions(+) diff --git a/apidoc/ostree-sections.txt b/apidoc/ostree-sections.txt index d60e8f2a..d3cf1c68 100644 --- a/apidoc/ostree-sections.txt +++ b/apidoc/ostree-sections.txt @@ -133,6 +133,7 @@ ostree_content_file_parse_at ostree_raw_file_to_archive_z2_stream ostree_raw_file_to_archive_z2_stream_with_options ostree_raw_file_to_content_stream +ostree_break_hardlink ostree_checksum_file_from_input ostree_checksum_file ostree_checksum_file_at diff --git a/src/libostree/libostree-devel.sym b/src/libostree/libostree-devel.sym index ca3afa87..582e8c92 100644 --- a/src/libostree/libostree-devel.sym +++ b/src/libostree/libostree-devel.sym @@ -22,6 +22,7 @@ LIBOSTREE_2017.15 { ostree_repo_fsck_object; ostree_repo_mark_commit_partial; + ostree_break_hardlink; } LIBOSTREE_2017.14; /* Stub section for the stable release *after* this development one; don't diff --git a/src/libostree/ostree-core.c b/src/libostree/ostree-core.c index c029aa47..7c1a3f99 100644 --- a/src/libostree/ostree-core.c +++ b/src/libostree/ostree-core.c @@ -749,6 +749,86 @@ ostree_content_file_parse (gboolean compressed, cancellable, error); } +/** + * ostree_break_hardlink: + * @dfd: Directory fd + * @path: Path relative to @dfd + * @skip_xattrs: Do not copy extended attributes + * @error: error + * + * In many cases using libostree, a program may need to "break" + * hardlinks by performing a copy. For example, in order to + * logically append to a file. + * + * This function performs full copying, including e.g. extended + * attributes and permissions of both regular files and symbolic links. + * + * If the file is not hardlinked, this function does nothing and + * returns successfully. + * + * This function does not perform synchronization via `fsync()` or + * `fdatasync()`; the idea is this will commonly be done as part + * of an `ostree_repo_commit_transaction()`, which itself takes + * care of synchronization. + * + * Since: 2017.15 + */ +gboolean ostree_break_hardlink (int dfd, + const char *path, + gboolean skip_xattrs, + GCancellable *cancellable, + GError **error) +{ + struct stat stbuf; + + if (!glnx_fstatat (dfd, path, &stbuf, AT_SYMLINK_NOFOLLOW, error)) + return FALSE; + + if (!S_ISLNK (stbuf.st_mode) && !S_ISREG (stbuf.st_mode)) + return glnx_throw (error, "Unsupported type for entry '%s'", path); + + const GLnxFileCopyFlags copyflags = + skip_xattrs ? GLNX_FILE_COPY_NOXATTRS : 0; + + if (stbuf.st_nlink > 1) + { + guint count; + gboolean copy_success = FALSE; + char *path_tmp = glnx_strjoina (path, ".XXXXXX"); + + for (count = 0; count < 100; count++) + { + g_autoptr(GError) tmp_error = NULL; + + glnx_gen_temp_name (path_tmp); + + if (!glnx_file_copy_at (dfd, path, &stbuf, dfd, path_tmp, copyflags, + cancellable, &tmp_error)) + { + if (g_error_matches (tmp_error, G_IO_ERROR, G_IO_ERROR_EXISTS)) + continue; + g_propagate_error (error, g_steal_pointer (&tmp_error)); + return FALSE; + } + + copy_success = TRUE; + break; + } + + if (!copy_success) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_EXISTS, + "Exceeded limit of %u file creation attempts", count); + return FALSE; + } + + if (!glnx_renameat (dfd, path_tmp, dfd, path, error)) + return FALSE; + } + + return TRUE; +} + /** * ostree_checksum_file_from_input: * @file_info: File information diff --git a/src/libostree/ostree-core.h b/src/libostree/ostree-core.h index 3e3631fb..01dded94 100644 --- a/src/libostree/ostree-core.h +++ b/src/libostree/ostree-core.h @@ -438,6 +438,13 @@ gboolean ostree_checksum_file (GFile *f, GCancellable *cancellable, GError **error); +_OSTREE_PUBLIC +gboolean ostree_break_hardlink (int dfd, + const char *path, + gboolean skip_xattrs, + GCancellable *cancellable, + GError **error); + /** * OstreeChecksumFlags: * diff --git a/tests/test-basic-c.c b/tests/test-basic-c.c index f7e85438..4ca379e8 100644 --- a/tests/test-basic-c.c +++ b/tests/test-basic-c.c @@ -22,6 +22,7 @@ #include #include #include +#include #include "libglnx.h" #include "libostreetest.h" @@ -236,6 +237,72 @@ test_object_writes (gconstpointer data) } } +static gboolean +impl_test_break_hardlink (int tmp_dfd, + const char *path, + GError **error) +{ + const char *linkedpath = glnx_strjoina (path, ".link"); + struct stat orig_stbuf; + if (!glnx_fstatat (tmp_dfd, path, &orig_stbuf, AT_SYMLINK_NOFOLLOW, error)) + return FALSE; + + /* Calling ostree_break_hardlink() should be a noop */ + struct stat stbuf; + if (!ostree_break_hardlink (tmp_dfd, path, TRUE, NULL, error)) + return FALSE; + if (!glnx_fstatat (tmp_dfd, path, &stbuf, AT_SYMLINK_NOFOLLOW, error)) + return FALSE; + + g_assert_cmpint (orig_stbuf.st_dev, ==, stbuf.st_dev); + g_assert_cmpint (orig_stbuf.st_ino, ==, stbuf.st_ino); + + if (linkat (tmp_dfd, path, tmp_dfd, linkedpath, 0) < 0) + return glnx_throw_errno_prefix (error, "linkat"); + + if (!ostree_break_hardlink (tmp_dfd, path, TRUE, NULL, error)) + return FALSE; + if (!glnx_fstatat (tmp_dfd, path, &stbuf, AT_SYMLINK_NOFOLLOW, error)) + return FALSE; + /* This file should be different */ + g_assert_cmpint (orig_stbuf.st_dev, ==, stbuf.st_dev); + g_assert_cmpint (orig_stbuf.st_ino, !=, stbuf.st_ino); + /* But this one is still the same */ + if (!glnx_fstatat (tmp_dfd, linkedpath, &stbuf, AT_SYMLINK_NOFOLLOW, error)) + return FALSE; + g_assert_cmpint (orig_stbuf.st_dev, ==, stbuf.st_dev); + g_assert_cmpint (orig_stbuf.st_ino, ==, stbuf.st_ino); + + (void) unlinkat (tmp_dfd, path, 0); + (void) unlinkat (tmp_dfd, linkedpath, 0); + + return TRUE; +} + +static void +test_break_hardlink (void) +{ + int tmp_dfd = AT_FDCWD; + g_autoptr(GError) error = NULL; + + /* Regular file */ + const char hello_hardlinked_content[] = "hello hardlinked content"; + glnx_file_replace_contents_at (tmp_dfd, "test-hardlink", + (guint8*)hello_hardlinked_content, + strlen (hello_hardlinked_content), + GLNX_FILE_REPLACE_NODATASYNC, + NULL, &error); + g_assert_no_error (error); + (void)impl_test_break_hardlink (tmp_dfd, "test-hardlink", &error); + g_assert_no_error (error); + + /* Symlink */ + if (symlinkat ("some-path", tmp_dfd, "test-symhardlink") < 0) + err (1, "symlinkat"); + (void)impl_test_break_hardlink (tmp_dfd, "test-symhardlink", &error); + g_assert_no_error (error); +} + static GVariant* xattr_cb (OstreeRepo *repo, const char *path, @@ -376,6 +443,7 @@ int main (int argc, char **argv) g_test_add_data_func ("/raw-file-to-archive-stream", repo, test_raw_file_to_archive_stream); g_test_add_data_func ("/objectwrites", repo, test_object_writes); 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); return g_test_run(); From 26b7637a39e804946e0aa3f4eed3f7570c842a70 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 14 Dec 2017 12:09:28 -0500 Subject: [PATCH 38/43] lib/core: Optimize breaking hardlinks for regfiles It'd all be really nice if there was some sort of `O_TMPFILE` for symlinks, but anyways the way we were doing a generic "make temp file than rename" actually defeats some of the point of `O_TMPFILE`. It's now fully safe to do "copy to self", so let's do that for regfiles. Closes: #1378 Approved by: jlebon --- src/libostree/ostree-core.c | 101 ++++++++++++++++++++++-------------- 1 file changed, 61 insertions(+), 40 deletions(-) diff --git a/src/libostree/ostree-core.c b/src/libostree/ostree-core.c index 7c1a3f99..679c9529 100644 --- a/src/libostree/ostree-core.c +++ b/src/libostree/ostree-core.c @@ -749,6 +749,50 @@ ostree_content_file_parse (gboolean compressed, cancellable, error); } +static gboolean +break_symhardlink (int dfd, + const char *path, + struct stat *stbuf, + GLnxFileCopyFlags copyflags, + GCancellable *cancellable, + GError **error) +{ + guint count; + gboolean copy_success = FALSE; + char *path_tmp = glnx_strjoina (path, ".XXXXXX"); + + for (count = 0; count < 100; count++) + { + g_autoptr(GError) tmp_error = NULL; + + glnx_gen_temp_name (path_tmp); + + if (!glnx_file_copy_at (dfd, path, stbuf, dfd, path_tmp, copyflags, + cancellable, &tmp_error)) + { + if (g_error_matches (tmp_error, G_IO_ERROR, G_IO_ERROR_EXISTS)) + continue; + g_propagate_error (error, g_steal_pointer (&tmp_error)); + return FALSE; + } + + copy_success = TRUE; + break; + } + + if (!copy_success) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_EXISTS, + "Exceeded limit of %u file creation attempts", count); + return FALSE; + } + + if (!glnx_renameat (dfd, path_tmp, dfd, path, error)) + return FALSE; + + return TRUE; +} + /** * ostree_break_hardlink: * @dfd: Directory fd @@ -784,48 +828,25 @@ gboolean ostree_break_hardlink (int dfd, if (!glnx_fstatat (dfd, path, &stbuf, AT_SYMLINK_NOFOLLOW, error)) return FALSE; - if (!S_ISLNK (stbuf.st_mode) && !S_ISREG (stbuf.st_mode)) + if (stbuf.st_nlink <= 1) + return TRUE; /* Note early return */ + + const GLnxFileCopyFlags copyflags = skip_xattrs ? GLNX_FILE_COPY_NOXATTRS : 0; + + if (S_ISREG (stbuf.st_mode)) + /* Note it's now completely safe to copy a file to itself, + * as glnx_file_copy_at() is documented to do an O_TMPFILE + rename() + * with GLNX_FILE_COPY_OVERWRITE. + */ + return glnx_file_copy_at (dfd, path, &stbuf, dfd, path, + copyflags | GLNX_FILE_COPY_OVERWRITE, + cancellable, error); + else if (S_ISLNK (stbuf.st_mode)) + return break_symhardlink (dfd, path, &stbuf, copyflags, + cancellable, error); + else return glnx_throw (error, "Unsupported type for entry '%s'", path); - const GLnxFileCopyFlags copyflags = - skip_xattrs ? GLNX_FILE_COPY_NOXATTRS : 0; - - if (stbuf.st_nlink > 1) - { - guint count; - gboolean copy_success = FALSE; - char *path_tmp = glnx_strjoina (path, ".XXXXXX"); - - for (count = 0; count < 100; count++) - { - g_autoptr(GError) tmp_error = NULL; - - glnx_gen_temp_name (path_tmp); - - if (!glnx_file_copy_at (dfd, path, &stbuf, dfd, path_tmp, copyflags, - cancellable, &tmp_error)) - { - if (g_error_matches (tmp_error, G_IO_ERROR, G_IO_ERROR_EXISTS)) - continue; - g_propagate_error (error, g_steal_pointer (&tmp_error)); - return FALSE; - } - - copy_success = TRUE; - break; - } - - if (!copy_success) - { - g_set_error (error, G_IO_ERROR, G_IO_ERROR_EXISTS, - "Exceeded limit of %u file creation attempts", count); - return FALSE; - } - - if (!glnx_renameat (dfd, path_tmp, dfd, path, error)) - return FALSE; - } - return TRUE; } From 85f388e05883d4a0e55d3dfcc5b8763c20dfba51 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 14 Dec 2017 12:54:24 -0500 Subject: [PATCH 39/43] bin/commit: Support creating "unbound" commits We had this basically forced on in the CLI; down the line I'd really like to make this an API option to commit or so, but given that we found a use case in the rpm-ostree test suite for "unbound" commits, let's support creating them from the cmdline. See: https://github.com/ostreedev/ostree/pull/1379 Closes: #1380 Approved by: jlebon --- src/ostree/ot-builtin-commit.c | 9 +++++++-- tests/basic-test.sh | 12 +++++++++++- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/ostree/ot-builtin-commit.c b/src/ostree/ot-builtin-commit.c index e53d7309..9c05428e 100644 --- a/src/ostree/ot-builtin-commit.c +++ b/src/ostree/ot-builtin-commit.c @@ -37,6 +37,7 @@ static char *opt_body_file; static gboolean opt_editor; static char *opt_parent; static gboolean opt_orphan; +static gboolean opt_no_bindings; static char **opt_bind_refs; static char *opt_branch; static char *opt_statoverride_file; @@ -90,6 +91,7 @@ static GOptionEntry options[] = { { "editor", 'e', 0, G_OPTION_ARG_NONE, &opt_editor, "Use an editor to write the commit message", NULL }, { "branch", 'b', 0, G_OPTION_ARG_STRING, &opt_branch, "Branch", "BRANCH" }, { "orphan", 0, 0, G_OPTION_ARG_NONE, &opt_orphan, "Create a commit without writing a ref", NULL }, + { "no-bindings", 0, 0, G_OPTION_ARG_NONE, &opt_no_bindings, "Do not write any ref bindings", NULL }, { "bind-ref", 0, 0, G_OPTION_ARG_STRING_ARRAY, &opt_bind_refs, "Add a ref to ref binding commit metadata", "BRANCH" }, { "tree", 0, 0, G_OPTION_ARG_STRING_ARRAY, &opt_trees, "Overlay the given argument as a tree", "dir=PATH or tar=TARFILE or ref=COMMIT" }, { "add-metadata-string", 0, 0, G_OPTION_ARG_STRING_ARRAY, &opt_metadata_strings, "Add a key/value pair to metadata", "KEY=VALUE" }, @@ -745,9 +747,12 @@ ostree_builtin_commit (int argc, char **argv, OstreeCommandInvocation *invocatio { gboolean update_summary; guint64 timestamp; - g_autoptr(GVariant) old_metadata = g_steal_pointer (&metadata); - fill_bindings (repo, old_metadata, &metadata); + if (!opt_no_bindings) + { + g_autoptr(GVariant) old_metadata = g_steal_pointer (&metadata); + fill_bindings (repo, old_metadata, &metadata); + } if (!opt_timestamp) { diff --git a/tests/basic-test.sh b/tests/basic-test.sh index 06092c12..87cb9fa2 100644 --- a/tests/basic-test.sh +++ b/tests/basic-test.sh @@ -19,7 +19,7 @@ set -euo pipefail -echo "1..$((77 + ${extra_basic_tests:-0}))" +echo "1..$((78 + ${extra_basic_tests:-0}))" CHECKOUT_U_ARG="" CHECKOUT_H_ARGS="-H" @@ -767,6 +767,16 @@ $OSTREE show --print-detached-metadata-key=SIGNATURE test2 > test2-meta assert_file_has_content test2-meta "HANCOCK" echo "ok metadata commit with strings" +cd ${test_tmpdir} +$OSTREE show --print-metadata-key=ostree.ref-binding test2 > test2-ref-binding +assert_file_has_content test2-ref-binding 'test2' + +$OSTREE commit ${COMMIT_ARGS} -b test2-unbound --no-bindings --tree=dir=${test_tmpdir}/checkout-test2 +if $OSTREE show --print-metadata-key=ostree.ref-binding; then + fatal "ref bindings found with --no-bindings?" +fi +echo "ok refbinding" + if ! skip_one_without_user_xattrs; then cd ${test_tmpdir} rm repo2 -rf From b822f337b541e3cc8be3454f0e423e0874209b82 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 14 Dec 2017 13:13:42 -0500 Subject: [PATCH 40/43] bin/refs: Disallow aliases to remote refs It can't really work in general; the client and server would have to agree on the name of the remote. Closes: https://github.com/ostreedev/ostree/issues/1342 Closes: #1381 Approved by: jlebon --- src/ostree/ot-builtin-refs.c | 2 ++ tests/test-refs.sh | 9 ++++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/ostree/ot-builtin-refs.c b/src/ostree/ot-builtin-refs.c index 3508a529..be5cf9d4 100644 --- a/src/ostree/ot-builtin-refs.c +++ b/src/ostree/ot-builtin-refs.c @@ -223,6 +223,8 @@ static gboolean do_ref (OstreeRepo *repo, const char *refspec_prefix, GCancellab if (opt_alias) { + if (remote) + return glnx_throw (error, "Cannot create alias to remote ref: %s", remote); if (!ostree_repo_set_alias_ref_immediate (repo, remote, ref, refspec_prefix, cancellable, error)) goto out; diff --git a/tests/test-refs.sh b/tests/test-refs.sh index 1f0dbdbd..316e3e10 100755 --- a/tests/test-refs.sh +++ b/tests/test-refs.sh @@ -23,7 +23,7 @@ set -euo pipefail setup_fake_remote_repo1 "archive" -echo '1..5' +echo '1..6' cd ${test_tmpdir} mkdir repo @@ -186,3 +186,10 @@ assert_file_has_content_literal refs.txt 'exampleos/x86_64/stable/server -> exam ${CMD_PREFIX} ostree --repo=repo summary -u echo "ok ref symlink" + +# https://github.com/ostreedev/ostree/issues/1342 +if ${CMD_PREFIX} ostree --repo=repo refs -A exampleos/x86_64/27/server --create=exampleos:exampleos/x86_64/stable/server 2>err.txt; then + fatal "Created alias ref to remote?" +fi +assert_file_has_content_literal err.txt 'Cannot create alias to remote ref' +echo "ok ref no alias remote" From 8c42e81f123a696d9f1c02764e53a38b75260f25 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 15 Dec 2017 09:20:58 -0500 Subject: [PATCH 41/43] build-sys: Use -fno-strict-aliasing by default See discussion in https://bugzilla.gnome.org/show_bug.cgi?id=791622 This is what e.g. systemd, the Linux kernel, and lots of other projects do. It's astonishingly hard to reliably get right; the optimization IMO only really matters for truly high performance inner loops, but if you're doing that kind of stuff today you're probably doing it on a GPU anyways. Closes: #1384 Approved by: pwithnall --- Makefile.am | 3 ++- configure.ac | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/Makefile.am b/Makefile.am index 6043b2aa..ea1863d3 100644 --- a/Makefile.am +++ b/Makefile.am @@ -31,7 +31,8 @@ AM_CPPFLAGS += -DDATADIR='"$(datadir)"' -DLIBEXECDIR='"$(libexecdir)"' \ -DOSTREE_GITREV='"$(OSTREE_GITREV)"' \ -DGLIB_VERSION_MIN_REQUIRED=GLIB_VERSION_2_40 '-DGLIB_VERSION_MAX_ALLOWED=G_ENCODE_VERSION(2,50)' \ -DSOUP_VERSION_MIN_REQUIRED=SOUP_VERSION_2_40 '-DSOUP_VERSION_MAX_ALLOWED=G_ENCODE_VERSION(2,48)' -AM_CFLAGS += -std=gnu99 $(WARN_CFLAGS) +# For strict aliasing, see https://bugzilla.gnome.org/show_bug.cgi?id=791622 +AM_CFLAGS += -std=gnu99 -fno-strict-aliasing $(WARN_CFLAGS) AM_DISTCHECK_CONFIGURE_FLAGS += \ --enable-gtk-doc \ --enable-man \ diff --git a/configure.ac b/configure.ac index e2c27822..36071c93 100644 --- a/configure.ac +++ b/configure.ac @@ -48,6 +48,7 @@ CC_CHECK_FLAGS_APPEND([WARN_CFLAGS], [CFLAGS], [\ -Werror=incompatible-pointer-types \ -Werror=misleading-indentation \ -Werror=missing-include-dirs -Werror=aggregate-return \ + -Wstrict-aliasing=2 \ -Werror=unused-result \ ])]) AC_SUBST(WARN_CFLAGS) From 5a77b8dafeac5200017b203e24ac3526fd59de29 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 14 Dec 2017 21:42:54 -0500 Subject: [PATCH 42/43] Bump libglnx, use "n items" progress for fsck Sooo much nicer. See also https://github.com/projectatomic/rpm-ostree/pull/1143 Update submodule: libglnx Closes: #1383 Approved by: jlebon --- libglnx | 2 +- src/ostree/ot-builtin-fsck.c | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/libglnx b/libglnx index b36606b3..96b1fd95 160000 --- a/libglnx +++ b/libglnx @@ -1 +1 @@ -Subproject commit b36606b366d39c7ddb90ee21d622c0cb1da118ed +Subproject commit 96b1fd9578b3d6ff2d8e0707068f5ef450eba98c diff --git a/src/ostree/ot-builtin-fsck.c b/src/ostree/ot-builtin-fsck.c index 79fd9e21..70a30210 100644 --- a/src/ostree/ot-builtin-fsck.c +++ b/src/ostree/ot-builtin-fsck.c @@ -111,8 +111,10 @@ fsck_reachable_objects_from_commits (OstreeRepo *repo, return FALSE; } + g_auto(GLnxConsoleRef) console = { 0, }; + glnx_console_lock (&console); + const guint count = g_hash_table_size (reachable_objects); - const guint mod = count / 10; guint i = 0; g_hash_table_iter_init (&hash_iter, reachable_objects); while (g_hash_table_iter_next (&hash_iter, &key, &value)) @@ -127,9 +129,8 @@ fsck_reachable_objects_from_commits (OstreeRepo *repo, cancellable, error)) return FALSE; - if (mod == 0 || (i % mod == 0)) - g_print ("%u/%u objects\n", i + 1, count); i++; + glnx_console_progress_n_items ("fsck objects", i, count); } return TRUE; From 19d08dab617bf060c6440ecbd8df3347b04741b5 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 19 Dec 2017 15:54:01 +0100 Subject: [PATCH 43/43] Release 2017.15 Let's do a new release with the locking preview, the http2 disable options and other misc bugfixes to close out the year. Closes: #1386 Approved by: jlebon --- configure.ac | 2 +- src/libostree/libostree-devel.sym | 8 ++------ src/libostree/libostree-released.sym | 6 ++++++ tests/test-symbols.sh | 2 +- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/configure.ac b/configure.ac index 36071c93..478d7349 100644 --- a/configure.ac +++ b/configure.ac @@ -7,7 +7,7 @@ m4_define([year_version], [2017]) m4_define([release_version], [15]) 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 582e8c92..5ab4b99c 100644 --- a/src/libostree/libostree-devel.sym +++ b/src/libostree/libostree-devel.sym @@ -18,12 +18,8 @@ ***/ /* Add new symbols here. Release commits should copy this section into -released.sym. */ - -LIBOSTREE_2017.15 { - ostree_repo_fsck_object; - ostree_repo_mark_commit_partial; - ostree_break_hardlink; -} LIBOSTREE_2017.14; +LIBOSTREE_2017.16 { +} LIBOSTREE_2017.15; /* 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 e4d50895..b7d57853 100644 --- a/src/libostree/libostree-released.sym +++ b/src/libostree/libostree-released.sym @@ -445,6 +445,12 @@ global: LIBOSTREE_2017.14 { } LIBOSTREE_2017.13; +LIBOSTREE_2017.15 { + ostree_repo_fsck_object; + ostree_repo_mark_commit_partial; + ostree_break_hardlink; +} LIBOSTREE_2017.14; + /* 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 6e71e2dd..51137d6c 100755 --- a/tests/test-symbols.sh +++ b/tests/test-symbols.sh @@ -52,7 +52,7 @@ echo 'ok documented symbols' # ONLY update this checksum in release commits! cat > released-sha256.txt <