diff options
| -rw-r--r-- | libglusterfs/src/syncop-utils.c | 18 | ||||
| -rw-r--r-- | tests/bugs/posix/bug-gfid-path.t | 70 | ||||
| -rw-r--r-- | xlators/features/bit-rot/src/stub/bit-rot-stub-helpers.c | 38 | ||||
| -rw-r--r-- | xlators/storage/posix/src/posix-gfid-path.c | 26 | ||||
| -rw-r--r-- | xlators/storage/posix/src/posix-gfid-path.h | 1 |
5 files changed, 127 insertions, 26 deletions
diff --git a/libglusterfs/src/syncop-utils.c b/libglusterfs/src/syncop-utils.c index 40ced03cb45..1ccda55e4c5 100644 --- a/libglusterfs/src/syncop-utils.c +++ b/libglusterfs/src/syncop-utils.c @@ -591,9 +591,21 @@ syncop_gfid_to_path_hard (inode_table_t *itable, xlator_t *subvol, uuid_t gfid, if (ret < 0) goto out; - ret = dict_get_str (xattr, hard_resolve ? - GFID2PATH_VIRT_XATTR_KEY : GFID_TO_PATH_KEY, - &path); + + /* + * posix will do dict_set_dynstr for GFID_TO_PATH_KEY i.e. + * for in memory search for the path. And for on disk xattr + * fetching of the path for the key GFID2PATH_VIRT_XATTR_KEY + * it uses dict_set_dynptr. So, for GFID2PATH_VIRT_XATTR_KEY + * use dict_get_ptr to avoid dict complaining about type + * mismatch (i.e. str vs ptr) + */ + if (!hard_resolve) + ret = dict_get_str (xattr, GFID_TO_PATH_KEY, &path); + else + ret = dict_get_ptr (xattr, GFID2PATH_VIRT_XATTR_KEY, + (void **)&path); + if (ret || !path) { ret = -EINVAL; goto out; diff --git a/tests/bugs/posix/bug-gfid-path.t b/tests/bugs/posix/bug-gfid-path.t new file mode 100644 index 00000000000..1bbbe9f0670 --- /dev/null +++ b/tests/bugs/posix/bug-gfid-path.t @@ -0,0 +1,70 @@ +#!/bin/bash + +# This test case is for the bug where, even though a file is +# created when gfid2path option is turned off (default is ON), +# getfattr of "glusterfs.gfidtopath" was succeeding for that +# file. Ideally the getfattr should fail, as the file does not +# have its path(s) stored as a extended attribute (because it +# was created when gfid2path option was off) + +. $(dirname $0)/../../include.rc +. $(dirname $0)/../../volume.rc + +cleanup; + +TEST glusterd +TEST pidof glusterd +TEST $CLI volume info; + +TEST $CLI volume create $V0 $H0:$B0/${V0}{1,2,3,4}; + +EXPECT "$V0" volinfo_field $V0 'Volume Name'; +EXPECT 'Created' volinfo_field $V0 'Status'; +EXPECT '4' brick_count $V0 + +TEST $CLI volume start $V0; +EXPECT 'Started' volinfo_field $V0 'Status'; + +TEST glusterfs -s $H0 --volfile-id $V0 $M0; + +TEST mkdir $M0/dir +TEST mkdir $M0/new +TEST mkdir $M0/3 + +TEST touch $M0/dir/file + +# except success as by default gfid2path is enabled +# and the glusterfs.gfidtopath xattr should give the +# path of the object as the value + +TEST getfattr -n glusterfs.gfidtopath $M0/dir/file + +# turn off gfid2path feature +TEST $CLI volume set $V0 storage.gfid2path off + +TEST touch $M0/new/foo + +# again enable gfid2path. This has to be enabled before +# trying the getfattr. Because, glusterfs.gfidtopath xattr +# request is handled only if gfid2path is enabled. If not, +# then getxattr on glusterfs.gfid2path fails anyways. In this +# context we want getfattr to fail, because the file was created +# when gfid2path feature was disabled and not because gfid2path +# feature itself is disabled. +TEST $CLI volume set $V0 storage.gfid2path on + +# getfattr should fail as it is attempted on a file +# which does not have its path stored as a xattr +# (because file got created after disabling gfid2path) +TEST ! getfattr -n glusterfs.gfidtopath $M0/new/foo; + + + +TEST touch $M0/3/new + +# should be successful +TEST getfattr -n glusterfs.gfidtopath $M0/3/new + +TEST rm -rf $M0/* + +cleanup; diff --git a/xlators/features/bit-rot/src/stub/bit-rot-stub-helpers.c b/xlators/features/bit-rot/src/stub/bit-rot-stub-helpers.c index 24aa9aa5ff4..fff7a7eedf2 100644 --- a/xlators/features/bit-rot/src/stub/bit-rot-stub-helpers.c +++ b/xlators/features/bit-rot/src/stub/bit-rot-stub-helpers.c @@ -722,35 +722,21 @@ br_stub_get_path_of_gfid (xlator_t *this, inode_t *parent, inode_t *inode, GF_VALIDATE_OR_GOTO (this->name, parent, out); GF_VALIDATE_OR_GOTO (this->name, path, out); - /* No need to validate the @inode for hard resolution. Because inode - * can be NULL and if it is NULL, then syncop_gfid_to_path_hard will - * allocate a new inode and proceed. So no need to bother about + /* Above, No need to validate the @inode for hard resolution. Because + * inode can be NULL and if it is NULL, then syncop_gfid_to_path_hard + * will allocate a new inode and proceed. So no need to bother about * @inode. Because we need it only to send a syncop_getxattr call * from inside syncop_gfid_to_path_hard. And getxattr fetches the * path from the backend. */ + ret = syncop_gfid_to_path_hard (parent->table, FIRST_CHILD (this), gfid, inode, path, _gf_true); - - /* - * This is to handle those corrupted files which does not contain - * the gfid2path xattr in the backend (because they were created - * when the option was OFF OR it was upgraded from a version before - * gfid2path was brought in. - * Ideally posix should be returning ret < 0 i.e. error if the - * gfid2path xattr is not present. But for some reason it is - * returning success and path as "". THis is causing problems. - * For now handling it by adding extra checks. But the better way - * is to make posix return error if gfid2path xattr is absent. - * When that is done remove below if block and also this entire - * comment. - */ - if (ret >= 0 && !strlen (*path)) { + if (ret < 0) gf_msg (this->name, GF_LOG_WARNING, 0, BRS_MSG_PATH_GET_FAILED, - "path for the object %s is %s. Going for in memory path", - uuid_utoa_r (gfid, gfid_str), *path); - ret = -1; - } + "failed to get the path xattr from disk for the " + " gfid %s. Trying to get path from the memory", + uuid_utoa_r (gfid, gfid_str)); /* * Try with soft resolution of path if hard resolve fails. Because @@ -767,10 +753,16 @@ br_stub_get_path_of_gfid (xlator_t *this, inode_t *parent, inode_t *inode, * found in the inode table and better not to do inode_path() on the * inode which has not been linked. */ - if (ret < 0 && inode) + if (ret < 0 && inode) { ret = syncop_gfid_to_path_hard (parent->table, FIRST_CHILD (this), gfid, inode, path, _gf_false); + if (ret < 0) + gf_msg (this->name, GF_LOG_WARNING, 0, + BRS_MSG_PATH_GET_FAILED, + "failed to get the path from the memory for gfid %s", + uuid_utoa_r (gfid, gfid_str)); + } out: return ret; diff --git a/xlators/storage/posix/src/posix-gfid-path.c b/xlators/storage/posix/src/posix-gfid-path.c index 2834c2fc4fd..9873e7e5a84 100644 --- a/xlators/storage/posix/src/posix-gfid-path.c +++ b/xlators/storage/posix/src/posix-gfid-path.c @@ -123,6 +123,7 @@ posix_get_gfid2path (xlator_t *this, inode_t *inode, const char *real_path, gf_boolean_t have_val = _gf_false; struct posix_private *priv = NULL; char pargfid_str[UUID_CANONICAL_FORM_LEN + 1] = {0,}; + gf_boolean_t found = _gf_false; priv = this->private; @@ -141,6 +142,7 @@ posix_get_gfid2path (xlator_t *this, inode_t *inode, const char *real_path, "value for key (%s)", GFID2PATH_VIRT_XATTR_KEY); goto err; } + found = _gf_true; } else { have_val = _gf_false; memset (value_buf, '\0', sizeof(value_buf)); @@ -202,6 +204,7 @@ posix_get_gfid2path (xlator_t *this, inode_t *inode, const char *real_path, goto ignore; } + found = _gf_true; memset (value_buf, '\0', sizeof(value_buf)); size = sys_lgetxattr (real_path, keybuffer, value_buf, sizeof (value_buf) - 1); @@ -231,6 +234,29 @@ ignore: list_offset += strlen (keybuffer) + 1; } /* while (remaining_size > 0) */ + /* gfid2path xattr is absent in the list of xattrs */ + if (!found) { + ret = -1; + /* + * ENODATA because xattr is not present in the + * list of xattrs. Thus the consumer should + * face error instead of a success and a empty + * string in the dict for the key. + */ + *op_errno = ENODATA; + goto err; + } + + /* + * gfid2path xattr is found in list of xattrs, but getxattr + * on the 1st gfid2path xattr itself failed and the while + * loop above broke. So there is nothing in the value. So + * it would be better not to send "" as the value for any + * key, as it is not true. + */ + if (found && !i) + goto err; /* both errno and ret are set before beak */ + /* Calculate memory to be allocated */ for (j = 0; j < i; j++) { bytes += strlen(paths[j]); diff --git a/xlators/storage/posix/src/posix-gfid-path.h b/xlators/storage/posix/src/posix-gfid-path.h index ac3d03e14c3..59799125bbb 100644 --- a/xlators/storage/posix/src/posix-gfid-path.h +++ b/xlators/storage/posix/src/posix-gfid-path.h @@ -13,6 +13,7 @@ #include "xlator.h" #include "common-utils.h" +#include "compat-errno.h" #define MAX_GFID2PATH_LINK_SUP 500 |