b7d4d7
From 3f1eee125a35c33ecb078e5d3bfd80d80e63881d Mon Sep 17 00:00:00 2001
b7d4d7
From: Barak Sason Rofman <bsasonro@redhat.com>
b7d4d7
Date: Wed, 15 Jan 2020 12:02:05 +0200
b7d4d7
Subject: [PATCH 502/511] dht - fixing a permission update issue
b7d4d7
b7d4d7
When bringing back a downed brick and performing lookup from the client
b7d4d7
side, the permission on said brick aren't updated on the first lookup,
b7d4d7
but only on the second.
b7d4d7
b7d4d7
This patch modifies permission update logic so the first lookup will
b7d4d7
trigger a permission update on the downed brick.
b7d4d7
b7d4d7
LIMITATIONS OF THE PATCH:
b7d4d7
As the choice of source depends on whether the directory has layout or not.
b7d4d7
Even the directories on the newly added brick will have layout xattr[zeroed], but the same is not true for a root directory.
b7d4d7
Hence, in case in the entire cluster only the newly added bricks are up [and others are down], then any change in permission during this time will be overwritten by the older permissions when the cluster is restarted.
b7d4d7
b7d4d7
Upstream:
b7d4d7
> Reviewed-on: https://review.gluster.org/#/c/glusterfs/+/24020/
b7d4d7
> fixes: #999
b7d4d7
> Change-Id: Ieb70246d41e59f9cae9f70bc203627a433dfbd33
b7d4d7
> Signed-off-by: Barak Sason Rofman <bsasonro@redhat.com>
b7d4d7
b7d4d7
BUG: 1663821
b7d4d7
Change-Id: Ieb70246d41e59f9cae9f70bc203627a433dfbd33
b7d4d7
Signed-off-by: Barak Sason Rofman <bsasonro@redhat.com>
b7d4d7
Reviewed-on: https://code.engineering.redhat.com/gerrit/221116
b7d4d7
Tested-by: RHGS Build Bot <nigelb@redhat.com>
b7d4d7
Reviewed-by: Sunil Kumar Heggodu Gopala Acharya <sheggodu@redhat.com>
b7d4d7
---
b7d4d7
 tests/bugs/bug-1064147.t                 | 71 ++++++++++++++++++++++++++++++++
b7d4d7
 xlators/cluster/dht/src/dht-common.c     | 28 ++++++++++---
b7d4d7
 xlators/cluster/dht/src/dht-selfheal.c   | 15 +++++--
b7d4d7
 xlators/storage/posix/src/posix-common.c | 16 +++----
b7d4d7
 4 files changed, 111 insertions(+), 19 deletions(-)
b7d4d7
 create mode 100755 tests/bugs/bug-1064147.t
