summaryrefslogtreecommitdiffstats
path: root/xlators
diff options
context:
space:
mode:
authorShyam <srangana@redhat.com>2014-09-11 10:52:57 +0530
committerNiels de Vos <ndevos@redhat.com>2014-10-01 03:36:35 -0700
commit5f31af8527e7f1f62e2c80fdf25f6d2cf71e49f1 (patch)
tree044be6b03dbc90395ac7f2edf6cb01dc311b5c42 /xlators
parent58f1273c0831a0c0ac99f4bc66461ec88a112f62 (diff)
cluster/dht: Rename should not fail post hardlink creation
In the rename path, we wind the creation of newname hardlink and linkto file in dst hashed a the same time. If the linkto creation fails, but the link creation succeeds, we enter the failure code and cleanup the created newname hardlink. In the interim if another client looks up newname and finds it as a hardlink from FUSE, it could send an unlink for oldname instead of a rename. This combined with the above cleanup code could end up losing all the files copies, and thereby losing data. This fix separates these steps into 2 parts, creating the linkto first and then the link file, so that post link file creation no failures would cleanup the newname file. If linkto fails then link is not attempted, thereby not polluting the name space with newname. Change-Id: I61da8e906060da16a31ea1076eec2f01fd617f44 BUG: 1140348 Signed-off-by: Shyam <srangana@redhat.com> Reviewed-on: http://review.gluster.org/8570 Tested-by: Gluster Build System <jenkins@build.gluster.com> Reviewed-by: Jeff Darcy <jdarcy@redhat.com> Reviewed-by: Vijay Bellur <vbellur@redhat.com> (cherry picked from commit 4ce3db8e508e715a43352b082e861fd0e729951f) Signed-off-by: Nithya Balachandran <nbalacha@redhat.com> Reviewed-on: http://review.gluster.org/8728 Reviewed-by: Niels de Vos <ndevos@redhat.com>
Diffstat (limited to 'xlators')
-rw-r--r--xlators/cluster/dht/src/dht-common.h10
-rw-r--r--xlators/cluster/dht/src/dht-rename.c133
2 files changed, 102 insertions, 41 deletions
diff --git a/xlators/cluster/dht/src/dht-common.h b/xlators/cluster/dht/src/dht-common.h
index f64c29bf41e..b9a696bb628 100644
--- a/xlators/cluster/dht/src/dht-common.h
+++ b/xlators/cluster/dht/src/dht-common.h
@@ -520,11 +520,11 @@ int dht_filter_loc_subvol_key (xlator_t *this, loc_t *loc, loc_t *new_loc,
xlator_t **subvol);
int dht_rename_cleanup (call_frame_t *frame);
-int dht_rename_links_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
- int32_t op_ret, int32_t op_errno,
- inode_t *inode, struct iatt *stbuf,
- struct iatt *preparent, struct iatt *postparent,
- dict_t *xdata);
+int dht_rename_link_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
+ int32_t op_ret, int32_t op_errno,
+ inode_t *inode, struct iatt *stbuf,
+ struct iatt *preparent, struct iatt *postparent,
+ dict_t *xdata);
int dht_fix_directory_layout (call_frame_t *frame,
dht_selfheal_dir_cbk_t dir_cbk,
diff --git a/xlators/cluster/dht/src/dht-rename.c b/xlators/cluster/dht/src/dht-rename.c
index 3e4607d337b..343beb0b896 100644
--- a/xlators/cluster/dht/src/dht-rename.c
+++ b/xlators/cluster/dht/src/dht-rename.c
@@ -826,9 +826,8 @@ dht_do_rename (call_frame_t *frame)
return 0;
}
-
int
-dht_rename_links_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
+dht_rename_link_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
int32_t op_ret, int32_t op_errno,
inode_t *inode, struct iatt *stbuf,
struct iatt *preparent, struct iatt *postparent,
@@ -836,8 +835,6 @@ dht_rename_links_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
{
dht_local_t *local = NULL;
call_frame_t *prev = NULL;
- int this_call_cnt = 0;
-
local = frame->local;
prev = cookie;
@@ -847,34 +844,85 @@ dht_rename_links_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
"link/file on %s failed (%s)",
prev->this->name, strerror (op_errno));
local->op_ret = -1;
- if (op_errno != ENOENT) {
- local->op_errno = op_errno;
- if (prev->this == local->src_cached) {
- local->added_link = _gf_false;
- }
- }
- } else if (local->src_cached == prev->this) {
- /* merge of attr returned only from linkfile creation */
+ local->op_errno = op_errno;
+ local->added_link = _gf_false;
+ } else
dht_iatt_merge (this, &local->stbuf, stbuf, prev->this);
+
+ if (local->op_ret == -1)
+ goto cleanup;
+
+ dht_do_rename (frame);
+
+ return 0;
+
+cleanup:
+ dht_rename_cleanup (frame);
+
+ return 0;
+}
+
+int
+dht_rename_linkto_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
+ int32_t op_ret, int32_t op_errno,
+ inode_t *inode, struct iatt *stbuf,
+ struct iatt *preparent, struct iatt *postparent,
+ dict_t *xdata)
+{
+ dht_local_t *local = NULL;
+ call_frame_t *prev = NULL;
+ xlator_t *src_cached = NULL;
+ dict_t *xattr = NULL;
+
+ DHT_MARK_FOP_INTERNAL (xattr);
+
+ local = frame->local;
+ prev = cookie;
+
+ src_cached = local->src_cached;
+
+ if (op_ret == -1) {
+ gf_log (this->name, GF_LOG_DEBUG,
+ "link/file on %s failed (%s)",
+ prev->this->name, strerror (op_errno));
+ local->op_ret = -1;
+ local->op_errno = op_errno;
}
- this_call_cnt = dht_frame_return (frame);
- if (is_last_call (this_call_cnt)) {
- if (local->op_ret == -1)
- goto cleanup;
+ /* If linkto creation failed move to failure cleanup code,
+ * instead of continuing with creating the link file */
+ if (local->op_ret != 0) {
+ goto cleanup;
+ }
- dht_do_rename (frame);
+ gf_log (this->name, GF_LOG_TRACE,
+ "link %s => %s (%s)", local->loc.path,
+ local->loc2.path, src_cached->name);
+ if (uuid_compare (local->loc.pargfid,
+ local->loc2.pargfid) == 0) {
+ DHT_MARKER_DONT_ACCOUNT(xattr);
}
+ local->added_link = _gf_true;
+
+ STACK_WIND (frame, dht_rename_link_cbk,
+ src_cached, src_cached->fops->link,
+ &local->loc, &local->loc2, xattr);
+
+ if (xattr)
+ dict_unref (xattr);
+
return 0;
cleanup:
dht_rename_cleanup (frame);
+ if (xattr)
+ dict_unref (xattr);
+
return 0;
}
-
int
dht_rename_unlink_links_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
int32_t op_ret, int32_t op_errno,
@@ -956,24 +1004,35 @@ dht_rename_create_links (call_frame_t *frame)
return 0;
}
- if (dst_hashed != src_hashed && dst_hashed != src_cached)
- call_cnt++;
-
- if (src_cached != dst_hashed)
+ if (src_cached != dst_hashed) {
+ /* needed to create the link file */
call_cnt++;
+ if (dst_hashed != src_hashed)
+ /* needed to create the linkto file */
+ call_cnt ++;
+ }
- local->call_cnt = call_cnt;
-
- if (dst_hashed != src_hashed && dst_hashed != src_cached) {
- gf_log (this->name, GF_LOG_TRACE,
+ /* We should not have any failures post the link creation, as this
+ * introduces the newname into the namespace. Clients could have cached
+ * the existence of the newname and may start taking actions based on
+ * the same. Hence create the linkto first, and then attempt the link.
+ *
+ * NOTE: If another client is attempting the same oldname -> newname
+ * rename, and finds both file names as existing, and are hard links
+ * to each other, then FUSE would send in an unlink for oldname. In
+ * this time duration if we treat the linkto as a critical error and
+ * unlink the newname we created, we would have effectively lost the
+ * file to rename operations. */
+ if (dst_hashed != src_hashed && src_cached != dst_hashed) {
+ gf_log (this->name, GF_LOG_TRACE,
"linkfile %s @ %s => %s",
- local->loc.path, dst_hashed->name, src_cached->name);
- memcpy (local->gfid, local->loc.inode->gfid, 16);
- dht_linkfile_create (frame, dht_rename_links_cbk, this,
- src_cached, dst_hashed, &local->loc);
- }
+ local->loc.path, dst_hashed->name,
+ src_cached->name);
- if (src_cached != dst_hashed) {
+ memcpy (local->gfid, local->loc.inode->gfid, 16);
+ dht_linkfile_create (frame, dht_rename_linkto_cbk, this,
+ src_cached, dst_hashed, &local->loc);
+ } else if (src_cached != dst_hashed) {
dict_t *xattr_new = NULL;
xattr_new = dict_copy_with_ref (xattr, NULL);
@@ -981,18 +1040,20 @@ dht_rename_create_links (call_frame_t *frame)
gf_log (this->name, GF_LOG_TRACE,
"link %s => %s (%s)", local->loc.path,
local->loc2.path, src_cached->name);
+
if (uuid_compare (local->loc.pargfid,
local->loc2.pargfid) == 0) {
DHT_MARKER_DONT_ACCOUNT(xattr_new);
}
local->added_link = _gf_true;
- STACK_WIND (frame, dht_rename_links_cbk,
- src_cached, src_cached->fops->link,
- &local->loc, &local->loc2, xattr_new);
+
+ STACK_WIND (frame, dht_rename_link_cbk,
+ src_cached, src_cached->fops->link,
+ &local->loc, &local->loc2, xattr_new);
dict_unref (xattr_new);
- }
+ }
nolinks:
if (!call_cnt) {