diff options
| -rw-r--r-- | tests/bugs/bug-1584517.t | 70 | ||||
| -rw-r--r-- | xlators/cluster/dht/src/dht-common.c | 81 | ||||
| -rw-r--r-- | xlators/cluster/dht/src/dht-common.h | 3 | ||||
| -rw-r--r-- | xlators/cluster/dht/src/dht-selfheal.c | 2 |
4 files changed, 147 insertions, 9 deletions
diff --git a/tests/bugs/bug-1584517.t b/tests/bugs/bug-1584517.t new file mode 100644 index 00000000000..7f48015a034 --- /dev/null +++ b/tests/bugs/bug-1584517.t @@ -0,0 +1,70 @@ +#!/bin/bash +. $(dirname $0)/../include.rc +. $(dirname $0)/../volume.rc +. $(dirname $0)/../dht.rc +cleanup; +#This test case verifies attributes (uid/gid/perm) for the +#directory are healed after stop/start brick. To verify the same +#test case change attributes of the directory after down a DHT subvolume +#and one AFR children. After start the volume with force and run lookup +#operation attributes should be healed on started bricks at the backend. + + +TEST glusterd +TEST pidof glusterd +TEST $CLI volume create $V0 replica 3 $H0:$B0/${V0}{0,1,2,3,4,5} +TEST $CLI volume start $V0 +TEST useradd dev -M +TEST groupadd QA + +TEST glusterfs --volfile-id=$V0 --volfile-server=$H0 $M0; + +TEST mkdir $M0/dironedown + +TEST kill_brick $V0 $H0 $B0/${V0}2 +EXPECT_WITHIN ${PROCESS_UP_TIMEOUT} "5" online_brick_count + +TEST kill_brick $V0 $H0 $B0/${V0}3 +EXPECT_WITHIN ${PROCESS_UP_TIMEOUT} "4" online_brick_count + +TEST kill_brick $V0 $H0 $B0/${V0}4 +EXPECT_WITHIN ${PROCESS_UP_TIMEOUT} "3" online_brick_count + +TEST kill_brick $V0 $H0 $B0/${V0}5 +EXPECT_WITHIN ${PROCESS_UP_TIMEOUT} "2" online_brick_count + +TEST chown dev $M0/dironedown +TEST chgrp QA $M0/dironedown +TEST chmod 777 $M0/dironedown + +#store the permissions for comparision +permission_onedown=`ls -l $M0 | grep dironedown | awk '{print $1}'` + +TEST $CLI volume start $V0 force +EXPECT_WITHIN ${PROCESS_UP_TIMEOUT} "6" online_brick_count + +TEST glusterfs --volfile-id=$V0 --volfile-server=$H0 $M0; + +#Run lookup two times to hit revalidate code path in dht +# to heal user attr + +TEST ls $M0/dironedown + +#check attributes those were created post brick going down +TEST brick_perm=`ls -l $B0/${V0}3 | grep dironedown | awk '{print $1}'` +TEST echo $brick_perm +TEST [ ${brick_perm} = ${permission_onedown} ] +uid=`ls -l $B0/${V0}3 | grep dironedown | awk '{print $3}'` +TEST echo $uid +TEST [ $uid = dev ] +gid=`ls -l $B0/${V0}3 | grep dironedown | awk '{print $4}'` +TEST echo $gid +TEST [ $gid = QA ] + +TEST umount $M0 +userdel --force dev +groupdel QA + +cleanup +exit + diff --git a/xlators/cluster/dht/src/dht-common.c b/xlators/cluster/dht/src/dht-common.c index 07f00b4a92d..900789ca8aa 100644 --- a/xlators/cluster/dht/src/dht-common.c +++ b/xlators/cluster/dht/src/dht-common.c @@ -1340,6 +1340,8 @@ dht_lookup_dir_cbk (call_frame_t *frame, void *cookie, xlator_t *this, char gfid_local[GF_UUID_BUF_SIZE] = {0}; char gfid_node[GF_UUID_BUF_SIZE] = {0}; int32_t mds_xattr_val[1] = {0}; + call_frame_t *copy = NULL; + dht_local_t *copy_local = NULL; GF_VALIDATE_OR_GOTO ("dht", frame, out); GF_VALIDATE_OR_GOTO ("dht", this, out); @@ -1412,6 +1414,23 @@ dht_lookup_dir_cbk (call_frame_t *frame, void *cookie, xlator_t *this, dht_aggregate_xattr (local->xattr, xattr); } + if (dict_get (xattr, conf->mds_xattr_key)) { + local->mds_subvol = prev; + local->mds_stbuf.ia_gid = stbuf->ia_gid; + local->mds_stbuf.ia_uid = stbuf->ia_uid; + local->mds_stbuf.ia_prot = stbuf->ia_prot; + } + + if (local->stbuf.ia_type != IA_INVAL) { + if (!__is_root_gfid (stbuf->ia_gfid) && + ((local->stbuf.ia_gid != stbuf->ia_gid) || + (local->stbuf.ia_uid != stbuf->ia_uid) || + (is_permission_different (&local->stbuf.ia_prot, + &stbuf->ia_prot)))) { + local->need_attrheal = 1; + } + } + if (local->inode == NULL) local->inode = inode_ref (inode); @@ -1507,6 +1526,43 @@ unlock: &local->postparent, 1); } + if (local->need_attrheal) { + local->need_attrheal = 0; + if (!__is_root_gfid (inode->gfid)) { + gf_uuid_copy (local->gfid, local->mds_stbuf.ia_gfid); + local->stbuf.ia_gid = local->mds_stbuf.ia_gid; + local->stbuf.ia_uid = local->mds_stbuf.ia_uid; + local->stbuf.ia_prot = local->mds_stbuf.ia_prot; + } + copy = create_frame (this, this->ctx->pool); + if (copy) { + copy_local = dht_local_init (copy, &local->loc, + NULL, 0); + if (!copy_local) { + DHT_STACK_DESTROY (copy); + goto skip_attr_heal; + } + copy_local->stbuf = local->stbuf; + copy_local->mds_stbuf = local->mds_stbuf; + copy_local->mds_subvol = local->mds_subvol; + copy->local = copy_local; + FRAME_SU_DO (copy, dht_local_t); + ret = synctask_new (this->ctx->env, + dht_dir_attr_heal, + dht_dir_attr_heal_done, + copy, copy); + if (ret) { + gf_msg (this->name, GF_LOG_ERROR, ENOMEM, + DHT_MSG_DIR_ATTR_HEAL_FAILED, + "Synctask creation failed to heal attr " + "for path %s gfid %s ", + local->loc.path, local->gfid); + DHT_STACK_DESTROY (copy); + } + } + } + +skip_attr_heal: DHT_STRIP_PHASE1_FLAGS (&local->stbuf); dht_set_fixed_dir_stat (&local->postparent); /* Delete mds xattr at the time of STACK UNWIND */ @@ -1527,7 +1583,7 @@ out: return ret; } -int static +int is_permission_different (ia_prot_t *prot1, ia_prot_t *prot2) { if ((prot1->owner.read != prot2->owner.read) || @@ -1688,12 +1744,12 @@ dht_revalidate_cbk (call_frame_t *frame, void *cookie, xlator_t *this, { if ((local->stbuf.ia_gid != stbuf->ia_gid) || (local->stbuf.ia_uid != stbuf->ia_uid) || - (__is_root_gfid (stbuf->ia_gfid) && is_permission_different (&local->stbuf.ia_prot, - &stbuf->ia_prot))) { + &stbuf->ia_prot)) { local->need_selfheal = 1; } } + if (!dict_get (xattr, conf->mds_xattr_key)) { gf_msg_debug (this->name, 0, "internal xattr %s is not present" @@ -1839,10 +1895,9 @@ out: local->need_selfheal = 0; if (!__is_root_gfid (inode->gfid)) { gf_uuid_copy (local->gfid, local->mds_stbuf.ia_gfid); - if (local->mds_stbuf.ia_gid || local->mds_stbuf.ia_uid) { - local->stbuf.ia_gid = local->mds_stbuf.ia_gid; - local->stbuf.ia_uid = local->mds_stbuf.ia_uid; - } + local->stbuf.ia_gid = local->mds_stbuf.ia_gid; + local->stbuf.ia_uid = local->mds_stbuf.ia_uid; + local->stbuf.ia_prot = local->mds_stbuf.ia_prot; } else { gf_uuid_copy (local->gfid, local->stbuf.ia_gfid); local->stbuf.ia_gid = local->prebuf.ia_gid; @@ -1854,8 +1909,10 @@ out: if (copy) { copy_local = dht_local_init (copy, &local->loc, NULL, 0); - if (!copy_local) + if (!copy_local) { + DHT_STACK_DESTROY (copy); goto cont; + } copy_local->stbuf = local->stbuf; copy_local->mds_stbuf = local->mds_stbuf; copy_local->mds_subvol = local->mds_subvol; @@ -1865,6 +1922,14 @@ out: dht_dir_attr_heal, dht_dir_attr_heal_done, copy, copy); + if (ret) { + gf_msg (this->name, GF_LOG_ERROR, ENOMEM, + DHT_MSG_DIR_ATTR_HEAL_FAILED, + "Synctask creation failed to heal attr " + "for path %s gfid %s ", + local->loc.path, local->gfid); + DHT_STACK_DESTROY (copy); + } } } cont: diff --git a/xlators/cluster/dht/src/dht-common.h b/xlators/cluster/dht/src/dht-common.h index 43bceb9fd2d..02560ef94ab 100644 --- a/xlators/cluster/dht/src/dht-common.h +++ b/xlators/cluster/dht/src/dht-common.h @@ -298,6 +298,7 @@ struct dht_local { xlator_t *mds_subvol; /* This is use for dir only */ char need_selfheal; char need_xattr_heal; + char need_attrheal; int file_count; int dir_count; call_frame_t *main_frame; @@ -1488,4 +1489,6 @@ int dht_request_iatt_in_xdata (xlator_t *this, dict_t *xattr_req); int dht_read_iatt_from_xdata (xlator_t *this, dict_t *xdata, struct iatt *stbuf); +int +is_permission_different (ia_prot_t *prot1, ia_prot_t *prot2); #endif/* _DHT_H */ diff --git a/xlators/cluster/dht/src/dht-selfheal.c b/xlators/cluster/dht/src/dht-selfheal.c index 6425bdc506e..2aa75bcc4cd 100644 --- a/xlators/cluster/dht/src/dht-selfheal.c +++ b/xlators/cluster/dht/src/dht-selfheal.c @@ -2500,7 +2500,7 @@ dht_dir_attr_heal (void *data) NULL, NULL, NULL, NULL); } else { ret = syncop_setattr (subvol, &local->loc, &local->mds_stbuf, - (GF_SET_ATTR_UID | GF_SET_ATTR_GID), + (GF_SET_ATTR_UID | GF_SET_ATTR_GID | GF_SET_ATTR_MODE), NULL, NULL, NULL, NULL); } |