diff options
| -rw-r--r-- | tests/bitrot/bug-internal-xattrs-check-1243391.t | 42 | ||||
| -rw-r--r-- | xlators/features/bit-rot/src/stub/bit-rot-stub.c | 77 | ||||
| -rw-r--r-- | xlators/features/bit-rot/src/stub/bit-rot-stub.h | 42 |
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 00000000000..bc9c12520b2 --- /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 f30b5a74307..9439f0cdfe7 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 c8442068e6b..9362c129303 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; |