b7d4d7
b7d4d7
diff --git a/tests/bugs/bug-1064147.t b/tests/bugs/bug-1064147.t
b7d4d7
new file mode 100755
b7d4d7
index 0000000..617a1aa
b7d4d7
--- /dev/null
b7d4d7
+++ b/tests/bugs/bug-1064147.t
b7d4d7
@@ -0,0 +1,71 @@
b7d4d7
+#!/bin/bash
b7d4d7
+
b7d4d7
+. $(dirname $0)/../include.rc
b7d4d7
+. $(dirname $0)/../volume.rc
b7d4d7
+
b7d4d7
+# Initialize
b7d4d7
+#------------------------------------------------------------
b7d4d7
+cleanup;
b7d4d7
+
b7d4d7
+# Start glusterd
b7d4d7
+TEST glusterd;
b7d4d7
+TEST pidof glusterd;
b7d4d7
+TEST $CLI volume info;
b7d4d7
+
b7d4d7
+# Create a volume
b7d4d7
+TEST $CLI volume create $V0 $H0:/${V0}{1,2};
b7d4d7
+
b7d4d7
+# Verify volume creation
b7d4d7
+ EXPECT "$V0" volinfo_field $V0 'Volume Name';
b7d4d7
+ EXPECT 'Created' volinfo_field $V0 'Status';
b7d4d7
+
b7d4d7
+# Start volume and verify successful start
b7d4d7
+ TEST $CLI volume start $V0;
b7d4d7
+ EXPECT 'Started' volinfo_field $V0 'Status';
b7d4d7
+ TEST glusterfs -s $H0 --volfile-id=$V0 $M0
b7d4d7
+#------------------------------------------------------------
b7d4d7
+
b7d4d7
+# Test case 1 - Subvolume down + Healing
b7d4d7
+#------------------------------------------------------------
b7d4d7
+# Kill 2nd brick process
b7d4d7
+TEST kill -9  `ps aux | grep glusterfsd | grep ${V0}2 | grep -v grep | awk '{print $2}'`;
b7d4d7
+
b7d4d7
+# Change root permissions
b7d4d7
+TEST chmod 444 $M0
b7d4d7
+
b7d4d7
+# Store permission for comparision
b7d4d7
+TEST permission_new=`stat -c "%A" $M0`
b7d4d7
+
b7d4d7
+# Bring up the killed brick process
b7d4d7
+TEST $CLI volume start $V0 force
b7d4d7
+
b7d4d7
+# Perform lookup
b7d4d7
+sleep 5
b7d4d7
+TEST ls $M0
b7d4d7
+
b7d4d7
+# Check brick permissions
b7d4d7
+TEST brick_perm=`stat -c "%A" /${V0}2`
b7d4d7
+TEST [ ${brick_perm} = ${permission_new} ]
b7d4d7
+#------------------------------------------------------------
b7d4d7
+
b7d4d7
+# Test case 2 - Add-brick + Healing
b7d4d7
+#------------------------------------------------------------
b7d4d7
+# Change root permissions
b7d4d7
+TEST chmod 777 $M0
b7d4d7
+
b7d4d7
+# Store permission for comparision
b7d4d7
+TEST permission_new_2=`stat -c "%A" $M0`
b7d4d7
+
b7d4d7
+# Add a 3rd brick
b7d4d7
+TEST $CLI volume add-brick $V0 $H0:/${V0}3
b7d4d7
+
b7d4d7
+# Perform lookup
b7d4d7
+sleep 5
b7d4d7
+TEST ls $M0
b7d4d7
+
b7d4d7
+# Check permissions on the new brick
b7d4d7
+TEST brick_perm2=`stat -c "%A" /${V0}3`
b7d4d7
+
b7d4d7
+TEST [ ${brick_perm2} = ${permission_new_2} ]
b7d4d7
+
b7d4d7
+cleanup;
b7d4d7
diff --git a/xlators/cluster/dht/src/dht-common.c b/xlators/cluster/dht/src/dht-common.c
b7d4d7
index 4db89df..fe1d0ee 100644
b7d4d7
--- a/xlators/cluster/dht/src/dht-common.c
b7d4d7
+++ b/xlators/cluster/dht/src/dht-common.c
b7d4d7
@@ -1363,13 +1363,29 @@ dht_lookup_dir_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
b7d4d7
             dht_aggregate_xattr(local->xattr, xattr);
b7d4d7
         }
b7d4d7
 
