afr: add null check for thin-arbiter gfid.

Problem:
Lookup/creation of thin-arbiter ID file happens in background during
mounting. On new volumes, if the  ID file creation is in progress, and a
FOP fails on data brick, a post-op (xattrop) is attemtped on TA. Since
the TA file's gfid is null at this point, the ASSERT checks in protocol/
client causes a crash.

Fix:
Given that we decided to do Lookup/creation of thin-arbiter in
background, fail the other AFR FOPS on TA if the ID file's gfid is null
instead of winding it down to protocol/client.

Also remove afr_changelog_thin_arbiter_post_op() which seems to be dead
code.

Updates: #763
Change-Id: I70dc666faf55cc5c8f7cf8e7d36085e4fa399c4d
Signed-off-by: Ravishankar N <ravishankar@redhat.com>
(cherry picked from commit f9b5074394e3d2f3b6728aab97230ba620879426)

4 files changed, 13 insertions, 88 deletions

diff --git a/xlators/cluster/afr/src/afr-common.c b/xlators/cluster/afr/src/afr-common.c
index 1aec722acb1..1da75c07f73 100644
--- a/xlators/cluster/afr/src/afr-common.c
+++ b/xlators/cluster/afr/src/afr-common.c
@@ -3034,7 +3034,7 @@ afr_ta_id_file_check(void *opaque)
     this = opaque;
     priv = this->private;
 
-    ret = afr_fill_ta_loc(this, &loc);
+    ret = afr_fill_ta_loc(this, &loc, _gf_false);
     if (ret) {
         gf_msg(this->name, GF_LOG_ERROR, -ret, AFR_MSG_THIN_ARB,
                "Failed to populate thin-arbiter loc for: %s.", loc.name);
diff --git a/xlators/cluster/afr/src/afr-read-txn.c b/xlators/cluster/afr/src/afr-read-txn.c
index 772b59f9a2f..88ea9b4cfc6 100644
--- a/xlators/cluster/afr/src/afr-read-txn.c
+++ b/xlators/cluster/afr/src/afr-read-txn.c
@@ -164,7 +164,7 @@ afr_ta_read_txn(void *opaque)
     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);
