From 0e2ad49888a0a1448f82da8103f1daccb49d1c97 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Fri, 2 Nov 2018 13:26:30 -0400 Subject: [PATCH 1/4] compose: Always put workdir on same filesystem as pkgcache This ensures that we always get hardlinks when checking out of the pkgcache. This works right now because we indirectly require the target bare-user repo and the pkgcache to be on the same filesystem by setting `no_copy_fallback` in the core (I say "indirectly" because that setting only enforces the workdir to be on the same filesystem as the pkgcache repo, but since the workdir is currently placed inside the bare-user repo...). However, I'd like to change the requirement of a bare-user repo so that one can commit into a repo on a different file system or a repo of a different type (e.g. archive repo). This is prep for that. --- src/app/rpmostree-compose-builtin-tree.c | 104 +++++++++++++++-------- 1 file changed, 67 insertions(+), 37 deletions(-) diff --git a/src/app/rpmostree-compose-builtin-tree.c b/src/app/rpmostree-compose-builtin-tree.c index b51d9fcebe..ccf46a6e26 100644 --- a/src/app/rpmostree-compose-builtin-tree.c +++ b/src/app/rpmostree-compose-builtin-tree.c @@ -231,16 +231,10 @@ install_packages (RpmOstreeTreeComposeContext *self, return FALSE; } - /* For unified core, we have a pkgcache repo. This may be auto-created under - * the workdir, or live explicitly in the dir for --cache. - */ + /* For unified core, we have a pkgcache repo. This is auto-created under the cachedir. */ if (opt_unified_core) { - self->pkgcache_repo = ostree_repo_create_at (self->cachedir_dfd, "pkgcache-repo", - OSTREE_REPO_MODE_BARE_USER, NULL, - cancellable, error); - if (!self->pkgcache_repo) - return FALSE; + g_assert (self->pkgcache_repo); if (!opt_cachedir) { @@ -496,52 +490,88 @@ rpm_ostree_compose_context_new (const char *treefile_pathstr, if (opt_unified_core) { + /* Unified mode works very differently. We ignore --workdir because we want to be sure + * that we're going to get hardlinks. The only way to be sure of this is to place the + * workdir underneath the pkgcache repo. */ + if (opt_workdir) g_printerr ("note: --workdir is ignored for --ex-unified-core\n"); - /* For unified core, our workdir must be underneath the repo tmp/ - * in order to use hardlinks. We also really want a bare-user repo. - * We hard require that for now, but down the line we may automatically - * do a pull-local from the bare-user repo to the archive. + /* We also really want a bare-user repo. We hard require that for now, but down the + * line we may automatically do a pull-local from the bare-user repo to the archive. */ if (ostree_repo_get_mode (self->repo) != OSTREE_REPO_MODE_BARE_USER) return glnx_throw (error, "--ex-unified-core requires a bare-user repository"); - if (!glnx_mkdtempat (ostree_repo_get_dfd (self->repo), "tmp/rpm-ostree-compose.XXXXXX", 0700, - &self->workdir_tmp, error)) - return FALSE; - /* Note special handling of this aliasing in _finalize() */ - self->workdir_dfd = self->workdir_tmp.fd; - } - else if (!opt_workdir) - { - if (!glnx_mkdtempat (AT_FDCWD, "/var/tmp/rpm-ostree.XXXXXX", 0700, &self->workdir_tmp, error)) + if (opt_cachedir) + { + if (!glnx_opendirat (AT_FDCWD, opt_cachedir, TRUE, &self->cachedir_dfd, error)) + { + g_prefix_error (error, "Opening cachedir '%s': ", opt_cachedir); + return FALSE; + } + + /* Put workdir beneath cachedir, which is where the pkgcache repo also is */ + if (!glnx_mkdtempat (self->cachedir_dfd, "rpm-ostree-compose.XXXXXX", 0700, + &self->workdir_tmp, error)) + return FALSE; + } + else + { + /* Put cachedir under the target repo: makes things more efficient if it's + * bare-user, and otherwise just restricts IO to within the same fs. If for + * whatever reason users don't want to run the compose there (e.g. weird + * filesystems that aren't fully POSIX compliant), they can just use --cachedir. + */ + if (!glnx_mkdtempat (ostree_repo_get_dfd (self->repo), + "tmp/rpm-ostree-compose.XXXXXX", 0700, + &self->workdir_tmp, error)) + return FALSE; + + self->cachedir_dfd = self->workdir_tmp.fd; + } + + self->pkgcache_repo = ostree_repo_create_at (self->cachedir_dfd, "pkgcache-repo", + OSTREE_REPO_MODE_BARE_USER, NULL, + cancellable, error); + if (!self->pkgcache_repo) return FALSE; + /* Note special handling of this aliasing in _finalize() */ self->workdir_dfd = self->workdir_tmp.fd; } else { - if (!glnx_opendirat (AT_FDCWD, opt_workdir, FALSE, &self->workdir_dfd, error)) - return FALSE; - } - - self->treefile_path = g_file_new_for_path (treefile_pathstr); + if (!opt_workdir) + { + if (!glnx_mkdtempat (AT_FDCWD, "/var/tmp/rpm-ostree.XXXXXX", 0700, &self->workdir_tmp, error)) + return FALSE; + /* Note special handling of this aliasing in _finalize() */ + self->workdir_dfd = self->workdir_tmp.fd; + } + else + { + if (!glnx_opendirat (AT_FDCWD, opt_workdir, FALSE, &self->workdir_dfd, error)) + return FALSE; + } - if (opt_cachedir) - { - if (!glnx_opendirat (AT_FDCWD, opt_cachedir, TRUE, &self->cachedir_dfd, error)) + if (opt_cachedir) { - g_prefix_error (error, "Opening cachedir '%s': ", opt_cachedir); - return FALSE; + if (!glnx_opendirat (AT_FDCWD, opt_cachedir, TRUE, &self->cachedir_dfd, error)) + { + g_prefix_error (error, "Opening cachedir '%s': ", opt_cachedir); + return FALSE; + } + } + else + { + self->cachedir_dfd = fcntl (self->workdir_dfd, F_DUPFD_CLOEXEC, 3); + if (self->cachedir_dfd < 0) + return glnx_throw_errno_prefix (error, "fcntl"); } } - else - { - self->cachedir_dfd = fcntl (self->workdir_dfd, F_DUPFD_CLOEXEC, 3); - if (self->cachedir_dfd < 0) - return glnx_throw_errno_prefix (error, "fcntl"); - } + + self->treefile_path = g_file_new_for_path (treefile_pathstr); self->metadata = rpmostree_composeutil_read_json_metadata (opt_metadata_json, error); if (!self->metadata) From ea6d83564fd717c6ccd26ec72bb8bf6d125ef0a2 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Fri, 2 Nov 2018 13:26:31 -0400 Subject: [PATCH 2/4] compose: Support all target repos in unified mode Previously, we were limiting the target repo in unified mode to be a bare-user repo located on the same filesystem (see message of previous commit). This patch lifts this restriction by making a distinction between the *build repo* and the *final* target repo. To do this, we create a bare-user repo located near the pkgcache to take advantage of hardlinks and devino caching at commit time. And only after committing do we essentially `pull-local` into the final target repo. This of course allows us to avoid potentially pulling across the two filesystems file objects that are already present in the target repo. This will be used by coreos-assembler: https://github.com/coreos/coreos-assembler/pull/190 Closes: #1490 --- src/app/rpmostree-compose-builtin-tree.c | 123 +++++++++++++++++------ 1 file changed, 90 insertions(+), 33 deletions(-) diff --git a/src/app/rpmostree-compose-builtin-tree.c b/src/app/rpmostree-compose-builtin-tree.c index ccf46a6e26..1202be4c50 100644 --- a/src/app/rpmostree-compose-builtin-tree.c +++ b/src/app/rpmostree-compose-builtin-tree.c @@ -114,8 +114,9 @@ typedef struct { int rootfs_dfd; int cachedir_dfd; gboolean unified_core_and_fuse; - OstreeRepo *repo; - OstreeRepo *pkgcache_repo; + OstreeRepo *repo; /* target repo provided by --repo */ + OstreeRepo *build_repo; /* unified mode: repo we build into */ + OstreeRepo *pkgcache_repo; /* unified mode: pkgcache repo where we import pkgs */ OstreeRepoDevInoCache *devino_cache; const char *ref; char *rojig_spec; @@ -145,6 +146,7 @@ rpm_ostree_tree_compose_context_free (RpmOstreeTreeComposeContext *ctx) glnx_close_fd (&ctx->rootfs_dfd); glnx_close_fd (&ctx->cachedir_dfd); g_clear_object (&ctx->repo); + g_clear_object (&ctx->build_repo); g_clear_object (&ctx->pkgcache_repo); g_clear_pointer (&ctx->devino_cache, (GDestroyNotify)ostree_repo_devino_cache_unref); g_free (ctx->previous_checksum); @@ -265,7 +267,7 @@ install_packages (RpmOstreeTreeComposeContext *self, rpmostree_context_set_devino_cache (self->corectx, self->devino_cache); } - rpmostree_context_set_repos (self->corectx, self->repo, self->pkgcache_repo); + rpmostree_context_set_repos (self->corectx, self->build_repo, self->pkgcache_repo); } if (!rpmostree_context_prepare (self->corectx, cancellable, error)) @@ -492,17 +494,11 @@ rpm_ostree_compose_context_new (const char *treefile_pathstr, { /* Unified mode works very differently. We ignore --workdir because we want to be sure * that we're going to get hardlinks. The only way to be sure of this is to place the - * workdir underneath the pkgcache repo. */ + * workdir underneath the cachedir; the same fs where the pkgcache repo is. */ if (opt_workdir) g_printerr ("note: --workdir is ignored for --ex-unified-core\n"); - /* We also really want a bare-user repo. We hard require that for now, but down the - * line we may automatically do a pull-local from the bare-user repo to the archive. - */ - if (ostree_repo_get_mode (self->repo) != OSTREE_REPO_MODE_BARE_USER) - return glnx_throw (error, "--ex-unified-core requires a bare-user repository"); - if (opt_cachedir) { if (!glnx_opendirat (AT_FDCWD, opt_cachedir, TRUE, &self->cachedir_dfd, error)) @@ -537,6 +533,15 @@ rpm_ostree_compose_context_new (const char *treefile_pathstr, if (!self->pkgcache_repo) return FALSE; + /* We use a temporary repo for building and committing on the same FS as the + * pkgcache to guarantee links and devino caching. We then pull-local into the "real" + * target repo. */ + self->build_repo = ostree_repo_create_at (self->cachedir_dfd, "repo-build", + OSTREE_REPO_MODE_BARE_USER, NULL, + cancellable, error); + if (!self->build_repo) + return glnx_prefix_error (error, "Creating repo-build"); + /* Note special handling of this aliasing in _finalize() */ self->workdir_dfd = self->workdir_tmp.fd; } @@ -569,6 +574,8 @@ rpm_ostree_compose_context_new (const char *treefile_pathstr, if (self->cachedir_dfd < 0) return glnx_throw_errno_prefix (error, "fcntl"); } + + self->build_repo = g_object_ref (self->repo); } self->treefile_path = g_file_new_for_path (treefile_pathstr); @@ -583,7 +590,8 @@ rpm_ostree_compose_context_new (const char *treefile_pathstr, return FALSE; } - self->corectx = rpmostree_context_new_tree (self->cachedir_dfd, self->repo, cancellable, error); + self->corectx = rpmostree_context_new_tree (self->cachedir_dfd, self->build_repo, + cancellable, error); if (!self->corectx) return FALSE; @@ -835,6 +843,49 @@ repo_is_on_netfs (OstreeRepo *repo) } } +/* See canonical version of this in ot-builtin-pull.c */ +static void +noninteractive_console_progress_changed (OstreeAsyncProgress *progress, + gpointer user_data) +{ + /* We do nothing here - we just want the final status */ +} + +static gboolean +pull_local_into_target_repo (OstreeRepo *src_repo, + OstreeRepo *dest_repo, + const char *checksum, + GCancellable *cancellable, + GError **error) +{ + const char *refs[] = { checksum, NULL }; + + /* really should enhance the pull API so we can just pass the src OstreeRepo directly */ + g_autofree char *src_repo_uri = + g_strdup_printf ("file:///proc/self/fd/%d", ostree_repo_get_dfd (src_repo)); + + g_auto(GLnxConsoleRef) console = { 0, }; + glnx_console_lock (&console); + g_autoptr(OstreeAsyncProgress) progress = ostree_async_progress_new_and_connect ( + console.is_tty ? ostree_repo_pull_default_console_progress_changed + : noninteractive_console_progress_changed, &console); + + /* no fancy flags here, so just use the old school simpler API */ + if (!ostree_repo_pull (dest_repo, src_repo_uri, (char**)refs, 0, progress, + cancellable, error)) + return FALSE; + + if (!console.is_tty) + { + const char *status = ostree_async_progress_get_status (progress); + if (status) + g_print ("%s\n", status); + } + ostree_async_progress_finish (progress); + + return TRUE; +} + /* Perform required postprocessing, and invoke rpmostree_compose_commit(). */ static gboolean impl_commit_tree (RpmOstreeTreeComposeContext *self, @@ -886,7 +937,7 @@ impl_commit_tree (RpmOstreeTreeComposeContext *self, if (use_txn) { - if (!ostree_repo_prepare_transaction (self->repo, NULL, cancellable, error)) + if (!ostree_repo_prepare_transaction (self->build_repo, NULL, cancellable, error)) return FALSE; } @@ -899,44 +950,50 @@ impl_commit_tree (RpmOstreeTreeComposeContext *self, /* The penultimate step, just basically `ostree commit` */ g_autofree char *new_revision = NULL; - if (!rpmostree_compose_commit (self->rootfs_dfd, self->repo, parent_revision, + if (!rpmostree_compose_commit (self->rootfs_dfd, self->build_repo, parent_revision, metadata, gpgkey, selinux, self->devino_cache, &new_revision, cancellable, error)) return FALSE; + OstreeRepoTransactionStats stats = { 0, }; + OstreeRepoTransactionStats *statsp = NULL; + + if (use_txn) + { + if (!ostree_repo_commit_transaction (self->build_repo, &stats, cancellable, error)) + return glnx_prefix_error (error, "Commit"); + statsp = &stats; + } + + if (!opt_unified_core) + g_assert (self->repo == self->build_repo); + else + { + /* Now we actually pull it into the target repo specified by the user */ + g_assert (self->repo != self->build_repo); + + if (!pull_local_into_target_repo (self->build_repo, self->repo, new_revision, + cancellable, error)) + return FALSE; + } + g_autoptr(GVariant) new_commit = NULL; - if (!ostree_repo_load_commit (self->repo, new_revision, &new_commit, - NULL, error)) + if (!ostree_repo_load_commit (self->repo, new_revision, &new_commit, NULL, error)) return FALSE; g_autoptr(GVariant) new_commit_inline_meta = g_variant_get_child_value (new_commit, 0); /* --write-commitid-to overrides writing the ref */ if (self->ref && !opt_write_commitid_to) { - if (use_txn) - ostree_repo_transaction_set_ref (self->repo, NULL, self->ref, new_revision); - else - { - if (!ostree_repo_set_ref_immediate (self->repo, NULL, self->ref, new_revision, - cancellable, error)) - return FALSE; - } + if (!ostree_repo_set_ref_immediate (self->repo, NULL, self->ref, new_revision, + cancellable, error)) + return FALSE; g_print ("%s => %s\n", self->ref, new_revision); g_variant_builder_add (&composemeta_builder, "{sv}", "ref", g_variant_new_string (self->ref)); } else g_print ("Wrote commit: %s\n", new_revision); - OstreeRepoTransactionStats stats = { 0, }; - OstreeRepoTransactionStats *statsp = NULL; - - if (use_txn) - { - if (!ostree_repo_commit_transaction (self->repo, &stats, cancellable, error)) - return glnx_prefix_error (error, "Commit"); - statsp = &stats; - } - if (!rpmostree_composeutil_write_composejson (self->repo, opt_write_composejson_to, statsp, new_revision, new_commit, From 44b33d0d2b094648c6b5a473d16c595344be0f7d Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Mon, 5 Nov 2018 15:35:15 -0500 Subject: [PATCH 3/4] fixup! compose: Always put workdir on same filesystem as pkgcache --- src/app/rpmostree-compose-builtin-tree.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/app/rpmostree-compose-builtin-tree.c b/src/app/rpmostree-compose-builtin-tree.c index 1202be4c50..d415c59cd9 100644 --- a/src/app/rpmostree-compose-builtin-tree.c +++ b/src/app/rpmostree-compose-builtin-tree.c @@ -502,10 +502,7 @@ rpm_ostree_compose_context_new (const char *treefile_pathstr, if (opt_cachedir) { if (!glnx_opendirat (AT_FDCWD, opt_cachedir, TRUE, &self->cachedir_dfd, error)) - { - g_prefix_error (error, "Opening cachedir '%s': ", opt_cachedir); - return FALSE; - } + return glnx_prefix_error (error, "Opening cachedir"); /* Put workdir beneath cachedir, which is where the pkgcache repo also is */ if (!glnx_mkdtempat (self->cachedir_dfd, "rpm-ostree-compose.XXXXXX", 0700, @@ -563,10 +560,7 @@ rpm_ostree_compose_context_new (const char *treefile_pathstr, if (opt_cachedir) { if (!glnx_opendirat (AT_FDCWD, opt_cachedir, TRUE, &self->cachedir_dfd, error)) - { - g_prefix_error (error, "Opening cachedir '%s': ", opt_cachedir); - return FALSE; - } + return glnx_prefix_error (error, "Opening cachedir"); } else { From 5b6ee6a2bf694694b6f9d7b02aba482f15c8367e Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Mon, 5 Nov 2018 15:43:27 -0500 Subject: [PATCH 4/4] fixup! compose: Support all target repos in unified mode --- src/app/rpmostree-compose-builtin-tree.c | 70 ++++++++++++++---------- 1 file changed, 40 insertions(+), 30 deletions(-) diff --git a/src/app/rpmostree-compose-builtin-tree.c b/src/app/rpmostree-compose-builtin-tree.c index d415c59cd9..48760671c8 100644 --- a/src/app/rpmostree-compose-builtin-tree.c +++ b/src/app/rpmostree-compose-builtin-tree.c @@ -460,6 +460,28 @@ process_touch_if_changed (GError **error) return TRUE; } +/* https://pagure.io/atomic-wg/issue/387 */ +static gboolean +repo_is_on_netfs (OstreeRepo *repo) +{ +#ifndef FUSE_SUPER_MAGIC +#define FUSE_SUPER_MAGIC 0x65735546 +#endif + + int dfd = ostree_repo_get_dfd (repo); + struct statfs stbuf; + if (fstatfs (dfd, &stbuf) != 0) + return FALSE; + switch (stbuf.f_type) + { + case NFS_SUPER_MAGIC: + case FUSE_SUPER_MAGIC: + return TRUE; + default: + return FALSE; + } +} + /* Prepare a context - this does some generic pre-compose initialization from * the arguments such as loading the treefile and any specified metadata. */ @@ -511,15 +533,25 @@ rpm_ostree_compose_context_new (const char *treefile_pathstr, } else { - /* Put cachedir under the target repo: makes things more efficient if it's - * bare-user, and otherwise just restricts IO to within the same fs. If for - * whatever reason users don't want to run the compose there (e.g. weird - * filesystems that aren't fully POSIX compliant), they can just use --cachedir. + /* Put cachedir under the target repo if it's not on NFS or fuse. It makes things + * more efficient if it's bare-user, and otherwise just restricts IO to within the + * same fs. If for whatever reason users don't want to run the compose there (e.g. + * weird filesystems that aren't fully POSIX compliant), they can just use + * --cachedir. */ - if (!glnx_mkdtempat (ostree_repo_get_dfd (self->repo), - "tmp/rpm-ostree-compose.XXXXXX", 0700, - &self->workdir_tmp, error)) - return FALSE; + if (!repo_is_on_netfs (self->repo)) + { + if (!glnx_mkdtempat (ostree_repo_get_dfd (self->repo), + "tmp/rpm-ostree-compose.XXXXXX", 0700, + &self->workdir_tmp, error)) + return FALSE; + } + else + { + if (!glnx_mkdtempat (AT_FDCWD, "/var/tmp/rpm-ostree-compose.XXXXXX", 0700, + &self->workdir_tmp, error)) + return FALSE; + } self->cachedir_dfd = self->workdir_tmp.fd; } @@ -815,28 +847,6 @@ impl_install_tree (RpmOstreeTreeComposeContext *self, return TRUE; } -/* https://pagure.io/atomic-wg/issue/387 */ -static gboolean -repo_is_on_netfs (OstreeRepo *repo) -{ -#ifndef FUSE_SUPER_MAGIC -#define FUSE_SUPER_MAGIC 0x65735546 -#endif - - int dfd = ostree_repo_get_dfd (repo); - struct statfs stbuf; - if (fstatfs (dfd, &stbuf) != 0) - return FALSE; - switch (stbuf.f_type) - { - case NFS_SUPER_MAGIC: - case FUSE_SUPER_MAGIC: - return TRUE; - default: - return FALSE; - } -} - /* See canonical version of this in ot-builtin-pull.c */ static void noninteractive_console_progress_changed (OstreeAsyncProgress *progress,