You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2013/05/18 13:24:45 UTC
svn commit: r1484092 - in /subversion/trunk/subversion: include/svn_io.h
libsvn_fs_fs/fs_fs.c libsvn_subr/io.c libsvn_wc/workqueue.c
mod_dav_svn/activity.c tests/libsvn_fs_fs/fs-pack-test.c
Author: rhuijben
Date: Sat May 18 11:24:45 2013
New Revision: 1484092
URL: http://svn.apache.org/r1484092
Log:
Introduce an io function that implements a flushed file write to a temporary
file, an optional permission copy and then a rename to a final name in one
step.
The problem with the old implementation as two separate functions was that the
flush was required for the atomic rename (as fsfs commit), but the
implementation was part of the write file function that also had other users.
Try to delete the temporary file in most error cases.
* subversion/include/svn_io.h
(svn_io_write_atomic): New function.
* subversion/libsvn_fs_fs/fs_fs.c
(write_format,
get_and_increment_txn_key_body,
svn_fs_fs__change_txn_props,
write_current,
svn_fs_fs__set_uuid): Replace code with call to svn_io_write_atomic.
* subversion/libsvn_subr/io.c
(svn_io_write_atomic): New function.
* subversion/libsvn_wc/workqueue.c
(run_postupgrade): Use svn_io_write_atomic.
* subversion/mod_dav_svn/activity.c
(dav_svn__store_activity): Use svn_io_write_atomic.
* subversion/tests/libsvn_fs_fs/fs-pack-test.c
(write_format): Use svn_io_write_atomic.
Modified:
subversion/trunk/subversion/include/svn_io.h
subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
subversion/trunk/subversion/libsvn_subr/io.c
subversion/trunk/subversion/libsvn_wc/workqueue.c
subversion/trunk/subversion/mod_dav_svn/activity.c
subversion/trunk/subversion/tests/libsvn_fs_fs/fs-pack-test.c
Modified: subversion/trunk/subversion/include/svn_io.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_io.h?rev=1484092&r1=1484091&r2=1484092&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_io.h (original)
+++ subversion/trunk/subversion/include/svn_io.h Sat May 18 11:24:45 2013
@@ -2091,6 +2091,24 @@ svn_io_file_write_full(apr_file_t *file,
apr_pool_t *pool);
/**
+ * Writes @a nbytes bytes from @a *buf to a temporary file inside the same
+ * directory as @a *final_path. Then syncs the temporary file to disk and
+ * closes the file. After this rename the temporary file to @a final_path,
+ * possibly replacing an existing file.
+ *
+ * If @a copy_perms_path is not NULL, copy the permissions applied on @a
+ * @a copy_perms_path on the temporary file before renaming.
+ *
+ * @since New in 1.9.
+ */
+svn_error_t *
+svn_io_write_atomic(const char *final_path,
+ const void *buf,
+ apr_size_t nbytes,
+ const char* copy_perms_path,
+ apr_pool_t *scratch_pool);
+
+/**
* Open a unique file in @a dirpath, and write @a nbytes from @a buf to
* the file before flushing it to disk and closing it. Return the name
* of the newly created file in @a *tmp_path, allocated in @a pool.
Modified: subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c?rev=1484092&r1=1484091&r2=1484092&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c Sat May 18 11:24:45 2013
@@ -1102,15 +1102,8 @@ write_format(const char *path, int forma
}
else
{
- const char *path_tmp;
-
- SVN_ERR(svn_io_write_unique(&path_tmp,
- svn_dirent_dirname(path, pool),
- sb->data, sb->len,
- svn_io_file_del_none, pool));
-
- /* rename the temp file as the real destination */
- SVN_ERR(svn_io_file_rename(path_tmp, path, pool));
+ SVN_ERR(svn_io_write_atomic(path, sb->data, sb->len,
+ NULL /* copy_perms_path */, pool));
}
/* And set the perms to make it read only */
@@ -6401,7 +6394,6 @@ get_and_increment_txn_key_body(void *bat
{
struct get_and_increment_txn_key_baton *cb = baton;
const char *txn_current_filename = path_txn_current(cb->fs, pool);
- const char *tmp_filename;
char next_txn_id[MAX_KEY_SIZE+3];
apr_size_t len;
@@ -6420,11 +6412,8 @@ get_and_increment_txn_key_body(void *bat
++len;
next_txn_id[len] = '\0';
- SVN_ERR(svn_io_write_unique(&tmp_filename,
- svn_dirent_dirname(txn_current_filename, pool),
- next_txn_id, len, svn_io_file_del_none, pool));
- SVN_ERR(move_into_place(tmp_filename, txn_current_filename,
- txn_current_filename, pool));
+ SVN_ERR(svn_io_write_atomic(txn_current_filename, next_txn_id, len,
+ txn_current_filename /* copy_perms path */, pool));
return SVN_NO_ERROR;
}
@@ -6602,7 +6591,6 @@ svn_fs_fs__change_txn_props(svn_fs_txn_t
const apr_array_header_t *props,
apr_pool_t *pool)
{
- const char *txn_prop_filename;
svn_stringbuf_t *buf;
svn_stream_t *stream;
apr_hash_t *txn_prop = apr_hash_make(pool);
@@ -6632,15 +6620,10 @@ svn_fs_fs__change_txn_props(svn_fs_txn_t
stream = svn_stream_from_stringbuf(buf, pool);
SVN_ERR(svn_hash_write2(txn_prop, stream, SVN_HASH_TERMINATOR, pool));
SVN_ERR(svn_stream_close(stream));
- SVN_ERR(svn_io_write_unique(&txn_prop_filename,
- path_txn_dir(txn->fs, txn->id, pool),
- buf->data,
- buf->len,
- svn_io_file_del_none,
- pool));
- return svn_io_file_rename(txn_prop_filename,
- path_txn_props(txn->fs, txn->id, pool),
- pool);
+ SVN_ERR(svn_io_write_atomic(path_txn_props(txn->fs, txn->id, pool),
+ buf->data, buf->len,
+ NULL /* copy_perms_path */, pool));
+ return SVN_NO_ERROR;
}
svn_error_t *
@@ -8245,7 +8228,7 @@ write_current(svn_fs_t *fs, svn_revnum_t
const char *next_copy_id, apr_pool_t *pool)
{
char *buf;
- const char *tmp_name, *name;
+ const char *name;
fs_fs_data_t *ffd = fs->fsap_data;
/* Now we can just write out this line. */
@@ -8255,12 +8238,10 @@ write_current(svn_fs_t *fs, svn_revnum_t
buf = apr_psprintf(pool, "%ld %s %s\n", rev, next_node_id, next_copy_id);
name = svn_fs_fs__path_current(fs, pool);
- SVN_ERR(svn_io_write_unique(&tmp_name,
- svn_dirent_dirname(name, pool),
- buf, strlen(buf),
- svn_io_file_del_none, pool));
+ SVN_ERR(svn_io_write_atomic(name, buf, strlen(buf),
+ name /* copy_perms_path */, pool));
- return move_into_place(tmp_name, name, name, pool);
+ return SVN_NO_ERROR;
}
/* Open a new svn_fs_t handle to FS, set that handle's concept of "current
@@ -9314,7 +9295,6 @@ svn_fs_fs__set_uuid(svn_fs_t *fs,
{
char *my_uuid;
apr_size_t my_uuid_len;
- const char *tmp_path;
const char *uuid_path = path_uuid(fs, pool);
if (! uuid)
@@ -9324,15 +9304,11 @@ svn_fs_fs__set_uuid(svn_fs_t *fs,
my_uuid = apr_pstrcat(fs->pool, uuid, "\n", (char *)NULL);
my_uuid_len = strlen(my_uuid);
- SVN_ERR(svn_io_write_unique(&tmp_path,
- svn_dirent_dirname(uuid_path, pool),
- my_uuid, my_uuid_len,
- svn_io_file_del_none, pool));
-
/* We use the permissions of the 'current' file, because the 'uuid'
file does not exist during repository creation. */
- SVN_ERR(move_into_place(tmp_path, uuid_path,
- svn_fs_fs__path_current(fs, pool), pool));
+ SVN_ERR(svn_io_write_atomic(uuid_path, my_uuid, my_uuid_len,
+ svn_fs_fs__path_current(fs, pool) /* perms */,
+ pool));
/* Remove the newline we added, and stash the UUID. */
my_uuid[my_uuid_len - 1] = '\0';
Modified: subversion/trunk/subversion/libsvn_subr/io.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?rev=1484092&r1=1484091&r2=1484092&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/io.c (original)
+++ subversion/trunk/subversion/libsvn_subr/io.c Sat May 18 11:24:45 2013
@@ -3502,6 +3502,60 @@ svn_io_write_unique(const char **tmp_pat
svn_io_file_close(new_file, pool)));
}
+svn_error_t *
+svn_io_write_atomic(const char *final_path,
+ const void *buf,
+ apr_size_t nbytes,
+ const char *copy_perms_path,
+ apr_pool_t *scratch_pool)
+{
+ apr_file_t *tmp_file;
+ const char *tmp_path;
+ svn_error_t *err;
+ const char *dirname = svn_dirent_dirname(final_path, scratch_pool);
+
+ SVN_ERR(svn_io_open_unique_file3(&tmp_file, &tmp_path, dirname,
+ svn_io_file_del_none,
+ scratch_pool, scratch_pool));
+
+ err = svn_io_file_write_full(tmp_file, buf, nbytes, NULL, scratch_pool);
+
+ if (!err)
+ err = svn_io_file_flush_to_disk(tmp_file, scratch_pool);
+
+ err = svn_error_compose_create(err,
+ svn_io_file_close(tmp_file, scratch_pool));
+
+ if (!err && copy_perms_path)
+ err = svn_io_copy_perms(copy_perms_path, tmp_path, scratch_pool);
+
+ if (err)
+ {
+ return svn_error_compose_create(err,
+ svn_io_remove_file2(tmp_path, FALSE,
+ scratch_pool));
+ }
+
+ SVN_ERR(svn_io_file_rename(tmp_path, final_path, scratch_pool));
+
+#ifdef __linux__
+ {
+ /* Linux has the unusual feature that fsync() on a file is not
+ enough to ensure that a file's directory entries have been
+ flushed to disk; you have to fsync the directory as well.
+ On other operating systems, we'd only be asking for trouble
+ by trying to open and fsync a directory. */
+ apr_file_t *file;
+
+ SVN_ERR(svn_io_file_open(&file, dirname, APR_READ, APR_OS_DEFAULT,
+ scratch_pool));
+ SVN_ERR(svn_io_file_flush_to_disk(file, pool));
+ SVN_ERR(svn_io_file_close(file, pool));
+ }
+#endif
+
+ return SVN_NO_ERROR;
+}
svn_error_t *
svn_io_file_trunc(apr_file_t *file, apr_off_t offset, apr_pool_t *pool)
Modified: subversion/trunk/subversion/libsvn_wc/workqueue.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/workqueue.c?rev=1484092&r1=1484091&r2=1484092&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/workqueue.c (original)
+++ subversion/trunk/subversion/libsvn_wc/workqueue.c Sat May 18 11:24:45 2013
@@ -395,7 +395,6 @@ run_postupgrade(work_item_baton_t *wqb,
const char *format_path;
const char *wcroot_abspath;
const char *adm_path;
- const char *temp_path;
svn_error_t *err;
err = svn_wc__wipe_postupgrade(wri_abspath, FALSE,
@@ -420,15 +419,13 @@ run_postupgrade(work_item_baton_t *wqb,
### The order may matter for some sufficiently old clients.. but
### this code only runs during upgrade after the files had been
### removed earlier during the upgrade. */
- SVN_ERR(svn_io_write_unique(&temp_path, adm_path, SVN_WC__NON_ENTRIES_STRING,
+ SVN_ERR(svn_io_write_atomic(format_path, SVN_WC__NON_ENTRIES_STRING,
sizeof(SVN_WC__NON_ENTRIES_STRING) - 1,
- svn_io_file_del_none, scratch_pool));
- SVN_ERR(svn_io_file_rename(temp_path, format_path, scratch_pool));
+ NULL, scratch_pool));
- SVN_ERR(svn_io_write_unique(&temp_path, adm_path, SVN_WC__NON_ENTRIES_STRING,
+ SVN_ERR(svn_io_write_atomic(entries_path, SVN_WC__NON_ENTRIES_STRING,
sizeof(SVN_WC__NON_ENTRIES_STRING) - 1,
- svn_io_file_del_none, scratch_pool));
- SVN_ERR(svn_io_file_rename(temp_path, entries_path, scratch_pool));
+ NULL, scratch_pool));
return SVN_NO_ERROR;
}
Modified: subversion/trunk/subversion/mod_dav_svn/activity.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_dav_svn/activity.c?rev=1484092&r1=1484091&r2=1484092&view=diff
==============================================================================
--- subversion/trunk/subversion/mod_dav_svn/activity.c (original)
+++ subversion/trunk/subversion/mod_dav_svn/activity.c Sat May 18 11:24:45 2013
@@ -209,11 +209,9 @@ dav_svn__store_activity(const dav_svn_re
activity_contents = apr_psprintf(repos->pool, "%s\n%s\n",
txn_name, activity_id);
- /* ### is there another directory we already have and can write to? */
- err = svn_io_write_unique(&tmp_path,
- svn_dirent_dirname(final_path, repos->pool),
+ err = svn_io_write_atomic(final_path,
activity_contents, strlen(activity_contents),
- svn_io_file_del_none, repos->pool);
+ NULL /* copy_perms path */, repos->pool);
if (err)
{
svn_error_t *serr = svn_error_quick_wrap(err,
@@ -225,15 +223,6 @@ dav_svn__store_activity(const dav_svn_re
repos->pool);
}
- err = svn_io_file_rename(tmp_path, final_path, repos->pool);
- if (err)
- {
- svn_error_clear(svn_io_remove_file2(tmp_path, TRUE, repos->pool));
- return dav_svn__convert_err(err, HTTP_INTERNAL_SERVER_ERROR,
- "could not replace files.",
- repos->pool);
- }
-
return NULL;
}
Modified: subversion/trunk/subversion/tests/libsvn_fs_fs/fs-pack-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_fs_fs/fs-pack-test.c?rev=1484092&r1=1484091&r2=1484092&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_fs_fs/fs-pack-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_fs_fs/fs-pack-test.c Sat May 18 11:24:45 2013
@@ -74,13 +74,8 @@ write_format(const char *path,
{
const char *path_tmp;
- SVN_ERR(svn_io_write_unique(&path_tmp,
- svn_dirent_dirname(path, pool),
- contents, strlen(contents),
- svn_io_file_del_none, pool));
-
- /* rename the temp file as the real destination */
- SVN_ERR(svn_io_file_rename(path_tmp, path, pool));
+ SVN_ERR(svn_io_write_atomic(path, contents, strlen(contents),
+ NULL /* copy perms */, pool));
}
/* And set the perms to make it read only */