summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRaghavendra Bhat <raghavendra@redhat.com>2018-05-04 12:25:48 -0400
committerShyamsundar Ranganathan <srangana@redhat.com>2018-05-22 10:28:30 +0000
commit8d22afce10157fa473b169754829b7646d10e108 (patch)
tree9853986efefaefc811a91deb0de3775af6c386ce
parent06fa5aff8de34a13db0e054800578cf8a45df152 (diff)
make posix return errors when gfid2path key is absent
Change-Id: I3a8d452d00560dac5e0b7ff0b1835d1f20a59f91 updates: bz#1580540 Signed-off-by: Raghavendra Bhat <raghavendra@redhat.com> (cherry picked from commit c2cf3f686f3ea0efd936d2eafc404fc9d2e0acc7)
-rw-r--r--libglusterfs/src/syncop-utils.c18
-rw-r--r--tests/bugs/posix/bug-gfid-path.t70
-rw-r--r--xlators/features/bit-rot/src/stub/bit-rot-stub-helpers.c38
-rw-r--r--xlators/storage/posix/src/posix-gfid-path.c26
-rw-r--r--xlators/storage/posix/src/posix-gfid-path.h1
5 files changed, 127 insertions, 26 deletions
diff --git a/libglusterfs/src/syncop-utils.c b/libglusterfs/src/syncop-utils.c
index 40ced03..1ccda55 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 0000000..1bbbe9f
--- /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 24aa9aa..fff7a7e 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 2834c2f..9873e7e 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 ac3d03e..5979912 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