From e8a94eab232c3b8297a9216b8c8d2a6bdb7656fb Mon Sep 17 00:00:00 2001

From: Saravanakumar Arumugam <sarumuga@redhat.com>

Date: Fri, 8 Jul 2016 19:10:45 +0530

Subject: [PATCH 397/406] geo-rep: filter out xtime attribute during getxattr

georep gsyncd's xtime needs to filtered irrespective

of any process access.

This way, we can avoid (unnecessarily)syncing xtime attribute

to slave, which may raise permission denied errors.

test case modified to check for xtime xattr only in backend.

Back port of>

>Change-Id: I2390b703048d5cc747d91fa2ae884dc55de58669

>BUG: 1353952

>Signed-off-by: Saravanakumar Arumugam <sarumuga@redhat.com>

>Signed-off-by: Mohammed Rafi KC <rkavunga@redhat.com>

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

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

>Reviewed-by: Kotresh HR <khiremat@redhat.com>

>Tested-by: Kotresh HR <khiremat@redhat.com>

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

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

>Reviewed-by: Pranith Kumar Karampuri <pkarampu@redhat.com>

Change-Id: Ibdee6f3093648a7e0fb1e2b6be8172e604ab657f

BUG: 1327045

Signed-off-by: Mohammed Rafi KC <rkavunga@redhat.com>

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

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

---

 tests/basic/geo-replication/marker-xattrs.t | 17 ++++-----

 tests/bugs/geo-replication/bug-1296496.t    |  3 +-

 tests/bugs/geo-replication/bug-877293.t     |  4 +--

 xlators/storage/posix/src/posix-helpers.c   | 53 ++++++++++++++++++++++++++++-

 xlators/storage/posix/src/posix.c           | 15 ++++----

 xlators/storage/posix/src/posix.h           |  3 +-

 6 files changed, 73 insertions(+), 22 deletions(-)

diff --git a/tests/basic/geo-replication/marker-xattrs.t b/tests/basic/geo-replication/marker-xattrs.t

index dd5483d..e5b26a6 100755

--- a/tests/basic/geo-replication/marker-xattrs.t

+++ b/tests/basic/geo-replication/marker-xattrs.t

@@ -24,11 +24,11 @@ TEST touch $M0

 vol_uuid=$(get_volume_mark $M1)

 xtime=trusted.glusterfs.$vol_uuid.xtime

-TEST "getfattr -n $xtime $M1 | grep -q ${xtime}="

+TEST "getfattr -n $xtime $B0/${V0}-1 | grep -q ${xtime}="

 TEST kill_brick $V0 $H0 $B0/${V0}-0

-TEST "getfattr -n $xtime $M1 | grep -q ${xtime}="

+TEST "getfattr -n $xtime $B0/${V0}-1 | grep -q ${xtime}="

 TEST getfattr -d -m. -e hex $M1

 EXPECT_WITHIN $UMOUNT_TIMEOUT "Y" force_umount $M0

@@ -61,13 +61,13 @@ vol_uuid=$(get_volume_mark $M1)

 xtime=trusted.glusterfs.$vol_uuid.xtime

 stime=trusted.glusterfs.$vol_uuid.stime

-stime_val=$(getfattr -e hex -n $xtime $M1 | grep ${xtime}= | cut -f2 -d'=')

+stime_val=$(getfattr -e hex -n $xtime $B0/${V0}-1 | grep ${xtime}= | cut -f2 -d'=')

 TEST "setfattr -n $stime -v $stime_val $B0/${V0}-1"

-TEST "getfattr -n $xtime $M1 | grep -q ${xtime}="

+TEST "getfattr -n $xtime $B0/${V0}-1 | grep -q ${xtime}="

 TEST kill_brick $V0 $H0 $B0/${V0}-0

-TEST "getfattr -n $xtime $M1 | grep -q ${xtime}="

+TEST "getfattr -n $xtime $B0/${V0}-1 | grep -q ${xtime}="

 TEST "getfattr -n $stime $M1 | grep -q ${stime}="

 TEST getfattr -d -m. -e hex $M1

@@ -98,12 +98,9 @@ TEST touch $M0

 vol_uuid=$(get_volume_mark $M1)

 xtime=trusted.glusterfs.$vol_uuid.xtime

-TEST "getfattr -n $xtime $M1 | grep -q ${xtime}="

+TEST "getfattr -n $xtime $B0/${V0}-0 | grep -q ${xtime}="

-TEST kill_brick $V0 $H0 $B0/${V0}-0

-

-#Stripe doesn't tolerate ENOTCONN

-TEST ! "getfattr -n $xtime $M1 | grep -q ${xtime}="

+TEST "getfattr -n $xtime $B0/${V0}-1 | grep -q ${xtime}="

 EXPECT_WITHIN $UMOUNT_TIMEOUT "Y" force_umount $M0

 EXPECT_WITHIN $UMOUNT_TIMEOUT "Y" force_umount $M1

diff --git a/tests/bugs/geo-replication/bug-1296496.t b/tests/bugs/geo-replication/bug-1296496.t

index 703fda6..a157be7 100644

--- a/tests/bugs/geo-replication/bug-1296496.t

+++ b/tests/bugs/geo-replication/bug-1296496.t

@@ -29,7 +29,8 @@ xtime="trusted.glusterfs.$vol_uuid.xtime"

 #TEST xtime

 TEST ! getfattr -n $xtime $M0

-TEST getfattr -n $xtime $M1

+TEST getfattr -n $xtime $B0/${V0}-0

+TEST getfattr -n $xtime $B0/${V0}-1

 #TEST stime

 slave_uuid=$(uuidgen)

