Skip to content

Commit

Permalink
lib/pull: Change fetcher to return O_TMPFILE
Browse files Browse the repository at this point in the history
A lot of the libostree code is honestly too complex for its
own good (this is mostly my fault).  The way we do HTTP writes
is still one of those.  The way the fetcher writes tempfiles,
then reads them back in is definitely one of those.

Now that we've dropped the "partial object" bits in:
#1176 i.e. commit
0488b48
we can simplify things a lot more by having the fetcher
return an `O_TMPFILE` rather than a filename.

For trusted archive mirroring, we need to enable linking
in the tmpfiles directly.

Otherwise for at least content objects they're compressed, so we couldn't link
them in. For metadata, we need to do similar logic to what we have around
`mmap()` to only grab a tmpfile if the size is large enough.

Closes: #1252
Approved by: jlebon
  • Loading branch information
cgwalters authored and rh-atomic-bot committed Oct 5, 2017
1 parent 7f6af94 commit 2e3889a
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 107 deletions.
35 changes: 12 additions & 23 deletions src/libostree/ostree-fetcher-curl.c
Original file line number Diff line number Diff line change
Expand Up @@ -269,13 +269,13 @@ ensure_tmpfile (FetcherRequest *req, GError **error)
{
if (!req->tmpf.initialized)
{
if (!glnx_open_tmpfile_linkable_at (req->fetcher->tmpdir_dfd, ".",
O_WRONLY | O_CLOEXEC, &req->tmpf,
error))
if (!_ostree_fetcher_tmpf_from_flags (req->flags, req->fetcher->tmpdir_dfd,
&req->tmpf, error))
return FALSE;
}
return TRUE;
}

/* Check for completed transfers, and remove their easy handles */
static void
check_multi_info (OstreeFetcher *fetcher)
Expand Down Expand Up @@ -378,25 +378,19 @@ check_multi_info (OstreeFetcher *fetcher)
g_autoptr(GError) local_error = NULL;
GError **error = &local_error;

