From 902a257fad46d58775824b5cf6abd3c3ab1da724 Mon Sep 17 00:00:00 2001

From: Jiffin Tony Thottan <jthottan@redhat.com>

Date: Thu, 18 May 2017 11:22:16 +0530

Subject: [PATCH 527/529] dht/hardlink : Remove stale linkto file incase of

 failure

This is a similar issue fixed for rename in https://review.gluster.org/#/c/16016/

For hardlinks, if cached and hashed subvolumes are different, then it will first

create linkto file in hashed using root permission, but actually hardlink creation

fails with EACESS and stale linkto file is never removed.All the followup hardlink

calls with file name will result ESTALE because linktofile creation fails with EEXIST

and follow up lookup on linkto file returns gfid-mismatching(old linkto file) and

finally fails with ESTALE

Steps to produce :

(From link/00.t test from posix-testsuite)

Steps executed in script

 * create a file "abc" using root

 * change the ownership of file to a non root user

 * create hardlink "link" for "abc" using a non root user, it fails with EACESS

 * delete "abc"

 * create directory "abc" using root

 * again try to create hadrlink "link" for "abc" using non root user, fails with ESTALE

Also tried to fix other bugs in dht_linkfile_create_cbk() and posix_lookup.

Thanks Susant for the help in debugging the issue and suggestion for this patch.

Upstrem reference :

>Change-Id: I7a5a1899d3fd1fdb13578b37f9d52a084492e35d

>BUG: 1452084

>Signed-off-by: Jiffin Tony Thottan <jthottan@redhat.com>

>Reviewed-on: https://review.gluster.org/17331

>Smoke: Gluster Build System <jenkins@build.gluster.org>

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

>CentOS-regression: Gluster Build System <jenkins@build.gluster.org>

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

>Signed-off-by: Jiffin Tony Thottan <jthottan@redhat.com>

Change-Id: I7a5a1899d3fd1fdb13578b37f9d52a084492e35d

BUG: 1452083

Signed-off-by: Jiffin Tony Thottan <jthottan@redhat.com>

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

Reviewed-by: Atin Mukherjee <amukherj@redhat.com>

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

---

 xlators/cluster/dht/src/dht-common.c   | 73 ++++++++++++++++++++++++++++++++++

 xlators/cluster/dht/src/dht-common.h   |  6 +++

 xlators/cluster/dht/src/dht-linkfile.c |  3 +-

 xlators/storage/posix/src/posix.c      |  3 ++

 4 files changed, 84 insertions(+), 1 deletion(-)

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

index 7465275..480dcbf 100644

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

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