diff --git a/tests/bugs/geo-replication/bug-877293.t b/tests/bugs/geo-replication/bug-877293.t

index 542774a..c5205e8 100755

--- a/tests/bugs/geo-replication/bug-877293.t

+++ b/tests/bugs/geo-replication/bug-877293.t

@@ -26,11 +26,11 @@ TEST touch $M0

 vol_uuid=`getfattr -n trusted.glusterfs.volume-mark -ehex $M1 | sed -n 's/^trusted.glusterfs.volume-mark=0x//p' | cut -b5-36 | sed 's/\([a-f0-9]\{8\}\)\([a-f0-9]\{4\}\)\([a-f0-9]\{4\}\)\([a-f0-9]\{4\}\)/\1-\2-\3-\4-/'`

 xtime=trusted.glusterfs.$vol_uuid.xtime

-TEST "getfattr -n $xtime $M1 | grep -q ${xtime}="

+TEST "getfattr -n $xtime $B0/${V0}-0 | grep -q ${xtime}="

 TEST kill_brick $V0 $H0 $B0/${V0}-0

-TEST "getfattr -n $xtime $M1 | grep -q ${xtime}="

+TEST "getfattr -n $xtime $B0/${V0}-1 | grep -q ${xtime}="

 EXPECT_WITHIN $UMOUNT_TIMEOUT "Y" force_umount $M0

 EXPECT_WITHIN $UMOUNT_TIMEOUT "Y" force_umount $M1

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

index 24b8bdd..dd6791c 100644

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

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

@@ -96,6 +96,57 @@ out:

         return flag;

 }

+int

+posix_handle_georep_xattrs (call_frame_t *frame, const char *name,

+                            int *op_errno, gf_boolean_t is_getxattr)

+{

+

+        int                i                = 0;

+        int                ret              = 0;

+        int                pid              = 1;

+        gf_boolean_t       filter_xattr     = _gf_true;

+        static const char *georep_xattr[]   = { "*.glusterfs.*.stime",

+                                                "*.glusterfs.*.xtime",

+                                                "*.glusterfs.*.entry_stime",

+                                                NULL

+                                              };

+        if (frame && frame->root) {

+                pid = frame->root->pid;

+        }

+

+        if (!name) {

+                /* No need to do anything here */

+                ret = 0;

+                goto out;

+        }

+

+        if (pid == GF_CLIENT_PID_GSYNCD && is_getxattr) {

+                filter_xattr = _gf_false;

+

+                /* getxattr from gsyncd process should return all the

+                 * internal xattr. In other cases ignore such xattrs

+                 */

+        }

+

+        for (i = 0; filter_xattr && georep_xattr[i]; i++) {

+                if (fnmatch (georep_xattr[i] , name, FNM_PERIOD) == 0) {

+                        ret = -1;

+                        if (op_errno)

+                               *op_errno = ENOATTR;

+

+                        gf_msg_debug ("posix", ENOATTR,

+                                      "Ignoring the key %s as an internal "

+                                      "xattrs.", name);

+                        goto out;

+                }

+        }

+

+        ret = 0;

+out:

+        return ret;

+}

+

+

 static gf_boolean_t

 _is_in_array (char **str_array, char *str)

 {

@@ -729,7 +780,7 @@ _handle_list_xattr (dict_t *xattr_req, const char *real_path, int fdnum,

                 if (posix_special_xattr (marker_xattrs, key))

                         goto next;

-                if (!fnmatch (GF_XATTR_STIME_PATTERN, key, 0))

+                if (posix_handle_georep_xattrs (NULL, key, NULL, _gf_false))

                         goto next;

                 if (dict_get (filler->xattr, key))

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

index 207c7fd..ed70782 100644

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

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

@@ -4313,11 +4313,10 @@ posix_getxattr (call_frame_t *frame, xlator_t *this,

         op_ret = -1;

         priv = this->private;

-        /* Allow access to stime xattr only to geo-rep worker */

-        if (frame->root->pid != GF_CLIENT_PID_GSYNCD && name &&

-            fnmatch ("*.glusterfs.*.stime", name, FNM_PERIOD) == 0) {

+        ret = posix_handle_georep_xattrs (frame, name, &op_errno, _gf_true);

+        if (ret == -1) {

                 op_ret = -1;

-                op_errno = ENOATTR;

+                /* errno should be set from the above function*/

                 goto out;

         }

@@ -4629,9 +4628,11 @@ posix_getxattr (call_frame_t *frame, xlator_t *this,

         remaining_size = size;

         list_offset = 0;

         while (remaining_size > 0) {

-                strcpy (keybuffer, list + list_offset);

-                if (frame->root->pid != GF_CLIENT_PID_GSYNCD &&

-                    fnmatch ("*.glusterfs.*.stime", keybuffer, FNM_PERIOD) == 0)

+                strncpy (keybuffer, list + list_offset, sizeof(keybuffer));

+

+                ret = posix_handle_georep_xattrs (frame, keybuffer, NULL,

+                                                  _gf_false);

+                if (ret == -1)

                         goto ignore;

                 size = sys_lgetxattr (real_path, keybuffer, NULL, 0);

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

index 87f91e5..6fd32c1 100644

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

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

@@ -259,7 +259,8 @@ int

 posix_get_ancestry (xlator_t *this, inode_t *leaf_inode,

                     gf_dirent_t *head, char **path, int type, int32_t *op_errno,

                     dict_t *xdata);

-

+int

+posix_handle_georep_xattrs (call_frame_t *, const char *, int *, gf_boolean_t);

 void

 posix_gfid_unset (xlator_t *this, dict_t *xdata);

-- 

1.8.3.1