From a2cb732a71d341edb65e4cdbf103a3fec35e8589 Mon Sep 17 00:00:00 2001
From: Susant Palai <spalai@redhat.com>
Date: Wed, 17 Aug 2016 15:10:04 +0530
Subject: [PATCH 72/86] cluster/dht: heal root permission post add-brick
Post add-brick event the new brick will have permission of 755
by default. If the root directory permission was other than 755,
that does not get healed to the new brick leading to permission
errors/inconsistencies.
For choosing source of attr heal we can trust the subvols which
have layouts with latest ctime(as part of missing directory heal,
we heal the proper attr). In case none of the subvols have layout,
return ESTALE to retrigger a fresh lookup.
Note: This patch heals the permission of the root directories only.
Since, permission healing of directory is not straight forward and
required intrusive fix, those are not addressed here.
> Reviewed-on: http://review.gluster.org/15195
> Smoke: Gluster Build System <jenkins@build.gluster.org>
> CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
> NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>
>Reviewed-by: Raghavendra G <rgowdapp@redhat.com>
>Reviewed-on: http://review.gluster.org/15465
>Smoke: Gluster Build System <jenkins@build.gluster.org>
>NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>
>CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
>Reviewed-by: Raghavendra G <rgowdapp@redhat.com>
Change-Id: If894e3895d070d46b62d2452e52c1eaafcf56c29
BUG: 1294035
Signed-off-by: Susant Palai <spalai@redhat.com>
Reviewed-on: https://code.engineering.redhat.com/gerrit/84877
Reviewed-by: Atin Mukherjee <amukherj@redhat.com>
Tested-by: Atin Mukherjee <amukherj@redhat.com>
---
tests/bugs/distribute/bug-1368012.t | 50 ++++++++++++++++++++++++++++++++
xlators/cluster/dht/src/dht-common.c | 32 +++++++++++++++++++-
xlators/cluster/dht/src/dht-selfheal.c | 16 +++++++--
3 files changed, 93 insertions(+), 5 deletions(-)
create mode 100644 tests/bugs/distribute/bug-1368012.t
diff --git a/tests/bugs/distribute/bug-1368012.t b/tests/bugs/distribute/bug-1368012.t
new file mode 100644
index 0000000..f89314b
--- /dev/null
+++ b/tests/bugs/distribute/bug-1368012.t
@@ -0,0 +1,50 @@
+#!/bin/bash
+
+. $(dirname $0)/../../include.rc
+. $(dirname $0)/../../volume.rc
+
+cleanup;
+
+function get_permission {
+ stat -c "%A" $1
+}
+
+## Start glusterd
+TEST glusterd;
+TEST pidof glusterd;
+TEST $CLI volume info;
+
+## Lets create volume
+TEST $CLI volume create $V0 $H0:/${V0}{1,2};
+
+## Verify volume is created
+EXPECT "$V0" volinfo_field $V0 'Volume Name';
+EXPECT 'Created' volinfo_field $V0 'Status';
+## Start volume and verify
+TEST $CLI volume start $V0;
+TEST $CLI volume set $V0 performance.stat-prefetch off
+EXPECT 'Started' volinfo_field $V0 'Status';
+TEST glusterfs -s $H0 --volfile-id=$V0 $M0
+
+##Test case: Add-brick
+#------------------------------------------------------------
+#change permission of both root
+TEST chmod 444 $M0
+
+#store permission for comparision
+TEST permission_root=`stat -c "%A" $M0`
+TEST echo $permission_root
+#Add-brick
+TEST $CLI volume add-brick $V0 $H0:/${V0}3
+
+#Allow one lookup to happen
+TEST pushd $M0
+TEST ls
+#Generate another lookup
+echo 3 > /proc/sys/vm/drop_caches
+TEST ls
+#check root permission
+EXPECT_WITHIN "5" $permission_root get_permission $M0
+#check permission on the new-brick
+EXPECT $permission_root get_permission /${V0}3
+cleanup
diff --git a/xlators/cluster/dht/src/dht-common.c b/xlators/cluster/dht/src/dht-common.c
index 5fd0a16..2eb44a1 100644
--- a/xlators/cluster/dht/src/dht-common.c
+++ b/xlators/cluster/dht/src/dht-common.c
@@ -735,6 +735,27 @@ out:
return ret;
}
+int static
+is_permission_different (ia_prot_t *prot1, ia_prot_t *prot2)
+{
+ if ((prot1->owner.read != prot2->owner.read) ||
+ (prot1->owner.write != prot2->owner.write) ||
+ (prot1->owner.exec != prot2->owner.exec) ||
+ (prot1->group.read != prot2->group.read) ||
+ (prot1->group.write != prot2->group.write) ||
+ (prot1->group.exec != prot2->group.exec) ||
+ (prot1->other.read != prot2->other.read) ||
+ (prot1->other.write != prot2->other.write) ||
+ (prot1->other.exec != prot2->other.exec) ||
+ (prot1->suid != prot2->suid) ||
+ (prot1->sgid != prot2->sgid) ||
+ (prot1->sticky != prot2->sticky)) {
+ return 1;
+ } else {
+ return 0;
+ }
+}
+
int
dht_revalidate_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
int op_ret, int op_errno,
@@ -856,14 +877,21 @@ dht_revalidate_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
local->stbuf.ia_ctime_nsec,
stbuf->ia_ctime,
stbuf->ia_ctime_nsec)) {
+ /* Choose source */
local->prebuf.ia_gid = stbuf->ia_gid;
local->prebuf.ia_uid = stbuf->ia_uid;
+
+ if (__is_root_gfid (stbuf->ia_gfid))
+ local->prebuf.ia_prot = stbuf->ia_prot;
}
}
if (local->stbuf.ia_type != IA_INVAL)
{
if ((local->stbuf.ia_gid != stbuf->ia_gid) ||
- (local->stbuf.ia_uid != stbuf->ia_uid)) {
+ (local->stbuf.ia_uid != stbuf->ia_uid) ||
+ (__is_root_gfid (stbuf->ia_gfid) &&
+ is_permission_different (&local->stbuf.ia_prot,
+ &stbuf->ia_prot))) {
local->need_selfheal = 1;
}
}
@@ -928,6 +956,8 @@ out:
gf_uuid_copy (local->gfid, local->stbuf.ia_gfid);
local->stbuf.ia_gid = local->prebuf.ia_gid;
local->stbuf.ia_uid = local->prebuf.ia_uid;
+ if (__is_root_gfid(local->stbuf.ia_gfid))
+ local->stbuf.ia_prot = local->prebuf.ia_prot;
copy = create_frame (this, this->ctx->pool);
if (copy) {
copy_local = dht_local_init (copy, &local->loc,
diff --git a/xlators/cluster/dht/src/dht-selfheal.c b/xlators/cluster/dht/src/dht-selfheal.c
index 520a3f3..f43a235 100644
--- a/xlators/cluster/dht/src/dht-selfheal.c
+++ b/xlators/cluster/dht/src/dht-selfheal.c
@@ -2240,11 +2240,19 @@ dht_dir_attr_heal (void *data)
for (i = 0; i < call_cnt; i++) {
subvol = conf->subvolumes[i];
- if (!subvol || (subvol == dht_first_up_subvol (this)))
+ if (!subvol)
continue;
- ret = syncop_setattr (subvol, &local->loc, &local->stbuf,
- (GF_SET_ATTR_UID | GF_SET_ATTR_GID),
- NULL, NULL, NULL, NULL);
+
+ if (__is_root_gfid (local->stbuf.ia_gfid)) {
+ ret = syncop_setattr (subvol, &local->loc, &local->stbuf,
+ (GF_SET_ATTR_UID | GF_SET_ATTR_GID | GF_SET_ATTR_MODE),
+ NULL, NULL, NULL, NULL);
+ } else {
+ ret = syncop_setattr (subvol, &local->loc, &local->stbuf,
+ (GF_SET_ATTR_UID | GF_SET_ATTR_GID),
+ NULL, NULL, NULL, NULL);
+ }
+
if (ret) {
gf_uuid_unparse(local->loc.gfid, gfid);
--
1.7.1