From 1c0e354571472a076d86757a4e9541e0c8524d36 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 1 Dec 2017 14:49:20 -0500 Subject: [PATCH] importer: Rework API Now that the importer *only* imports into OSTree repos, let's clean up the API so that the `OstreeRepo` and `OstreeSePolicy` are passed as constructor args. Also rework things so there's only one constructor API that steals the fd. This is prep for adding another async import API. Closes: #1124 Approved by: jlebon --- src/daemon/rpmostreed-transaction-types.c | 37 +++++++--- src/libpriv/rpmostree-core.c | 8 +-- src/libpriv/rpmostree-importer.c | 83 ++++++++--------------- src/libpriv/rpmostree-importer.h | 19 ++---- tests/check/test-utils.c | 7 +- 5 files changed, 72 insertions(+), 82 deletions(-) diff --git a/src/daemon/rpmostreed-transaction-types.c b/src/daemon/rpmostreed-transaction-types.c index 7f19d551..28a1f4ee 100644 --- a/src/daemon/rpmostreed-transaction-types.c +++ b/src/daemon/rpmostreed-transaction-types.c @@ -468,7 +468,7 @@ deploy_transaction_finalize (GObject *object) static gboolean import_local_rpm (OstreeRepo *repo, - int fd, + int *fd, char **sha256_nevra, GCancellable *cancellable, GError **error) @@ -482,12 +482,11 @@ import_local_rpm (OstreeRepo *repo, if (policy == NULL) return FALSE; - g_autoptr(RpmOstreeImporter) unpacker = rpmostree_importer_new_fd (fd, NULL, 0, error); + g_autoptr(RpmOstreeImporter) unpacker = rpmostree_importer_new_take_fd (fd, repo, NULL, 0, policy, error); if (unpacker == NULL) return FALSE; - if (!rpmostree_importer_run (unpacker, repo, policy, - NULL, cancellable, error)) + if (!rpmostree_importer_run (unpacker, NULL, cancellable, error)) return FALSE; g_autofree char *nevra = rpmostree_importer_get_nevra (unpacker); @@ -497,6 +496,25 @@ import_local_rpm (OstreeRepo *repo, return TRUE; } +static void +ptr_close_fd (gpointer fdp) +{ + int fd = GPOINTER_TO_INT (fdp); + glnx_close_fd (&fd); +} + +/* GUnixFDList doesn't allow stealing individual members */ +static GPtrArray * +unixfdlist_to_ptrarray (GUnixFDList *fdl) +{ + gint len; + gint *fds = g_unix_fd_list_steal_fds (fdl, &len); + GPtrArray *ret = g_ptr_array_new_with_free_func ((GDestroyNotify)ptr_close_fd); + for (int i = 0; i < len; i++) + g_ptr_array_add (ret, GINT_TO_POINTER (fds[i])); + return ret; +} + static gboolean import_many_local_rpms (OstreeRepo *repo, GUnixFDList *fdl, @@ -516,12 +534,15 @@ import_many_local_rpms (OstreeRepo *repo, g_autoptr(GPtrArray) pkgs = g_ptr_array_new_with_free_func (g_free); - gint nfds = 0; - const gint *fds = g_unix_fd_list_peek_fds (fdl, &nfds); - for (guint i = 0; i < nfds; i++) + g_autoptr(GPtrArray) fds = unixfdlist_to_ptrarray (fdl); + for (guint i = 0; i < fds->len; i++) { + /* Steal fd from the ptrarray */ + glnx_autofd int fd = GPOINTER_TO_INT (fds->pdata[i]); + fds->pdata[i] = GINT_TO_POINTER (-1); g_autofree char *sha256_nevra = NULL; - if (!import_local_rpm (repo, fds[i], &sha256_nevra, cancellable, error)) + /* Transfer fd to import */ + if (!import_local_rpm (repo, &fd, &sha256_nevra, cancellable, error)) return FALSE; g_ptr_array_add (pkgs, g_steal_pointer (&sha256_nevra)); diff --git a/src/libpriv/rpmostree-core.c b/src/libpriv/rpmostree-core.c index 560c32e7..f9e6d537 100644 --- a/src/libpriv/rpmostree-core.c +++ b/src/libpriv/rpmostree-core.c @@ -2055,7 +2055,9 @@ import_one_package (RpmOstreeContext *self, } /* TODO - tweak the unpacker flags for containers */ - g_autoptr(RpmOstreeImporter) unpacker = rpmostree_importer_new_fd (fd, pkg, flags, error); + OstreeRepo *ostreerepo = get_pkgcache_repo (self); + g_autoptr(RpmOstreeImporter) unpacker = + rpmostree_importer_new_take_fd (&fd, ostreerepo, pkg, flags, sepolicy, error); if (!unpacker) return FALSE; @@ -2065,10 +2067,8 @@ import_one_package (RpmOstreeContext *self, rpmostree_importer_set_jigdo_mode (unpacker, jigdo_xattr_table, jigdo_xattrs); } - OstreeRepo *ostreerepo = get_pkgcache_repo (self); g_autofree char *ostree_commit = NULL; - if (!rpmostree_importer_run (unpacker, ostreerepo, sepolicy, - &ostree_commit, cancellable, error)) + if (!rpmostree_importer_run (unpacker, &ostree_commit, cancellable, error)) return glnx_prefix_error (error, "Unpacking %s", dnf_package_get_nevra (pkg)); diff --git a/src/libpriv/rpmostree-importer.c b/src/libpriv/rpmostree-importer.c index d5735d30..d69a7004 100644 --- a/src/libpriv/rpmostree-importer.c +++ b/src/libpriv/rpmostree-importer.c @@ -53,9 +53,10 @@ typedef GObjectClass RpmOstreeImporterClass; struct RpmOstreeImporter { GObject parent_instance; + OstreeRepo *repo; + OstreeSePolicy *sepolicy; struct archive *archive; int fd; - gboolean owns_fd; Header hdr; rpmfi fi; off_t cpio_offset; @@ -87,10 +88,11 @@ rpmostree_importer_finalize (GObject *object) archive_read_free (self->archive); if (self->fi) (void) rpmfiFree (self->fi); - if (self->owns_fd) - glnx_close_fd (&self->fd); + glnx_close_fd (&self->fd); g_string_free (self->tmpfiles_d, TRUE); g_free (self->ostree_branch); + g_clear_object (&self->repo); + g_clear_object (&self->sepolicy); g_clear_pointer (&self->rpmfi_overrides, (GDestroyNotify)g_hash_table_unref); g_clear_pointer (&self->doc_files, (GDestroyNotify)g_hash_table_unref); @@ -115,6 +117,7 @@ rpmostree_importer_class_init (RpmOstreeImporterClass *klass) static void rpmostree_importer_init (RpmOstreeImporter *self) { + self->fd = -1; self->rpmfi_overrides = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL); self->tmpfiles_d = g_string_new (""); @@ -226,10 +229,12 @@ build_rpmfi_overrides (RpmOstreeImporter *self) } /* - * rpmostree_importer_new_fd: + * rpmostree_importer_new_take_fd: * @fd: Fd + * @repo: repo * @pkg: (optional): Package reference, used for metadata * @flags: flags + * @sepolicy: (optional): SELinux policy * @error: error * * Create a new unpacker instance. The @pkg argument, if @@ -237,10 +242,12 @@ build_rpmfi_overrides (RpmOstreeImporter *self) * origin repo will be added to the final commit. */ RpmOstreeImporter * -rpmostree_importer_new_fd (int fd, - DnfPackage *pkg, - RpmOstreeImporterFlags flags, - GError **error) +rpmostree_importer_new_take_fd (int *fd, + OstreeRepo *repo, + DnfPackage *pkg, + RpmOstreeImporterFlags flags, + OstreeSePolicy *sepolicy, + GError **error) { RpmOstreeImporter *ret = NULL; g_auto(Header) hdr = NULL; @@ -248,15 +255,17 @@ rpmostree_importer_new_fd (int fd, struct archive *archive; gsize cpio_offset; - archive = rpmostree_unpack_rpm2cpio (fd, error); + archive = rpmostree_unpack_rpm2cpio (*fd, error); if (archive == NULL) goto out; - if (!rpmostree_importer_read_metainfo (fd, &hdr, &cpio_offset, &fi, error)) + if (!rpmostree_importer_read_metainfo (*fd, &hdr, &cpio_offset, &fi, error)) goto out; ret = g_object_new (RPMOSTREE_TYPE_IMPORTER, NULL); - ret->fd = fd; + ret->fd = glnx_steal_fd (fd); + ret->repo = g_object_ref (repo); + ret->sepolicy = sepolicy ? g_object_ref (sepolicy) : NULL; ret->fi = g_steal_pointer (&fi); ret->archive = g_steal_pointer (&archive); ret->flags = flags; @@ -278,38 +287,6 @@ rpmostree_importer_new_fd (int fd, return ret; } -/* - * rpmostree_importer_new_at: - * @dfd: Fd - * @path: Path - * @pkg: (optional): Package reference, used for metadata - * @flags: flags - * @error: error - * - * Create a new unpacker instance. The @pkg argument, if - * specified, will be inspected and metadata such as the - * origin repo will be added to the final commit. - */ -RpmOstreeImporter * -rpmostree_importer_new_at (int dfd, const char *path, - DnfPackage *pkg, - RpmOstreeImporterFlags flags, - GError **error) -{ - glnx_autofd int fd = -1; - if (!glnx_openat_rdonly (dfd, path, TRUE, &fd, error)) - return FALSE; - - RpmOstreeImporter *ret = rpmostree_importer_new_fd (fd, pkg, flags, error); - if (ret == NULL) - return NULL; - - ret->owns_fd = TRUE; - fd = -1; - - return g_steal_pointer (&ret); -} - void rpmostree_importer_set_jigdo_mode (RpmOstreeImporter *self, GVariant *xattr_table, @@ -410,7 +387,6 @@ repo_metadata_for_package (DnfRepo *repo) static gboolean build_metadata_variant (RpmOstreeImporter *self, - OstreeSePolicy *sepolicy, GVariant **out_variant, GCancellable *cancellable, GError **error) @@ -460,10 +436,10 @@ build_metadata_variant (RpmOstreeImporter *self, /* The current sepolicy that was used to label the unpacked files is important * to record. It will help us during future overlays to determine whether the * files should be relabeled. */ - if (sepolicy) + if (self->sepolicy) g_variant_builder_add (&metadata_builder, "{sv}", "rpmostree.sepolicy", g_variant_new_string - (ostree_sepolicy_get_csum (sepolicy))); + (ostree_sepolicy_get_csum (self->sepolicy))); /* let's be nice to our future selves just in case */ g_variant_builder_add (&metadata_builder, "{sv}", "rpmostree.unpack_version", @@ -788,12 +764,11 @@ handle_translate_pathname (OstreeRepo *repo, static gboolean import_rpm_to_repo (RpmOstreeImporter *self, - OstreeRepo *repo, - OstreeSePolicy *sepolicy, char **out_csum, GCancellable *cancellable, GError **error) { + OstreeRepo *repo = self->repo; /* Passed to the commit modifier */ GError *cb_error = NULL; cb_data fdata = { self, &cb_error }; @@ -821,12 +796,12 @@ import_rpm_to_repo (RpmOstreeImporter *self, { ostree_repo_commit_modifier_set_xattr_callback (modifier, jigdo_xattr_cb, NULL, self); - g_assert (sepolicy == NULL); + g_assert (self->sepolicy == NULL); } else { ostree_repo_commit_modifier_set_xattr_callback (modifier, xattr_cb, NULL, self); - ostree_repo_commit_modifier_set_sepolicy (modifier, sepolicy); + ostree_repo_commit_modifier_set_sepolicy (modifier, self->sepolicy); } OstreeRepoImportArchiveOptions opts = { 0 }; @@ -883,7 +858,7 @@ import_rpm_to_repo (RpmOstreeImporter *self, return FALSE; g_autoptr(GVariant) metadata = NULL; - if (!build_metadata_variant (self, sepolicy, &metadata, cancellable, error)) + if (!build_metadata_variant (self, &metadata, cancellable, error)) return FALSE; g_variant_ref_sink (metadata); @@ -903,18 +878,16 @@ import_rpm_to_repo (RpmOstreeImporter *self, gboolean rpmostree_importer_run (RpmOstreeImporter *self, - OstreeRepo *repo, - OstreeSePolicy *sepolicy, char **out_csum, GCancellable *cancellable, GError **error) { g_autofree char *csum = NULL; - if (!import_rpm_to_repo (self, repo, sepolicy, &csum, cancellable, error)) + if (!import_rpm_to_repo (self, &csum, cancellable, error)) return FALSE; const char *branch = rpmostree_importer_get_ostree_branch (self); - ostree_repo_transaction_set_ref (repo, NULL, branch, csum); + ostree_repo_transaction_set_ref (self->repo, NULL, branch, csum); if (out_csum) *out_csum = g_steal_pointer (&csum); diff --git a/src/libpriv/rpmostree-importer.h b/src/libpriv/rpmostree-importer.h index b94a4adc..65af301d 100644 --- a/src/libpriv/rpmostree-importer.h +++ b/src/libpriv/rpmostree-importer.h @@ -47,17 +47,12 @@ typedef enum { } RpmOstreeImporterFlags; RpmOstreeImporter* -rpmostree_importer_new_fd (int fd, - DnfPackage *pkg, - RpmOstreeImporterFlags flags, - GError **error); - -RpmOstreeImporter* -rpmostree_importer_new_at (int dfd, - const char *path, - DnfPackage *pkg, /* for metadata */ - RpmOstreeImporterFlags flags, - GError **error); +rpmostree_importer_new_take_fd (int *fd, + OstreeRepo *repo, + DnfPackage *pkg, + RpmOstreeImporterFlags flags, + OstreeSePolicy *sepolicy, + GError **error); void rpmostree_importer_set_jigdo_mode (RpmOstreeImporter *self, GVariant *xattr_table, @@ -75,8 +70,6 @@ rpmostree_importer_get_ostree_branch (RpmOstreeImporter *unpacker); gboolean rpmostree_importer_run (RpmOstreeImporter *unpacker, - OstreeRepo *repo, - OstreeSePolicy *sepolicy, char **out_commit, GCancellable *cancellable, GError **error); diff --git a/tests/check/test-utils.c b/tests/check/test-utils.c index bdbf7e42..8b8164d6 100644 --- a/tests/check/test-utils.c +++ b/tests/check/test-utils.c @@ -142,11 +142,14 @@ test_variant_to_nevra(void) g_autoptr(RpmOstreeImporter) importer = NULL; g_autofree char *foo_rpm = g_strdup_printf ("yumrepo/packages/%s/%s.rpm", arch, nevra); - importer = rpmostree_importer_new_at (AT_FDCWD, foo_rpm, NULL, 0, &error); + glnx_autofd int foo_fd = -1; + glnx_openat_rdonly (AT_FDCWD, foo_rpm, TRUE, &foo_fd, &error); + g_assert_no_error (error); + importer = rpmostree_importer_new_take_fd (&foo_fd, repo, NULL, 0, NULL, &error); g_assert_no_error (error); g_assert (importer); - ret = rpmostree_importer_run (importer, repo, NULL, NULL, NULL, &error); + ret = rpmostree_importer_run (importer, NULL, NULL, &error); g_assert_no_error (error); g_assert (ret);