cluster/dht: Don't rely on linkto xattr to find destination subvol during phase 2 of migration.

linkto xattr on source file cannot be relied to find where the data
file currently resides. This can happen if there are multiple
migrations before phase 2 detection by a client. For eg.,

* migration (M1, node1, node2) starts.
* application writes some data. DHT correctly stores the state in
  inode context that phase-1 of migration is in progress
* migration M1 completes
* migration (M2, node2, node3) is triggered and completed
* application resumes writes to the file. DHT identifies it as phase-2
  of migration. However, linkto xattr on node1 points to node2, but
  the file is on node3. A lookup correctly identifies node3 as cached
  subvol

TBD:
   When we identify phase-2 of a previous migration (say M1), there
   might be a migration in progress - say (M3, node3, node4). In this
   case we need to send writes to both (node3, node4) not just
   node3. Also, the inode state needs to correctly indicate that its in
   phase-1 of migration. I'll send this as a different patch.

Change-Id: I1a861f766258170af2f6c0935468edb6be687b95
BUG: 1142423
Signed-off-by: Raghavendra G <rgowdapp@redhat.com>
Reviewed-on: http://review.gluster.org/10805
Tested-by: NetBSD Build System

1 files changed, 31 insertions, 101 deletions

diff --git a/xlators/cluster/dht/src/dht-helper.c b/xlators/cluster/dht/src/dht-helper.c
index 312717198b1..2f85066569d 100644
--- a/xlators/cluster/dht/src/dht-helper.c
+++ b/xlators/cluster/dht/src/dht-helper.c
@@ -835,10 +835,9 @@ dht_migration_complete_check_task (void *data)
 {
         int           ret      = -1;
         xlator_t     *src_node = NULL;
-        xlator_t     *dst_node = NULL;
+        xlator_t     *dst_node = NULL, *linkto_target = NULL;
         dht_local_t  *local    = NULL;
         dict_t       *dict     = NULL;
-        dht_layout_t *layout   = NULL;
         struct iatt   stbuf    = {0,};
         xlator_t     *this     = NULL;
         call_frame_t *frame    = NULL;
@@ -890,120 +889,50 @@ dht_migration_complete_check_task (void *data)
         }
 
         if (!ret)
-                dst_node = dht_linkfile_subvol (this, NULL, NULL, dict);
-
-        if (ret) {
-                if (!dht_inode_missing(-ret) || (!local->loc.inode)) {
-                        local->op_errno = -ret;
-                        gf_log (this->name, GF_LOG_ERROR,
-                                "%s: failed to get the 'linkto' xattr %s",
-                                local->loc.path, strerror (-ret));
-                        ret = -1;
-                        goto out;
-                }
-
-                /* Need to do lookup on hashed subvol, then get the file */
-                ret = syncop_lookup (this, &local->loc, &stbuf, NULL,
-                                     NULL, NULL);
-                if (ret) {
-                        local->op_errno = -ret;
-                        ret = -1;
-                        goto out;
-                }
-
-                dst_node = dht_subvol_get_cached (this, local->loc.inode);
-        }
-
-        if (!dst_node) {
-                gf_log (this->name, GF_LOG_ERROR,
-                        "%s: failed to get the destination node",
-                        local->loc.path);
-                ret = -1;
-                local->op_errno = EINVAL;
-                goto out;
-        }
+                linkto_target = dht_linkfile_subvol (this, NULL, NULL, dict);
 
