diff options
Diffstat (limited to 'xlators/features/bit-rot/src/stub/bit-rot-stub.c')
| -rw-r--r-- | xlators/features/bit-rot/src/stub/bit-rot-stub.c | 475 |
1 files changed, 291 insertions, 184 deletions
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 c6c9b6aafce..447dd47ff41 100644 --- a/xlators/features/bit-rot/src/stub/bit-rot-stub.c +++ b/xlators/features/bit-rot/src/stub/bit-rot-stub.c @@ -12,12 +12,11 @@ #include <sys/uio.h> #include <signal.h> -#include "glusterfs.h" -#include "xlator.h" -#include "logging.h" +#include <glusterfs/glusterfs.h> +#include <glusterfs/logging.h> #include "changelog.h" -#include "compat-errno.h" -#include "call-stub.h" +#include <glusterfs/compat-errno.h> +#include <glusterfs/call-stub.h> #include "bit-rot-stub.h" #include "bit-rot-stub-mem-types.h" @@ -26,6 +25,15 @@ #define BR_STUB_REQUEST_COOKIE 0x1 +void +br_stub_lock_cleaner(void *arg) +{ + pthread_mutex_t *clean_mutex = arg; + + pthread_mutex_unlock(clean_mutex); + return; +} + void * br_stub_signth(void *); @@ -48,8 +56,7 @@ mem_acct_init(xlator_t *this) ret = xlator_mem_acct_init(this, gf_br_stub_mt_end + 1); if (ret != 0) { - gf_msg(this->name, GF_LOG_WARNING, 0, BRS_MSG_MEM_ACNT_FAILED, - "Memory accounting init failed"); + gf_smsg(this->name, GF_LOG_WARNING, 0, BRS_MSG_MEM_ACNT_FAILED, NULL); return ret; } @@ -64,29 +71,29 @@ br_stub_bad_object_container_init(xlator_t *this, br_stub_private_t *priv) ret = pthread_cond_init(&priv->container.bad_cond, NULL); if (ret != 0) { - gf_msg(this->name, GF_LOG_ERROR, 0, BRS_MSG_BAD_OBJ_THREAD_FAIL, - "pthread_cond_init failed (%d)", ret); + gf_smsg(this->name, GF_LOG_ERROR, 0, BRS_MSG_BAD_OBJ_THREAD_FAIL, + "cond_init ret=%d", ret, NULL); goto out; } ret = pthread_mutex_init(&priv->container.bad_lock, NULL); if (ret != 0) { - gf_msg(this->name, GF_LOG_ERROR, 0, BRS_MSG_BAD_OBJ_THREAD_FAIL, - "pthread_mutex_init failed (%d)", ret); + gf_smsg(this->name, GF_LOG_ERROR, 0, BRS_MSG_BAD_OBJ_THREAD_FAIL, + "mutex_init ret=%d", ret, NULL); goto cleanup_cond; } ret = pthread_attr_init(&w_attr); if (ret != 0) { - gf_msg(this->name, GF_LOG_ERROR, 0, BRS_MSG_BAD_OBJ_THREAD_FAIL, - "pthread_attr_init failed (%d)", ret); + gf_smsg(this->name, GF_LOG_ERROR, 0, BRS_MSG_BAD_OBJ_THREAD_FAIL, + "attr_init ret=%d", ret, NULL); goto cleanup_lock; } ret = pthread_attr_setstacksize(&w_attr, BAD_OBJECT_THREAD_STACK_SIZE); if (ret == EINVAL) { - gf_msg(this->name, GF_LOG_WARNING, 0, BRS_MSG_BAD_OBJ_THREAD_FAIL, - "Using default thread stack size"); + gf_smsg(this->name, GF_LOG_WARNING, 0, + BRS_MSG_USING_DEFAULT_THREAD_SIZE, NULL); } INIT_LIST_HEAD(&priv->container.bad_queue); @@ -122,8 +129,7 @@ init(xlator_t *this) br_stub_private_t *priv = NULL; if (!this->children) { - gf_msg(this->name, GF_LOG_ERROR, 0, BRS_MSG_NO_CHILD, - "FATAL: no children"); + gf_smsg(this->name, GF_LOG_ERROR, 0, BRS_MSG_NO_CHILD, NULL); goto error_return; } @@ -161,16 +167,20 @@ init(xlator_t *this) * assigned inside the thread. So setting this->private here. */ this->private = priv; + if (!priv->do_versioning) + return 0; ret = gf_thread_create(&priv->signth, NULL, br_stub_signth, this, "brssign"); - if (ret != 0) + if (ret != 0) { + gf_smsg(this->name, GF_LOG_WARNING, 0, BRS_MSG_SPAWN_SIGN_THRD_FAILED, + NULL); goto cleanup_lock; + } ret = br_stub_bad_object_container_init(this, priv); if (ret) { - gf_msg(this->name, GF_LOG_ERROR, 0, BRS_MSG_BAD_CONTAINER_FAIL, - "failed to launch the thread for storing bad gfids"); + gf_smsg(this->name, GF_LOG_ERROR, 0, BRS_MSG_BAD_CONTAINER_FAIL, NULL); goto cleanup_lock; } @@ -183,6 +193,7 @@ cleanup_lock: pthread_mutex_destroy(&priv->lock); free_mempool: mem_pool_destroy(priv->local_pool); + priv->local_pool = NULL; free_priv: GF_FREE(priv); this->private = NULL; @@ -211,10 +222,62 @@ reconfigure(xlator_t *this, dict_t *options) priv = this->private; - GF_OPTION_RECONF("bitrot", priv->do_versioning, options, bool, out); + GF_OPTION_RECONF("bitrot", priv->do_versioning, options, bool, err); + if (priv->do_versioning && !priv->signth) { + ret = gf_thread_create(&priv->signth, NULL, br_stub_signth, this, + "brssign"); + if (ret != 0) { + gf_smsg(this->name, GF_LOG_WARNING, 0, + BRS_MSG_SPAWN_SIGN_THRD_FAILED, NULL); + goto err; + } + + ret = br_stub_bad_object_container_init(this, priv); + if (ret) { + gf_smsg(this->name, GF_LOG_ERROR, 0, BRS_MSG_BAD_CONTAINER_FAIL, + NULL); + goto err; + } + } else { + if (priv->signth) { + if (gf_thread_cleanup_xint(priv->signth)) { + gf_smsg(this->name, GF_LOG_ERROR, 0, + BRS_MSG_CANCEL_SIGN_THREAD_FAILED, NULL); + } else { + gf_smsg(this->name, GF_LOG_INFO, 0, BRS_MSG_KILL_SIGN_THREAD, + NULL); + priv->signth = 0; + } + } + + if (priv->container.thread) { + if (gf_thread_cleanup_xint(priv->container.thread)) { + gf_smsg(this->name, GF_LOG_ERROR, 0, + BRS_MSG_CANCEL_SIGN_THREAD_FAILED, NULL); + } + priv->container.thread = 0; + } + } ret = 0; -out: + return ret; +err: + if (priv->signth) { + if (gf_thread_cleanup_xint(priv->signth)) { + gf_smsg(this->name, GF_LOG_ERROR, 0, + BRS_MSG_CANCEL_SIGN_THREAD_FAILED, NULL); + } + priv->signth = 0; + } + + if (priv->container.thread) { + if (gf_thread_cleanup_xint(priv->container.thread)) { + gf_smsg(this->name, GF_LOG_ERROR, 0, + BRS_MSG_CANCEL_SIGN_THREAD_FAILED, NULL); + } + priv->container.thread = 0; + } + ret = -1; return ret; } @@ -245,10 +308,13 @@ fini(xlator_t *this) if (!priv) return; + if (!priv->do_versioning) + goto cleanup; + ret = gf_thread_cleanup_xint(priv->signth); if (ret) { - gf_msg(this->name, GF_LOG_ERROR, 0, BRS_MSG_CANCEL_SIGN_THREAD_FAILED, - "Could not cancel sign serializer thread"); + gf_smsg(this->name, GF_LOG_ERROR, 0, BRS_MSG_CANCEL_SIGN_THREAD_FAILED, + NULL); goto out; } priv->signth = 0; @@ -262,13 +328,10 @@ fini(xlator_t *this) GF_FREE(sigstub); } - pthread_mutex_destroy(&priv->lock); - pthread_cond_destroy(&priv->cond); - ret = gf_thread_cleanup_xint(priv->container.thread); if (ret) { - gf_msg(this->name, GF_LOG_ERROR, 0, BRS_MSG_CANCEL_SIGN_THREAD_FAILED, - "Could not cancel sign serializer thread"); + gf_smsg(this->name, GF_LOG_ERROR, 0, BRS_MSG_CANCEL_SIGN_THREAD_FAILED, + NULL); goto out; } @@ -280,14 +343,18 @@ fini(xlator_t *this) call_stub_destroy(stub); } + pthread_mutex_destroy(&priv->container.bad_lock); + pthread_cond_destroy(&priv->container.bad_cond); + +cleanup: + pthread_mutex_destroy(&priv->lock); + pthread_cond_destroy(&priv->cond); + if (priv->local_pool) { mem_pool_destroy(priv->local_pool); priv->local_pool = NULL; } - pthread_mutex_destroy(&priv->container.bad_lock); - pthread_cond_destroy(&priv->container.bad_cond); - this->private = NULL; GF_FREE(priv); @@ -357,8 +424,8 @@ br_stub_prepare_version_request(xlator_t *this, dict_t *dict, priv = this->private; br_set_ongoingversion(obuf, oversion, priv->boot); - return dict_set_static_bin(dict, BITROT_CURRENT_VERSION_KEY, (void *)obuf, - sizeof(br_version_t)); + return dict_set_bin(dict, BITROT_CURRENT_VERSION_KEY, (void *)obuf, + sizeof(br_version_t)); } static int @@ -369,8 +436,7 @@ br_stub_prepare_signing_request(dict_t *dict, br_signature_t *sbuf, br_set_signature(sbuf, sign, signaturelen, &size); - return dict_set_static_bin(dict, BITROT_SIGNING_VERSION_KEY, (void *)sbuf, - size); + return dict_set_bin(dict, BITROT_SIGNING_VERSION_KEY, (void *)sbuf, size); } /** @@ -410,7 +476,7 @@ br_stub_init_inode_versions(xlator_t *this, fd_t *fd, inode_t *inode, goto free_ctx; if (ctx_addr) - *ctx_addr = (uint64_t)ctx; + *ctx_addr = (uint64_t)(uintptr_t)ctx; return 0; free_ctx: @@ -510,11 +576,9 @@ br_stub_need_versioning(xlator_t *this, fd_t *fd, gf_boolean_t *versioning, ret = br_stub_init_inode_versions(this, fd, fd->inode, version, _gf_true, _gf_false, &ctx_addr); if (ret) { - gf_msg(this->name, GF_LOG_ERROR, 0, - BRS_MSG_GET_INODE_CONTEXT_FAILED, - "failed to " - " init the inode context for the inode %s", - uuid_utoa(fd->inode->gfid)); + gf_smsg(this->name, GF_LOG_ERROR, 0, + BRS_MSG_GET_INODE_CONTEXT_FAILED, "gfid=%s", + uuid_utoa(fd->inode->gfid), NULL); goto error_return; } } @@ -548,10 +612,8 @@ br_stub_anon_fd_ctx(xlator_t *this, fd_t *fd, br_stub_inode_ctx_t *ctx) if (!br_stub_fd) { ret = br_stub_add_fd_to_inode(this, fd, ctx); if (ret) { - gf_msg(this->name, GF_LOG_ERROR, 0, BRS_MSG_ADD_FD_TO_INODE, - "failed to add fd to " - "the inode (gfid: %s)", - uuid_utoa(fd->inode->gfid)); + gf_smsg(this->name, GF_LOG_ERROR, 0, BRS_MSG_ADD_FD_TO_INODE, + "gfid=%s", uuid_utoa(fd->inode->gfid), NULL); goto out; } } @@ -571,9 +633,8 @@ br_stub_versioning_prep(call_frame_t *frame, xlator_t *this, fd_t *fd, local = br_stub_alloc_local(this); if (!local) { - gf_msg(this->name, GF_LOG_ERROR, ENOMEM, BRS_MSG_NO_MEMORY, - "local allocation failed (gfid: %s)", - uuid_utoa(fd->inode->gfid)); + gf_smsg(this->name, GF_LOG_ERROR, ENOMEM, BRS_MSG_NO_MEMORY, "gfid=%s", + uuid_utoa(fd->inode->gfid), NULL); goto error_return; } @@ -643,8 +704,8 @@ br_stub_check_bad_object(xlator_t *this, inode_t *inode, int32_t *op_ret, 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)); + gf_smsg(this->name, GF_LOG_ERROR, 0, BRS_MSG_BAD_OBJECT_ACCESS, + "gfid=%s", uuid_utoa(inode->gfid), NULL); *op_ret = -1; *op_errno = EIO; } @@ -653,9 +714,9 @@ br_stub_check_bad_object(xlator_t *this, inode_t *inode, int32_t *op_ret, ret = br_stub_init_inode_versions(this, NULL, inode, version, _gf_true, _gf_false, NULL); if (ret) { - gf_msg( - this->name, GF_LOG_ERROR, 0, BRS_MSG_GET_INODE_CONTEXT_FAILED, - "failed to init inode context for %s", uuid_utoa(inode->gfid)); + gf_smsg(this->name, GF_LOG_ERROR, 0, + BRS_MSG_GET_INODE_CONTEXT_FAILED, "gfid=%s", + uuid_utoa(inode->gfid), NULL); *op_ret = -1; *op_errno = EINVAL; } @@ -792,23 +853,27 @@ br_stub_perform_incversioning(xlator_t *this, call_frame_t *frame, op_errno = ENOMEM; dict = dict_new(); if (!dict) - goto done; + goto out; ret = br_stub_alloc_versions(&obuf, NULL, 0); - if (ret) - goto dealloc_dict; + if (ret) { + gf_smsg(this->name, GF_LOG_ERROR, 0, BRS_MSG_ALLOC_MEM_FAILED, + "gfid=%s", uuid_utoa(fd->inode->gfid), NULL); + goto out; + } ret = br_stub_prepare_version_request(this, dict, obuf, writeback_version); - if (ret) - goto dealloc_versions; + if (ret) { + gf_smsg(this->name, GF_LOG_ERROR, 0, BRS_MSG_VERSION_PREPARE_FAIL, + "gfid=%s", uuid_utoa(fd->inode->gfid), NULL); + br_stub_dealloc_versions(obuf); + goto out; + } ret = br_stub_fd_versioning( this, frame, stub, dict, fd, br_stub_fd_incversioning_cbk, writeback_version, BR_STUB_INCREMENTAL_VERSIONING, !WRITEBACK_DURABLE); - -dealloc_versions: - br_stub_dealloc_versions(obuf); -dealloc_dict: - dict_unref(dict); -done: +out: + if (dict) + dict_unref(dict); if (ret) { if (local) frame->local = NULL; @@ -846,6 +911,24 @@ br_stub_signth(void *arg) THIS = this; while (1) { + /* + * Disabling bit-rot feature leads to this particular thread + * getting cleaned up by reconfigure via a call to the function + * gf_thread_cleanup_xint (which in turn calls pthread_cancel + * and pthread_join). But, if this thread had held the mutex + * &priv->lock at the time of cancellation, then it leads to + * deadlock in future when bit-rot feature is enabled (which + * again spawns this thread which cant hold the lock as the + * mutex is still held by the previous instance of the thread + * which got killed). Also, the br_stub_handle_object_signature + * function which is called whenever file has to be signed + * also gets blocked as it too attempts to acquire &priv->lock. + * + * So, arrange for the lock to be unlocked as part of the + * cleanup of this thread using pthread_cleanup_push and + * pthread_cleanup_pop. + */ + pthread_cleanup_push(br_stub_lock_cleaner, &priv->lock); pthread_mutex_lock(&priv->lock); { while (list_empty(&priv->squeue)) @@ -856,6 +939,7 @@ br_stub_signth(void *arg) list_del_init(&sigstub->list); } pthread_mutex_unlock(&priv->lock); + pthread_cleanup_pop(0); call_resume(sigstub->stub); @@ -931,10 +1015,9 @@ br_stub_compare_sign_version(xlator_t *this, inode_t *inode, if (invalid) { ret = -1; - gf_msg(this->name, GF_LOG_WARNING, 0, BRS_MSG_SIGN_VERSION_ERROR, - "Signing version exceeds " - "current version [%lu > %lu]", - sbuf->signedversion, ctx->currentversion); + gf_smsg(this->name, GF_LOG_WARNING, 0, BRS_MSG_SIGN_VERSION_ERROR, + "Signing-ver=%lu", sbuf->signedversion, "current-ver=%lu", + ctx->currentversion, NULL); } out: @@ -945,31 +1028,36 @@ static int br_stub_prepare_signature(xlator_t *this, dict_t *dict, inode_t *inode, br_isignature_t *sign, int *fakesuccess) { - int32_t ret = 0; + int32_t ret = -1; size_t signaturelen = 0; br_signature_t *sbuf = NULL; if (!br_is_signature_type_valid(sign->signaturetype)) - goto error_return; + goto out; signaturelen = sign->signaturelen; ret = br_stub_alloc_versions(NULL, &sbuf, signaturelen); - if (ret) - goto error_return; + if (ret) { + gf_smsg(this->name, GF_LOG_ERROR, 0, BRS_MSG_ALLOC_MEM_FAILED, + "gfid=%s", uuid_utoa(inode->gfid), NULL); + ret = -1; + goto out; + } ret = br_stub_prepare_signing_request(dict, sbuf, sign, signaturelen); - if (ret) - goto dealloc_versions; + if (ret) { + gf_smsg(this->name, GF_LOG_ERROR, 0, BRS_MSG_SIGN_PREPARE_FAIL, + "gfid=%s", uuid_utoa(inode->gfid), NULL); + ret = -1; + br_stub_dealloc_versions(sbuf); + goto out; + } + /* At this point sbuf has been added to dict, so the memory will be freed + * when the data from the dict is destroyed + */ ret = br_stub_compare_sign_version(this, inode, sbuf, dict, fakesuccess); - if (ret) - goto dealloc_versions; - - return 0; - -dealloc_versions: - br_stub_dealloc_versions(sbuf); -error_return: - return -1; +out: + return ret; } static void @@ -986,12 +1074,18 @@ br_stub_handle_object_signature(call_frame_t *frame, xlator_t *this, fd_t *fd, priv = this->private; - if (frame->root->pid != GF_CLIENT_PID_BITD) + if (frame->root->pid != GF_CLIENT_PID_BITD) { + gf_smsg(this->name, GF_LOG_WARNING, op_errno, BRS_MSG_NON_BITD_PID, + "PID=%d", frame->root->pid, NULL); goto dofop; + } ret = br_stub_prepare_signature(this, dict, fd->inode, sign, &fakesuccess); - if (ret) + if (ret) { + gf_smsg(this->name, GF_LOG_WARNING, 0, BRS_MSG_SIGN_PREPARE_FAIL, + "gfid=%s", uuid_utoa(fd->inode->gfid), NULL); goto dofop; + } if (fakesuccess) { op_ret = op_errno = 0; goto dofop; @@ -1141,10 +1235,8 @@ br_stub_handle_object_reopen(call_frame_t *frame, xlator_t *this, fd_t *fd, stub = fop_fsetxattr_cbk_stub(frame, br_stub_fsetxattr_resume, 0, 0, NULL); if (!stub) { - gf_msg(this->name, GF_LOG_ERROR, 0, BRS_MSG_STUB_ALLOC_FAILED, - "failed to allocate stub for fsetxattr fop (gfid: %s)," - " unwinding", - uuid_utoa(fd->inode->gfid)); + gf_smsg(this->name, GF_LOG_ERROR, 0, BRS_MSG_STUB_ALLOC_FAILED, + "fsetxattr gfid=%s", uuid_utoa(fd->inode->gfid), NULL); goto cleanup_local; } @@ -1198,9 +1290,8 @@ br_stub_fsetxattr_bad_object_cbk(call_frame_t *frame, void *cookie, */ ret = br_stub_mark_object_bad(this, local->u.context.inode); if (ret) - gf_msg(this->name, GF_LOG_ERROR, 0, BRS_MSG_BAD_OBJ_MARK_FAIL, - "failed to mark object %s as bad", - uuid_utoa(local->u.context.inode->gfid)); + gf_smsg(this->name, GF_LOG_ERROR, 0, BRS_MSG_BAD_OBJ_MARK_FAIL, + "gfid=%s", uuid_utoa(local->u.context.inode->gfid), NULL); ret = br_stub_add(this, local->u.context.inode->gfid); @@ -1220,18 +1311,15 @@ br_stub_handle_bad_object_key(call_frame_t *frame, xlator_t *this, fd_t *fd, int32_t op_errno = EINVAL; if (frame->root->pid != GF_CLIENT_PID_SCRUB) { - gf_msg(this->name, GF_LOG_ERROR, 0, BRS_MSG_NON_SCRUB_BAD_OBJ_MARK, - "bad object marking " - "on %s is not from the scrubber", - uuid_utoa(fd->inode->gfid)); + gf_smsg(this->name, GF_LOG_ERROR, 0, BRS_MSG_NON_SCRUB_BAD_OBJ_MARK, + "gfid=%s", uuid_utoa(fd->inode->gfid), NULL); goto unwind; } local = br_stub_alloc_local(this); if (!local) { - gf_msg(this->name, GF_LOG_ERROR, 0, BRS_MSG_NO_MEMORY, - "failed to allocate memory for fsetxattr on %s", - uuid_utoa(fd->inode->gfid)); + gf_smsg(this->name, GF_LOG_ERROR, 0, BRS_MSG_ALLOC_MEM_FAILED, + "fsetxattr gfid=%s", uuid_utoa(fd->inode->gfid), NULL); op_ret = -1; op_errno = ENOMEM; goto unwind; @@ -1270,10 +1358,9 @@ br_stub_handle_internal_xattr(call_frame_t *frame, xlator_t *this, fd_t *fd, int32_t op_ret = -1; int32_t op_errno = EINVAL; - gf_msg(this->name, GF_LOG_ERROR, 0, BRS_MSG_SET_INTERNAL_XATTR, - "setxattr called" - " on the internal xattr %s for inode %s", - key, uuid_utoa(fd->inode->gfid)); + gf_smsg(this->name, GF_LOG_ERROR, 0, BRS_MSG_SET_INTERNAL_XATTR, + "setxattr key=%s", key, "inode-gfid=%s", uuid_utoa(fd->inode->gfid), + NULL); STACK_UNWIND_STRICT(fsetxattr, frame, op_ret, op_errno, NULL); return 0; @@ -1291,10 +1378,8 @@ br_stub_dump_xattr(xlator_t *this, dict_t *dict, int *op_errno) goto out; } dict_dump_to_str(dict, dump, BR_STUB_DUMP_STR_SIZE, format); - gf_msg(this->name, GF_LOG_ERROR, 0, BRS_MSG_SET_INTERNAL_XATTR, - "fsetxattr called on " - "internal xattr %s", - dump); + gf_smsg(this->name, GF_LOG_ERROR, 0, BRS_MSG_SET_INTERNAL_XATTR, + "fsetxattr dump=%s", dump, NULL); out: if (dump) { GF_FREE(dump); @@ -1331,6 +1416,8 @@ br_stub_fsetxattr(call_frame_t *frame, xlator_t *this, fd_t *fd, dict_t *dict, /* object signature request */ ret = dict_get_bin(dict, GLUSTERFS_SET_OBJECT_SIGNATURE, (void **)&sign); if (!ret) { + gf_msg_debug(this->name, 0, "got SIGNATURE request on %s", + uuid_utoa(fd->inode->gfid)); br_stub_handle_object_signature(frame, this, fd, dict, sign, xdata); goto done; } @@ -1423,10 +1510,8 @@ br_stub_removexattr(call_frame_t *frame, xlator_t *this, loc_t *loc, if (!strcmp(BITROT_OBJECT_BAD_KEY, name) || !strcmp(BITROT_SIGNING_VERSION_KEY, name) || !strcmp(BITROT_CURRENT_VERSION_KEY, name)) { - gf_msg(this->name, GF_LOG_WARNING, 0, BRS_MSG_REMOVE_INTERNAL_XATTR, - "removexattr called" - " on internal xattr %s for file %s", - name, loc->path); + gf_smsg(this->name, GF_LOG_WARNING, 0, BRS_MSG_REMOVE_INTERNAL_XATTR, + "name=%s", name, "file-path=%s", loc->path, NULL); goto unwind; } @@ -1448,10 +1533,9 @@ br_stub_fremovexattr(call_frame_t *frame, xlator_t *this, fd_t *fd, if (!strcmp(BITROT_OBJECT_BAD_KEY, name) || !strcmp(BITROT_SIGNING_VERSION_KEY, name) || !strcmp(BITROT_CURRENT_VERSION_KEY, name)) { - gf_msg(this->name, GF_LOG_WARNING, 0, BRS_MSG_REMOVE_INTERNAL_XATTR, - "removexattr called" - " on internal xattr %s for inode %s", - name, uuid_utoa(fd->inode->gfid)); + gf_smsg(this->name, GF_LOG_WARNING, 0, BRS_MSG_REMOVE_INTERNAL_XATTR, + "name=%s", name, "inode-gfid=%s", uuid_utoa(fd->inode->gfid), + NULL); goto unwind; } @@ -1476,7 +1560,7 @@ br_stub_listxattr_cbk(call_frame_t *frame, void *cookie, xlator_t *this, if (op_ret < 0) goto unwind; - br_stub_remove_vxattrs(xattr); + br_stub_remove_vxattrs(xattr, _gf_true); unwind: STACK_UNWIND_STRICT(getxattr, frame, op_ret, op_errno, xattr, xdata); @@ -1537,10 +1621,8 @@ br_stub_is_object_stale(xlator_t *this, call_frame_t *frame, inode_t *inode, ret = br_stub_get_inode_ctx(this, inode, &ctx_addr); if (ret) { - gf_msg(this->name, GF_LOG_ERROR, 0, BRS_MSG_GET_INODE_CONTEXT_FAILED, - "failed to get the " - "inode context for %s", - uuid_utoa(inode->gfid)); + gf_smsg(this->name, GF_LOG_ERROR, 0, BRS_MSG_GET_INODE_CONTEXT_FAILED, + "gfid=%s", uuid_utoa(inode->gfid), NULL); goto out; } @@ -1655,7 +1737,7 @@ br_stub_getxattr_cbk(call_frame_t *frame, void *cookie, xlator_t *this, op_ret = totallen; delkeys: - br_stub_remove_vxattrs(xattr); + br_stub_remove_vxattrs(xattr, _gf_true); unwind: STACK_UNWIND_STRICT(getxattr, frame, op_ret, op_errno, xattr, xdata); @@ -1711,9 +1793,7 @@ br_stub_getxattr(call_frame_t *frame, xlator_t *this, loc_t *loc, const char *name, dict_t *xdata) { void *cookie = NULL; - uuid_t rootgfid = { - 0, - }; + static uuid_t rootgfid = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}; fop_getxattr_cbk_t cbk = br_stub_getxattr_cbk; int32_t op_ret = -1; int32_t op_errno = EINVAL; @@ -1725,8 +1805,6 @@ br_stub_getxattr(call_frame_t *frame, xlator_t *this, loc_t *loc, GF_VALIDATE_OR_GOTO(this->name, this->private, unwind); GF_VALIDATE_OR_GOTO(this->name, loc->inode, unwind); - rootgfid[15] = 1; - if (!name) { cbk = br_stub_listxattr_cbk; goto wind; @@ -1796,16 +1874,13 @@ br_stub_fgetxattr(call_frame_t *frame, xlator_t *this, fd_t *fd, const char *name, dict_t *xdata) { void *cookie = NULL; - uuid_t rootgfid = { - 0, - }; + static uuid_t rootgfid = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}; fop_fgetxattr_cbk_t cbk = br_stub_getxattr_cbk; int32_t op_ret = -1; int32_t op_errno = EINVAL; br_stub_local_t *local = NULL; br_stub_private_t *priv = NULL; - rootgfid[15] = 1; priv = this->private; if (!name) { @@ -2025,10 +2100,8 @@ br_stub_writev(call_frame_t *frame, xlator_t *this, fd_t *fd, offset, flags, iobref, xdata); if (!stub) { - gf_msg(this->name, GF_LOG_ERROR, 0, BRS_MSG_STUB_ALLOC_FAILED, - "failed to allocate stub for write fop (gfid: %s), " - "unwinding", - uuid_utoa(fd->inode->gfid)); + gf_smsg(this->name, GF_LOG_ERROR, 0, BRS_MSG_STUB_ALLOC_FAILED, + "write gfid=%s", uuid_utoa(fd->inode->gfid), NULL); goto cleanup_local; } @@ -2141,10 +2214,8 @@ br_stub_ftruncate(call_frame_t *frame, xlator_t *this, fd_t *fd, off_t offset, stub = fop_ftruncate_stub(frame, br_stub_ftruncate_resume, fd, offset, xdata); if (!stub) { - gf_msg(this->name, GF_LOG_ERROR, 0, BRS_MSG_STUB_ALLOC_FAILED, - "failed to allocate stub for ftruncate fop (gfid: %s)," - " unwinding", - uuid_utoa(fd->inode->gfid)); + gf_smsg(this->name, GF_LOG_ERROR, 0, BRS_MSG_STUB_ALLOC_FAILED, + "ftruncate gfid=%s", uuid_utoa(fd->inode->gfid), NULL); goto cleanup_local; } @@ -2248,10 +2319,8 @@ br_stub_truncate(call_frame_t *frame, xlator_t *this, loc_t *loc, off_t offset, fd = fd_anonymous(loc->inode); if (!fd) { - gf_msg(this->name, GF_LOG_ERROR, 0, BRS_MSG_CREATE_ANONYMOUS_FD_FAILED, - "failed to create " - "anonymous fd for the inode %s", - uuid_utoa(loc->inode->gfid)); + gf_smsg(this->name, GF_LOG_ERROR, 0, BRS_MSG_CREATE_ANONYMOUS_FD_FAILED, + "inode-gfid=%s", uuid_utoa(loc->inode->gfid), NULL); goto unwind; } @@ -2281,10 +2350,8 @@ br_stub_truncate(call_frame_t *frame, xlator_t *this, loc_t *loc, off_t offset, stub = fop_truncate_stub(frame, br_stub_truncate_resume, loc, offset, xdata); if (!stub) { - gf_msg(this->name, GF_LOG_ERROR, 0, BRS_MSG_STUB_ALLOC_FAILED, - "failed to allocate stub for truncate fop (gfid: %s), " - "unwinding", - uuid_utoa(fd->inode->gfid)); + gf_smsg(this->name, GF_LOG_ERROR, 0, BRS_MSG_STUB_ALLOC_FAILED, + "truncate gfid=%s", uuid_utoa(fd->inode->gfid), NULL); goto cleanup_local; } @@ -2357,11 +2424,9 @@ br_stub_open(call_frame_t *frame, xlator_t *this, loc_t *loc, int32_t flags, ret = br_stub_init_inode_versions(this, fd, fd->inode, version, _gf_true, _gf_false, &ctx_addr); if (ret) { - gf_msg(this->name, GF_LOG_ERROR, 0, - BRS_MSG_GET_INODE_CONTEXT_FAILED, - "failed to init the inode context for " - "the file %s (gfid: %s)", - loc->path, uuid_utoa(fd->inode->gfid)); + gf_smsg(this->name, GF_LOG_ERROR, 0, + BRS_MSG_GET_INODE_CONTEXT_FAILED, "path=%s", loc->path, + "gfid=%s", uuid_utoa(fd->inode->gfid), NULL); goto unwind; } } @@ -2380,9 +2445,8 @@ br_stub_open(call_frame_t *frame, xlator_t *this, loc_t *loc, int32_t flags, ret = br_stub_add_fd_to_inode(this, fd, ctx); if (ret) { - gf_msg(this->name, GF_LOG_ERROR, 0, BRS_MSG_ADD_FD_TO_LIST_FAILED, - "failed add fd to the list (gfid: %s)", - uuid_utoa(fd->inode->gfid)); + gf_smsg(this->name, GF_LOG_ERROR, 0, BRS_MSG_ADD_FD_TO_LIST_FAILED, + "gfid=%s", uuid_utoa(fd->inode->gfid), NULL); goto unwind; } @@ -2413,10 +2477,8 @@ br_stub_add_fd_to_inode(xlator_t *this, fd_t *fd, br_stub_inode_ctx_t *ctx) ret = br_stub_require_release_call(this, fd, &br_stub_fd); if (ret) { - gf_msg(this->name, GF_LOG_ERROR, 0, BRS_MSG_SET_FD_CONTEXT_FAILED, - "failed to set the fd " - "context for the file (gfid: %s)", - uuid_utoa(fd->inode->gfid)); + gf_smsg(this->name, GF_LOG_ERROR, 0, BRS_MSG_SET_FD_CONTEXT_FAILED, + "gfid=%s", uuid_utoa(fd->inode->gfid), NULL); goto out; } @@ -2703,17 +2765,37 @@ br_stub_readdirp_cbk(call_frame_t *frame, void *cookie, xlator_t *this, if (!IA_ISREG(entry->d_stat.ia_type)) continue; + /* + * Readdirp for most part is a bulk lookup for all the entries + * present in the directory being read. Ideally, for each + * entry, the handling should be similar to that of a lookup + * callback. But for now, just keeping this as it has been + * until now (which means, this comment has been added much + * later as part of a change that wanted to send the flag + * of true/false to br_stub_remove_vxattrs to indicate whether + * the bad-object xattr should be removed from the entry->dict + * or not). Until this change, the function br_stub_remove_vxattrs + * was just removing all the xattrs associated with bit-rot-stub + * (like version, bad-object, signature etc). But, there are + * scenarios where we only want to send bad-object xattr and not + * others. So this comment is part of that change which also + * mentions about another possible change that might be needed + * in future. + * But for now, adding _gf_true means functionally its same as + * what this function was doing before. Just remove all the stub + * related xattrs. + */ ret = br_stub_get_inode_ctx(this, entry->inode, &ctxaddr); if (ret < 0) ctxaddr = 0; if (ctxaddr) { /* already has the context */ - br_stub_remove_vxattrs(entry->dict); + br_stub_remove_vxattrs(entry->dict, _gf_true); continue; } ret = br_stub_lookup_version(this, entry->inode->gfid, entry->inode, entry->dict); - br_stub_remove_vxattrs(entry->dict); + br_stub_remove_vxattrs(entry->dict, _gf_true); if (ret) { /** * there's no per-file granularity support in case of @@ -2849,13 +2931,22 @@ br_stub_lookup_cbk(call_frame_t *frame, void *cookie, xlator_t *this, int32_t ret = 0; br_stub_private_t *priv = NULL; gf_boolean_t ver_enabled = _gf_false; + gf_boolean_t remove_bad_file_marker = _gf_true; BR_STUB_VER_ENABLED_IN_CALLPATH(frame, ver_enabled); priv = this->private; if (op_ret < 0) { (void)br_stub_handle_lookup_error(this, inode, op_errno); - goto unwind; + + /* + * If the lookup error is not ENOENT, then it is better + * to send the bad file marker to the higher layer (if + * it has been set) + */ + if (op_errno != ENOENT) + remove_bad_file_marker = _gf_false; + goto delkey; } BR_STUB_VER_COND_GOTO(priv, (!ver_enabled), delkey); @@ -2876,7 +2967,13 @@ br_stub_lookup_cbk(call_frame_t *frame, void *cookie, xlator_t *this, if (ret) { op_ret = -1; op_errno = EIO; - goto unwind; + /* + * This flag ensures that in the label @delkey below, + * bad file marker is not removed from the dictinary, + * but other virtual xattrs (such as version, signature) + * are removed. + */ + remove_bad_file_marker = _gf_false; } goto delkey; } @@ -2900,11 +2997,11 @@ br_stub_lookup_cbk(call_frame_t *frame, void *cookie, xlator_t *this, */ op_ret = -1; op_errno = EIO; - goto unwind; + goto delkey; } delkey: - br_stub_remove_vxattrs(xattr); + br_stub_remove_vxattrs(xattr, remove_bad_file_marker); unwind: STACK_UNWIND_STRICT(lookup, frame, op_ret, op_errno, inode, stbuf, xattr, postparent); @@ -3090,8 +3187,7 @@ br_stub_unlink_cbk(call_frame_t *frame, void *cookie, xlator_t *this, goto unwind; if (!local) { - gf_msg(this->name, GF_LOG_WARNING, 0, BRS_MSG_NULL_LOCAL, - "local is NULL"); + gf_smsg(this->name, GF_LOG_WARNING, 0, BRS_MSG_NULL_LOCAL, NULL); goto unwind; } inode = local->u.context.inode; @@ -3109,9 +3205,8 @@ br_stub_unlink_cbk(call_frame_t *frame, void *cookie, xlator_t *this, * has to be removed manually. Its not a good idea to fail * the fop, as the object has already been deleted. */ - gf_msg(this->name, GF_LOG_WARNING, 0, BRS_MSG_GET_INODE_CONTEXT_FAILED, - "failed to get the context for the inode %s", - uuid_utoa(inode->gfid)); + gf_smsg(this->name, GF_LOG_WARNING, 0, BRS_MSG_GET_INODE_CONTEXT_FAILED, + "inode-gfid=%s", uuid_utoa(inode->gfid), NULL); goto unwind; } @@ -3154,9 +3249,9 @@ br_stub_unlink(call_frame_t *frame, xlator_t *this, loc_t *loc, int flag, if (!local) { op_ret = -1; op_errno = ENOMEM; - gf_msg(this->name, GF_LOG_ERROR, ENOMEM, BRS_MSG_NO_MEMORY, - "failed to allocate memory for local (path: %s, gfid: %s)", - loc->path, uuid_utoa(loc->inode->gfid)); + gf_smsg(this->name, GF_LOG_ERROR, ENOMEM, BRS_MSG_ALLOC_MEM_FAILED, + "local path=%s", loc->path, "gfid=%s", + uuid_utoa(loc->inode->gfid), NULL); goto unwind; } @@ -3231,23 +3326,21 @@ br_stub_send_ipc_fop(xlator_t *this, fd_t *fd, unsigned long releaseversion, xdata = dict_new(); if (!xdata) { - gf_msg(this->name, GF_LOG_WARNING, ENOMEM, BRS_MSG_NO_MEMORY, - "dict allocation failed: cannot send IPC FOP " - "to changelog"); + gf_smsg(this->name, GF_LOG_WARNING, ENOMEM, BRS_MSG_DICT_ALLOC_FAILED, + NULL); goto out; } ret = dict_set_static_bin(xdata, "RELEASE-EVENT", &ev, CHANGELOG_EV_SIZE); if (ret) { - gf_msg(this->name, GF_LOG_WARNING, 0, BRS_MSG_SET_EVENT_FAILED, - "cannot set release event in dict"); + gf_smsg(this->name, GF_LOG_WARNING, 0, BRS_MSG_SET_EVENT_FAILED, NULL); goto dealloc_dict; } frame = create_frame(this, this->ctx->pool); if (!frame) { - gf_msg(this->name, GF_LOG_WARNING, 0, BRS_MSG_CREATE_FRAME_FAILED, - "create_frame() failure"); + gf_smsg(this->name, GF_LOG_WARNING, 0, BRS_MSG_CREATE_FRAME_FAILED, + NULL); goto dealloc_dict; } @@ -3382,8 +3475,8 @@ br_stub_releasedir(xlator_t *this, fd_t *fd) if (fctx->bad_object.dir) { ret = sys_closedir(fctx->bad_object.dir); if (ret) - gf_msg(this->name, GF_LOG_ERROR, 0, BRS_MSG_BAD_OBJ_DIR_CLOSE_FAIL, - "closedir error: %s", strerror(errno)); + gf_smsg(this->name, GF_LOG_ERROR, 0, BRS_MSG_BAD_OBJ_DIR_CLOSE_FAIL, + "error=%s", strerror(errno), NULL); } GF_FREE(fctx); @@ -3411,14 +3504,14 @@ br_stub_ictxmerge(xlator_t *this, fd_t *fd, inode_t *inode, ret = br_stub_get_inode_ctx(this, inode, &ctxaddr); if (ret < 0) goto done; - ctx = (br_stub_inode_ctx_t *)ctxaddr; + ctx = (br_stub_inode_ctx_t *)(uintptr_t)ctxaddr; LOCK(&linked_inode->lock); { ret = __br_stub_get_inode_ctx(this, linked_inode, &lctxaddr); if (ret < 0) goto unblock; - lctx = (br_stub_inode_ctx_t *)lctxaddr; + lctx = (br_stub_inode_ctx_t *)(uintptr_t)lctxaddr; GF_ASSERT(list_is_singular(&ctx->fd_list)); br_stub_fd = list_first_entry(&ctx->fd_list, br_stub_fd_t, list); @@ -3481,3 +3574,17 @@ struct volume_options options[] = { .default_value = "{{ brick.path }}"}, {.key = {NULL}}, }; + +xlator_api_t xlator_api = { + .init = init, + .fini = fini, + .notify = notify, + .reconfigure = reconfigure, + .mem_acct_init = mem_acct_init, + .op_version = {1}, /* Present from the initial version */ + .fops = &fops, + .cbks = &cbks, + .options = options, + .identifier = "bitrot-stub", + .category = GF_MAINTAINED, +}; |
