From 500bd0014128e6727e83b6cb77e8ac94304b8f4a Mon Sep 17 00:00:00 2001 From: Ravishankar N Date: Thu, 7 Mar 2019 17:02:36 +0530 Subject: afr: thin-arbiter read txn fixes - Fixes afr_ta_read_txn() to handle inode refresh failures. code-path. - Fixes a double free issue of dict. Note: This patch address post-merge review comments for commit 69532c141be160b3fea03c1579ae4ac13018dcdf fixes: bz#1686398 Change-Id: Id5299b45b68569d47df6b73755918237a1592cb4 Signed-off-by: Ravishankar N --- xlators/cluster/afr/src/afr-common.c | 20 +++++++++++------- xlators/cluster/afr/src/afr-read-txn.c | 38 ++++++++++++++++++++-------------- xlators/cluster/afr/src/afr.h | 1 + 3 files changed, 37 insertions(+), 22 deletions(-) (limited to 'xlators/cluster') diff --git a/xlators/cluster/afr/src/afr-common.c b/xlators/cluster/afr/src/afr-common.c index 47854445716..002f97276f9 100644 --- a/xlators/cluster/afr/src/afr-common.c +++ b/xlators/cluster/afr/src/afr-common.c @@ -1269,20 +1269,25 @@ afr_inode_refresh_done(call_frame_t *frame, xlator_t *this, int error) success_replies = alloca0(priv->child_count); afr_fill_success_replies(local, priv, success_replies); - if (!afr_has_quorum(success_replies, this, frame)) { - error = afr_final_errno(frame->local, this->private); - if (!error) - error = afr_quorum_errno(priv); - goto refresh_done; - } - if (priv->thin_arbiter_count && local->is_read_txn && AFR_COUNT(success_replies, priv->child_count) != priv->child_count) { /* We need to query the good bricks and/or thin-arbiter.*/ + if (success_replies[0]) { + local->read_txn_query_child = AFR_CHILD_ZERO; + } else if (success_replies[1]) { + local->read_txn_query_child = AFR_CHILD_ONE; + } error = EINVAL; goto refresh_done; } + if (!afr_has_quorum(success_replies, this, frame)) { + error = afr_final_errno(frame->local, this->private); + if (!error) + error = afr_quorum_errno(priv); + goto refresh_done; + } + ret = afr_replies_interpret(frame, this, local->refreshinode, &start_heal); if (ret && afr_selfheal_enabled(this) && start_heal) { @@ -5696,6 +5701,7 @@ afr_local_init(afr_local_t *local, afr_private_t *priv, int32_t *op_errno) if (priv->thin_arbiter_count) { local->ta_child_up = priv->ta_child_up; local->ta_failed_subvol = AFR_CHILD_UNKNOWN; + local->read_txn_query_child = AFR_CHILD_UNKNOWN; } local->is_new_entry = _gf_false; diff --git a/xlators/cluster/afr/src/afr-read-txn.c b/xlators/cluster/afr/src/afr-read-txn.c index 9a91f2e56fc..7e258049005 100644 --- a/xlators/cluster/afr/src/afr-read-txn.c +++ b/xlators/cluster/afr/src/afr-read-txn.c @@ -114,7 +114,7 @@ afr_ta_read_txn(void *opaque) call_frame_t *frame = NULL; xlator_t *this = NULL; int read_subvol = -1; - int up_child = AFR_CHILD_UNKNOWN; + int query_child = AFR_CHILD_UNKNOWN; int possible_bad_child = AFR_CHILD_UNKNOWN; int ret = 0; int op_errno = ENOMEM; @@ -134,18 +134,18 @@ afr_ta_read_txn(void *opaque) this = frame->this; local = frame->local; priv = this->private; + query_child = local->read_txn_query_child; - if (local->child_up[AFR_CHILD_ZERO]) { - up_child = AFR_CHILD_ZERO; + if (query_child == AFR_CHILD_ZERO) { possible_bad_child = AFR_CHILD_ONE; - } else if (local->child_up[AFR_CHILD_ONE]) { - up_child = AFR_CHILD_ONE; + } else if (query_child == AFR_CHILD_ONE) { possible_bad_child = AFR_CHILD_ZERO; + } else { + /*read_txn_query_child is AFR_CHILD_UNKNOWN*/ + goto out; } - GF_ASSERT(up_child != AFR_CHILD_UNKNOWN); - - /* Query the up_child to see if it blames the down one. */ + /* Ask the query_child to see if it blames the possibly bad one. */ xdata_req = dict_new(); if (!xdata_req) goto out; @@ -159,29 +159,32 @@ afr_ta_read_txn(void *opaque) goto out; if (local->fd) { - ret = syncop_fxattrop(priv->children[up_child], local->fd, + ret = syncop_fxattrop(priv->children[query_child], local->fd, GF_XATTROP_ADD_ARRAY, xdata_req, NULL, &xdata_rsp, NULL); } else { - ret = syncop_xattrop(priv->children[up_child], &local->loc, + ret = syncop_xattrop(priv->children[query_child], &local->loc, GF_XATTROP_ADD_ARRAY, xdata_req, NULL, &xdata_rsp, NULL); } if (ret || !xdata_rsp) { gf_msg(this->name, GF_LOG_ERROR, -ret, AFR_MSG_THIN_ARB, "Failed xattrop for gfid %s on %s", - uuid_utoa(local->inode->gfid), priv->children[up_child]->name); + uuid_utoa(local->inode->gfid), + priv->children[query_child]->name); op_errno = -ret; goto out; } if (afr_ta_dict_contains_pending_xattr(xdata_rsp, priv, possible_bad_child)) { - read_subvol = up_child; + read_subvol = query_child; goto out; } dict_unref(xdata_rsp); - /* Query thin-arbiter to see if it blames any data brick. */ + xdata_rsp = NULL; + + /* It doesn't. So query thin-arbiter to see if it blames any data brick. */ ret = afr_fill_ta_loc(this, &loc); if (ret) { gf_msg(this->name, GF_LOG_ERROR, -ret, AFR_MSG_THIN_ARB, @@ -211,8 +214,8 @@ afr_ta_read_txn(void *opaque) goto unlock; } - if (!afr_ta_dict_contains_pending_xattr(xdata_rsp, priv, up_child)) { - read_subvol = up_child; + if (!afr_ta_dict_contains_pending_xattr(xdata_rsp, priv, query_child)) { + read_subvol = query_child; } else { gf_msg(this->name, GF_LOG_ERROR, EIO, AFR_MSG_THIN_ARB, "Failing read for gfid %s since good brick %s is down", @@ -450,6 +453,11 @@ afr_read_txn(call_frame_t *frame, xlator_t *this, inode_t *inode, if (priv->thin_arbiter_count && AFR_COUNT(local->child_up, priv->child_count) != priv->child_count) { + if (local->child_up[0]) { + local->read_txn_query_child = AFR_CHILD_ZERO; + } else if (local->child_up[1]) { + local->read_txn_query_child = AFR_CHILD_ONE; + } afr_ta_read_txn_synctask(frame, this); return 0; } diff --git a/xlators/cluster/afr/src/afr.h b/xlators/cluster/afr/src/afr.h index 8c0fe5efd8b..a044b2f0dc3 100644 --- a/xlators/cluster/afr/src/afr.h +++ b/xlators/cluster/afr/src/afr.h @@ -878,6 +878,7 @@ typedef struct _afr_local { afr_inode_ctx_t *inode_ctx; /*For thin-arbiter transactions.*/ + unsigned char read_txn_query_child; unsigned char ta_child_up; struct list_head ta_waitq; struct list_head ta_onwireq; -- cgit