features/gfid-access: Handle loc modification correctly for virtual dirs

Change-Id: I6e3321534dc2f711519b18e8bffb691ab952a8ba
BUG: 1112659
Signed-off-by: Pranith Kumar K <pkarampu@redhat.com>
Reviewed-on: http://review.gluster.org/8163
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Raghavendra G <rgowdapp@redhat.com>
Reviewed-by: Vijay Bellur <vbellur@redhat.com>

3 files changed, 202 insertions, 120 deletions

diff --git a/tests/basic/gfid-access.t b/tests/basic/gfid-access.t
new file mode 100644
index 00000000000..24477acba5c
--- /dev/null
+++ b/tests/basic/gfid-access.t
@@ -0,0 +1,60 @@
+#!/bin/bash
+
+. $(dirname $0)/../include.rc
+. $(dirname $0)/../volume.rc
+
+cleanup;
+
+TEST glusterd
+TEST pidof glusterd
+TEST $CLI volume create $V0 $H0:$B0/${V0}0
+TEST $CLI volume start $V0
+TEST glusterfs --volfile-id=/$V0 --volfile-server=$H0 $M0 --attribute-timeout=0 --entry-timeout=0 --aux-gfid-mount
+TEST mkdir $M0/a
+TEST touch $M0/b
+a_gfid_str=$(gf_gfid_xattr_to_str $(gf_get_gfid_xattr $B0/${V0}0/a))
+b_gfid_str=$(gf_gfid_xattr_to_str $(gf_get_gfid_xattr $B0/${V0}0/b))
+
+#Operations on Directory
+TEST setfattr -n trusted.abc -v abc $M0/a
+EXPECT "abc" echo $(getfattr -n trusted.abc $M0/a)
+EXPECT "abc" echo $(getfattr -n trusted.abc $M0/.gfid/$a_gfid_str)
+TEST setfattr -x trusted.abc $M0/a
+TEST ! getfattr -n trusted.abc $M0/a
+TEST ! getfattr -n trusted.abc $M0/.gfid/$a_gfid_str
+TEST chmod 0777 $M0/a
+EXPECT "777" stat -c "%a" $M0/a
+EXPECT "777" stat -c "%a" $M0/.gfid/$a_gfid_str
+
+TEST setfattr -n trusted.abc -v def $M0/.gfid/$a_gfid_str
+EXPECT "def" echo $(getfattr -n trusted.abc $M0/a)
+EXPECT "def" echo $(getfattr -n trusted.abc $M0/.gfid/$a_gfid_str)
+TEST setfattr -x trusted.abc $M0/.gfid/$a_gfid_str
+TEST ! getfattr -n trusted.abc $M0/a
+TEST ! getfattr -n trusted.abc $M0/.gfid/$a_gfid_str
+TEST chmod 0777 $M0/.gfid/$a_gfid_str
+EXPECT "777" stat -c "%a" $M0/a
+EXPECT "777" stat -c "%a" $M0/.gfid/$a_gfid_str
+
+#Operations on File
+TEST setfattr -n trusted.abc -v abc $M0/b
+EXPECT "abc" echo $(getfattr -n trusted.abc $M0/b)
+EXPECT "abc" echo $(getfattr -n trusted.abc $M0/.gfid/$b_gfid_str)
+TEST setfattr -x trusted.abc $M0/b
+TEST ! getfattr -n trusted.abc $M0/b
+TEST ! getfattr -n trusted.abc $M0/.gfid/$b_gfid_str
+TEST chmod 0777 $M0/b
+EXPECT "777" stat -c "%a" $M0/b
+EXPECT "777" stat -c "%a" $M0/.gfid/$b_gfid_str
+
+TEST setfattr -n trusted.abc -v def $M0/.gfid/$b_gfid_str
+EXPECT "def" echo $(getfattr -n trusted.abc $M0/b)
+EXPECT "def" echo $(getfattr -n trusted.abc $M0/.gfid/$b_gfid_str)
+TEST setfattr -x trusted.abc $M0/.gfid/$b_gfid_str
+TEST ! getfattr -n trusted.abc $M0/b
+TEST ! getfattr -n trusted.abc $M0/.gfid/$b_gfid_str
+TEST chmod 0777 $M0/.gfid/$b_gfid_str
+EXPECT "777" stat -c "%a" $M0/b
+EXPECT "777" stat -c "%a" $M0/.gfid/$b_gfid_str
+
+cleanup
diff --git a/xlators/features/gfid-access/src/gfid-access.c b/xlators/features/gfid-access/src/gfid-access.c
index 40b2f37e83e..8b699ffc9d4 100644
--- a/xlators/features/gfid-access/src/gfid-access.c
+++ b/xlators/features/gfid-access/src/gfid-access.c
@@ -17,6 +17,48 @@
 #include "byte-order.h"
 
 
