From c1cf679813e86984bfa72964840d0e7b0a19e7e2 Mon Sep 17 00:00:00 2001 From: Pranith Kumar K Date: Mon, 23 Jan 2017 14:58:45 +0530 Subject: [PATCH 292/294] cluster/afr: Perform new entry mark before creating new entry There is a chance for the source brick to go down just after the new entry is created and before source brick is marked with necessary pending markers. If after this any I/O happens then new entry will become source and reverse heal will happen. To prevent this mark the pending xattrs before creating the new entry. >BUG: 1417466 >Change-Id: I233b87e694d32e5d734df5a83b4d2ca711c17503 >Signed-off-by: Pranith Kumar K >Reviewed-on: https://review.gluster.org/16474 >Smoke: Gluster Build System >NetBSD-regression: NetBSD Build System >CentOS-regression: Gluster Build System >Reviewed-by: Ravishankar N >Reviewed-by: Krutika Dhananjay BUG: 1403180 Change-Id: I32a02c9e06edf700bbfd8acd274f37c99dfb9083 Signed-off-by: Pranith Kumar K Reviewed-on: https://code.engineering.redhat.com/gerrit/97977 --- xlators/cluster/afr/src/afr-self-heal-common.c | 17 +++++-- xlators/cluster/afr/src/afr-self-heal-entry.c | 61 ++++++++++---------------- xlators/cluster/afr/src/afr-self-heal-name.c | 19 ++++---- xlators/cluster/afr/src/afr-self-heal.h | 8 ++-- 4 files changed, 51 insertions(+), 54 deletions(-) diff --git a/xlators/cluster/afr/src/afr-self-heal-common.c b/xlators/cluster/afr/src/afr-self-heal-common.c index f731d42..8b2c3fa 100644 --- a/xlators/cluster/afr/src/afr-self-heal-common.c +++ b/xlators/cluster/afr/src/afr-self-heal-common.c @@ -27,6 +27,8 @@ afr_selfheal_post_op_cbk (call_frame_t *frame, void *cookie, xlator_t *this, local = frame->local; + local->op_ret = op_ret; + local->op_errno = op_errno; syncbarrier_wake (&local->barrier); return 0; @@ -40,6 +42,7 @@ afr_selfheal_post_op (call_frame_t *frame, xlator_t *this, inode_t *inode, afr_private_t *priv = NULL; afr_local_t *local = NULL; loc_t loc = {0, }; + int ret = 0; priv = this->private; local = frame->local; @@ -47,15 +50,20 @@ afr_selfheal_post_op (call_frame_t *frame, xlator_t *this, inode_t *inode, loc.inode = inode_ref (inode); gf_uuid_copy (loc.gfid, inode->gfid); + local->op_ret = 0; + STACK_WIND (frame, afr_selfheal_post_op_cbk, priv->children[subvol], priv->children[subvol]->fops->xattrop, &loc, GF_XATTROP_ADD_ARRAY, xattr, xdata); syncbarrier_wait (&local->barrier, 1); + if (local->op_ret < 0) + ret = -local->op_errno; loc_wipe (&loc); + local->op_ret = 0; - return 0; + return ret; } int @@ -1945,13 +1953,16 @@ afr_selfheal_newentry_mark (call_frame_t *frame, xlator_t *this, inode_t *inode, changelog = afr_mark_pending_changelog (priv, newentry, xattr, replies[source].poststat.ia_type); - if (!changelog) + if (!changelog) { + ret = -ENOMEM; goto out; + } for (i = 0; i < priv->child_count; i++) { if (!sources[i]) continue; - afr_selfheal_post_op (frame, this, inode, i, xattr, NULL); + ret |= afr_selfheal_post_op (frame, this, inode, i, xattr, + NULL); } out: if (changelog) diff --git a/xlators/cluster/afr/src/afr-self-heal-entry.c b/xlators/cluster/afr/src/afr-self-heal-entry.c index 2efeedf..6b79da0 100644 --- a/xlators/cluster/afr/src/afr-self-heal-entry.c +++ b/xlators/cluster/afr/src/afr-self-heal-entry.c @@ -17,11 +17,6 @@ #include "syncop-utils.h" #include "events.h" -/* Max file name length is 255 this filename is of length 256. No file with - * this name can ever come, entry-lock with this name is going to prevent - * self-heals from older versions while the granular entry-self-heal is going - * on in newer version.*/ - static int afr_selfheal_entry_delete (xlator_t *this, inode_t *dir, const char *name, inode_t *inode, int child, struct afr_reply *replies) @@ -71,26 +66,29 @@ afr_selfheal_entry_delete (xlator_t *this, inode_t *dir, const char *name, int -afr_selfheal_recreate_entry (xlator_t *this, int dst, int source, inode_t *dir, +afr_selfheal_recreate_entry (call_frame_t *frame, int dst, int source, + unsigned char *sources, inode_t *dir, const char *name, inode_t *inode, - struct afr_reply *replies, - unsigned char *newentry) + struct afr_reply *replies) { int ret = 0; loc_t loc = {0,}; loc_t srcloc = {0,}; + xlator_t *this = frame->this; afr_private_t *priv = NULL; dict_t *xdata = NULL; struct iatt *iatt = NULL; char *linkname = NULL; mode_t mode = 0; struct iatt newent = {0,}; - priv = this->private; + unsigned char *newentry = NULL; + priv = this->private; xdata = dict_new(); if (!xdata) return -ENOMEM; + newentry = alloca0 (priv->child_count); loc.parent = inode_ref (dir); gf_uuid_copy (loc.pargfid, dir->gfid); loc.name = name; @@ -109,6 +107,15 @@ afr_selfheal_recreate_entry (xlator_t *this, int dst, int source, inode_t *dir, srcloc.inode = inode_ref (inode); gf_uuid_copy (srcloc.gfid, iatt->ia_gfid); + if (iatt->ia_type != IA_IFDIR) + ret = syncop_lookup (priv->children[dst], &srcloc, 0, 0, 0, 0); + if (iatt->ia_type == IA_IFDIR || ret == -ENOENT || ret == -ESTALE) { + newentry[dst] = 1; + ret = afr_selfheal_newentry_mark (frame, this, inode, source, + replies, sources, newentry); + if (ret) + goto out; + } mode = st_mode_from_ia (iatt->ia_prot, iatt->ia_type); @@ -116,12 +123,9 @@ afr_selfheal_recreate_entry (xlator_t *this, int dst, int source, inode_t *dir, case IA_IFDIR: ret = syncop_mkdir (priv->children[dst], &loc, mode, 0, xdata, NULL); - if (ret == 0) - newentry[dst] = 1; break; case IA_IFLNK: - ret = syncop_lookup (priv->children[dst], &srcloc, 0, 0, 0, 0); - if (ret == 0) { + if (!newentry[dst]) { ret = syncop_link (priv->children[dst], &srcloc, &loc, &newent, NULL, NULL); } else { @@ -131,8 +135,6 @@ afr_selfheal_recreate_entry (xlator_t *this, int dst, int source, inode_t *dir, goto out; ret = syncop_symlink (priv->children[dst], &loc, linkname, NULL, xdata, NULL); - if (ret == 0) - newentry[dst] = 1; } break; default: @@ -142,10 +144,6 @@ afr_selfheal_recreate_entry (xlator_t *this, int dst, int source, inode_t *dir, ret = syncop_mknod (priv->children[dst], &loc, mode, makedev (ia_major(iatt->ia_rdev), ia_minor (iatt->ia_rdev)), &newent, xdata, NULL); - if (ret == 0 && newent.ia_nlink == 1) { - /* New entry created. Mark @dst pending on all sources */ - newentry[dst] = 1; - } break; } @@ -168,12 +166,9 @@ __afr_selfheal_heal_dirent (call_frame_t *frame, xlator_t *this, fd_t *fd, int ret = 0; afr_private_t *priv = NULL; int i = 0; - unsigned char *newentry = NULL; priv = this->private; - newentry = alloca0 (priv->child_count); - if (!replies[source].valid) return -EIO; @@ -196,17 +191,15 @@ __afr_selfheal_heal_dirent (call_frame_t *frame, xlator_t *this, fd_t *fd, replies[source].poststat.ia_gfid)) continue; - ret = afr_selfheal_recreate_entry (this, i, source, - fd->inode, name, inode, - replies, newentry); + ret = afr_selfheal_recreate_entry (frame, i, source, + sources, fd->inode, + name, inode, + replies); } if (ret < 0) break; } - if (AFR_COUNT (newentry, priv->child_count)) - afr_selfheal_newentry_mark (frame, this, inode, source, replies, - sources, newentry); return ret; } @@ -290,13 +283,10 @@ __afr_selfheal_merge_dirent (call_frame_t *frame, xlator_t *this, fd_t *fd, int ret = 0; int i = 0; int source = -1; - unsigned char *newentry = NULL; afr_private_t *priv = NULL; priv = this->private; - newentry = alloca0 (priv->child_count); - for (i = 0; i < priv->child_count; i++) { if (replies[i].valid && replies[i].op_ret == 0) { source = i; @@ -331,14 +321,11 @@ __afr_selfheal_merge_dirent (call_frame_t *frame, xlator_t *this, fd_t *fd, if (replies[i].op_errno != ENOENT) continue; - ret = afr_selfheal_recreate_entry (this, i, source, fd->inode, - name, inode, replies, - newentry); + ret = afr_selfheal_recreate_entry (frame, i, source, sources, + fd->inode, name, inode, + replies); } - if (AFR_COUNT (newentry, priv->child_count)) - afr_selfheal_newentry_mark (frame, this, inode, source, replies, - sources, newentry); return ret; } diff --git a/xlators/cluster/afr/src/afr-self-heal-name.c b/xlators/cluster/afr/src/afr-self-heal-name.c index 1d7b0cc..3ac5219 100644 --- a/xlators/cluster/afr/src/afr-self-heal-name.c +++ b/xlators/cluster/afr/src/afr-self-heal-name.c @@ -110,12 +110,10 @@ __afr_selfheal_name_impunge (call_frame_t *frame, xlator_t *this, int i = 0; afr_private_t *priv = NULL; int ret = 0; - unsigned char *newentry = NULL; unsigned char *sources = NULL; priv = this->private; - newentry = alloca0 (priv->child_count); sources = alloca0 (priv->child_count); gf_uuid_copy (parent->gfid, pargfid); @@ -129,15 +127,17 @@ __afr_selfheal_name_impunge (call_frame_t *frame, xlator_t *this, sources[i] = 1; continue; } + } + + for (i = 0; i < priv->child_count; i++) { + if (sources[i]) + continue; - ret |= afr_selfheal_recreate_entry (this, i, gfid_idx, parent, - bname, inode, replies, - newentry); + ret |= afr_selfheal_recreate_entry (frame, i, gfid_idx, sources, + parent, bname, inode, + replies); } - if (AFR_COUNT (newentry, priv->child_count)) - afr_selfheal_newentry_mark (frame, this, inode, gfid_idx, replies, - sources, newentry); return ret; } @@ -484,8 +484,7 @@ __afr_selfheal_name_do (call_frame_t *frame, xlator_t *this, inode_t *parent, } ret = __afr_selfheal_name_impunge (frame, this, parent, pargfid, - bname, inode, - replies, gfid_idx); + bname, inode, replies, gfid_idx); if (ret == -EIO) ret = -1; diff --git a/xlators/cluster/afr/src/afr-self-heal.h b/xlators/cluster/afr/src/afr-self-heal.h index a339050..2b3a87e 100644 --- a/xlators/cluster/afr/src/afr-self-heal.h +++ b/xlators/cluster/afr/src/afr-self-heal.h @@ -174,10 +174,10 @@ afr_selfheal_undo_pending (call_frame_t *frame, xlator_t *this, inode_t *inode, unsigned char *locked_on); int -afr_selfheal_recreate_entry (xlator_t *this, int dst, int source, inode_t *dir, - const char *name, inode_t *inode, - struct afr_reply *replies, - unsigned char *newentry); +afr_selfheal_recreate_entry (call_frame_t *frame, int dst, int source, + unsigned char *sources, + inode_t *dir, const char *name, inode_t *inode, + struct afr_reply *replies); int afr_selfheal_post_op (call_frame_t *frame, xlator_t *this, inode_t *inode, -- 2.9.3