Skip to content

Commit

Permalink
Update libglnx
Browse files Browse the repository at this point in the history
Update libglnx, which is mostly port the repo stagedir code
to the new tmpdir API.  This turned out to require some
libglnx changes to support de-allocating the tmpdir ref while
still maintaining the on-disk dir.

Update submodule: libglnx

Closes: #1172
Approved by: jlebon
  • Loading branch information
cgwalters authored and rh-atomic-bot committed Sep 18, 2017
1 parent b39a61b commit d0b0578
Show file tree
Hide file tree
Showing 6 changed files with 62 additions and 98 deletions.
2 changes: 1 addition & 1 deletion libglnx
Submodule libglnx updated from e226cc to e5856c
39 changes: 12 additions & 27 deletions src/libostree/ostree-repo-commit.c
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ _ostree_repo_commit_tmpf_final (OstreeRepo *self,

int dest_dfd;
if (self->in_transaction)
dest_dfd = self->commit_stagedir_fd;
dest_dfd = self->commit_stagedir.fd;
else
dest_dfd = self->objects_dir_fd;

Expand Down Expand Up @@ -173,7 +173,7 @@ _ostree_repo_commit_path_final (OstreeRepo *self,

int dest_dfd;
if (self->in_transaction)
dest_dfd = self->commit_stagedir_fd;
dest_dfd = self->commit_stagedir.fd;
else
dest_dfd = self->objects_dir_fd;

Expand Down Expand Up @@ -1114,8 +1114,7 @@ ostree_repo_prepare_transaction (OstreeRepo *self,
gboolean ret_transaction_resume = FALSE;
if (!_ostree_repo_allocate_tmpdir (self->tmp_dir_fd,
self->stagedir_prefix,
&self->commit_stagedir_name,
&self->commit_stagedir_fd,
&self->commit_stagedir,
&self->commit_stagedir_lock,
&ret_transaction_resume,
cancellable, error))
Expand All @@ -1134,7 +1133,7 @@ rename_pending_loose_objects (OstreeRepo *self,
GLNX_AUTO_PREFIX_ERROR ("rename pending", error);
g_auto(GLnxDirFdIterator) dfd_iter = { 0, };

if (!glnx_dirfd_iterator_init_at (self->commit_stagedir_fd, ".", FALSE, &dfd_iter, error))
if (!glnx_dirfd_iterator_init_at (self->commit_stagedir.fd, ".", FALSE, &dfd_iter, error))
return FALSE;

/* Iterate over the outer checksum dir */
Expand Down Expand Up @@ -1212,8 +1211,7 @@ rename_pending_loose_objects (OstreeRepo *self,
if (fsync (self->objects_dir_fd) == -1)
return glnx_throw_errno_prefix (error, "fsync");

if (!glnx_shutil_rm_rf_at (self->tmp_dir_fd, self->commit_stagedir_name,
cancellable, error))
if (!glnx_tmpdir_delete (&self->commit_stagedir, cancellable, error))
return FALSE;

return TRUE;
Expand Down Expand Up @@ -1549,15 +1547,8 @@ ostree_repo_commit_transaction (OstreeRepo *self,
return FALSE;
g_clear_pointer (&self->txn_collection_refs, g_hash_table_destroy);

if (self->commit_stagedir_fd != -1)
{
(void) close (self->commit_stagedir_fd);
self->commit_stagedir_fd = -1;

glnx_release_lock_file (&self->commit_stagedir_lock);
}

g_clear_pointer (&self->commit_stagedir_name, g_free);
glnx_tmpdir_unset (&self->commit_stagedir);
glnx_release_lock_file (&self->commit_stagedir_lock);

self->in_transaction = FALSE;

Expand Down Expand Up @@ -1588,14 +1579,8 @@ ostree_repo_abort_transaction (OstreeRepo *self,
g_clear_pointer (&self->txn_refs, g_hash_table_destroy);
g_clear_pointer (&self->txn_collection_refs, g_hash_table_destroy);

if (self->commit_stagedir_fd != -1)
{
(void) close (self->commit_stagedir_fd);
self->commit_stagedir_fd = -1;

glnx_release_lock_file (&self->commit_stagedir_lock);
}
g_clear_pointer (&self->commit_stagedir_name, g_free);
glnx_tmpdir_unset (&self->commit_stagedir);
glnx_release_lock_file (&self->commit_stagedir_lock);

self->in_transaction = FALSE;

Expand Down Expand Up @@ -2181,8 +2166,8 @@ ostree_repo_read_commit_detached_metadata (OstreeRepo *self,
_ostree_loose_path (buf, checksum, OSTREE_OBJECT_TYPE_COMMIT_META, self->mode);

g_autoptr(GVariant) ret_metadata = NULL;
if (self->commit_stagedir_fd != -1 &&
!ot_util_variant_map_at (self->commit_stagedir_fd, buf,
if (self->commit_stagedir.initialized &&
!ot_util_variant_map_at (self->commit_stagedir.fd, buf,
G_VARIANT_TYPE ("a{sv}"),
OT_VARIANT_MAP_ALLOW_NOENT | OT_VARIANT_MAP_TRUSTED, &ret_metadata, error))
return glnx_prefix_error (error, "Unable to read existing detached metadata");
Expand Down Expand Up @@ -2229,7 +2214,7 @@ ostree_repo_write_commit_detached_metadata (OstreeRepo *self,
int dest_dfd;

if (self->in_transaction)
dest_dfd = self->commit_stagedir_fd;
dest_dfd = self->commit_stagedir.fd;
else
dest_dfd = self->objects_dir_fd;

Expand Down
6 changes: 2 additions & 4 deletions src/libostree/ostree-repo-private.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,7 @@ struct OstreeRepo {
GObject parent;

char *stagedir_prefix;
int commit_stagedir_fd;
char *commit_stagedir_name;
GLnxTmpDir commit_stagedir;
GLnxLockFile commit_stagedir_lock;

/* A cached fd-relative version, distinct from the case where we may have a
Expand Down Expand Up @@ -209,8 +208,7 @@ G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(OstreeRepoMemoryCacheRef, _ostree_repo_memory_c
gboolean
_ostree_repo_allocate_tmpdir (int tmpdir_dfd,
const char *tmpdir_prefix,
char **tmpdir_name_out,
int *tmpdir_fd_out,
GLnxTmpDir *tmpdir_out,
GLnxLockFile *file_lock_out,
gboolean * reusing_dir_out,
GCancellable *cancellable,
Expand Down
71 changes: 32 additions & 39 deletions src/libostree/ostree-repo.c
Original file line number Diff line number Diff line change
Expand Up @@ -487,9 +487,7 @@ ostree_repo_finalize (GObject *object)
g_clear_object (&self->repodir);
if (self->repo_dir_fd != -1)
(void) close (self->repo_dir_fd);
if (self->commit_stagedir_fd != -1)
(void) close (self->commit_stagedir_fd);
g_free (self->commit_stagedir_name);
glnx_tmpdir_unset (&self->commit_stagedir);
glnx_release_lock_file (&self->commit_stagedir_lock);
if (self->tmp_dir_fd != -1)
(void) close (self->tmp_dir_fd);
Expand Down Expand Up @@ -687,7 +685,6 @@ ostree_repo_init (OstreeRepo *self)
self->repo_dir_fd = -1;
self->cache_dir_fd = -1;
self->tmp_dir_fd = -1;
self->commit_stagedir_fd = -1;
self->objects_dir_fd = -1;
self->uncompressed_objects_dir_fd = -1;
self->commit_stagedir_lock = empty_lockfile;
Expand Down Expand Up @@ -2793,9 +2790,9 @@ load_metadata_internal (OstreeRepo *self,
error))
return FALSE;

if (fd < 0 && self->commit_stagedir_fd != -1)
if (fd < 0 && self->commit_stagedir.initialized)
{
if (!ot_openat_ignore_enoent (self->commit_stagedir_fd, loose_path_buf, &fd,
if (!ot_openat_ignore_enoent (self->commit_stagedir.fd, loose_path_buf, &fd,
error))
return FALSE;
}
Expand Down Expand Up @@ -2906,9 +2903,9 @@ repo_load_file_archive (OstreeRepo *self,
error))
return FALSE;

if (fd < 0 && self->commit_stagedir_fd != -1)
if (fd < 0 && self->commit_stagedir.initialized)
{
if (!ot_openat_ignore_enoent (self->commit_stagedir_fd, loose_path_buf, &fd,
if (!ot_openat_ignore_enoent (self->commit_stagedir.fd, loose_path_buf, &fd,
error))
return FALSE;
}
Expand Down Expand Up @@ -2967,9 +2964,9 @@ _ostree_repo_load_file_bare (OstreeRepo *self,
int objdir_fd = self->objects_dir_fd;
int res;
if ((res = TEMP_FAILURE_RETRY (fstatat (objdir_fd, loose_path_buf, &stbuf, AT_SYMLINK_NOFOLLOW))) < 0
&& errno == ENOENT && self->commit_stagedir_fd != -1)
&& errno == ENOENT && self->commit_stagedir.initialized)
{
objdir_fd = self->commit_stagedir_fd;
objdir_fd = self->commit_stagedir.fd;
res = TEMP_FAILURE_RETRY (fstatat (objdir_fd, loose_path_buf, &stbuf, AT_SYMLINK_NOFOLLOW));
}
if (res < 0 && errno != ENOENT)
Expand Down Expand Up @@ -3214,7 +3211,9 @@ _ostree_repo_has_loose_object (OstreeRepo *self,

gboolean found = FALSE;
/* It's easier to share code if we make this an array */
const int dfd_searches[] = { self->commit_stagedir_fd, self->objects_dir_fd };
int dfd_searches[] = { -1, self->objects_dir_fd };
if (self->commit_stagedir.initialized)
dfd_searches[0] = self->commit_stagedir.fd;
for (guint i = 0; i < G_N_ELEMENTS (dfd_searches); i++)
{
int dfd = dfd_searches[i];
Expand Down Expand Up @@ -3654,8 +3653,8 @@ ostree_repo_query_object_storage_size (OstreeRepo *self,

struct stat stbuf;
res = TEMP_FAILURE_RETRY (fstatat (self->objects_dir_fd, loose_path, &stbuf, AT_SYMLINK_NOFOLLOW));
if (res < 0 && errno == ENOENT && self->commit_stagedir_fd != -1)
res = TEMP_FAILURE_RETRY (fstatat (self->commit_stagedir_fd, loose_path, &stbuf, AT_SYMLINK_NOFOLLOW));
if (res < 0 && errno == ENOENT && self->commit_stagedir.initialized)
res = TEMP_FAILURE_RETRY (fstatat (self->commit_stagedir.fd, loose_path, &stbuf, AT_SYMLINK_NOFOLLOW));

if (res < 0)
return glnx_throw_errno_prefix (error, "Querying object %s.%s", sha256, ostree_object_type_to_string (objtype));
Expand Down Expand Up @@ -5104,8 +5103,7 @@ _ostree_repo_try_lock_tmpdir (int tmpdir_dfd,
gboolean
_ostree_repo_allocate_tmpdir (int tmpdir_dfd,
const char *tmpdir_prefix,
char **tmpdir_name_out,
int *tmpdir_fd_out,
GLnxTmpDir *tmpdir_out,
GLnxLockFile *file_lock_out,
gboolean *reusing_dir_out,
GCancellable *cancellable,
Expand All @@ -5120,9 +5118,8 @@ _ostree_repo_allocate_tmpdir (int tmpdir_dfd,

gboolean reusing_dir = FALSE;
gboolean did_lock = FALSE;
g_autofree char *tmpdir_name = NULL;
glnx_fd_close int tmpdir_fd = -1;
while (tmpdir_name == NULL)
g_auto(GLnxTmpDir) ret_tmpdir = { 0, };
while (!ret_tmpdir.initialized)
{
struct dirent *dent;
glnx_fd_close int existing_tmpdir_fd = -1;
Expand All @@ -5142,8 +5139,9 @@ _ostree_repo_allocate_tmpdir (int tmpdir_dfd,
dent->d_type != DT_DIR)
continue;

glnx_fd_close int target_dfd = -1;
if (!glnx_opendirat (dfd_iter.fd, dent->d_name, FALSE,
&existing_tmpdir_fd, &local_error))
&target_dfd, &local_error))
{
if (g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_NOT_DIRECTORY))
continue;
Expand All @@ -5166,24 +5164,22 @@ _ostree_repo_allocate_tmpdir (int tmpdir_dfd,
/* Touch the reused directory so that we don't accidentally
* remove it due to being old when cleaning up the tmpdir.
*/
(void)futimens (existing_tmpdir_fd, NULL);
(void)futimens (target_dfd, NULL);

/* We found an existing tmpdir which we managed to lock */
tmpdir_name = g_strdup (dent->d_name);
tmpdir_fd = glnx_steal_fd (&existing_tmpdir_fd);
reusing_dir = TRUE;
ret_tmpdir.src_dfd = tmpdir_dfd;
ret_tmpdir.fd = glnx_steal_fd (&target_dfd);
ret_tmpdir.path = g_strdup (dent->d_name);
ret_tmpdir.initialized = TRUE;
}

while (tmpdir_name == NULL)
while (!ret_tmpdir.initialized)
{
g_auto(GLnxTmpDir) new_tmpdir = { 0, };
/* No existing tmpdir found, create a new */
g_autofree char *tmpdir_name_template = g_strconcat (tmpdir_prefix, "XXXXXX", NULL);
if (!glnx_mkdtempat (tmpdir_dfd, tmpdir_name_template, 0777, error))
return FALSE;

glnx_fd_close int new_tmpdir_fd = -1;
if (!glnx_opendirat (tmpdir_dfd, tmpdir_name_template, FALSE,
&new_tmpdir_fd, error))
const char *tmpdir_name_template = glnx_strjoina (tmpdir_prefix, "XXXXXX");
if (!glnx_mkdtempat (tmpdir_dfd, tmpdir_name_template, 0755,
&new_tmpdir, error))
return FALSE;

/* Note, at this point we can race with another process that picks up this
Expand All @@ -5195,16 +5191,13 @@ _ostree_repo_allocate_tmpdir (int tmpdir_dfd,
if (!did_lock)
continue;

tmpdir_name = g_steal_pointer (&tmpdir_name_template);
tmpdir_fd = glnx_steal_fd (&new_tmpdir_fd);
ret_tmpdir = new_tmpdir; /* Transfer ownership */
new_tmpdir.initialized = FALSE;
}

if (tmpdir_name_out)
*tmpdir_name_out = g_steal_pointer (&tmpdir_name);
if (tmpdir_fd_out)
*tmpdir_fd_out = glnx_steal_fd (&tmpdir_fd);
if (reusing_dir_out)
*reusing_dir_out = reusing_dir;
*tmpdir_out = ret_tmpdir; /* Transfer ownership */
ret_tmpdir.initialized = FALSE;
*reusing_dir_out = reusing_dir;
return TRUE;
}

Expand Down
20 changes: 7 additions & 13 deletions tests/test-repo-finder-config.c
Original file line number Diff line number Diff line change
Expand Up @@ -39,28 +39,25 @@
typedef struct
{
OstreeRepo *parent_repo; /* owned */
int working_dfd; /* owned */
GLnxTmpDir tmpdir; /* owned */
GFile *working_dir; /* owned */
} Fixture;

static void
setup (Fixture *fixture,
gconstpointer test_data)
{
g_autofree gchar *tmp_name = NULL;
g_autoptr(GError) error = NULL;

tmp_name = g_strdup ("test-repo-finder-config-XXXXXX");
glnx_mkdtempat_open_in_system (tmp_name, 0700, &fixture->working_dfd, &error);
(void)glnx_mkdtemp ("test-repo-finder-config-XXXXXX", 0700, &fixture->tmpdir, &error);
g_assert_no_error (error);

g_test_message ("Using temporary directory: %s", tmp_name);
g_test_message ("Using temporary directory: %s", fixture->tmpdir.path);

glnx_shutil_mkdir_p_at (fixture->working_dfd, "repo", 0700, NULL, &error);
glnx_shutil_mkdir_p_at (fixture->tmpdir.fd, "repo", 0700, NULL, &error);
g_assert_no_error (error);

g_autoptr(GFile) tmp_dir = g_file_new_for_path (g_get_tmp_dir ());
fixture->working_dir = g_file_get_child (tmp_dir, tmp_name);
fixture->working_dir = g_file_new_for_path (fixture->tmpdir.path);

fixture->parent_repo = ot_test_setup_repo (NULL, &error);
g_assert_no_error (error);
Expand All @@ -73,10 +70,7 @@ teardown (Fixture *fixture,
g_autoptr(GError) error = NULL;

/* Recursively remove the temporary directory. */
glnx_shutil_rm_rf_at (fixture->working_dfd, ".", NULL, NULL);

close (fixture->working_dfd);
fixture->working_dfd = -1;
(void)glnx_tmpdir_delete (&fixture->tmpdir, NULL, NULL);

/* The repo also needs its source files to be removed. This is the inverse
* of setup_test_repository() in libtest.sh. */
Expand Down Expand Up @@ -177,7 +171,7 @@ assert_create_remote (Fixture *fixture,
g_autoptr(GError) error = NULL;
const gchar *repo_name = (collection_id != NULL) ? collection_id : "no-collection";

glnx_shutil_mkdir_p_at (fixture->working_dfd, repo_name, 0700, NULL, &error);
glnx_shutil_mkdir_p_at (fixture->tmpdir.fd, repo_name, 0700, NULL, &error);
g_assert_no_error (error);

g_autoptr(GFile) repo_path = g_file_get_child (fixture->working_dir, repo_name);
Expand Down
Loading

0 comments on commit d0b0578

Please sign in to comment.