From 96eed957205c5a318c2849100778d48aad748469 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Fri, 11 Dec 2015 19:30:20 +0100 Subject: [PATCH] repo: Allocate a tmpdir for each OstreeFetcher to isolate concurrent downloads This way two pulls will not use the same tmpdir and accidentally overwrite each other. However, consecutive OstreeFetchers will reuse the tmpdirs, so that we can properly resume downloading large objects. https://bugzilla.gnome.org/show_bug.cgi?id=757611 --- src/libostree/ostree-fetcher.c | 37 +++++++++++++++++++++++++++++--- src/libostree/ostree-fetcher.h | 6 ++++-- src/libostree/ostree-repo-pull.c | 32 +++++++++++++++------------ src/libostree/ostree-repo.c | 4 +++- 4 files changed, 59 insertions(+), 20 deletions(-) diff --git a/src/libostree/ostree-fetcher.c b/src/libostree/ostree-fetcher.c index 981f4f44..86ab26a6 100644 --- a/src/libostree/ostree-fetcher.c +++ b/src/libostree/ostree-fetcher.c @@ -30,6 +30,7 @@ #include "ostree-tls-cert-interaction.h" #endif #include "ostree.h" +#include "ostree-repo-private.h" #include "otutil.h" typedef enum { @@ -63,6 +64,9 @@ struct OstreeFetcher GObject parent_instance; int tmpdir_dfd; + char *tmpdir_name; + GLnxLockFile tmpdir_lock; + int base_tmpdir_dfd; GTlsCertificate *client_cert; @@ -114,6 +118,17 @@ _ostree_fetcher_finalize (GObject *object) self = OSTREE_FETCHER (object); + if (self->tmpdir_dfd != -1) + close (self->tmpdir_dfd); + + /* Note: We don't remove the tmpdir here, because that would cause + us to not reuse it on resume. This happens because we use two + fetchers for each pull, so finalizing the first one would remove + all the files to be resumed from the previous second one */ + + g_free (self->tmpdir_name); + glnx_release_lock_file (&self->tmpdir_lock); + g_clear_object (&self->session); g_clear_object (&self->client_cert); @@ -140,6 +155,7 @@ _ostree_fetcher_init (OstreeFetcher *self) { gint max_conns; const char *http_proxy; + GLnxLockFile empty_lockfile = GLNX_LOCK_FILE_INIT; g_queue_init (&self->pending_queue); self->session = soup_session_async_new_with_options (SOUP_SESSION_USER_AGENT, "ostree ", @@ -174,18 +190,33 @@ _ostree_fetcher_init (OstreeFetcher *self) self->output_stream_set = g_hash_table_new_full (NULL, NULL, NULL, (GDestroyNotify)g_object_unref); self->outstanding = g_hash_table_new_full (NULL, NULL, NULL, NULL); + + self->tmpdir_dfd = -1; + self->tmpdir_lock = empty_lockfile; + } OstreeFetcher * _ostree_fetcher_new (int tmpdir_dfd, - OstreeFetcherConfigFlags flags) + OstreeFetcherConfigFlags flags, + GCancellable *cancellable, + GError **error) { OstreeFetcher *self = (OstreeFetcher*)g_object_new (OSTREE_TYPE_FETCHER, NULL); - self->tmpdir_dfd = tmpdir_dfd; + if (!_ostree_repo_allocate_tmpdir (tmpdir_dfd, + "fetcher-", + &self->tmpdir_name, + &self->tmpdir_dfd, + &self->tmpdir_lock, + NULL, + cancellable, error)) + return NULL; + + self->base_tmpdir_dfd = tmpdir_dfd; if ((flags & OSTREE_FETCHER_FLAGS_TLS_PERMISSIVE) > 0) g_object_set ((GObject*)self->session, "ssl-strict", FALSE, NULL); - + return self; } diff --git a/src/libostree/ostree-fetcher.h b/src/libostree/ostree-fetcher.h index 577a4d62..f9c0e042 100644 --- a/src/libostree/ostree-fetcher.h +++ b/src/libostree/ostree-fetcher.h @@ -54,8 +54,10 @@ typedef enum { GType _ostree_fetcher_get_type (void) G_GNUC_CONST; -OstreeFetcher *_ostree_fetcher_new (int tmpdir_dfd, - OstreeFetcherConfigFlags flags); +OstreeFetcher *_ostree_fetcher_new (int tmpdir_dfd, + OstreeFetcherConfigFlags flags, + GCancellable *cancellable, + GError **error); int _ostree_fetcher_get_dfd (OstreeFetcher *fetcher); diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 57c36891..907ed454 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -646,6 +646,7 @@ content_fetch_on_complete (GObject *object, GAsyncResult *result, gpointer user_data) { + OstreeFetcher *fetcher = (OstreeFetcher *)object; FetchObjectData *fetch_data = user_data; OtPullData *pull_data = fetch_data->pull_data; GError *local_error = NULL; @@ -660,7 +661,7 @@ content_fetch_on_complete (GObject *object, const char *checksum; OstreeObjectType objtype; - temp_path = _ostree_fetcher_request_uri_with_partial_finish ((OstreeFetcher*)object, result, error); + temp_path = _ostree_fetcher_request_uri_with_partial_finish (fetcher, result, error); if (!temp_path) goto out; @@ -680,7 +681,7 @@ content_fetch_on_complete (GObject *object, if (!have_object) { if (!_ostree_repo_commit_loose_final (pull_data->repo, checksum, OSTREE_OBJECT_TYPE_FILE, - pull_data->tmpdir_dfd, temp_path, + _ostree_fetcher_get_dfd (fetcher), temp_path, cancellable, error)) goto out; } @@ -689,13 +690,14 @@ content_fetch_on_complete (GObject *object, else { /* Non-mirroring path */ - - if (!ostree_content_file_parse_at (TRUE, pull_data->tmpdir_dfd, temp_path, FALSE, + + if (!ostree_content_file_parse_at (TRUE, _ostree_fetcher_get_dfd (fetcher), + temp_path, FALSE, &file_in, &file_info, &xattrs, cancellable, error)) { /* If it appears corrupted, delete it */ - (void) unlinkat (pull_data->tmpdir_dfd, temp_path, 0); + (void) unlinkat (_ostree_fetcher_get_dfd (fetcher), temp_path, 0); goto out; } @@ -703,8 +705,8 @@ content_fetch_on_complete (GObject *object, * a reference to the fd. If we fail to write later, then * the temp space will be cleaned up. */ - (void) unlinkat (pull_data->tmpdir_dfd, temp_path, 0); - + (void) unlinkat (_ostree_fetcher_get_dfd (fetcher), temp_path, 0); + if (!ostree_raw_file_to_content_stream (file_in, file_info, xattrs, &object_input, &length, cancellable, error)) @@ -772,6 +774,7 @@ meta_fetch_on_complete (GObject *object, GAsyncResult *result, gpointer user_data) { + OstreeFetcher *fetcher = (OstreeFetcher *)object; FetchObjectData *fetch_data = user_data; OtPullData *pull_data = fetch_data->pull_data; g_autoptr(GVariant) metadata = NULL; @@ -786,7 +789,7 @@ meta_fetch_on_complete (GObject *object, g_debug ("fetch of %s%s complete", ostree_object_to_string (checksum, objtype), fetch_data->is_detached_meta ? " (detached)" : ""); - temp_path = _ostree_fetcher_request_uri_with_partial_finish ((OstreeFetcher*)object, result, error); + temp_path = _ostree_fetcher_request_uri_with_partial_finish (fetcher, result, error); if (!temp_path) { if (g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) @@ -823,7 +826,7 @@ meta_fetch_on_complete (GObject *object, if (objtype == OSTREE_OBJECT_TYPE_TOMBSTONE_COMMIT) goto out; - fd = openat (pull_data->tmpdir_dfd, temp_path, O_RDONLY | O_CLOEXEC); + fd = openat (_ostree_fetcher_get_dfd (fetcher), temp_path, O_RDONLY | O_CLOEXEC); if (fd == -1) { gs_set_error_from_errno (error, errno); @@ -837,7 +840,7 @@ meta_fetch_on_complete (GObject *object, goto out; /* Now delete it, see comment in corresponding content fetch path */ - (void) unlinkat (pull_data->tmpdir_dfd, temp_path, 0); + (void) unlinkat (_ostree_fetcher_get_dfd (fetcher), temp_path, 0); if (!ostree_repo_write_commit_detached_metadata (pull_data->repo, checksum, metadata, pull_data->cancellable, error)) @@ -852,7 +855,7 @@ meta_fetch_on_complete (GObject *object, FALSE, &metadata, error)) goto out; - (void) unlinkat (pull_data->tmpdir_dfd, temp_path, 0); + (void) unlinkat (_ostree_fetcher_get_dfd (fetcher), temp_path, 0); /* Write the commitpartial file now while we're still fetching data */ if (objtype == OSTREE_OBJECT_TYPE_COMMIT) @@ -926,6 +929,7 @@ static_deltapart_fetch_on_complete (GObject *object, GAsyncResult *result, gpointer user_data) { + OstreeFetcher *fetcher = (OstreeFetcher *)object; FetchStaticDeltaData *fetch_data = user_data; OtPullData *pull_data = fetch_data->pull_data; g_autoptr(GVariant) metadata = NULL; @@ -939,11 +943,11 @@ static_deltapart_fetch_on_complete (GObject *object, g_debug ("fetch static delta part %s complete", fetch_data->expected_checksum); - temp_path = _ostree_fetcher_request_uri_with_partial_finish ((OstreeFetcher*)object, result, error); + temp_path = _ostree_fetcher_request_uri_with_partial_finish (fetcher, result, error); if (!temp_path) goto out; - fd = openat (pull_data->tmpdir_dfd, temp_path, O_RDONLY | O_CLOEXEC); + fd = openat (_ostree_fetcher_get_dfd (fetcher), temp_path, O_RDONLY | O_CLOEXEC); if (fd == -1) { gs_set_error_from_errno (error, errno); @@ -982,7 +986,7 @@ static_deltapart_fetch_on_complete (GObject *object, * or error, the file will be gone. This is particularly * important if say we hit e.g. ENOSPC. */ - (void) unlinkat (pull_data->tmpdir_dfd, temp_path, 0); + (void) unlinkat (_ostree_fetcher_get_dfd (fetcher), temp_path, 0); _ostree_static_delta_part_execute_async (pull_data->repo, fetch_data->objects, diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 57d81c64..cc3bd6f7 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -419,7 +419,9 @@ _ostree_repo_remote_new_fetcher (OstreeRepo *self, if (tls_permissive) fetcher_flags |= OSTREE_FETCHER_FLAGS_TLS_PERMISSIVE; - fetcher = _ostree_fetcher_new (self->tmp_dir_fd, fetcher_flags); + fetcher = _ostree_fetcher_new (self->tmp_dir_fd, fetcher_flags, NULL, error); + if (fetcher == NULL) + goto out; { g_autofree char *tls_client_cert_path = NULL;