+int
+ga_valid_inode_loc_copy (loc_t *dst, loc_t *src, xlator_t *this)
+{
+        int      ret        = 0;
+        uint64_t value      = 0;
+
+        /* if its an entry operation, on the virtual */
+        /* directory inode as parent, we need to handle */
+        /* it properly */
+        ret = loc_copy (dst, src);
+        if (ret < 0)
+                goto out;
+
+        if (dst->parent) {
+                ret = inode_ctx_get (dst->parent, this, &value);
+                if (ret < 0) {
+                        ret = 0; //real-inode
+                        goto out;
+                }
+                inode_unref (dst->parent);
+                dst->parent = inode_ref ((inode_t*)value);
+                /* if parent is virtual, no need to handle */
+                /* loc->inode */
+                goto out;
+        }
+
+        /* if its an inode operation, on the virtual */
+        /* directory inode itself, we need to handle */
+        /* it properly */
+        if (dst->inode) {
+                ret = inode_ctx_get (dst->inode, this, &value);
+                if (ret < 0) {
+                        ret = 0; //real-inode
+                        goto out;
+                }
+                inode_unref (dst->inode);
+                dst->inode = inode_ref ((inode_t*)value);
+                goto out;
+        }
+out:
+        return ret;
+}
 
 void
 ga_newfile_args_free (ga_newfile_args_t *args)
