Skip to content

Commit

Permalink
importer: Rework API
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@cgwalters
cgwalters committed Dec 1, 2017
1 parent 389ab0f commit 32678fc
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 82 deletions.
37 changes: 29 additions & 8 deletions src/daemon/rpmostreed-transaction-types.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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);
Expand All @@ -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,
Expand All @@ -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));
Expand Down
8 changes: 4 additions & 4 deletions src/libpriv/rpmostree-core.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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));

Expand Down
83 changes: 28 additions & 55 deletions src/libpriv/rpmostree-importer.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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 ("");
Expand Down Expand Up @@ -226,37 +229,43 @@ 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
* specified, will be inspected and metadata such as the
* 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;
rpmfi fi = NULL;
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;
Expand All @@ -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,
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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 };
Expand Down Expand Up @@ -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 };
Expand Down Expand Up @@ -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);

Expand All @@ -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);
Expand Down
19 changes: 6 additions & 13 deletions src/libpriv/rpmostree-importer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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);
Expand Down
7 changes: 5 additions & 2 deletions tests/check/test-utils.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down

0 comments on commit 32678fc

Please sign in to comment.