lib/commit: Reuse txn dir for tmpfiles

This closes a race condition I was seeing with `test-concurrency.py`. If we
don't have `O_TMPFILE` (or for symlinks) we'll create temporary files;
previously these would be subject to the date-based pruning because we set the
timestamp to 0 for objects.

Having our temporary files also in the txn staging dir ensures that they're
covered by the locking we do for that directory, and it's also generally cleaner
since the lifecycle of all the temporary data for a txn is in one place.

Closes: #1352
Approved by: dbnicholson
This commit is contained in:
Colin Walters 2017-11-28 06:14:29 -05:00 committed by Atomic Bot
parent 277ce6b36d
commit 72304a272c
1 changed files with 23 additions and 9 deletions

View File

@ -51,6 +51,20 @@ commit_dest_dfd (OstreeRepo *self)
return self->objects_dir_fd; return self->objects_dir_fd;
} }
/* If we don't have O_TMPFILE, or for symlinks we'll create temporary
* files. If we have a txn, use the staging dir to ensure that
* things are consistently locked against concurrent cleanup, and
* in general we have all of our data in one place.
*/
static int
commit_tmp_dfd (OstreeRepo *self)
{
if (self->in_transaction)
return self->commit_stagedir.fd;
else
return self->tmp_dir_fd;
}
/* The objects/ directory has a two-character directory prefix for checksums /* The objects/ directory has a two-character directory prefix for checksums
* to avoid putting lots of files in a single directory. This technique * to avoid putting lots of files in a single directory. This technique
* is quite old, but Git also uses it for example. * is quite old, but Git also uses it for example.
@ -439,7 +453,7 @@ _ostree_repo_bare_content_open (OstreeRepo *self,
g_assert (!real->initialized); g_assert (!real->initialized);
real->initialized = TRUE; real->initialized = TRUE;
g_assert (S_ISREG (mode)); g_assert (S_ISREG (mode));
if (!glnx_open_tmpfile_linkable_at (self->tmp_dir_fd, ".", O_WRONLY|O_CLOEXEC, if (!glnx_open_tmpfile_linkable_at (commit_tmp_dfd (self), ".", O_WRONLY|O_CLOEXEC,
&real->tmpf, error)) &real->tmpf, error))
return FALSE; return FALSE;
ot_checksum_init (&real->checksum); ot_checksum_init (&real->checksum);
@ -530,7 +544,7 @@ create_regular_tmpfile_linkable_with_content (OstreeRepo *self,
GError **error) GError **error)
{ {
g_auto(GLnxTmpfile) tmpf = { 0, }; g_auto(GLnxTmpfile) tmpf = { 0, };
if (!glnx_open_tmpfile_linkable_at (self->tmp_dir_fd, ".", O_WRONLY|O_CLOEXEC, if (!glnx_open_tmpfile_linkable_at (commit_tmp_dfd (self), ".", O_WRONLY|O_CLOEXEC,
&tmpf, error)) &tmpf, error))
return FALSE; return FALSE;
@ -668,7 +682,7 @@ write_content_object (OstreeRepo *self,
* *
* We use GLnxTmpfile for regular files, and OtCleanupUnlinkat for symlinks. * We use GLnxTmpfile for regular files, and OtCleanupUnlinkat for symlinks.
*/ */
g_auto(OtCleanupUnlinkat) tmp_unlinker = { self->tmp_dir_fd, NULL }; g_auto(OtCleanupUnlinkat) tmp_unlinker = { commit_tmp_dfd (self), NULL };
g_auto(GLnxTmpfile) tmpf = { 0, }; g_auto(GLnxTmpfile) tmpf = { 0, };
goffset unpacked_size = 0; goffset unpacked_size = 0;
gboolean indexable = FALSE; gboolean indexable = FALSE;
@ -677,7 +691,7 @@ write_content_object (OstreeRepo *self,
{ {
/* This will not be hit for bare-user or archive */ /* This will not be hit for bare-user or archive */
g_assert (self->mode == OSTREE_REPO_MODE_BARE || self->mode == OSTREE_REPO_MODE_BARE_USER_ONLY); g_assert (self->mode == OSTREE_REPO_MODE_BARE || self->mode == OSTREE_REPO_MODE_BARE_USER_ONLY);
if (!_ostree_make_temporary_symlink_at (self->tmp_dir_fd, if (!_ostree_make_temporary_symlink_at (commit_tmp_dfd (self),
g_file_info_get_symlink_target (file_info), g_file_info_get_symlink_target (file_info),
&tmp_unlinker.path, &tmp_unlinker.path,
cancellable, error)) cancellable, error))
@ -700,7 +714,7 @@ write_content_object (OstreeRepo *self,
if (self->generate_sizes) if (self->generate_sizes)
indexable = TRUE; indexable = TRUE;
if (!glnx_open_tmpfile_linkable_at (self->tmp_dir_fd, ".", O_WRONLY|O_CLOEXEC, if (!glnx_open_tmpfile_linkable_at (commit_tmp_dfd (self), ".", O_WRONLY|O_CLOEXEC,
&tmpf, error)) &tmpf, error))
return FALSE; return FALSE;
temp_out = g_unix_output_stream_new (tmpf.fd, FALSE); temp_out = g_unix_output_stream_new (tmpf.fd, FALSE);
@ -790,14 +804,14 @@ write_content_object (OstreeRepo *self,
* Note, this does not apply for bare-user repos, as they store symlinks * Note, this does not apply for bare-user repos, as they store symlinks
* as regular files. * as regular files.
*/ */
if (G_UNLIKELY (fchownat (self->tmp_dir_fd, tmp_unlinker.path, if (G_UNLIKELY (fchownat (tmp_unlinker.dfd, tmp_unlinker.path,
uid, gid, AT_SYMLINK_NOFOLLOW) == -1)) uid, gid, AT_SYMLINK_NOFOLLOW) == -1))
return glnx_throw_errno_prefix (error, "fchownat"); return glnx_throw_errno_prefix (error, "fchownat");
if (xattrs != NULL) if (xattrs != NULL)
{ {
ot_security_smack_reset_dfd_name (self->tmp_dir_fd, tmp_unlinker.path); ot_security_smack_reset_dfd_name (tmp_unlinker.dfd, tmp_unlinker.path);
if (!glnx_dfd_name_set_all_xattrs (self->tmp_dir_fd, tmp_unlinker.path, if (!glnx_dfd_name_set_all_xattrs (tmp_unlinker.dfd, tmp_unlinker.path,
xattrs, cancellable, error)) xattrs, cancellable, error))
return FALSE; return FALSE;
} }
@ -1039,7 +1053,7 @@ write_metadata_object (OstreeRepo *self,
/* Write the metadata to a temporary file */ /* Write the metadata to a temporary file */
g_auto(GLnxTmpfile) tmpf = { 0, }; g_auto(GLnxTmpfile) tmpf = { 0, };
if (!glnx_open_tmpfile_linkable_at (self->tmp_dir_fd, ".", O_WRONLY|O_CLOEXEC, if (!glnx_open_tmpfile_linkable_at (commit_tmp_dfd (self), ".", O_WRONLY|O_CLOEXEC,
&tmpf, error)) &tmpf, error))
return FALSE; return FALSE;
if (!glnx_try_fallocate (tmpf.fd, 0, len, error)) if (!glnx_try_fallocate (tmpf.fd, 0, len, error))