From e76a777f3820e62948256a45a38d5e97f3eb08a9 Mon Sep 17 00:00:00 2001

From: N Balachandran <nbalacha@redhat.com>

Date: Tue, 28 Aug 2018 12:00:33 +0530

Subject: [PATCH 438/444] cluster/dht: In rename, unlink after creating linkto

 file

The linkto file creation for the dst was done in parallel with

the unlink of the old src linkto. If these operations reached

the brick out of order, we end up with a dst linkto file without

a .glusterfs handle.

Fixed by unlinking only after the linkto file creation has

completed.

upstream patch: https://review.gluster.org/#/c/glusterfs/+/21023/

> Change-Id: I4246f7655f5bc180f5ded7fd34d263b7828a8110

> fixes: bz#1621981

> Signed-off-by: N Balachandran <nbalacha@redhat.com>

Change-Id: Ia845a68bb314997cadab57887a84dff9373400c4

BUG: 1622001

Signed-off-by: N Balachandran <nbalacha@redhat.com>

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

Tested-by: RHGS Build Bot <nigelb@redhat.com>

Reviewed-by: Raghavendra Gowdappa <rgowdapp@redhat.com>

---

 tests/bugs/posix/bug-1619720.t       |   1 +

 xlators/cluster/dht/src/dht-rename.c | 254 ++++++++++++++++++-----------------

 2 files changed, 133 insertions(+), 122 deletions(-)

diff --git a/tests/bugs/posix/bug-1619720.t b/tests/bugs/posix/bug-1619720.t

index 5e0d0f7..bfd304d 100755

--- a/tests/bugs/posix/bug-1619720.t

+++ b/tests/bugs/posix/bug-1619720.t

@@ -48,6 +48,7 @@ TEST mv $M0/tmp/file-2 $M0/tmp/file-3

 TEST mv -f $M0/tmp/file-1 $M0/tmp/file-3

+

 TEST getfattr -n $pgfid_xattr_name $B0/${V0}0/tmp/file-3

 TEST getfattr -n $pgfid_xattr_name $B0/${V0}1/tmp/file-3

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

index 1d0c2bb..378cb0a 100644

--- a/xlators/cluster/dht/src/dht-rename.c

+++ b/xlators/cluster/dht/src/dht-rename.c

@@ -849,8 +849,8 @@ dht_rename_cleanup (call_frame_t *frame)

         if (src_cached == dst_cached)

                 goto nolinks;

