summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--tests/bitrot/bug-internal-xattrs-check-1243391.t42
-rw-r--r--xlators/features/bit-rot/src/stub/bit-rot-stub.c77
-rw-r--r--xlators/features/bit-rot/src/stub/bit-rot-stub.h42
3 files changed, 138 insertions, 23 deletions
diff --git a/tests/bitrot/bug-internal-xattrs-check-1243391.t b/tests/bitrot/bug-internal-xattrs-check-1243391.t
new file mode 100644
index 0000000..bc9c125
--- /dev/null
+++ b/tests/bitrot/bug-internal-xattrs-check-1243391.t
@@ -0,0 +1,42 @@
+#!/bin/bash
+
+. $(dirname $0)/../include.rc
+. $(dirname $0)/../volume.rc
+
+cleanup;
+
+TEST glusterd
+TEST pidof glusterd
+
+## Create a distribute volume (B=2)
+TEST $CLI volume create $V0 $H0:$B0/${V0}1 $H0:$B0/${V0}2;
+EXPECT "$V0" volinfo_field $V0 'Volume Name';
+EXPECT 'Created' volinfo_field $V0 'Status';
+EXPECT '2' brick_count $V0
+
+
+## Start the volume
+TEST $CLI volume start $V0;
+EXPECT 'Started' volinfo_field $V0 'Status';
+
+## Mount the volume
+TEST $GFS --volfile-server=$H0 --volfile-id=$V0 $M0;
+
+echo "123" >> $M0/file;
+
+TEST ! setfattr -n "trusted.glusterfs.set-signature" -v "123" $M0/file;
+TEST ! setfattr -n "trusted.glusterfs.get-signature" -v "123" $M0/file;
+
+# sign xattr
+TEST ! setfattr -n "trusted.bit-rot.signature" -v "123" $M0/file;
+TEST ! setfattr -x "trusted.bit-rot.signature" $M0/file;
+
+# versioning xattr
+TEST ! setfattr -n "trusted.bit-rot.version" -v "123" $M0/file;
+TEST ! setfattr -x "trusted.bit-rot.version" $M0/file;
+
+# bad file xattr
+TEST ! setfattr -n "trusted.bit-rot.bad-file" -v "123" $M0/file;
+TEST ! setfattr -x "trusted.bit-rot.bad-file" $M0/file;
+
+cleanup;
diff --git a/xlators/features/bit-rot/src/stub/bit-rot-stub.c b/xlators/features/bit-rot/src/stub/bit-rot-stub.c
index f30b5a7..9439f0c 100644
--- a/xlators/features/bit-rot/src/stub/bit-rot-stub.c
+++ b/xlators/features/bit-rot/src/stub/bit-rot-stub.c
@@ -452,6 +452,40 @@ br_stub_mark_inode_modified (xlator_t *this, br_stub_local_t *local)
}
/**
+ * The possible return values from br_stub_is_bad_object () are:
+ * 1) 0 => as per the inode context object is not bad
+ * 2) -1 => Failed to get the inode context itself
+ * 3) -2 => As per the inode context object is bad
+ * Both -ve values means the fop which called this function is failed
+ * and error is returned upwards.
+ */
+static int
+br_stub_check_bad_object (xlator_t *this, inode_t *inode, int32_t *op_ret,
+ int32_t *op_errno)
+{
+ int ret = -1;
+
+ ret = br_stub_is_bad_object (this, inode);
+ if (ret == -2) {
+ gf_msg (this->name, GF_LOG_ERROR, 0, BRS_MSG_BAD_OBJECT_ACCESS,
+ "%s is a bad object. Returning",
+ uuid_utoa (inode->gfid));
+ *op_ret = -1;
+ *op_errno = EIO;
+ }
+
+ if (ret == -1) {
+ gf_msg (this->name, GF_LOG_ERROR, 0,
+ BRS_MSG_GET_INODE_CONTEXT_FAILED, "could not get inode"
+ " context for %s", uuid_utoa (inode->gfid));
+ *op_ret = -1;
+ *op_errno = EINVAL;
+ }
+
+ return ret;
+}
+
+/**
* callback for inode/fd versioning
*/
int
@@ -1095,6 +1129,12 @@ br_stub_fsetxattr (call_frame_t *frame, xlator_t *this,
goto done;
}
+ if (dict_get (dict, GLUSTERFS_GET_OBJECT_SIGNATURE)) {
+ br_stub_handle_internal_xattr (frame, this, fd,
+ GLUSTERFS_GET_OBJECT_SIGNATURE);
+ goto done;
+ }
+
/* object reopen request */
ret = dict_get_uint32 (dict, BR_REOPEN_SIGN_HINT_KEY, &val);
if (!ret) {
@@ -1135,6 +1175,7 @@ br_stub_setxattr (call_frame_t *frame, xlator_t *this,
char *format = "(%s:%s)";
if (dict_get (dict, GLUSTERFS_SET_OBJECT_SIGNATURE) ||
+ dict_get (dict, GLUSTERFS_GET_OBJECT_SIGNATURE) ||
dict_get (dict, BR_REOPEN_SIGN_HINT_KEY) ||
dict_get (dict, BITROT_OBJECT_BAD_KEY) ||
dict_get (dict, BITROT_SIGNING_VERSION_KEY) ||
@@ -1579,13 +1620,16 @@ br_stub_readv (call_frame_t *frame, xlator_t *this,
{
int32_t op_ret = -1;
int32_t op_errno = EINVAL;
+ int32_t ret = -1;
GF_VALIDATE_OR_GOTO ("bit-rot-stub", this, unwind);
GF_VALIDATE_OR_GOTO (this->name, frame, unwind);
GF_VALIDATE_OR_GOTO (this->name, fd, unwind);
GF_VALIDATE_OR_GOTO (this->name, fd->inode, unwind);
- BR_STUB_HANDLE_BAD_OBJECT (this, fd->inode, op_ret, op_errno, unwind);
+ ret = br_stub_check_bad_object (this, fd->inode, &op_ret, &op_errno);
+ if (ret)
+ goto unwind;
STACK_WIND_TAIL (frame, FIRST_CHILD(this),
FIRST_CHILD(this)->fops->readv, fd, size, offset,
@@ -1679,7 +1723,9 @@ br_stub_writev (call_frame_t *frame, xlator_t *this, fd_t *fd,
if (ret)
goto unwind;
- BR_STUB_HANDLE_BAD_OBJECT (this, fd->inode, op_ret, op_errno, unwind);
+ ret = br_stub_check_bad_object (this, fd->inode, &op_ret, &op_errno);
+ if (ret)
+ goto unwind;
/**
* The inode is not dirty and also witnessed atleast one successful
@@ -1800,7 +1846,9 @@ br_stub_ftruncate (call_frame_t *frame, xlator_t *this, fd_t *fd,
if (ret)
goto unwind;
- BR_STUB_HANDLE_BAD_OBJECT (this, fd->inode, op_ret, op_errno, unwind);
+ ret = br_stub_check_bad_object (this, fd->inode, &op_ret, &op_errno);
+ if (ret)
+ goto unwind;
if (!inc_version && modified)
goto wind;
@@ -1932,7 +1980,9 @@ br_stub_truncate (call_frame_t *frame, xlator_t *this, loc_t *loc,
if (ret)
goto cleanup_fd;
- BR_STUB_HANDLE_BAD_OBJECT (this, fd->inode, op_ret, op_errno, unwind);
+ ret = br_stub_check_bad_object (this, fd->inode, &op_ret, &op_errno);
+ if (ret)
+ goto unwind;
if (!inc_version && modified)
goto wind;
@@ -2026,7 +2076,9 @@ br_stub_open (call_frame_t *frame, xlator_t *this,
ctx = (br_stub_inode_ctx_t *)(long)ctx_addr;
- BR_STUB_HANDLE_BAD_OBJECT (this, loc->inode, op_ret, op_errno, unwind);
+ ret = br_stub_check_bad_object (this, fd->inode, &op_ret, &op_errno);
+ if (ret)
+ goto unwind;
if (frame->root->pid == GF_CLIENT_PID_SCRUB)
goto wind;
@@ -2208,6 +2260,13 @@ unwind:
* calls can be avoided and bad objects can be caught instantly. Fetching the
* xattr is needed only in lookups when there is a brick restart or inode
* forget.
+ *
+ * If the dict (@xattr) is NULL, then how should that be handled? Fail the
+ * lookup operation? Or let it continue with version being initialized to
+ * BITROT_DEFAULT_CURRENT_VERSION. But what if the version was different
+ * on disk (and also a right signature was there), but posix failed to
+ * successfully allocate the dict? Posix does not treat call back xdata
+ * creattion failure as the lookup failure.
*/
static inline int32_t
br_stub_lookup_version (xlator_t *this,
@@ -2232,6 +2291,14 @@ br_stub_lookup_version (xlator_t *this,
|| (status == BR_VXATTR_STATUS_UNSIGNED))
? obuf->ongoingversion : BITROT_DEFAULT_CURRENT_VERSION;
+ /**
+ * If signature is there, but version is not therem then that status is
+ * is treated as INVALID. So in that case, we should not initialize the
+ * inode context with wrong version names etc.
+ */
+ if (status == BR_VXATTR_STATUS_INVALID)
+ return -1;
+
return br_stub_init_inode_versions (this, NULL, inode, version,
_gf_true, bad_object);
}
diff --git a/xlators/features/bit-rot/src/stub/bit-rot-stub.h b/xlators/features/bit-rot/src/stub/bit-rot-stub.h
index c844206..9362c12 100644
--- a/xlators/features/bit-rot/src/stub/bit-rot-stub.h
+++ b/xlators/features/bit-rot/src/stub/bit-rot-stub.h
@@ -385,32 +385,32 @@ br_stub_remove_vxattrs (dict_t *xattr)
}
}
-#define BR_STUB_HANDLE_BAD_OBJECT(this, inode, op_ret, op_errno, label) \
- do { \
- if (br_stub_is_bad_object (this, inode)) { \
- gf_msg (this->name, GF_LOG_ERROR, 0, \
- BRS_MSG_BAD_OBJECT_ACCESS, \
- "%s is a bad object. Returning", \
- uuid_utoa (inode->gfid)); \
- op_ret = -1; \
- op_errno = EIO; \
- goto label; \
- } \
- } while (0)
-
-static inline gf_boolean_t
+/**
+ * This function returns the below values for different situations
+ * 0 => as per the inode context object is not bad
+ * -1 => Failed to get the inode context itself
+ * -2 => As per the inode context object is bad
+ * Both -ve values means the fop which called this function is failed
+ * and error is returned upwards.
+ * In future if needed or more errors have to be handled, then those
+ * errors can be made into enums.
+ */
+static inline int
br_stub_is_bad_object (xlator_t *this, inode_t *inode)
{
- gf_boolean_t bad_object = _gf_false;
+ int bad_object = 0;
+ gf_boolean_t tmp = _gf_false;
uint64_t ctx_addr = 0;
br_stub_inode_ctx_t *ctx = NULL;
int32_t ret = -1;
ret = br_stub_get_inode_ctx (this, inode, &ctx_addr);
if (ret) {
- gf_msg (this->name, GF_LOG_ERROR, 0, BRS_MSG_SET_CONTEXT_FAILED,
+ gf_msg (this->name, GF_LOG_ERROR, 0,
+ BRS_MSG_GET_INODE_CONTEXT_FAILED,
"failed to get the inode context for the inode %s",
uuid_utoa (inode->gfid));
+ bad_object = -1;
goto out;
}
@@ -418,7 +418,9 @@ br_stub_is_bad_object (xlator_t *this, inode_t *inode)
LOCK (&inode->lock);
{
- bad_object = __br_stub_is_bad_object (ctx);
+ tmp = __br_stub_is_bad_object (ctx);
+ if (tmp)
+ bad_object = -2;
}
UNLOCK (&inode->lock);
@@ -454,12 +456,16 @@ out:
return ret;
}
+/**
+ * There is a possibility that dict_set might fail. The o/p of dict_set is
+ * given to the caller and the caller has to decide what to do.
+ */
static inline int32_t
br_stub_mark_xdata_bad_object (xlator_t *this, inode_t *inode, dict_t *xdata)
{
int32_t ret = 0;
- if (br_stub_is_bad_object (this, inode))
+ if (br_stub_is_bad_object (this, inode) == -2)
ret = dict_set_int32 (xdata, GLUSTERFS_BAD_INODE, 1);
return ret;