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.

Change-Id: I70a2ccd750a2af92c5fc36e0eefb2b6125404b4a
BUG: 1482923
Signed-off-by: Pranith Kumar K <pkarampu@redhat.com>
Signed-off-by: Ravishankar N <ravishankar@redhat.com>
Reviewed-on: https://review.gluster.org/18065
Smoke: Gluster Build System <jenkins@build.gluster.org>
CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
This commit is contained in:
Ravishankar N 2017-08-18 18:05:54 +05:30 committed by Pranith Kumar Karampuri
parent 24b95089a1
commit d594900dbc
5 changed files with 71 additions and 47 deletions

View File

@ -1699,6 +1699,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)
@ -1706,15 +1719,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]);
}
}

View File

@ -549,39 +549,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)
@ -650,6 +654,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;

View File

@ -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)
@ -343,8 +352,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 */

View File

@ -215,6 +215,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);

View File

@ -1158,6 +1158,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);