Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

checkout/commit: Use glnx_regfile_copy_bytes() if possible #817

Closed
wants to merge 1 commit into from
Closed

checkout/commit: Use glnx_regfile_copy_bytes() if possible #817

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion libglnx
Open in desktop
Submodule libglnx updated from 602fdd to 3a4d0f
44 changes: 28 additions & 16 deletions src/libostree/ostree-repo-checkout.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,48 +102,62 @@ fsync_is_enabled (OstreeRepo *self,
static gboolean
write_regular_file_content (OstreeRepo *self,
OstreeRepoCheckoutAtOptions *options,
GOutputStream *output,
int outfd,
GFileInfo *file_info,
GVariant *xattrs,
GInputStream *input,
GCancellable *cancellable,
GError **error)
{
const OstreeRepoCheckoutMode mode = options->mode;
g_autoptr(GOutputStream) outstream = NULL;

if (g_output_stream_splice (output, input, 0,
cancellable, error) < 0)
return FALSE;
if (G_IS_FILE_DESCRIPTOR_BASED (input))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this condition ever fail?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We expose as public API:

gboolean      ostree_repo_write_content (OstreeRepo       *self,
                                         const char       *expected_checksum,
                                         GInputStream     *object_input,
                                         guint64           length,
                                         guchar          **out_csum,
                                         GCancellable     *cancellable,
                                         GError          **error);

So we have to support it. In practice using the command line tools, we will go through this code path when pulling from an archive repo to a bare one, since the content objects are zlib-compressed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right, that makes sense.

{
int infd = g_file_descriptor_based_get_fd ((GFileDescriptorBased*) input);
guint64 len = g_file_info_get_size (file_info);

if (!g_output_stream_flush (output, cancellable, error))
return FALSE;
if (glnx_regfile_copy_bytes (infd, outfd, (off_t)len, TRUE) < 0)
return glnx_throw_errno_prefix (error, "regfile copy");
}
else
{
outstream = g_unix_output_stream_new (outfd, FALSE);
if (g_output_stream_splice (outstream, input, 0,
cancellable, error) < 0)
return FALSE;

int fd = g_file_descriptor_based_get_fd ((GFileDescriptorBased*)output);
if (!g_output_stream_flush (outstream, cancellable, error))
return FALSE;
}

if (mode != OSTREE_REPO_CHECKOUT_MODE_USER)
{
if (TEMP_FAILURE_RETRY (fchown (fd, g_file_info_get_attribute_uint32 (file_info, "unix::uid"),
if (TEMP_FAILURE_RETRY (fchown (outfd, g_file_info_get_attribute_uint32 (file_info, "unix::uid"),
g_file_info_get_attribute_uint32 (file_info, "unix::gid"))) < 0)
return glnx_throw_errno (error);

if (TEMP_FAILURE_RETRY (fchmod (fd, g_file_info_get_attribute_uint32 (file_info, "unix::mode"))) < 0)
if (TEMP_FAILURE_RETRY (fchmod (outfd, g_file_info_get_attribute_uint32 (file_info, "unix::mode"))) < 0)
return glnx_throw_errno (error);

if (xattrs)
{
if (!glnx_fd_set_all_xattrs (fd, xattrs, cancellable, error))
if (!glnx_fd_set_all_xattrs (outfd, xattrs, cancellable, error))
return FALSE;
}
}

if (fsync_is_enabled (self, options))
{
if (fsync (fd) == -1)
if (fsync (outfd) == -1)
return glnx_throw_errno (error);
}

if (!g_output_stream_close (output, cancellable, error))
return FALSE;
if (outstream)
{
if (!g_output_stream_close (outstream, cancellable, error))
return FALSE;
}

return TRUE;
}
Expand Down Expand Up @@ -231,7 +245,6 @@ create_file_copy_from_input_at (OstreeRepo *repo,
else if (g_file_info_get_file_type (file_info) == G_FILE_TYPE_REGULAR)
{
glnx_fd_close int temp_fd = -1;
g_autoptr(GOutputStream) temp_out = NULL;
guint32 file_mode;
GLnxLinkTmpfileReplaceMode replace_mode;

Expand All @@ -244,7 +257,6 @@ create_file_copy_from_input_at (OstreeRepo *repo,
&temp_fd, &temp_filename,
error))
return FALSE;
temp_out = g_unix_output_stream_new (temp_fd, FALSE);

if (sepolicy_enabled)
{
Expand All @@ -258,7 +270,7 @@ create_file_copy_from_input_at (OstreeRepo *repo,
return glnx_throw_errno_prefix (error, "Setting security.selinux");
}

if (!write_regular_file_content (repo, options, temp_out, file_info, xattrs, input,
if (!write_regular_file_content (repo, options, temp_fd, file_info, xattrs, input,
cancellable, error))
return FALSE;

Expand Down
61 changes: 28 additions & 33 deletions src/libostree/ostree-repo-commit.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -459,29 +459,19 @@ add_size_index_to_metadata (OstreeRepo *self,
}

static gboolean
fallocate_stream (GFileDescriptorBased *stream,
goffset size,
GCancellable *cancellable,
GError **error)
ot_fallocate (int fd, goffset size, GError **error)
{
gboolean ret = FALSE;
int fd = g_file_descriptor_based_get_fd (stream);
if (size == 0)
return TRUE;

if (size > 0)
int r = posix_fallocate (fd, 0, size);
if (r != 0)
{
int r = posix_fallocate (fd, 0, size);
if (r != 0)
{
/* posix_fallocate is a weird deviation from errno standards */
errno = r;
glnx_set_error_from_errno (error);
goto out;
}
/* posix_fallocate is a weird deviation from errno standards */
errno = r;
return glnx_throw_errno_prefix (error, "fallocate");
}

ret = TRUE;
out:
return ret;
return TRUE;
}

gboolean
Expand Down Expand Up @@ -511,11 +501,10 @@ _ostree_repo_open_content_bare (OstreeRepo *self,
&fd, &temp_filename, error))
goto out;

ret_stream = g_unix_output_stream_new (fd, TRUE);

if (!fallocate_stream ((GFileDescriptorBased*)ret_stream, content_len,
cancellable, error))
if (!ot_fallocate (fd, content_len, error))
goto out;

ret_stream = g_unix_output_stream_new (fd, TRUE);
}

ret = TRUE;
Expand Down Expand Up @@ -570,28 +559,34 @@ create_regular_tmpfile_linkable_with_content (OstreeRepo *self,
GCancellable *cancellable,
GError **error)
{
g_autoptr(GOutputStream) temp_out = NULL;
glnx_fd_close int temp_fd = -1;
g_autofree char *temp_filename = NULL;

if (!glnx_open_tmpfile_linkable_at (self->tmp_dir_fd, ".", O_WRONLY|O_CLOEXEC,
&temp_fd, &temp_filename,
error))
return FALSE;
temp_out = g_unix_output_stream_new (temp_fd, FALSE);

if (!fallocate_stream ((GFileDescriptorBased*)temp_out, length,
cancellable, error))
if (!ot_fallocate (temp_fd, length, error))
return FALSE;

if (g_output_stream_splice (temp_out, input, 0,
cancellable, error) < 0)
return FALSE;
if (fchmod (temp_fd, 0644) < 0)
if (G_IS_FILE_DESCRIPTOR_BASED (input))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here too. Seems like they're always fd-based, no? I'm not very familiar with GIO.

{
glnx_set_error_from_errno (error);
return FALSE;
int infd = g_file_descriptor_based_get_fd ((GFileDescriptorBased*) input);
if (glnx_regfile_copy_bytes (infd, temp_fd, (off_t)length, TRUE) < 0)
return glnx_throw_errno_prefix (error, "regfile copy");
}
else
{
g_autoptr(GOutputStream) temp_out = g_unix_output_stream_new (temp_fd, FALSE);
if (g_output_stream_splice (temp_out, input, 0,
cancellable, error) < 0)
return FALSE;
}

if (fchmod (temp_fd, 0644) < 0)
return glnx_throw_errno_prefix (error, "fchmod");

*out_fd = temp_fd; temp_fd = -1;
*out_path = g_steal_pointer (&temp_filename);
return TRUE;
Expand Down
1 change: 1 addition & 0 deletions tests/ci-commitmessage-submodules.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ git log --pretty=oneline origin/master.. | while read logline; do
echo "Commit $commit modifies submodule: $submodule"
expected_match="Update submodule: $submodule"
if ! grep -q -e "$expected_match" ${tmpd}/log.txt; then
sed -e 's,^,# ,' < ${tmpd}/log.txt
echo "error: Commit message for ${commit} changes a submodule, but does not match regex ${expected_match}"
exit 1
fi
Expand Down