From 52341e3c2b836e5b9815a546fa5364ab8c091364 Mon Sep 17 00:00:00 2001

From: Raghavendra G <rgowdapp@redhat.com>

Date: Wed, 13 May 2015 19:56:47 +0530

Subject: [PATCH 12/18] 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: 1140506

Signed-off-by: Raghavendra G <rgowdapp@redhat.com>

Reviewed-on: http://review.gluster.org/10805

Reviewed-on: http://review.gluster.org/10965

Reviewed-on: https://code.engineering.redhat.com/gerrit/50188

Reviewed-by: Nithya Balachandran <nbalacha@redhat.com>

---

 xlators/cluster/dht/src/dht-helper.c | 132 ++++++++---------------------------

 1 file changed, 31 insertions(+), 101 deletions(-)

diff --git a/xlators/cluster/dht/src/dht-helper.c b/xlators/cluster/dht/src/dht-helper.c

index 3127171..1b5ad41 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;

 }

-- 

1.9.3