+    ret = afr_fill_ta_loc(this, &loc, _gf_true);
     if (ret) {
         gf_msg(this->name, GF_LOG_ERROR, -ret, AFR_MSG_THIN_ARB,
                "Failed to populate thin-arbiter loc for: %s.", loc.name);
diff --git a/xlators/cluster/afr/src/afr-transaction.c b/xlators/cluster/afr/src/afr-transaction.c
index de8f7cab69d..e8a1bcf5109 100644
--- a/xlators/cluster/afr/src/afr-transaction.c
+++ b/xlators/cluster/afr/src/afr-transaction.c
@@ -124,9 +124,9 @@ afr_release_notify_lock_for_ta(void *opaque)
 
     this = (xlator_t *)opaque;
     priv = this->private;
-    ret = afr_fill_ta_loc(this, &loc);
+    ret = afr_fill_ta_loc(this, &loc, _gf_true);
     if (ret) {
-        gf_msg(this->name, GF_LOG_ERROR, ENOMEM, AFR_MSG_THIN_ARB,
+        gf_msg(this->name, GF_LOG_ERROR, -ret, AFR_MSG_THIN_ARB,
                "Failed to populate loc for thin-arbiter.");
         goto out;
     }
@@ -1029,7 +1029,7 @@ set_response:
 }
 
 int
-afr_fill_ta_loc(xlator_t *this, loc_t *loc)
+afr_fill_ta_loc(xlator_t *this, loc_t *loc, gf_boolean_t is_gfid_based_fop)
 {
     afr_private_t *priv = NULL;
 
@@ -1037,6 +1037,11 @@ afr_fill_ta_loc(xlator_t *this, loc_t *loc)
     loc->parent = inode_ref(priv->root_inode);
     gf_uuid_copy(loc->pargfid, loc->parent->gfid);
     loc->name = priv->pending_key[THIN_ARBITER_BRICK_INDEX];
+    if (is_gfid_based_fop && gf_uuid_is_null(priv->ta_gfid)) {
+        /* Except afr_ta_id_file_check() which is path based, all other gluster
+         * FOPS need gfid.*/
+        return -EINVAL;
+    }
     gf_uuid_copy(loc->gfid, priv->ta_gfid);
     loc->inode = inode_new(loc->parent->table);
     if (!loc->inode) {
@@ -1046,86 +1051,6 @@ afr_fill_ta_loc(xlator_t *this, loc_t *loc)
     return 0;
 }
 
-int
-afr_changelog_thin_arbiter_post_op(xlator_t *this, afr_local_t *local)
-{
-    int ret = 0;
-    afr_private_t *priv = NULL;
-    dict_t *xattr = NULL;
-    int failed_count = 0;
-    struct gf_flock flock = {
-        0,
-    };
-    loc_t loc = {
-        0,
-    };
-    int i = 0;
-
-    priv = this->private;
-    if (!priv->thin_arbiter_count)
-        return 0;
-
-    failed_count = AFR_COUNT(local->transaction.failed_subvols,
-                             priv->child_count);
-    if (!failed_count)
-        return 0;
-
-    GF_ASSERT(failed_count == 1);
-    ret = afr_fill_ta_loc(this, &loc);
-    if (ret) {
-        gf_msg(this->name, GF_LOG_ERROR, -ret, AFR_MSG_THIN_ARB,
-               "Failed to populate thin-arbiter loc for: %s.", loc.name);
-        goto out;
-    }
-
-    xattr = dict_new();
-    if (!xattr) {
-        ret = -ENOMEM;
-        goto out;
-    }
-    for (i = 0; i < priv->child_count; i++) {
-        ret = dict_set_static_bin(xattr, priv->pending_key[i],
-                                  local->pending[i],
-                                  AFR_NUM_CHANGE_LOGS * sizeof(int));
-        if (ret)
-            goto out;
-    }
-
-    flock.l_type = F_WRLCK;
-    flock.l_start = 0;
-    flock.l_len = 0;
-
-    /*TODO: Convert to two domain locking. */
-    ret = syncop_inodelk(priv->children[THIN_ARBITER_BRICK_INDEX],
-                         AFR_TA_DOM_NOTIFY, &loc, F_SETLKW, &flock, NULL, NULL);
-    if (ret)
-        goto out;
-
-    ret = syncop_xattrop(priv->children[THIN_ARBITER_BRICK_INDEX], &loc,
-                         GF_XATTROP_ADD_ARRAY, xattr, NULL, NULL, NULL);
-
-    if (ret == -EINVAL) {
-        gf_msg(this->name, GF_LOG_INFO, -ret, AFR_MSG_THIN_ARB,
-               "Thin-arbiter has denied post-op on %s for gfid %s.",
-               priv->pending_key[THIN_ARBITER_BRICK_INDEX],
-               uuid_utoa(local->inode->gfid));
-
-    } else if (ret) {
-        gf_msg(this->name, GF_LOG_ERROR, -ret, AFR_MSG_THIN_ARB,
-               "Post-op on thin-arbiter id file %s failed for gfid %s.",
-               priv->pending_key[THIN_ARBITER_BRICK_INDEX],
-               uuid_utoa(local->inode->gfid));
-    }
-    flock.l_type = F_UNLCK;
-    syncop_inodelk(priv->children[THIN_ARBITER_BRICK_INDEX], AFR_TA_DOM_NOTIFY,
-                   &loc, F_SETLK, &flock, NULL, NULL);
-out:
-    if (xattr)
-        dict_unref(xattr);
-
-    return ret;
-}
-
 static int
 afr_ta_post_op_done(int ret, call_frame_t *frame, void *opaque)
 {
@@ -1220,9 +1145,9 @@ afr_ta_post_op_do(void *opaque)
     this = local->transaction.frame->this;
     priv = this->private;
 
-    ret = afr_fill_ta_loc(this, &loc);
+    ret = afr_fill_ta_loc(this, &loc, _gf_true);
     if (ret) {
-        gf_msg(this->name, GF_LOG_ERROR, ENOMEM, AFR_MSG_THIN_ARB,
+        gf_msg(this->name, GF_LOG_ERROR, -ret, AFR_MSG_THIN_ARB,
                "Failed to populate loc for thin-arbiter.");
         goto out;
     }
diff --git a/xlators/cluster/afr/src/afr.h b/xlators/cluster/afr/src/afr.h
index 555941cf341..c756d813530 100644
--- a/xlators/cluster/afr/src/afr.h
+++ b/xlators/cluster/afr/src/afr.h
@@ -1315,7 +1315,7 @@ int
 afr_set_inode_local(xlator_t *this, afr_local_t *local, inode_t *inode);
 
 int
-afr_fill_ta_loc(xlator_t *this, loc_t *loc);
+afr_fill_ta_loc(xlator_t *this, loc_t *loc, gf_boolean_t is_gfid_based_fop);
 
 int
 afr_ta_post_op_lock(xlator_t *this, loc_t *loc);