g_autofree char *tmpfile_path =
ostree_fetcher_generate_url_tmpname (eff_url);
if (!ensure_tmpfile (req, error))
{
g_task_return_error (task, g_steal_pointer (&local_error));
}
/* This should match the libsoup chmod */
else if (fchmod (req->tmpf.fd, 0644) < 0)
else if (lseek (req->tmpf.fd, 0, SEEK_SET) < 0)
{
glnx_set_error_from_errno (error);
g_task_return_error (task, g_steal_pointer (&local_error));
}
else if (!glnx_link_tmpfile_at (&req->tmpf, GLNX_LINK_TMPFILE_REPLACE,
fetcher->tmpdir_dfd, tmpfile_path,
error))
g_task_return_error (task, g_steal_pointer (&local_error));
else
{
g_task_return_pointer (task, g_steal_pointer (&tmpfile_path), g_free);
/* We return the tmpfile in the _finish wrapper */
g_task_return_boolean (task, TRUE);
}
}
}
Expand Down Expand Up @@ -887,26 +881,21 @@ _ostree_fetcher_request_to_tmpfile (OstreeFetcher *self,
gboolean
_ostree_fetcher_request_to_tmpfile_finish (OstreeFetcher *self,
GAsyncResult *result,
char **out_filename,
GLnxTmpfile *out_tmpf,
GError **error)
{
GTask *task;
FetcherRequest *req;
gpointer ret;

g_return_val_if_fail (g_task_is_valid (result, self), FALSE);
g_return_val_if_fail (g_async_result_is_tagged (result, _ostree_fetcher_request_async), FALSE);

task = (GTask*)result;
req = g_task_get_task_data (task);
GTask *task = (GTask*)result;
FetcherRequest *req = g_task_get_task_data (task);

ret = g_task_propagate_pointer (task, error);
if (!ret)
if (!g_task_propagate_boolean (task, error))
return FALSE;

g_assert (!req->is_membuf);
g_assert (out_filename);
*out_filename = ret;
*out_tmpf = req->tmpf;
req->tmpf.initialized = FALSE; /* Transfer ownership */

return TRUE;
}
Expand Down
30 changes: 11 additions & 19 deletions src/libostree/ostree-fetcher-soup.c
Original file line number Diff line number Diff line change
Expand Up @@ -905,11 +905,8 @@ on_stream_read (GObject *object,
{
if (!pending->is_membuf)
{
if (!glnx_open_tmpfile_linkable_at (pending->thread_closure->base_tmpdir_dfd, ".",
O_WRONLY | O_CLOEXEC, &pending->tmpf, &local_error))
goto out;
/* This should match the libcurl chmod */
if (!glnx_fchmod (pending->tmpf.fd, 0644, &local_error))
if (!_ostree_fetcher_tmpf_from_flags (pending->flags, pending->thread_closure->base_tmpdir_dfd,
&pending->tmpf, &local_error))
goto out;
pending->out_stream = g_unix_output_stream_new (pending->tmpf.fd, FALSE);
}
Expand Down Expand Up @@ -943,18 +940,13 @@ on_stream_read (GObject *object,
}
else
{
g_autofree char *uristring =
soup_uri_to_string (soup_request_get_uri (pending->request), FALSE);
g_autofree char *tmpfile_path =
ostree_fetcher_generate_url_tmpname (uristring);
if (!glnx_link_tmpfile_at (&pending->tmpf, GLNX_LINK_TMPFILE_REPLACE,
pending->thread_closure->base_tmpdir_dfd, tmpfile_path,
&local_error))
g_task_return_error (task, g_steal_pointer (&local_error));
if (lseek (pending->tmpf.fd, 0, SEEK_SET) < 0)
{
glnx_set_error_from_errno (&local_error);
g_task_return_error (task, g_steal_pointer (&local_error));
}
else
g_task_return_pointer (task,
g_steal_pointer (&tmpfile_path),
(GDestroyNotify) g_free);
g_task_return_boolean (task, TRUE);
}
remove_pending (pending);
}
Expand Down Expand Up @@ -1174,7 +1166,7 @@ _ostree_fetcher_request_to_tmpfile (OstreeFetcher *self,
gboolean
_ostree_fetcher_request_to_tmpfile_finish (OstreeFetcher *self,
GAsyncResult *result,
char **out_filename,
GLnxTmpfile *out_tmpf,
GError **error)
{
GTask *task;
Expand All @@ -1192,8 +1184,8 @@ _ostree_fetcher_request_to_tmpfile_finish (OstreeFetcher *self,
return FALSE;

g_assert (!pending->is_membuf);
g_assert (out_filename);
*out_filename = ret;
*out_tmpf = pending->tmpf;
pending->tmpf.initialized = FALSE; /* Transfer ownership */

return TRUE;
}
Expand Down
23 changes: 16 additions & 7 deletions src/libostree/ostree-fetcher-util.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,23 @@

G_BEGIN_DECLS

/* FIXME - delete this and replace by having fetchers simply
* return O_TMPFILE fds, not file paths.
*/
static inline char *
ostree_fetcher_generate_url_tmpname (const char *url)
static inline gboolean
_ostree_fetcher_tmpf_from_flags (OstreeFetcherRequestFlags flags,
int dfd,
GLnxTmpfile *tmpf,
GError **error)
{
return g_compute_checksum_for_string (G_CHECKSUM_SHA256,
url, strlen (url));
if ((flags & OSTREE_FETCHER_REQUEST_LINKABLE) > 0)
{
if (!glnx_open_tmpfile_linkable_at (dfd, ".", O_RDWR | O_CLOEXEC, tmpf, error))
return FALSE;
}
else if (!glnx_open_anonymous_tmpfile (O_RDWR | O_CLOEXEC, tmpf, error))
return FALSE;

if (!glnx_fchmod (tmpf->fd, 0644, error))
return FALSE;
return TRUE;
}

gboolean _ostree_fetcher_mirrored_request_to_membuf (OstreeFetcher *fetcher,
Expand Down
5 changes: 3 additions & 2 deletions src/libostree/ostree-fetcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ typedef enum {

typedef enum {
OSTREE_FETCHER_REQUEST_NUL_TERMINATION = (1 << 0),
OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT = (1 << 1)
OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT = (1 << 1),
OSTREE_FETCHER_REQUEST_LINKABLE = (1 << 2),
} OstreeFetcherRequestFlags;

void
Expand Down Expand Up @@ -124,7 +125,7 @@ void _ostree_fetcher_request_to_tmpfile (OstreeFetcher *self,

gboolean _ostree_fetcher_request_to_tmpfile_finish (OstreeFetcher *self,
GAsyncResult *result,
char **out_filename,
GLnxTmpfile *out_tmpf,
GError **error);

void _ostree_fetcher_request_to_membuf (OstreeFetcher *self,
Expand Down
Loading

0 comments on commit 2e3889a

Please sign in to comment.