From 4d121709766c10bdb7900dd9066dcec5b16678c3 Mon Sep 17 00:00:00 2001 From: Ravishankar N Date: Fri, 18 Aug 2017 18:05:54 +0530 Subject: afr: check validity of afr_reply ...in various self-heal code paths. Originally found by Pranith in __afr_selfheal_name_impunge () Also change __afr_selfheal_assign_gfid() to send lookup only on those bricks that don't have a gfid matching that of the source. > Reviewed-on: https://review.gluster.org/18065 > Smoke: Gluster Build System > CentOS-regression: Gluster Build System (cherry picked from commit d594900dbca92c356152be65fce16f77c402117c) Change-Id: I70a2ccd750a2af92c5fc36e0eefb2b6125404b4a BUG: 1491995 Signed-off-by: Pranith Kumar K Signed-off-by: Ravishankar N Reviewed-on: https://review.gluster.org/18303 Smoke: Gluster Build System CentOS-regression: Gluster Build System Reviewed-by: Shyamsundar Ranganathan --- xlators/cluster/afr/src/afr-common.c | 23 ++++++++----- xlators/cluster/afr/src/afr-self-heal-common.c | 47 +++++++++++++++----------- xlators/cluster/afr/src/afr-self-heal-name.c | 43 +++++++++++++---------- xlators/cluster/afr/src/afr-self-heal.h | 2 ++ xlators/cluster/afr/src/afr.h | 3 ++ 5 files changed, 71 insertions(+), 47 deletions(-) (limited to 'xlators') diff --git a/xlators/cluster/afr/src/afr-common.c b/xlators/cluster/afr/src/afr-common.c index 41077bb905d..bd8acb6abb7 100644 --- a/xlators/cluster/afr/src/afr-common.c +++ b/xlators/cluster/afr/src/afr-common.c @@ -1693,6 +1693,19 @@ afr_local_transaction_cleanup (afr_local_t *local, xlator_t *this) } +void +afr_reply_wipe (struct afr_reply *reply) +{ + if (reply->xdata) { + dict_unref (reply->xdata); + reply->xdata = NULL; + } + + if (reply->xattr) { + dict_unref (reply->xattr); + reply->xattr = NULL; + } +} void afr_replies_wipe (struct afr_reply *replies, int count) @@ -1700,15 +1713,7 @@ afr_replies_wipe (struct afr_reply *replies, int count) int i = 0; for (i = 0; i < count; i++) { - if (replies[i].xdata) { - dict_unref (replies[i].xdata); - replies[i].xdata = NULL; - } - - if (replies[i].xattr) { - dict_unref (replies[i].xattr); - replies[i].xattr = NULL; - } + afr_reply_wipe (&replies[i]); } } diff --git a/xlators/cluster/afr/src/afr-self-heal-common.c b/xlators/cluster/afr/src/afr-self-heal-common.c index 6b5e50d6c56..484b7dca54e 100644 --- a/xlators/cluster/afr/src/afr-self-heal-common.c +++ b/xlators/cluster/afr/src/afr-self-heal-common.c @@ -299,39 +299,43 @@ afr_selfheal_undo_pending (call_frame_t *frame, xlator_t *this, inode_t *inode, return 0; } +void +afr_reply_copy (struct afr_reply *dst, struct afr_reply *src) +{ + dict_t *xdata = NULL; + + dst->valid = src->valid; + dst->op_ret = src->op_ret; + dst->op_errno = src->op_errno; + dst->prestat = src->prestat; + dst->poststat = src->poststat; + dst->preparent = src->preparent; + dst->postparent = src->postparent; + dst->preparent2 = src->preparent2; + dst->postparent2 = src->postparent2; + if (src->xdata) + xdata = dict_ref (src->xdata); + else + xdata = NULL; + if (dst->xdata) + dict_unref (dst->xdata); + dst->xdata = xdata; + memcpy (dst->checksum, src->checksum, MD5_DIGEST_LENGTH); +} void afr_replies_copy (struct afr_reply *dst, struct afr_reply *src, int count) { int i = 0; - dict_t *xdata = NULL; if (dst == src) return; for (i = 0; i < count; i++) { - dst[i].valid = src[i].valid; - dst[i].op_ret = src[i].op_ret; - dst[i].op_errno = src[i].op_errno; - dst[i].prestat = src[i].prestat; - dst[i].poststat = src[i].poststat; - dst[i].preparent = src[i].preparent; - dst[i].postparent = src[i].postparent; - dst[i].preparent2 = src[i].preparent2; - dst[i].postparent2 = src[i].postparent2; - if (src[i].xdata) - xdata = dict_ref (src[i].xdata); - else - xdata = NULL; - if (dst[i].xdata) - dict_unref (dst[i].xdata); - dst[i].xdata = xdata; - memcpy (dst[i].checksum, src[i].checksum, - MD5_DIGEST_LENGTH); + afr_reply_copy (&dst[i], &src[i]); } } - int afr_selfheal_fill_dirty (xlator_t *this, int *dirty, int subvol, int idx, dict_t *xdata) @@ -400,6 +404,9 @@ afr_selfheal_extract_xattr (xlator_t *this, struct afr_reply *replies, priv = this->private; for (i = 0; i < priv->child_count; i++) { + if (!replies[i].valid || replies[i].op_ret != 0) + continue; + if (!replies[i].xdata) continue; diff --git a/xlators/cluster/afr/src/afr-self-heal-name.c b/xlators/cluster/afr/src/afr-self-heal-name.c index 8372cb6e376..89c8073c766 100644 --- a/xlators/cluster/afr/src/afr-self-heal-name.c +++ b/xlators/cluster/afr/src/afr-self-heal-name.c @@ -24,13 +24,16 @@ __afr_selfheal_assign_gfid (xlator_t *this, inode_t *parent, uuid_t pargfid, int ret = 0; int up_count = 0; int locked_count = 0; + int i = 0; afr_private_t *priv = NULL; dict_t *xdata = NULL; loc_t loc = {0, }; call_frame_t *new_frame = NULL; afr_local_t *new_local = NULL; + unsigned char *wind_on = NULL; priv = this->private; + wind_on = alloca0 (priv->child_count); new_frame = afr_frame_create (this); if (!new_frame) { @@ -76,20 +79,26 @@ __afr_selfheal_assign_gfid (xlator_t *this, inode_t *parent, uuid_t pargfid, } } - /* Clear out old replies here and wind lookup on all locked - * subvolumes to achieve two things: - * a. gfid heal on those subvolumes that do not have gfid associated - * with the inode, and - * b. refresh replies, which can be consumed by - * __afr_selfheal_name_impunge(). + /* gfid heal on those subvolumes that do not have gfid associated + * with the inode and update those replies. */ + for (i = 0; i < priv->child_count; i++) { + if (replies[i].valid && replies[i].op_ret == 0 && + !gf_uuid_is_null (replies[i].poststat.ia_gfid) && + !gf_uuid_compare (replies[i].poststat.ia_gfid, gfid)) + continue; + wind_on[i] = 1; + } - AFR_ONLIST (locked_on, new_frame, afr_selfheal_discover_cbk, lookup, + AFR_ONLIST (wind_on, new_frame, afr_selfheal_discover_cbk, lookup, &loc, xdata); - afr_replies_wipe (replies, priv->child_count); - - afr_replies_copy (replies, new_local->replies, priv->child_count); + for (i = 0; i < priv->child_count; i++) { + if (!wind_on[i]) + continue; + afr_reply_wipe (&replies[i]); + afr_reply_copy (&replies[i], &new_local->replies[i]); + } out: loc_wipe (&loc); @@ -119,7 +128,7 @@ __afr_selfheal_name_impunge (call_frame_t *frame, xlator_t *this, gf_uuid_copy (parent->gfid, pargfid); for (i = 0; i < priv->child_count; i++) { - if (!replies[i].valid) + if (!replies[i].valid || replies[i].op_ret != 0) continue; if (gf_uuid_compare (replies[i].poststat.ia_gfid, @@ -213,7 +222,7 @@ afr_selfheal_gfid_idx_get (xlator_t *this, struct afr_reply *replies, priv = this->private; for (i = 0; i < priv->child_count; i++) { - if (!replies[i].valid) + if (!replies[i].valid || replies[i].op_ret != 0) continue; if (!sources[i]) @@ -281,7 +290,7 @@ afr_selfheal_name_type_mismatch_check (xlator_t *this, struct afr_reply *replies priv = this->private; for (i = 0; i < priv->child_count; i++) { - if (!replies[i].valid) + if (!replies[i].valid || replies[i].op_ret != 0) continue; if (replies[i].poststat.ia_type == IA_INVAL) @@ -342,8 +351,8 @@ afr_selfheal_name_gfid_mismatch_check (xlator_t *this, struct afr_reply *replies priv = this->private; for (i = 0; i < priv->child_count; i++) { - if (!replies[i].valid) - continue; + if (!replies[i].valid || replies[i].op_ret != 0) + continue; if (gf_uuid_is_null (replies[i].poststat.ia_gfid)) continue; @@ -496,7 +505,6 @@ int __afr_selfheal_name_finalize_source (xlator_t *this, unsigned char *sources, unsigned char *healed_sinks, unsigned char *locked_on, - struct afr_reply *replies, uint64_t *witness) { int i = 0; @@ -565,8 +573,7 @@ __afr_selfheal_name_prepare (call_frame_t *frame, xlator_t *this, inode_t *paren source = __afr_selfheal_name_finalize_source (this, sources, healed_sinks, - locked_on, replies, - witness); + locked_on, witness); if (source < 0) { /* If source is < 0 (typically split-brain), we perform a conservative merge of entries rather than erroring out */ diff --git a/xlators/cluster/afr/src/afr-self-heal.h b/xlators/cluster/afr/src/afr-self-heal.h index 0a3d6482ca3..04463cccfc9 100644 --- a/xlators/cluster/afr/src/afr-self-heal.h +++ b/xlators/cluster/afr/src/afr-self-heal.h @@ -203,6 +203,8 @@ afr_selfheal_discover_cbk (call_frame_t *frame, void *cookie, xlator_t *this, int op_ret, int op_errno, inode_t *inode, struct iatt *buf, dict_t *xdata, struct iatt *parbuf); +void +afr_reply_copy (struct afr_reply *dst, struct afr_reply *src); void afr_replies_copy (struct afr_reply *dst, struct afr_reply *src, int count); diff --git a/xlators/cluster/afr/src/afr.h b/xlators/cluster/afr/src/afr.h index c9759c07a97..283acd269ad 100644 --- a/xlators/cluster/afr/src/afr.h +++ b/xlators/cluster/afr/src/afr.h @@ -1151,6 +1151,9 @@ afr_handle_open_fd_count (call_frame_t *frame, xlator_t *this); void afr_remove_eager_lock_stub (afr_local_t *local); +void +afr_reply_wipe (struct afr_reply *reply); + void afr_replies_wipe (struct afr_reply *replies, int count); -- cgit