b7d4d7
+        if (__is_root_gfid(stbuf->ia_gfid)) {
b7d4d7
+            ret = dht_dir_has_layout(xattr, conf->xattr_name);
b7d4d7
+            if (ret >= 0) {
b7d4d7
+                if (is_greater_time(local->prebuf.ia_ctime,
b7d4d7
+                                    local->prebuf.ia_ctime_nsec,
b7d4d7
+                                    stbuf->ia_ctime, stbuf->ia_ctime_nsec)) {
b7d4d7
+                    /* Choose source */
b7d4d7
+                    local->prebuf.ia_gid = stbuf->ia_gid;
b7d4d7
+                    local->prebuf.ia_uid = stbuf->ia_uid;
b7d4d7
+
b7d4d7
+                    local->prebuf.ia_ctime = stbuf->ia_ctime;
b7d4d7
+                    local->prebuf.ia_ctime_nsec = stbuf->ia_ctime_nsec;
b7d4d7
+                    local->prebuf.ia_prot = stbuf->ia_prot;
b7d4d7
+                }
b7d4d7
+            }
b7d4d7
+        }
b7d4d7
+
b7d4d7
         if (local->stbuf.ia_type != IA_INVAL) {
b7d4d7
             /* This is not the first subvol to respond */
b7d4d7
-            if (!__is_root_gfid(stbuf->ia_gfid) &&
b7d4d7
-                ((local->stbuf.ia_gid != stbuf->ia_gid) ||
b7d4d7
-                 (local->stbuf.ia_uid != stbuf->ia_uid) ||
b7d4d7
-                 (is_permission_different(&local->stbuf.ia_prot,
b7d4d7
-                                          &stbuf->ia_prot)))) {
b7d4d7
+            if ((local->stbuf.ia_gid != stbuf->ia_gid) ||
b7d4d7
+                (local->stbuf.ia_uid != stbuf->ia_uid) ||
b7d4d7
+                (is_permission_different(&local->stbuf.ia_prot,
b7d4d7
+                                         &stbuf->ia_prot))) {
b7d4d7
                 local->need_attrheal = 1;
b7d4d7
             }
b7d4d7
         }
b7d4d7
@@ -10969,7 +10985,7 @@ dht_notify(xlator_t *this, int event, void *data, ...)
b7d4d7
                 if ((cmd == GF_DEFRAG_CMD_STATUS) ||
b7d4d7
                     (cmd == GF_DEFRAG_CMD_STATUS_TIER) ||
b7d4d7
                     (cmd == GF_DEFRAG_CMD_DETACH_STATUS))
b7d4d7
-                	gf_defrag_status_get(conf, output, _gf_false);
b7d4d7
+                    gf_defrag_status_get(conf, output, _gf_false);
b7d4d7
                 else if (cmd == GF_DEFRAG_CMD_START_DETACH_TIER)
b7d4d7
                     gf_defrag_start_detach_tier(defrag);
b7d4d7
                 else if (cmd == GF_DEFRAG_CMD_DETACH_START)
b7d4d7
diff --git a/xlators/cluster/dht/src/dht-selfheal.c b/xlators/cluster/dht/src/dht-selfheal.c
b7d4d7
index f5dfff9..f4e17d1 100644
b7d4d7
--- a/xlators/cluster/dht/src/dht-selfheal.c
b7d4d7
+++ b/xlators/cluster/dht/src/dht-selfheal.c
b7d4d7
@@ -2097,9 +2097,18 @@ dht_selfheal_directory(call_frame_t *frame, dht_selfheal_dir_cbk_t dir_cbk,
b7d4d7
     local->selfheal.dir_cbk = dir_cbk;
b7d4d7
     local->selfheal.layout = dht_layout_ref(this, layout);
b7d4d7
 
b7d4d7
-    if (local->need_attrheal && !IA_ISINVAL(local->mds_stbuf.ia_type)) {
b7d4d7
-        /*Use the one in the mds_stbuf*/
b7d4d7
-        local->stbuf = local->mds_stbuf;
b7d4d7
+    if (local->need_attrheal) {
b7d4d7
+        if (__is_root_gfid(local->stbuf.ia_gfid)) {
b7d4d7
+            local->stbuf.ia_gid = local->prebuf.ia_gid;
b7d4d7
+            local->stbuf.ia_uid = local->prebuf.ia_uid;
b7d4d7
+
b7d4d7
+            local->stbuf.ia_ctime = local->prebuf.ia_ctime;
b7d4d7
+            local->stbuf.ia_ctime_nsec = local->prebuf.ia_ctime_nsec;
b7d4d7
+            local->stbuf.ia_prot = local->prebuf.ia_prot;
b7d4d7
+
b7d4d7
+        } else if (!IA_ISINVAL(local->mds_stbuf.ia_type)) {
b7d4d7
+            local->stbuf = local->mds_stbuf;
b7d4d7
+        }
b7d4d7
     }
b7d4d7
 
b7d4d7
     if (!__is_root_gfid(local->stbuf.ia_gfid)) {
b7d4d7
diff --git a/xlators/storage/posix/src/posix-common.c b/xlators/storage/posix/src/posix-common.c
b7d4d7
index c5a43a1..e5c6e62 100644
b7d4d7
--- a/xlators/storage/posix/src/posix-common.c
b7d4d7
+++ b/xlators/storage/posix/src/posix-common.c
b7d4d7
@@ -598,6 +598,7 @@ posix_init(xlator_t *this)
b7d4d7
     int force_directory = -1;
b7d4d7
     int create_mask = -1;
b7d4d7
     int create_directory_mask = -1;
b7d4d7
+    char value;
b7d4d7
 
b7d4d7
     dir_data = dict_get(this->options, "directory");
b7d4d7
 
b7d4d7
@@ -654,16 +655,11 @@ posix_init(xlator_t *this)
b7d4d7
     }
b7d4d7
 
b7d4d7
     /* Check for Extended attribute support, if not present, log it */
b7d4d7
-    op_ret = sys_lsetxattr(dir_data->data, "trusted.glusterfs.test", "working",
b7d4d7
-                           8, 0);
b7d4d7
-    if (op_ret != -1) {
b7d4d7
-        ret = sys_lremovexattr(dir_data->data, "trusted.glusterfs.test");
b7d4d7
-        if (ret) {
b7d4d7
-            gf_msg(this->name, GF_LOG_DEBUG, errno, P_MSG_INVALID_OPTION,
b7d4d7
-                   "failed to remove xattr: "
b7d4d7
-                   "trusted.glusterfs.test");
b7d4d7
-        }
b7d4d7
-    } else {
b7d4d7
+    size = sys_lgetxattr(dir_data->data, "user.x", &value, sizeof(value));
b7d4d7
+
b7d4d7
+    if ((size == -1) && (errno == EOPNOTSUPP)) {
b7d4d7
+        gf_msg(this->name, GF_LOG_DEBUG, 0, P_MSG_XDATA_GETXATTR,
b7d4d7
+               "getxattr returned %zd", size);
b7d4d7
         tmp_data = dict_get(this->options, "mandate-attribute");
b7d4d7
         if (tmp_data) {
b7d4d7
             if (gf_string2boolean(tmp_data->data, &tmp_bool) == -1) {
b7d4d7
-- 
b7d4d7
1.8.3.1
b7d4d7