|
 |
a3470f |
From 9a95de08eb49f10c0a342099826d5e4c445749aa Mon Sep 17 00:00:00 2001
|
|
|
a3470f |
From: Mohit Agrawal <moagrawa@redhat.com>
|
|
|
a3470f |
Date: Thu, 31 May 2018 12:29:35 +0530
|
|
|
a3470f |
Subject: [PATCH 313/325] dht: Inconsistent permission for directories after
|
|
|
a3470f |
brick stop/start
|
|
|
a3470f |
|
|
|
a3470f |
Problem: Inconsistent access permissions on directories after
|
|
|
a3470f |
bringing back the down sub-volumes, in case of directories dht_setattr
|
|
|
a3470f |
first wind a call on MDS once call is finished on MDS then wind a call
|
|
|
a3470f |
on NON-MDS.At the time of revalidating dht just compare the uid/gid with
|
|
|
a3470f |
stbuf uid/gid and if anyone differs set a flag to heal the same.
|
|
|
a3470f |
|
|
|
a3470f |
Solution: Add a condition to compare permission also in dht_revalidate_cbk
|
|
|
a3470f |
to set a flag to call dht_dir_attr_heal.
|
|
|
a3470f |
|
|
|
a3470f |
> BUG: 1584517
|
|
|
a3470f |
> Change-Id: I3e039607148005015b5d93364536158380d4c5aa
|
|
|
a3470f |
> fixes: bz#1584517
|
|
|
a3470f |
> (cherry picked from commit e57cbae0bcc3d8649b869eda5ec20f3c6a6d34f0)
|
|
|
a3470f |
> (Reviewed on upstream link https://review.gluster.org/#/c/20108/)
|
|
|
a3470f |
|
|
|
a3470f |
BUG: 1582066
|
|
|
a3470f |
Change-Id: I985445521aeeddce52c0a56c20287e523aa3398b
|
|
|
a3470f |
Signed-off-by: Mohit Agrawal <moagrawa@redhat.com>
|
|
|
a3470f |
Reviewed-on: https://code.engineering.redhat.com/gerrit/143721
|
|
|
a3470f |
Tested-by: RHGS Build Bot <nigelb@redhat.com>
|
|
|
a3470f |
Reviewed-by: Sunil Kumar Heggodu Gopala Acharya <sheggodu@redhat.com>
|
|
|
a3470f |
---
|
|
|
a3470f |
tests/bugs/bug-1584517.t | 70 +++++++++++++++++++++++++++++
|
|
|
a3470f |
xlators/cluster/dht/src/dht-common.c | 81 ++++++++++++++++++++++++++++++----
|
|
|
a3470f |
xlators/cluster/dht/src/dht-common.h | 3 ++
|
|
|
a3470f |
xlators/cluster/dht/src/dht-selfheal.c | 2 +-
|
|
|
a3470f |
4 files changed, 147 insertions(+), 9 deletions(-)
|
|
|
a3470f |
create mode 100644 tests/bugs/bug-1584517.t
|
|
|
a3470f |
|
|
|
a3470f |
diff --git a/tests/bugs/bug-1584517.t b/tests/bugs/bug-1584517.t
|
|
|
a3470f |
new file mode 100644
|
|
|
a3470f |
index 0000000..7f48015
|
|
|
a3470f |
--- /dev/null
|
|
|
a3470f |
+++ b/tests/bugs/bug-1584517.t
|
|
|
a3470f |
@@ -0,0 +1,70 @@
|
|
|
a3470f |
+#!/bin/bash
|
|
|
a3470f |
+. $(dirname $0)/../include.rc
|
|
|
a3470f |
+. $(dirname $0)/../volume.rc
|
|
|
a3470f |
+. $(dirname $0)/../dht.rc
|
|
|
a3470f |
+cleanup;
|
|
|
a3470f |
+#This test case verifies attributes (uid/gid/perm) for the
|
|
|
a3470f |
+#directory are healed after stop/start brick. To verify the same
|
|
|
a3470f |
+#test case change attributes of the directory after down a DHT subvolume
|
|
|
a3470f |
+#and one AFR children. After start the volume with force and run lookup
|
|
|
a3470f |
+#operation attributes should be healed on started bricks at the backend.
|
|
|
a3470f |
+
|
|
|
a3470f |
+
|
|
|
a3470f |
+TEST glusterd
|
|
|
a3470f |
+TEST pidof glusterd
|
|
|
a3470f |
+TEST $CLI volume create $V0 replica 3 $H0:$B0/${V0}{0,1,2,3,4,5}
|
|
|
a3470f |
+TEST $CLI volume start $V0
|
|
|
a3470f |
+TEST useradd dev -M
|
|
|
a3470f |
+TEST groupadd QA
|
|
|
a3470f |
+
|
|
|
a3470f |
+TEST glusterfs --volfile-id=$V0 --volfile-server=$H0 $M0;
|
|
|
a3470f |
+
|
|
|
a3470f |
+TEST mkdir $M0/dironedown
|
|
|
a3470f |
+
|
|
|
a3470f |
+TEST kill_brick $V0 $H0 $B0/${V0}2
|
|
|
a3470f |
+EXPECT_WITHIN ${PROCESS_UP_TIMEOUT} "5" online_brick_count
|
|
|
a3470f |
+
|
|
|
a3470f |
+TEST kill_brick $V0 $H0 $B0/${V0}3
|
|
|
a3470f |
+EXPECT_WITHIN ${PROCESS_UP_TIMEOUT} "4" online_brick_count
|
|
|
a3470f |
+
|
|
|
a3470f |
+TEST kill_brick $V0 $H0 $B0/${V0}4
|
|
|
a3470f |
+EXPECT_WITHIN ${PROCESS_UP_TIMEOUT} "3" online_brick_count
|
|
|
a3470f |
+
|
|
|
a3470f |
+TEST kill_brick $V0 $H0 $B0/${V0}5
|
|
|
a3470f |
+EXPECT_WITHIN ${PROCESS_UP_TIMEOUT} "2" online_brick_count
|
|
|
a3470f |
+
|
|
|
a3470f |
+TEST chown dev $M0/dironedown
|
|
|
a3470f |
+TEST chgrp QA $M0/dironedown
|
|
|
a3470f |
+TEST chmod 777 $M0/dironedown
|
|
|
a3470f |
+
|
|
|
a3470f |
+#store the permissions for comparision
|
|
|
a3470f |
+permission_onedown=`ls -l $M0 | grep dironedown | awk '{print $1}'`
|
|
|
a3470f |
+
|
|
|
a3470f |
+TEST $CLI volume start $V0 force
|
|
|
a3470f |
+EXPECT_WITHIN ${PROCESS_UP_TIMEOUT} "6" online_brick_count
|
|
|
a3470f |
+
|
|
|
a3470f |
+TEST glusterfs --volfile-id=$V0 --volfile-server=$H0 $M0;
|
|
|
a3470f |
+
|
|
|
a3470f |
+#Run lookup two times to hit revalidate code path in dht
|
|
|
a3470f |
+# to heal user attr
|
|
|
a3470f |
+
|
|
|
a3470f |
+TEST ls $M0/dironedown
|
|
|
a3470f |
+
|
|
|
a3470f |
+#check attributes those were created post brick going down
|
|
|
a3470f |
+TEST brick_perm=`ls -l $B0/${V0}3 | grep dironedown | awk '{print $1}'`
|
|
|
a3470f |
+TEST echo $brick_perm
|
|
|
a3470f |
+TEST [ ${brick_perm} = ${permission_onedown} ]
|
|
|
a3470f |
+uid=`ls -l $B0/${V0}3 | grep dironedown | awk '{print $3}'`
|
|
|
a3470f |
+TEST echo $uid
|
|
|
a3470f |
+TEST [ $uid = dev ]
|
|
|
a3470f |
+gid=`ls -l $B0/${V0}3 | grep dironedown | awk '{print $4}'`
|
|
|
a3470f |
+TEST echo $gid
|
|
|
a3470f |
+TEST [ $gid = QA ]
|
|
|
a3470f |
+
|
|
|
a3470f |
+TEST umount $M0
|
|
|
a3470f |
+userdel --force dev
|
|
|
a3470f |
+groupdel QA
|
|
|
a3470f |
+
|
|
|
a3470f |
+cleanup
|
|
|
a3470f |
+exit
|
|
|
a3470f |
+
|
|
|
a3470f |
diff --git a/xlators/cluster/dht/src/dht-common.c b/xlators/cluster/dht/src/dht-common.c
|
|
|
a3470f |
index c6adce4..23049b6 100644
|
|
|
a3470f |
--- a/xlators/cluster/dht/src/dht-common.c
|
|
|
a3470f |
+++ b/xlators/cluster/dht/src/dht-common.c
|
|
|
a3470f |
@@ -1329,6 +1329,8 @@ dht_lookup_dir_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
|
|
|
a3470f |
char gfid_local[GF_UUID_BUF_SIZE] = {0};
|
|
|
a3470f |
char gfid_node[GF_UUID_BUF_SIZE] = {0};
|
|
|
a3470f |
int32_t mds_xattr_val[1] = {0};
|
|
|
a3470f |
+ call_frame_t *copy = NULL;
|
|
|
a3470f |
+ dht_local_t *copy_local = NULL;
|
|
|
a3470f |
|
|
|
a3470f |
GF_VALIDATE_OR_GOTO ("dht", frame, out);
|
|
|
a3470f |
GF_VALIDATE_OR_GOTO ("dht", this, out);
|
|
|
a3470f |
@@ -1401,6 +1403,23 @@ dht_lookup_dir_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
|
|
|
a3470f |
dht_aggregate_xattr (local->xattr, xattr);
|
|
|
a3470f |
}
|
|
|
a3470f |
|
|
|
a3470f |
+ if (dict_get (xattr, conf->mds_xattr_key)) {
|
|
|
a3470f |
+ local->mds_subvol = prev;
|
|
|
a3470f |
+ local->mds_stbuf.ia_gid = stbuf->ia_gid;
|
|
|
a3470f |
+ local->mds_stbuf.ia_uid = stbuf->ia_uid;
|
|
|
a3470f |
+ local->mds_stbuf.ia_prot = stbuf->ia_prot;
|
|
|
a3470f |
+ }
|
|
|
a3470f |
+
|
|
|
a3470f |
+ if (local->stbuf.ia_type != IA_INVAL) {
|
|
|
a3470f |
+ if (!__is_root_gfid (stbuf->ia_gfid) &&
|
|
|
a3470f |
+ ((local->stbuf.ia_gid != stbuf->ia_gid) ||
|
|
|
a3470f |
+ (local->stbuf.ia_uid != stbuf->ia_uid) ||
|
|
|
a3470f |
+ (is_permission_different (&local->stbuf.ia_prot,
|
|
|
a3470f |
+ &stbuf->ia_prot)))) {
|
|
|
a3470f |
+ local->need_attrheal = 1;
|
|
|
a3470f |
+ }
|
|
|
a3470f |
+ }
|
|
|
a3470f |
+
|
|
|
a3470f |
if (local->inode == NULL)
|
|
|
a3470f |
local->inode = inode_ref (inode);
|
|
|
a3470f |
|
|
|
a3470f |
@@ -1496,6 +1515,43 @@ unlock:
|
|
|
a3470f |
&local->postparent, 1);
|
|
|
a3470f |
}
|
|
|
a3470f |
|
|
|
a3470f |
+ if (local->need_attrheal) {
|
|
|
a3470f |
+ local->need_attrheal = 0;
|
|
|
a3470f |
+ if (!__is_root_gfid (inode->gfid)) {
|
|
|
a3470f |
+ gf_uuid_copy (local->gfid, local->mds_stbuf.ia_gfid);
|
|
|
a3470f |
+ local->stbuf.ia_gid = local->mds_stbuf.ia_gid;
|
|
|
a3470f |
+ local->stbuf.ia_uid = local->mds_stbuf.ia_uid;
|
|
|
a3470f |
+ local->stbuf.ia_prot = local->mds_stbuf.ia_prot;
|
|
|
a3470f |
+ }
|
|
|
a3470f |
+ copy = create_frame (this, this->ctx->pool);
|
|
|
a3470f |
+ if (copy) {
|
|
|
a3470f |
+ copy_local = dht_local_init (copy, &local->loc,
|
|
|
a3470f |
+ NULL, 0);
|
|
|
a3470f |
+ if (!copy_local) {
|
|
|
a3470f |
+ DHT_STACK_DESTROY (copy);
|
|
|
a3470f |
+ goto skip_attr_heal;
|
|
|
a3470f |
+ }
|
|
|
a3470f |
+ copy_local->stbuf = local->stbuf;
|
|
|
a3470f |
+ copy_local->mds_stbuf = local->mds_stbuf;
|
|
|
a3470f |
+ copy_local->mds_subvol = local->mds_subvol;
|
|
|
a3470f |
+ copy->local = copy_local;
|
|
|
a3470f |
+ FRAME_SU_DO (copy, dht_local_t);
|
|
|
a3470f |
+ ret = synctask_new (this->ctx->env,
|
|
|
a3470f |
+ dht_dir_attr_heal,
|
|
|
a3470f |
+ dht_dir_attr_heal_done,
|
|
|
a3470f |
+ copy, copy);
|
|
|
a3470f |
+ if (ret) {
|
|
|
a3470f |
+ gf_msg (this->name, GF_LOG_ERROR, ENOMEM,
|
|
|
a3470f |
+ DHT_MSG_DIR_ATTR_HEAL_FAILED,
|
|
|
a3470f |
+ "Synctask creation failed to heal attr "
|
|
|
a3470f |
+ "for path %s gfid %s ",
|
|
|
a3470f |
+ local->loc.path, local->gfid);
|
|
|
a3470f |
+ DHT_STACK_DESTROY (copy);
|
|
|
a3470f |
+ }
|
|
|
a3470f |
+ }
|
|
|
a3470f |
+ }
|
|
|
a3470f |
+
|
|
|
a3470f |
+skip_attr_heal:
|
|
|
a3470f |
DHT_STRIP_PHASE1_FLAGS (&local->stbuf);
|
|
|
a3470f |
dht_set_fixed_dir_stat (&local->postparent);
|
|
|
a3470f |
/* Delete mds xattr at the time of STACK UNWIND */
|
|
|
a3470f |
@@ -1516,7 +1572,7 @@ out:
|
|
|
a3470f |
return ret;
|
|
|
a3470f |
}
|
|
|
a3470f |
|
|
|
a3470f |
-int static
|
|
|
a3470f |
+int
|
|
|
a3470f |
is_permission_different (ia_prot_t *prot1, ia_prot_t *prot2)
|
|
|
a3470f |
{
|
|
|
a3470f |
if ((prot1->owner.read != prot2->owner.read) ||
|
|
|
a3470f |
@@ -1677,12 +1733,12 @@ dht_revalidate_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
|
|
|
a3470f |
{
|
|
|
a3470f |
if ((local->stbuf.ia_gid != stbuf->ia_gid) ||
|
|
|
a3470f |
(local->stbuf.ia_uid != stbuf->ia_uid) ||
|
|
|
a3470f |
- (__is_root_gfid (stbuf->ia_gfid) &&
|
|
|
a3470f |
is_permission_different (&local->stbuf.ia_prot,
|
|
|
a3470f |
- &stbuf->ia_prot))) {
|
|
|
a3470f |
+ &stbuf->ia_prot)) {
|
|
|
a3470f |
local->need_selfheal = 1;
|
|
|
a3470f |
}
|
|
|
a3470f |
}
|
|
|
a3470f |
+
|
|
|
a3470f |
if (!dict_get (xattr, conf->mds_xattr_key)) {
|
|
|
a3470f |
gf_msg_debug (this->name, 0,
|
|
|
a3470f |
"internal xattr %s is not present"
|
|
|
a3470f |
@@ -1828,10 +1884,9 @@ out:
|
|
|
a3470f |
local->need_selfheal = 0;
|
|
|
a3470f |
if (!__is_root_gfid (inode->gfid)) {
|
|
|
a3470f |
gf_uuid_copy (local->gfid, local->mds_stbuf.ia_gfid);
|
|
|
a3470f |
- if (local->mds_stbuf.ia_gid || local->mds_stbuf.ia_uid) {
|
|
|
a3470f |
- local->stbuf.ia_gid = local->mds_stbuf.ia_gid;
|
|
|
a3470f |
- local->stbuf.ia_uid = local->mds_stbuf.ia_uid;
|
|
|
a3470f |
- }
|
|
|
a3470f |
+ local->stbuf.ia_gid = local->mds_stbuf.ia_gid;
|
|
|
a3470f |
+ local->stbuf.ia_uid = local->mds_stbuf.ia_uid;
|
|
|
a3470f |
+ local->stbuf.ia_prot = local->mds_stbuf.ia_prot;
|
|
|
a3470f |
} else {
|
|
|
a3470f |
gf_uuid_copy (local->gfid, local->stbuf.ia_gfid);
|
|
|
a3470f |
local->stbuf.ia_gid = local->prebuf.ia_gid;
|
|
|
a3470f |
@@ -1843,8 +1898,10 @@ out:
|
|
|
a3470f |
if (copy) {
|
|
|
a3470f |
copy_local = dht_local_init (copy, &local->loc,
|
|
|
a3470f |
NULL, 0);
|
|
|
a3470f |
- if (!copy_local)
|
|
|
a3470f |
+ if (!copy_local) {
|
|
|
a3470f |
+ DHT_STACK_DESTROY (copy);
|
|
|
a3470f |
goto cont;
|
|
|
a3470f |
+ }
|
|
|
a3470f |
copy_local->stbuf = local->stbuf;
|
|
|
a3470f |
copy_local->mds_stbuf = local->mds_stbuf;
|
|
|
a3470f |
copy_local->mds_subvol = local->mds_subvol;
|
|
|
a3470f |
@@ -1854,6 +1911,14 @@ out:
|
|
|
a3470f |
dht_dir_attr_heal,
|
|
|
a3470f |
dht_dir_attr_heal_done,
|
|
|
a3470f |
copy, copy);
|
|
|
a3470f |
+ if (ret) {
|
|
|
a3470f |
+ gf_msg (this->name, GF_LOG_ERROR, ENOMEM,
|
|
|
a3470f |
+ DHT_MSG_DIR_ATTR_HEAL_FAILED,
|
|
|
a3470f |
+ "Synctask creation failed to heal attr "
|
|
|
a3470f |
+ "for path %s gfid %s ",
|
|
|
a3470f |
+ local->loc.path, local->gfid);
|
|
|
a3470f |
+ DHT_STACK_DESTROY (copy);
|
|
|
a3470f |
+ }
|
|
|
a3470f |
}
|
|
|
a3470f |
}
|
|
|
a3470f |
cont:
|
|
|
a3470f |
diff --git a/xlators/cluster/dht/src/dht-common.h b/xlators/cluster/dht/src/dht-common.h
|
|
|
a3470f |
index a70342f..b40815c 100644
|
|
|
a3470f |
--- a/xlators/cluster/dht/src/dht-common.h
|
|
|
a3470f |
+++ b/xlators/cluster/dht/src/dht-common.h
|
|
|
a3470f |
@@ -298,6 +298,7 @@ struct dht_local {
|
|
|
a3470f |
xlator_t *mds_subvol; /* This is use for dir only */
|
|
|
a3470f |
char need_selfheal;
|
|
|
a3470f |
char need_xattr_heal;
|
|
|
a3470f |
+ char need_attrheal;
|
|
|
a3470f |
int file_count;
|
|
|
a3470f |
int dir_count;
|
|
|
a3470f |
call_frame_t *main_frame;
|
|
|
a3470f |
@@ -1491,4 +1492,6 @@ int
|
|
|
a3470f |
dht_selfheal_dir_setattr (call_frame_t *frame, loc_t *loc, struct iatt *stbuf,
|
|
|
a3470f |
int32_t valid, dht_layout_t *layout);
|
|
|
a3470f |
|
|
|
a3470f |
+int
|
|
|
a3470f |
+is_permission_different (ia_prot_t *prot1, ia_prot_t *prot2);
|
|
|
a3470f |
#endif/* _DHT_H */
|
|
|
a3470f |
diff --git a/xlators/cluster/dht/src/dht-selfheal.c b/xlators/cluster/dht/src/dht-selfheal.c
|
|
|
a3470f |
index e9b1db9..035a709 100644
|
|
|
a3470f |
--- a/xlators/cluster/dht/src/dht-selfheal.c
|
|
|
a3470f |
+++ b/xlators/cluster/dht/src/dht-selfheal.c
|
|
|
a3470f |
@@ -2495,7 +2495,7 @@ dht_dir_attr_heal (void *data)
|
|
|
a3470f |
NULL, NULL, NULL, NULL);
|
|
|
a3470f |
} else {
|
|
|
a3470f |
ret = syncop_setattr (subvol, &local->loc, &local->mds_stbuf,
|
|
|
a3470f |
- (GF_SET_ATTR_UID | GF_SET_ATTR_GID),
|
|
|
a3470f |
+ (GF_SET_ATTR_UID | GF_SET_ATTR_GID | GF_SET_ATTR_MODE),
|
|
|
a3470f |
NULL, NULL, NULL, NULL);
|
|
|
a3470f |
}
|
|
|
a3470f |
|
|
|
a3470f |
--
|
|
|
a3470f |
1.8.3.1
|
|
|
a3470f |
|