@@ -574,11 +616,10 @@ int32_t
 ga_setxattr (call_frame_t *frame, xlator_t *this, loc_t *loc, dict_t *dict,
              int32_t flags, dict_t *xdata)
 {
-
         data_t  *data     = NULL;
         int      op_errno = ENOMEM;
         int      ret      = 0;
-        inode_t *unref    = NULL;
+        loc_t   ga_loc    = {0, };
 
         GFID_ACCESS_INODE_OP_CHECK (loc, op_errno, err);
 
@@ -600,15 +641,15 @@ ga_setxattr (call_frame_t *frame, xlator_t *this, loc_t *loc, dict_t *dict,
 
         //If the inode is a virtual inode change the inode otherwise perform
         //the operation on same inode
-        GFID_ACCESS_GET_VALID_DIR_INODE (this, loc, unref, wind);
+        ret = ga_valid_inode_loc_copy (&ga_loc, loc, this);
+        if (ret < 0)
+                goto err;
 
-wind:
         STACK_WIND (frame, ga_setxattr_cbk, FIRST_CHILD(this),
-                    FIRST_CHILD(this)->fops->setxattr, loc, dict, flags,
+                    FIRST_CHILD(this)->fops->setxattr, &ga_loc, dict, flags,
                     xdata);
-        if (unref)
-                inode_unref (unref);
 
+        loc_wipe (&ga_loc);
         return 0;
 err:
         STACK_UNWIND_STRICT (setxattr, frame, -1, op_errno, xdata);
@@ -903,7 +944,7 @@ int
 ga_mkdir (call_frame_t *frame, xlator_t *this, loc_t *loc, mode_t mode,
           mode_t umask, dict_t *xdata)
 {
-        int op_errno = 0;
+        int op_errno = ENOMEM;
 
         GFID_ACCESS_ENTRY_OP_CHECK (loc, op_errno, err);
 
@@ -924,7 +965,7 @@ int
 ga_create (call_frame_t *frame, xlator_t *this, loc_t *loc, int flags,
            mode_t mode, mode_t umask, fd_t *fd, dict_t *xdata)
 {
-        int op_errno = 0;
+        int op_errno = ENOMEM;
 
         GFID_ACCESS_ENTRY_OP_CHECK (loc, op_errno, err);
 
@@ -944,7 +985,7 @@ int
 ga_symlink (call_frame_t *frame, xlator_t *this, const char *linkname,
             loc_t *loc, mode_t umask, dict_t *xdata)
 {
-        int op_errno = 0;
+        int op_errno = ENOMEM;
 
         GFID_ACCESS_ENTRY_OP_CHECK (loc, op_errno, err);
 
@@ -963,7 +1004,7 @@ int
 ga_mknod (call_frame_t *frame, xlator_t *this, loc_t *loc, mode_t mode,
           dev_t rdev, mode_t umask, dict_t *xdata)
 {
-        int op_errno = 0;
+        int op_errno = ENOMEM;
 
         GFID_ACCESS_ENTRY_OP_CHECK (loc, op_errno, err);
 
@@ -983,20 +1024,21 @@ int
 ga_rmdir (call_frame_t *frame, xlator_t *this, loc_t *loc, int flag,
           dict_t *xdata)
 {
-        int op_errno = 0;
-        inode_t *unref = NULL;
+        int   op_errno = ENOMEM;
+        int   ret      = -1;
+        loc_t ga_loc   = {0, };
 
         GFID_ACCESS_ENTRY_OP_CHECK (loc, op_errno, err);
 
-        GFID_ACCESS_GET_VALID_DIR_INODE (this, loc, unref, wind);
+        ret = ga_valid_inode_loc_copy (&ga_loc, loc, this);
+        if (ret < 0)
+                goto err;
 
-wind:
         STACK_WIND (frame, default_rmdir_cbk,
                     FIRST_CHILD(this), FIRST_CHILD(this)->fops->rmdir,
-                    loc, flag, xdata);
-        if (unref)
-                inode_unref (unref);
+                    &ga_loc, flag, xdata);
 
+        loc_wipe (&ga_loc);
         return 0;
 err:
         STACK_UNWIND_STRICT (rmdir, frame, -1, op_errno, NULL,
@@ -1009,21 +1051,21 @@ int
 ga_unlink (call_frame_t *frame, xlator_t *this, loc_t *loc, int32_t xflag,
            dict_t *xdata)
 {
-        int op_errno = 0;
-        inode_t *unref = NULL;
+        int   op_errno = ENOMEM;
+        int   ret      = -1;
+        loc_t ga_loc   = {0, };
 
         GFID_ACCESS_ENTRY_OP_CHECK (loc, op_errno, err);
 
-        GFID_ACCESS_GET_VALID_DIR_INODE (this, loc, unref, wind);
+        ret = ga_valid_inode_loc_copy (&ga_loc, loc, this);
+        if (ret < 0)
+                goto err;
 
-wind:
         STACK_WIND (frame, default_unlink_cbk,
                     FIRST_CHILD(this), FIRST_CHILD(this)->fops->unlink,
-                    loc, xflag, xdata);
-
-        if (unref)
-                inode_unref (unref);
+                    &ga_loc, xflag, xdata);
 
+        loc_wipe (&ga_loc);
         return 0;
 err:
         STACK_UNWIND_STRICT (unlink, frame, -1, op_errno, NULL,
@@ -1036,30 +1078,30 @@ int
 ga_rename (call_frame_t *frame, xlator_t *this,
            loc_t *oldloc, loc_t *newloc, dict_t *xdata)
 {
-        int op_errno = 0;
-        inode_t *oldloc_unref = NULL;
-        inode_t *newloc_unref = NULL;
+        int   op_errno  = ENOMEM;
+        int   ret       = 0;
+        loc_t ga_oldloc = {0, };
+        loc_t ga_newloc = {0, };
 
         GFID_ACCESS_ENTRY_OP_CHECK (oldloc, op_errno, err);
         GFID_ACCESS_ENTRY_OP_CHECK (newloc, op_errno, err);
 
-        GFID_ACCESS_GET_VALID_DIR_INODE (this, oldloc, oldloc_unref,
-                                         handle_newloc);
+        ret = ga_valid_inode_loc_copy (&ga_oldloc, oldloc, this);
+        if (ret < 0)
+                goto err;
 
-handle_newloc:
-        GFID_ACCESS_GET_VALID_DIR_INODE (this, newloc, newloc_unref, wind);
+        ret = ga_valid_inode_loc_copy (&ga_newloc, newloc, this);
+        if (ret < 0) {
+                loc_wipe (&ga_oldloc);
+                goto err;
+        }
 
-wind:
         STACK_WIND (frame, default_rename_cbk,
                     FIRST_CHILD(this), FIRST_CHILD(this)->fops->rename,
-                    oldloc, newloc, xdata);
-
-        if (oldloc_unref)
-                inode_unref (oldloc_unref);
-
-        if (newloc_unref)
-                inode_unref (newloc_unref);
+                    &ga_oldloc, &ga_newloc, xdata);
 
+        loc_wipe (&ga_newloc);
+        loc_wipe (&ga_oldloc);
         return 0;
 err:
         STACK_UNWIND_STRICT (rename, frame, -1, op_errno, NULL,
@@ -1073,31 +1115,32 @@ int
 ga_link (call_frame_t *frame, xlator_t *this,
          loc_t *oldloc, loc_t *newloc, dict_t *xdata)
 {
-        int op_errno = 0;
-        inode_t *oldloc_unref = NULL;
-        inode_t *newloc_unref = NULL;
+        int   op_errno  = ENOMEM;
+        int   ret       = 0;
+        loc_t ga_oldloc = {0, };
+        loc_t ga_newloc = {0, };
 
         GFID_ACCESS_ENTRY_OP_CHECK (oldloc, op_errno, err);
         GFID_ACCESS_ENTRY_OP_CHECK (newloc, op_errno, err);
 
-        GFID_ACCESS_GET_VALID_DIR_INODE (this, oldloc, oldloc_unref,
-                                         handle_newloc);
+        ret = ga_valid_inode_loc_copy (&ga_oldloc, oldloc, this);
+        if (ret < 0)
+                goto err;
 
-handle_newloc:
-        GFID_ACCESS_GET_VALID_DIR_INODE (this, newloc, newloc_unref, wind);
+        ret = ga_valid_inode_loc_copy (&ga_newloc, newloc, this);
+        if (ret < 0) {
+                loc_wipe (&ga_oldloc);
+                goto err;
+        }
 
-wind:
         STACK_WIND (frame, default_link_cbk,
                     FIRST_CHILD(this), FIRST_CHILD(this)->fops->link,
-                    oldloc, newloc, xdata);
-
-        if (oldloc_unref)
-                inode_unref (oldloc_unref);
-
-        if (newloc_unref)
-                inode_unref (newloc_unref);
+                    &ga_oldloc, &ga_newloc, xdata);
 
+        loc_wipe (&ga_newloc);
+        loc_wipe (&ga_oldloc);
         return 0;
+
 err:
         STACK_UNWIND_STRICT (link, frame, -1, op_errno, NULL,
                              NULL, NULL, NULL, xdata);
@@ -1109,7 +1152,7 @@ int32_t
 ga_opendir (call_frame_t *frame, xlator_t *this, loc_t *loc,
             fd_t *fd, dict_t *xdata)
 {
-        int op_errno = 0;
+        int op_errno = ENOMEM;
 
         GFID_ACCESS_INODE_OP_CHECK (loc, op_errno, err);
 
@@ -1135,23 +1178,24 @@ int32_t
 ga_getxattr (call_frame_t *frame, xlator_t *this, loc_t *loc,
              const char *name, dict_t *xdata)
 {
-        inode_t     *unref     = NULL;
-        int         op_errno   = 0;
+        int   op_errno = ENOMEM;
+        int   ret      = -1;
+        loc_t ga_loc   = {0, };
 
         GFID_ACCESS_INODE_OP_CHECK (loc, op_errno, err);
-        GFID_ACCESS_GET_VALID_DIR_INODE (this, loc, unref, wind);
+        ret = ga_valid_inode_loc_copy (&ga_loc, loc, this);
+        if (ret < 0)
+                goto err;
 
-wind:
         STACK_WIND (frame, default_getxattr_cbk, FIRST_CHILD(this),
-                    FIRST_CHILD(this)->fops->getxattr, loc, name, xdata);
+                    FIRST_CHILD(this)->fops->getxattr, &ga_loc, name, xdata);
 
-        if (unref)
-                inode_unref (unref);
+        loc_wipe (&ga_loc);
 
         return 0;
-
 err:
         STACK_UNWIND_STRICT (getxattr, frame, -1, op_errno, NULL, xdata);
+
         return 0;
 }
 
@@ -1159,26 +1203,30 @@ int32_t
 ga_stat (call_frame_t *frame, xlator_t *this, loc_t *loc,
          dict_t *xdata)
 {
-        inode_t          *unref         = NULL;
-        ga_private_t     *priv          = NULL;
+        int          op_errno = ENOMEM;
+        int          ret      = -1;
+        loc_t        ga_loc   = {0, };
+        ga_private_t *priv    = NULL;
 
-        GF_ASSERT (this);
         priv = this->private;
-        GF_ASSERT (priv);
-
         /* If stat is on ".gfid" itself, do not wind further,
          * return fake stat and return success.
          */
         if (__is_gfid_access_dir(loc->gfid))
                 goto out;
 
-        GFID_ACCESS_GET_VALID_DIR_INODE (this, loc, unref, wind);
+        ret = ga_valid_inode_loc_copy (&ga_loc, loc, this);
+        if (ret < 0)
+                goto err;
 
-wind:
         STACK_WIND (frame, default_stat_cbk, FIRST_CHILD(this),
-                    FIRST_CHILD(this)->fops->stat, loc, xdata);
-        if (unref)
-                inode_unref (unref);
+                    FIRST_CHILD(this)->fops->stat, &ga_loc, xdata);
+
+        loc_wipe (&ga_loc);
+        return 0;
+
+err:
+        STACK_UNWIND_STRICT (stat, frame, -1, op_errno, NULL, xdata);
 
         return 0;
 
@@ -1192,22 +1240,24 @@ ga_setattr (call_frame_t *frame, xlator_t *this, loc_t *loc,
             struct iatt *stbuf, int32_t valid,
             dict_t *xdata)
 {
-        inode_t     *unref       = NULL;
-        int         op_errno     = 0;
+        int   op_errno = ENOMEM;
+        int   ret      = -1;
+        loc_t ga_loc   = {0, };
 
         GFID_ACCESS_INODE_OP_CHECK (loc, op_errno, err);
-        GFID_ACCESS_GET_VALID_DIR_INODE (this, loc, unref, wind);
+        ret = ga_valid_inode_loc_copy (&ga_loc, loc, this);
+        if (ret < 0)
+                goto err;
 
-wind:
         STACK_WIND (frame, default_setattr_cbk, FIRST_CHILD (this),
-                    FIRST_CHILD (this)->fops->setattr, loc, stbuf, valid,
+                    FIRST_CHILD (this)->fops->setattr, &ga_loc, stbuf, valid,
                     xdata);
-        if (unref)
-                inode_unref (unref);
 
+        loc_wipe (&ga_loc);
         return 0;
 err:
         STACK_UNWIND_STRICT (setattr, frame, -1, op_errno, NULL, NULL, xdata);
+
         return 0;
 }
 
@@ -1215,22 +1265,25 @@ int32_t
 ga_removexattr (call_frame_t *frame, xlator_t *this, loc_t *loc,
                 const char *name, dict_t *xdata)
 {
-        inode_t     *unref       = NULL;
-        int         op_errno     = 0;
+        int   op_errno = ENOMEM;
+        int   ret      = -1;
+        loc_t ga_loc   = {0, };
 
         GFID_ACCESS_INODE_OP_CHECK (loc, op_errno, err);
-        GFID_ACCESS_GET_VALID_DIR_INODE (this, loc, unref, wind);
+        ret = ga_valid_inode_loc_copy (&ga_loc, loc, this);
+        if (ret < 0)
+                goto err;
 
-wind:
         STACK_WIND (frame, default_removexattr_cbk, FIRST_CHILD(this),
-                    FIRST_CHILD(this)->fops->removexattr, loc, name,
+                    FIRST_CHILD(this)->fops->removexattr, &ga_loc, name,
                     xdata);
-        if (unref)
-                inode_unref (unref);
 
+        loc_wipe (&ga_loc);
         return 0;
+
 err:
         STACK_UNWIND_STRICT (removexattr, frame, -1, op_errno, xdata);
+
         return 0;
 }
 
diff --git a/xlators/features/gfid-access/src/gfid-access.h b/xlators/features/gfid-access/src/gfid-access.h
index 11d576191b4..5c7a95af4c8 100644
--- a/xlators/features/gfid-access/src/gfid-access.h
+++ b/xlators/features/gfid-access/src/gfid-access.h
@@ -31,37 +31,6 @@
 #define GF_GFID_DIR ".gfid"
 #define GF_AUX_GFID 0xd
 
-#define GFID_ACCESS_GET_VALID_DIR_INODE(x,l,unref,lbl) do {             \
-                int       ret       = 0;                                \
-                uint64_t  value     = 0;                                \
-                                                                        \
-                /* if its an entry operation, on the virtual */         \
-                /* directory inode as parent, we need to handle */      \
-                /* it properly */                                       \
-                if (l->parent) {                                        \
-                        ret = inode_ctx_get (l->parent, x, &value);     \
-                        if (ret)                                        \
-                                goto lbl;                               \
-                        unref = (inode_t *)value;                       \
-                        l->parent = inode_ref (unref);                  \
-                        /* if parent is virtual, no need to handle */   \
-                        /* loc->inode */                                \
-                        break;                                          \
-                }                                                       \
-                                                                        \
-                /* if its an inode operation, on the virtual */         \
-                /* directory inode itself, we need to handle */         \
-                /* it properly */                                       \
-                if (l->inode) {                                         \
-                        ret = inode_ctx_get (l->inode, x, &value);      \
-                        if (ret)                                        \
-                                goto lbl;                               \
-                        unref = (inode_t *)value;                       \
-                        l->inode = inode_ref (unref);                   \
-                }                                                       \
-                                                                        \
-        } while (0)
-
 #define GFID_ACCESS_ENTRY_OP_CHECK(loc,err,lbl)    do {                 \
                 /* need to check if the lookup is on virtual dir */     \
                 if ((loc->name && !strcmp (GF_GFID_DIR, loc->name)) &&  \