-        if (local->linked && (dst_hashed != src_hashed )&&

-                (dst_hashed != src_cached)) {

+        if (local->linked && (dst_hashed != src_hashed) &&

+            (dst_hashed != src_cached)) {

                 call_cnt++;

         }

@@ -935,6 +935,120 @@ nolinks:

 int

+dht_rename_unlink (call_frame_t *frame, xlator_t *this)

+{

+        dht_local_t  *local = NULL;

+        xlator_t     *src_hashed = NULL;

+        xlator_t     *src_cached = NULL;

+        xlator_t     *dst_hashed = NULL;

+        xlator_t     *dst_cached = NULL;

+        xlator_t     *rename_subvol = NULL;

+        dict_t       *xattr     = NULL;

+

+        local = frame->local;

+

+        src_hashed = local->src_hashed;

+        src_cached = local->src_cached;

+        dst_hashed = local->dst_hashed;

+        dst_cached = local->dst_cached;

+

+        local->call_cnt = 0;

+

+        /* NOTE: rename_subvol is the same subvolume from which dht_rename_cbk

+         * is called. since rename has already happened on rename_subvol,

+         * unlink shouldn't be sent for oldpath (either linkfile or cached-file)

+         * on rename_subvol. */

+        if (src_cached == dst_cached)

+                rename_subvol = src_cached;

+        else

+                rename_subvol = dst_hashed;

+

+        /* TODO: delete files in background */

+

+        if (src_cached != dst_hashed && src_cached != dst_cached)

+                local->call_cnt++;

+

+        if (src_hashed != rename_subvol && src_hashed != src_cached)

+                local->call_cnt++;

+

+        if (dst_cached && dst_cached != dst_hashed && dst_cached != src_cached)

+                local->call_cnt++;

+

+        if (local->call_cnt == 0)

+                goto unwind;

+

+        DHT_MARK_FOP_INTERNAL (xattr);

+

+        if (src_cached != dst_hashed && src_cached != dst_cached) {

+                dict_t *xattr_new = NULL;

+

+                xattr_new = dict_copy_with_ref (xattr, NULL);

+

+                gf_msg_trace (this->name, 0,

+                              "deleting old src datafile %s @ %s",

+                              local->loc.path, src_cached->name);

+

+                if (gf_uuid_compare (local->loc.pargfid,

+                                     local->loc2.pargfid) == 0) {

+                        DHT_MARKER_DONT_ACCOUNT(xattr_new);

+                }

+

+                DHT_CHANGELOG_TRACK_AS_RENAME(xattr_new, &local->loc,

+                                              &local->loc2);

+                STACK_WIND_COOKIE (frame, dht_rename_unlink_cbk, src_cached,

+                                   src_cached, src_cached->fops->unlink,

+                                   &local->loc, 0, xattr_new);

+

+                dict_unref (xattr_new);

+                xattr_new = NULL;

+        }

+

+        if (src_hashed != rename_subvol && src_hashed != src_cached) {

+                dict_t *xattr_new = NULL;

+

+                xattr_new = dict_copy_with_ref (xattr, NULL);

+

+                gf_msg_trace (this->name, 0,

+                              "deleting old src linkfile %s @ %s",

+                              local->loc.path, src_hashed->name);

+

+                DHT_MARKER_DONT_ACCOUNT(xattr_new);

+

+                STACK_WIND_COOKIE (frame, dht_rename_unlink_cbk, src_hashed,

+                                   src_hashed, src_hashed->fops->unlink,

+                                   &local->loc, 0, xattr_new);

+

+                dict_unref (xattr_new);

+                xattr_new = NULL;

+        }

+

+        if (dst_cached &&

+            (dst_cached != dst_hashed) &&

+            (dst_cached != src_cached)) {

+                gf_msg_trace (this->name, 0,

+                              "deleting old dst datafile %s @ %s",

+                              local->loc2.path, dst_cached->name);

+

+                STACK_WIND_COOKIE (frame, dht_rename_unlink_cbk, dst_cached,

+                                   dst_cached, dst_cached->fops->unlink,

+                                   &local->loc2, 0, xattr);

+        }

+        if (xattr)

+                dict_unref (xattr);

+        return 0;

+

+unwind:

+        WIPE (&local->preoldparent);

+        WIPE (&local->postoldparent);

+        WIPE (&local->preparent);

+        WIPE (&local->postparent);

+

+        dht_rename_done (frame, this);

+

+        return 0;

+}

+

+int

 dht_rename_links_create_cbk (call_frame_t *frame, void *cookie, xlator_t *this,

                              int32_t op_ret, int32_t op_errno,

                              inode_t *inode, struct iatt *stbuf,

@@ -947,6 +1061,7 @@ dht_rename_links_create_cbk (call_frame_t *frame, void *cookie, xlator_t *this,

         prev = cookie;

         local = frame->local;

+        /* TODO: Handle this case in lookup-optimize */

         if (op_ret == -1) {

                 gf_msg (this->name, GF_LOG_WARNING, op_errno,

                         DHT_MSG_CREATE_LINK_FAILED,

@@ -958,8 +1073,8 @@ dht_rename_links_create_cbk (call_frame_t *frame, void *cookie, xlator_t *this,

                 local->linked = _gf_false;

                 dht_linkfile_attr_heal (frame, this);

         }

-        DHT_STACK_DESTROY (frame);

+        dht_rename_unlink (frame, this);

         return 0;

 }

@@ -973,19 +1088,14 @@ dht_rename_cbk (call_frame_t *frame, void *cookie, xlator_t *this,

 {

         dht_local_t  *local = NULL;

         xlator_t     *prev = NULL;

-        xlator_t     *src_hashed = NULL;

         xlator_t     *src_cached = NULL;

         xlator_t     *dst_hashed = NULL;

         xlator_t     *dst_cached = NULL;

-        xlator_t     *rename_subvol = NULL;

-        call_frame_t *link_frame = NULL;

-        dht_local_t *link_local = NULL;

-        dict_t       *xattr     = NULL;

+        loc_t         link_loc  = {0};

         local = frame->local;

         prev = cookie;

-        src_hashed = local->src_hashed;

         src_cached = local->src_cached;

         dst_hashed = local->dst_hashed;

         dst_cached = local->dst_cached;

@@ -1043,31 +1153,6 @@ dht_rename_cbk (call_frame_t *frame, void *cookie, xlator_t *this,

                         local->xattr = dict_copy_with_ref (xdata, local->xattr);

         }

-        if ((src_cached == dst_cached) && (dst_hashed != dst_cached)) {

-                link_frame = copy_frame (frame);

-                if (!link_frame) {

-                        goto err;

-                }

-

-                /* fop value sent as maxvalue because it is not used

-                   anywhere in this case */

-                link_local = dht_local_init (link_frame, &local->loc2, NULL,

-                                             GF_FOP_MAXVALUE);

-                if (!link_local) {

-                        goto err;

-                }

-

-                if (link_local->loc.inode)

-                        inode_unref (link_local->loc.inode);

-                link_local->loc.inode = inode_ref (local->loc.inode);

-                gf_uuid_copy (link_local->gfid, local->loc.inode->gfid);

-

-                dht_linkfile_create (link_frame, dht_rename_links_create_cbk,

-                                     this, src_cached, dst_hashed,

-                                     &link_local->loc);

-        }

-

-err:

         /* Merge attrs only from src_cached. In case there of src_cached !=

          * dst_hashed, this ignores linkfile attrs. */

         if (prev == src_cached) {

@@ -1080,98 +1165,23 @@ err:

                 dht_iatt_merge (this, &local->postparent, postnewparent, prev);

         }

+        /* Create the linkto file for the dst file */

+        if ((src_cached == dst_cached) && (dst_hashed != dst_cached)) {

-        /* NOTE: rename_subvol is the same subvolume from which dht_rename_cbk

-         *       is called. since rename has already happened on rename_subvol,

-         *       unlink should not be sent for oldpath (either linkfile or cached-file)

-         *       on rename_subvol. */

-        if (src_cached == dst_cached)

-                rename_subvol = src_cached;

-        else

-                rename_subvol = dst_hashed;

-

-        /* TODO: delete files in background */

-

-        if (src_cached != dst_hashed && src_cached != dst_cached)

-                local->call_cnt++;

-

-        if (src_hashed != rename_subvol && src_hashed != src_cached)

-                local->call_cnt++;

-

-        if (dst_cached && dst_cached != dst_hashed && dst_cached != src_cached)

-                local->call_cnt++;

-

-        if (local->call_cnt == 0)

-                goto unwind;

-

-        DHT_MARK_FOP_INTERNAL (xattr);

-

-        if (src_cached != dst_hashed && src_cached != dst_cached) {

-                dict_t *xattr_new = NULL;

-

-                xattr_new = dict_copy_with_ref (xattr, NULL);

-

-                gf_msg_trace (this->name, 0,

-                              "deleting old src datafile %s @ %s",

-                              local->loc.path, src_cached->name);

-

-                if (gf_uuid_compare (local->loc.pargfid,

-                                  local->loc2.pargfid) == 0) {

-                        DHT_MARKER_DONT_ACCOUNT(xattr_new);

-                }

-

-                DHT_CHANGELOG_TRACK_AS_RENAME(xattr_new, &local->loc,

-                                              &local->loc2);

-                STACK_WIND_COOKIE (frame, dht_rename_unlink_cbk, src_cached,

-                                   src_cached, src_cached->fops->unlink,

-                                   &local->loc, 0, xattr_new);

-

-                dict_unref (xattr_new);

-                xattr_new = NULL;

-        }

-

-        if (src_hashed != rename_subvol && src_hashed != src_cached) {

-                dict_t *xattr_new = NULL;

-

-                xattr_new = dict_copy_with_ref (xattr, NULL);

-

-                gf_msg_trace (this->name, 0,

-                              "deleting old src linkfile %s @ %s",

-                              local->loc.path, src_hashed->name);

-

-                DHT_MARKER_DONT_ACCOUNT(xattr_new);

-

-                STACK_WIND_COOKIE (frame, dht_rename_unlink_cbk, src_hashed,

-                                   src_hashed, src_hashed->fops->unlink,

-                                   &local->loc, 0, xattr_new);

-

-                dict_unref (xattr_new);

-                xattr_new = NULL;

-        }

+                loc_copy (&link_loc, &local->loc2);

+                if (link_loc.inode)

+                        inode_unref (link_loc.inode);

+                link_loc.inode = inode_ref (local->loc.inode);

+                gf_uuid_copy (local->gfid, local->loc.inode->gfid);

+                gf_uuid_copy (link_loc.gfid, local->loc.inode->gfid);

-        if (dst_cached

-            && (dst_cached != dst_hashed)

-            && (dst_cached != src_cached)) {

-                gf_msg_trace (this->name, 0,

-                              "deleting old dst datafile %s @ %s",

-                              local->loc2.path, dst_cached->name);

-

-                STACK_WIND_COOKIE (frame, dht_rename_unlink_cbk, dst_cached,

-                                   dst_cached, dst_cached->fops->unlink,

-                                   &local->loc2, 0, xattr);

+                dht_linkfile_create (frame, dht_rename_links_create_cbk,

+                                     this, src_cached, dst_hashed,

+                                     &link_loc);

+                return 0;

         }

-        if (xattr)

-                dict_unref (xattr);

-        return 0;

-

-unwind:

-        WIPE (&local->preoldparent);

-        WIPE (&local->postoldparent);

-        WIPE (&local->preparent);

-        WIPE (&local->postparent);

-

-        dht_rename_done (frame, this);

+        dht_rename_unlink (frame, this);

         return 0;

 cleanup:

-- 

1.8.3.1