@@ -6438,10 +6438,32 @@ dht_link_cbk (call_frame_t *frame, void *cookie, xlator_t *this,

         int           ret = -1;

         gf_boolean_t  stbuf_merged = _gf_false;

         xlator_t     *subvol = NULL;

+        call_frame_t *cleanup_frame = NULL;

+        dht_local_t  *cleanup_local = NULL;

         local = frame->local;

         if (op_ret == -1) {

+                /* Remove the linkto if exists */

+                if (local->linked) {

+                        cleanup_frame = create_frame (this, this->ctx->pool);

+                        if (cleanup_frame) {

+                                cleanup_local = dht_local_init (cleanup_frame,

+                                                                &local->loc2,

+                                                                NULL, 0);

+                                if (!cleanup_local || !local->link_subvol) {

+                                        DHT_STACK_DESTROY (cleanup_frame);

+                                        goto out;

+                                }

+                                cleanup_local->link_subvol = local->link_subvol;

+                                FRAME_SU_DO (cleanup_frame, dht_local_t);

+                                ret = synctask_new (this->ctx->env,

+                                                    dht_remove_stale_linkto,

+                                                    dht_remove_stale_linkto_cbk,

+                                                    cleanup_frame,

+                                                    cleanup_frame);

+                        }

+                }

                 /* No continuation on DHT inode missing errors, as we should

                  * then have a good stbuf that states P2 happened. We would

                  * get inode missing if, the file completed migrated between

@@ -9592,3 +9614,54 @@ dht_release (xlator_t *this, fd_t *fd)

 {

         return dht_fd_ctx_destroy (this, fd);

 }

+

+int

+dht_remove_stale_linkto (void *data)

+{

+        call_frame_t    *frame = NULL;

+        dht_local_t     *local = NULL;

+        xlator_t        *this  = NULL;

+        dict_t          *xdata_in = NULL;

+        int             ret = 0;

+

+        GF_VALIDATE_OR_GOTO ("dht", data, out);

+

+        frame = data;

+        local = frame->local;

+        this = frame->this;

+        GF_VALIDATE_OR_GOTO ("dht", this, out);

+        GF_VALIDATE_OR_GOTO ("dht", local, out);

+        GF_VALIDATE_OR_GOTO ("dht", local->link_subvol, out);

+

+        xdata_in = dict_new ();

+        if (!xdata_in)

+                goto out;

+

+        ret = dht_fill_dict_to_avoid_unlink_of_migrating_file (xdata_in);

+        if (ret) {

+                  gf_msg (this->name, GF_LOG_WARNING, -ret, 0,

+                                "Failed to set keys for stale linkto"

+                                "deletion on path %s", local->loc.path);

+                  goto out;

+        }

+

+        ret = syncop_unlink (local->link_subvol, &local->loc, xdata_in, NULL);

+        if (ret) {

+                  gf_msg (this->name, GF_LOG_WARNING, -ret, 0,

+                                "Removal of linkto failed"

+                                " on path %s at subvol %s",

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

+

+        }

+out:

+        if (xdata_in)

+                dict_unref (xdata_in);

+        return ret;

+}

+

+int

+dht_remove_stale_linkto_cbk (int ret, call_frame_t *sync_frame, void *data)

+{

+        DHT_STACK_DESTROY (sync_frame);

+        return 0;

+}

diff --git a/xlators/cluster/dht/src/dht-common.h b/xlators/cluster/dht/src/dht-common.h

index 81471f7..b35ef0c 100644

--- a/xlators/cluster/dht/src/dht-common.h

+++ b/xlators/cluster/dht/src/dht-common.h

@@ -1260,4 +1260,10 @@ getChoices (const char *value);

 int

 dht_aggregate_split_brain_xattr (dict_t *dst, char *key, data_t *value);

+int

+dht_remove_stale_linkto (void *data);

+

+int

+dht_remove_stale_linkto_cbk (int ret, call_frame_t *sync_frame, void *data);

+

 #endif/* _DHT_H */

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

index 355d830..101d939 100644

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

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

@@ -88,7 +88,7 @@ dht_linkfile_create_cbk (call_frame_t *frame, void *cookie, xlator_t *this,

                 }

                 STACK_WIND_COOKIE (frame, dht_linkfile_lookup_cbk, subvol,

-                                   subvol, subvol->fops->lookup, &local->loc,

+                                   subvol, subvol->fops->lookup, &local->linkfile.loc,

                                    xattrs);

                 if (xattrs)

                         dict_unref (xattrs);

@@ -119,6 +119,7 @@ dht_linkfile_create (call_frame_t *frame, fop_mknod_cbk_t linkfile_cbk,

         local = frame->local;

         local->linkfile.linkfile_cbk = linkfile_cbk;

         local->linkfile.srcvol = tovol;

+        loc_copy (&local->linkfile.loc, loc);

         local->linked = _gf_false;

diff --git a/xlators/storage/posix/src/posix.c b/xlators/storage/posix/src/posix.c

index 0524d50..a08d858 100644

--- a/xlators/storage/posix/src/posix.c

+++ b/xlators/storage/posix/src/posix.c

@@ -268,6 +268,9 @@ out:

                 op_ret = -1;

                 op_errno = ENODATA;

         }

+

+        if (op_ret == 0)

+                op_errno = 0;

         STACK_UNWIND_STRICT (lookup, frame, op_ret, op_errno,

                              (loc)?loc->inode:NULL, &buf, xattr, &postparent);

-- 

1.8.3.1