From af939370ad20fe1be8e52ea953996e190e86c4ee Mon Sep 17 00:00:00 2001 From: Venky Shankar Date: Sat, 9 Mar 2013 16:36:56 +0530 Subject: cluster/afr: sync xattrs removed on source to sink(s) xattrs are first removed from sink followed by setting source xattrs. Change-Id: I181cb5b785b667bbfc6e40787a2183a8f45de06b BUG: 906646 Signed-off-by: Venky Shankar Reviewed-on: http://review.gluster.org/4656 Reviewed-by: Pranith Kumar Karampuri Tested-by: Gluster Build System Reviewed-by: Jeff Darcy --- tests/bugs/bug-906646.t | 93 +++++++++++++++++++++++ tests/volume.rc | 9 +++ xlators/cluster/afr/src/afr-self-heal-metadata.c | 97 +++++++++++++++++++++--- xlators/storage/posix/src/posix.c | 39 ++++++++++ xlators/storage/posix/src/posix.h | 1 + 5 files changed, 227 insertions(+), 12 deletions(-) create mode 100644 tests/bugs/bug-906646.t diff --git a/tests/bugs/bug-906646.t b/tests/bugs/bug-906646.t new file mode 100644 index 00000000..0e6a3bcb --- /dev/null +++ b/tests/bugs/bug-906646.t @@ -0,0 +1,93 @@ +#!/bin/bash + +. $(dirname $0)/../include.rc +. $(dirname $0)/../volume.rc + +cleanup; + +REPLICA=2 + +TEST glusterd +TEST pidof glusterd + +TEST $CLI volume create $V0 replica $REPLICA $H0:$B0/${V0}-00 $H0:$B0/${V0}-01 $H0:$B0/${V0}-10 $H0:$B0/${V0}-11 +TEST $CLI volume start $V0 + +TEST $CLI volume set $V0 cluster.self-heal-daemon off +TEST $CLI volume set $V0 cluster.background-self-heal-count 0 + +## Mount FUSE with caching disabled +TEST glusterfs --entry-timeout=0 --attribute-timeout=0 -s $H0 --volfile-id $V0 $M0; + +function xattr_query_check() +{ + local path=$1 + local xa_name=$2 + + local ret=`getfattr -m . -n $xa_name $path 2>&1 | grep -o "$xa_name: No such attribute" | wc -l` + echo $ret +} + +function set_xattr() +{ + local path=$1 + local xa_name=$2 + local xa_val=$3 + + setfattr -n $xa_name -v $xa_val $path + echo $? +} + +function remove_xattr() +{ + local path=$1 + local xa_name=$2 + + setfattr -x $xa_name $path + echo $? +} + +f=f00f +pth=$M0/$f + +touch $pth + +# fetch backend paths +backend_paths=`get_backend_paths $pth` + +# convert it into and array +backend_paths_array=($backend_paths) + +# setxattr xattr for this file +EXPECT 0 set_xattr $pth "trusted.name" "test" + +# confirm the set on backend +EXPECT 0 xattr_query_check ${backend_paths_array[0]} "trusted.name" +EXPECT 0 xattr_query_check ${backend_paths_array[1]} "trusted.name" + +brick_path=`echo ${backend_paths_array[0]} | sed -n 's/\(.*\)\/'$f'/\1/p'` +brick_id=`$CLI volume info $V0 | grep "Brick[[:digit:]]" | grep -n $brick_path | cut -f1 -d:` + +# Kill a brick process +TEST kill_brick $V0 $H0 $brick_path + +# remove the xattr from the mount point +EXPECT 0 remove_xattr $pth "trusted.name" + +# we killed ${backend_paths[0]} - so expect the xattr to be there +# on the backend there +EXPECT 0 xattr_query_check ${backend_paths_array[0]} "trusted.name" +EXPECT 1 xattr_query_check ${backend_paths_array[1]} "trusted.name" + +# restart the brick process +TEST $CLI volume start $V0 force + +EXPECT_WITHIN 20 "1" afr_child_up_status $V0 `expr $brick_id - 1` + +stat $pth + +# check backends - xattr should not be present anywhere +EXPECT 1 xattr_query_check ${backend_paths_array[0]} "trusted.name" +EXPECT 1 xattr_query_check ${backend_paths_array[1]} "trusted.name" + +cleanup; diff --git a/tests/volume.rc b/tests/volume.rc index 1b9f8b2c..33a38ee7 100644 --- a/tests/volume.rc +++ b/tests/volume.rc @@ -220,3 +220,12 @@ function dht_get_layout { local my_xa=trusted.glusterfs.dht getfattr -d -e hex -n $my_xa $1 2> /dev/null | grep "$my_xa=" | cut -d= -f2 } + +## + # query pathinfo xattr and extract POSIX pathname(s) + ## +function get_backend_paths { + local path=$1 + + getfattr -m . -n trusted.glusterfs.pathinfo $path | tr ' ' '\n' | sed -n 's/.*/\1/p' +} diff --git a/xlators/cluster/afr/src/afr-self-heal-metadata.c b/xlators/cluster/afr/src/afr-self-heal-metadata.c index 9d512c28..c6a183b1 100644 --- a/xlators/cluster/afr/src/afr-self-heal-metadata.c +++ b/xlators/cluster/afr/src/afr-self-heal-metadata.c @@ -180,8 +180,13 @@ afr_sh_metadata_sync_cbk (call_frame_t *frame, void *cookie, xlator_t *this, call_count = afr_frame_return (frame); - if (call_count == 0) + if (call_count == 0) { + if (local->xattr_req) { + dict_unref (local->xattr_req); + local->xattr_req = NULL; + } afr_sh_metadata_erase_pending (frame, this); + } return 0; } @@ -207,6 +212,78 @@ afr_sh_metadata_xattr_cbk (call_frame_t *frame, void *cookie, xlator_t *this, return 0; } +int +afr_sh_removexattr_cbk (call_frame_t *frame, void *cookie, xlator_t *this, + int32_t op_ret, int32_t op_errno, + dict_t *xdata) +{ + int i = 0; + afr_private_t *priv = NULL; + afr_local_t *local = NULL; + + priv = this->private; + local = frame->local; + + if (op_ret < 0) { + afr_sh_metadata_sync_cbk (frame, cookie, + this, -1, op_errno, xdata); + goto out; + } + + i = (long) cookie; + + STACK_WIND_COOKIE (frame, afr_sh_metadata_xattr_cbk, + (void *) (long) i, + priv->children[i], + priv->children[i]->fops->setxattr, + &local->loc, local->xattr_req, 0, NULL); + + out: + return 0; +} + +inline void +afr_prune_pending_keys (dict_t *xattr_dict, afr_private_t *priv) +{ + int i = 0; + + for (; i < priv->child_count; i++) { + dict_del (xattr_dict, priv->pending_key[i]); + } +} + +int +afr_sh_getxattr_cbk (call_frame_t *frame, void *cookie, xlator_t *this, + int32_t op_ret, int32_t op_errno, dict_t *xattr, + dict_t *xdata) +{ + int i = 0; + afr_private_t *priv = NULL; + afr_local_t *local = NULL; + + priv = this->private; + local = frame->local; + + if (op_ret < 0) { + afr_sh_metadata_sync_cbk (frame, cookie, + this, -1, op_errno, xdata); + goto out; + } + + afr_prune_pending_keys (xattr, priv); + + i = (long) cookie; + + /* send removexattr in bulk via xdata */ + STACK_WIND_COOKIE (frame, afr_sh_removexattr_cbk, + cookie, + priv->children[i], + priv->children[i]->fops->removexattr, + &local->loc, "", xattr); + + out: + return 0; +} int afr_sh_metadata_sync (call_frame_t *frame, xlator_t *this, dict_t *xattr) @@ -232,9 +309,10 @@ afr_sh_metadata_sync (call_frame_t *frame, xlator_t *this, dict_t *xattr) /* * 2 calls per sink - setattr, setxattr */ - if (xattr) + if (xattr) { call_count = active_sinks * 2; - else + local->xattr_req = dict_ref (xattr); + } else call_count = active_sinks; local->call_count = call_count; @@ -277,11 +355,11 @@ afr_sh_metadata_sync (call_frame_t *frame, xlator_t *this, dict_t *xattr) if (!xattr) continue; - STACK_WIND_COOKIE (frame, afr_sh_metadata_xattr_cbk, + STACK_WIND_COOKIE (frame, afr_sh_getxattr_cbk, (void *) (long) i, priv->children[i], - priv->children[i]->fops->setxattr, - &local->loc, xattr, 0, NULL); + priv->children[i]->fops->getxattr, + &local->loc, NULL, NULL); call_count--; } @@ -299,8 +377,6 @@ afr_sh_metadata_getxattr_cbk (call_frame_t *frame, void *cookie, xlator_t *this, afr_private_t *priv = NULL; int source = 0; - int i; - local = frame->local; sh = &local->self_heal; priv = this->private; @@ -315,10 +391,7 @@ afr_sh_metadata_getxattr_cbk (call_frame_t *frame, void *cookie, xlator_t *this, afr_sh_metadata_sync (frame, this, NULL); } else { - for (i = 0; i < priv->child_count; i++) { - dict_del (xattr, priv->pending_key[i]); - } - + afr_prune_pending_keys (xattr, priv); afr_sh_metadata_sync (frame, this, xattr); } diff --git a/xlators/storage/posix/src/posix.c b/xlators/storage/posix/src/posix.c index 9465e1d6..4a786543 100644 --- a/xlators/storage/posix/src/posix.c +++ b/xlators/storage/posix/src/posix.c @@ -2976,6 +2976,28 @@ out: return 0; } +int +_posix_remove_xattr (dict_t *dict, char *key, data_t *value, void *data) +{ + int32_t op_ret = 0; + xlator_t *this = NULL; + posix_xattr_filler_t *filler = NULL; + + filler = (posix_xattr_filler_t *) data; + this = filler->this; + + op_ret = sys_lremovexattr (filler->real_path, key); + if (op_ret == -1) { + filler->op_errno = errno; + if (errno != ENOATTR && errno != EPERM) + gf_log (this->name, GF_LOG_ERROR, + "removexattr failed on %s (for %s): %s", + filler->real_path, key, strerror (errno)); + } + + return op_ret; +} + int32_t posix_removexattr (call_frame_t *frame, xlator_t *this, @@ -2984,6 +3006,7 @@ posix_removexattr (call_frame_t *frame, xlator_t *this, int32_t op_ret = -1; int32_t op_errno = 0; char * real_path = NULL; + posix_xattr_filler_t filler = {0,}; DECLARE_OLD_FS_ID_VAR; @@ -2999,6 +3022,22 @@ posix_removexattr (call_frame_t *frame, xlator_t *this, SET_FS_ID (frame->root->uid, frame->root->gid); + /** + * sending an empty key name with xdata containing the + * list of key(s) to be removed implies "bulk remove request" + * for removexattr. + */ + if (name && (strcmp (name, "") == 0) && xdata) { + filler.real_path = real_path; + filler.this = this; + op_ret = dict_foreach (xdata, _posix_remove_xattr, &filler); + if (op_ret) { + op_errno = filler.op_errno; + } + + goto out; + } + op_ret = sys_lremovexattr (real_path, name); if (op_ret == -1) { op_errno = errno; diff --git a/xlators/storage/posix/src/posix.h b/xlators/storage/posix/src/posix.h index 58f445c6..696422f6 100644 --- a/xlators/storage/posix/src/posix.h +++ b/xlators/storage/posix/src/posix.h @@ -135,6 +135,7 @@ typedef struct { inode_t *inode; /* for all do_xattrop() key handling */ int fd; int flags; + int32_t op_errno; } posix_xattr_filler_t; -- cgit