-        /* lookup on dst */
         if (local->loc.inode) {
-                ret = syncop_lookup (dst_node, &local->loc, &stbuf, NULL,
-                                     NULL, NULL);
-
-                if (ret) {
-                        gf_log (this->name, GF_LOG_ERROR,
-                                "%s: failed to lookup the file on %s",
-                                local->loc.path, dst_node->name);
-                        local->op_errno = -ret;
-                        ret = -1;
-                        goto out;
-                }
-
-                if (gf_uuid_compare (stbuf.ia_gfid, local->loc.inode->gfid)) {
-                        gf_msg (this->name, GF_LOG_ERROR, 0,
-                                DHT_MSG_GFID_MISMATCH,
-                                "%s: gfid different on the target file on %s",
-                                local->loc.path, dst_node->name);
-                        ret = -1;
-                        local->op_errno = EIO;
-                        goto out;
-                }
+                loc_copy (&tmp_loc, &local->loc);
         } else {
-                tmp_loc.inode = inode;
+                tmp_loc.inode = inode_ref (inode);
                 gf_uuid_copy (tmp_loc.gfid, inode->gfid);
-                ret = syncop_lookup (dst_node, &tmp_loc, &stbuf, 0, 0, 0);
-                if (ret) {
-                        gf_log (this->name, GF_LOG_ERROR,
-                                "%s: failed to lookup the file on %s",
-                                tmp_loc.path, dst_node->name);
-                        local->op_errno = -ret;
-                        ret = -1;
-                        goto out;
-                }
-
-                if (gf_uuid_compare (stbuf.ia_gfid, tmp_loc.inode->gfid)) {
-                        gf_msg (this->name, GF_LOG_ERROR, 0,
-                                DHT_MSG_GFID_MISMATCH,
-                                "%s: gfid different on the target file on %s",
-                                tmp_loc.path, dst_node->name);
-                        ret = -1;
-                        local->op_errno = EIO;
-                        goto out;
-                }
         }
 
-        /* update inode ctx (the layout) */
-        dht_layout_unref (this, local->layout);
-
-        ret = dht_layout_preset (this, dst_node, inode);
-        if (ret != 0) {
-                gf_msg_debug (this->name, 0,
-                              "%s: could not set preset layout "
-                              "for subvol %s", local->loc.path,
-                              dst_node->name);
-                ret   = -1;
-                local->op_errno = EINVAL;
+        ret = syncop_lookup (this, &tmp_loc, &stbuf, 0, 0, 0);
+        if (ret) {
+                gf_log (this->name, GF_LOG_ERROR,
+                        "%s: failed to lookup the file on %s (%s)",
+                        tmp_loc.path, this->name, strerror (-ret));
+                local->op_errno = -ret;
+                ret = -1;
                 goto out;
         }
 
-        layout = dht_layout_for_subvol (this, dst_node);
-        if (!layout) {
-                gf_log (this->name, GF_LOG_INFO,
-                        "%s: no pre-set layout for subvolume %s",
-                        local->loc.path, dst_node ? dst_node->name : "<nil>");
+        if (gf_uuid_compare (stbuf.ia_gfid, tmp_loc.inode->gfid)) {
+                gf_msg (this->name, GF_LOG_ERROR, 0,
+                        DHT_MSG_GFID_MISMATCH,
+                        "%s: gfid different on the target file on %s",
+                        tmp_loc.path, dst_node->name);
                 ret = -1;
-                local->op_errno = EINVAL;
+                local->op_errno = EIO;
                 goto out;
         }
 
-        ret = dht_layout_set (this, inode, layout);
-        if (ret) {
-                gf_log (this->name, GF_LOG_ERROR,
-                        "%s: failed to set the new layout",
-                        local->loc.path);
-                local->op_errno = EINVAL;
-                goto out;
+        dst_node = dht_subvol_get_cached (this, tmp_loc.inode);
+        if (linkto_target && dst_node != linkto_target) {
+                gf_log (this->name, GF_LOG_WARNING, "linkto target (%s) is "
+                        "different from cached-subvol (%s). Treating %s as "
+                        "destination subvol", linkto_target->name,
+                        dst_node->name, dst_node->name);
         }
 
+        /* update local. A layout is set in inode-ctx in lookup already */
+
+        dht_layout_unref (this, local->layout);
+
+        local->layout   = dht_layout_get (frame->this, inode);
         local->cached_subvol = dst_node;
+
         ret = 0;
 
         /* once we detect the migration complete, the inode-ctx2 is no more
@@ -1046,7 +975,6 @@ dht_migration_complete_check_task (void *data)
                         ret = -1;
                 }
         }
-        GF_FREE (path);
 
         SYNCTASK_SETID (frame->root->uid, frame->root->gid);
 
@@ -1057,6 +985,8 @@ dht_migration_complete_check_task (void *data)
         ret = 0;
 out:
 
+        loc_wipe (&tmp_loc);
+
         return ret;
 }