From e9290dc0db7bee31cda1cbed1d9eb68d9c404746 Mon Sep 17 00:00:00 2001 From: Venky Shankar Date: Tue, 26 May 2015 21:51:31 +0530 Subject: features/bitrot: stub improvements and fixes This patch refactors the signing trigger mechanism used by bitrot daemon as a "catch up" meachanism to sign files which _missed_ signing on the last run either due to bitrot being disabled and enabled again or if bitrot is enabled for a volume with existing data. Existing implementation relies on overloading writev() to trigger signing which just by the looks sounded dangerous and I hated it to the core. This change moves all that business to the setxattr interface thereby keeping the writev path strictly for client IO. Why not use IPC fop to trigger signing? There's a need to access the object's inode to perform various maintainance operations. inode is not _directly_ accessible in the IPC fop (although, it can be found via inode_grep() for the object's GFID - the inode just needs to be pinned in memory, which is the case if there's an active fd on the inode). This patch relies on good old technique of overloading fsetxattr() to do the job instead of using IPC fop. There are some pretty nice cleanups along the lines of memory deallocations, unncessary allocations and redundant ref()ing of structures (such as fd's) provided by this patch. All in all - much improved code navigation. Change-Id: Id93fe90b1618802d1a95a5072517dac342b96cb8 BUG: 1224600 Signed-off-by: Venky Shankar Reviewed-on: http://review.gluster.org/10942 Tested-by: NetBSD Build System Reviewed-by: Vijay Bellur --- xlators/features/bit-rot/src/bitd/bit-rot.c | 150 +++++++++------------------- 1 file changed, 47 insertions(+), 103 deletions(-) (limited to 'xlators/features/bit-rot/src/bitd/bit-rot.c') diff --git a/xlators/features/bit-rot/src/bitd/bit-rot.c b/xlators/features/bit-rot/src/bitd/bit-rot.c index b9adbd6647c..2652f02b4ea 100644 --- a/xlators/features/bit-rot/src/bitd/bit-rot.c +++ b/xlators/features/bit-rot/src/bitd/bit-rot.c @@ -485,94 +485,72 @@ br_log_object_path (xlator_t *this, char *op, } static void -br_send_dummy_write (xlator_t *this, fd_t *fd, br_child_t *child, - dict_t *xdata) +br_trigger_sign (xlator_t *this, br_child_t *child, + inode_t *linked_inode, loc_t *loc, gf_boolean_t need_reopen) { - struct iovec iov = {0, }; - struct iobref *iobref = NULL; - struct iobuf *iobuf = NULL; - char *msg = NULL; - size_t size = 0; - int32_t ret = -1; - - GF_VALIDATE_OR_GOTO ("bit-rot", this, out); - GF_VALIDATE_OR_GOTO (this->name, fd, out); - GF_VALIDATE_OR_GOTO (this->name, child, out); - - msg = gf_strdup ("GLUSTERFS"); - if (!msg) - goto out; + fd_t *fd = NULL; + int32_t ret = -1; + uint32_t val = 0; + dict_t *dict = NULL; + pid_t pid = GF_CLIENT_PID_BITD; - size = strlen (msg); + syncopctx_setfspid (&pid); - iov.iov_base = msg; - iov.iov_len = size; + val = (need_reopen == _gf_true) ? BR_OBJECT_REOPEN : BR_OBJECT_RESIGN; - iobref = iobref_new (); - if (!iobref) - goto free_msg; + dict = dict_new (); + if (!dict) + goto out; - iobuf = iobuf_get2 (this->ctx->iobuf_pool, size); - if (!iobuf) - goto free_iobref; + ret = dict_set_uint32 (dict, BR_REOPEN_SIGN_HINT_KEY, val); + if (ret) + goto cleanup_dict; - iobref_add (iobref, iobuf); + ret = -1; + fd = fd_create (linked_inode, 0); + if (!fd) { + gf_log (this->name, GF_LOG_ERROR, "Failed to create fd " + "[GFID %s]", uuid_utoa (linked_inode->gfid)); + goto cleanup_dict; + } - iov_unload (iobuf_ptr (iobuf), &iov, 1); /* FIXME!!! */ + ret = syncop_open (child->xl, loc, O_RDWR, fd, NULL, NULL); + if (ret) { + br_log_object (this, "open", linked_inode->gfid, -ret); + goto unref_fd; + } - iov.iov_base = iobuf_ptr (iobuf); - iov.iov_len = size; + fd_bind (fd); - ret = syncop_writev (child->xl, fd, &iov, 1, 0, iobref, 0, xdata, NULL); - if (ret <= 0) { - ret = -1; - gf_log (this->name, GF_LOG_ERROR, - "dummy write failed (%s)", strerror (errno)); - goto free_iobuf; - } + ret = syncop_fsetxattr (child->xl, fd, dict, 0, NULL, NULL); + if (ret) + br_log_object (this, "fsetxattr", linked_inode->gfid, -ret); - /* iobref_unbref() takes care of iobuf unref */ - ret = 0; + /* passthough: fd_unref() */ - free_iobuf: - iobuf_unref (iobuf); - free_iobref: - iobref_unref (iobref); - free_msg: - GF_FREE (msg); + unref_fd: + fd_unref (fd); + cleanup_dict: + dict_unref (dict); out: - return; + if (ret) { + gf_log (this->name, GF_LOG_WARNING, + "Could not trigger signingd for %s (reopen hint: %d)", + uuid_utoa (linked_inode->gfid), val); + } } static void -br_object_handle_reopen (xlator_t *this, - br_object_t *object, inode_t *linked_inode) +br_object_resign (xlator_t *this, + br_object_t *object, inode_t *linked_inode) { - int32_t ret = -1; - dict_t *dict = NULL; - loc_t loc = {0, }; - - /** - * Here dict is purposefully not checked for NULL, because at any cost - * sending a re-open should not be missed. This re-open is an indication - * for the stub to properly mark inode's status. - */ - dict = dict_new (); - if (dict) { - /* TODO: Make it a #define */ - ret = dict_set_int32 (dict, "br-fd-reopen", 1); - if (ret) - gf_log (this->name, GF_LOG_WARNING, - "Object reopen would trigger versioning."); - } + loc_t loc = {0, }; loc.inode = inode_ref (linked_inode); gf_uuid_copy (loc.gfid, linked_inode->gfid); - br_trigger_sign (this, object->child, linked_inode, &loc, dict); + br_trigger_sign (this, object->child, linked_inode, &loc, _gf_false); - if (dict) - dict_unref (dict); loc_wipe (&loc); } @@ -618,7 +596,7 @@ static inline int32_t br_sign_object (br_object_t *object) * actual signing of the object. */ if (sign_info == BR_SIGN_REOPEN_WAIT) { - br_object_handle_reopen (this, object, linked_inode); + br_object_resign (this, object, linked_inode); goto unref_inode; } @@ -903,41 +881,7 @@ out: return need_sign; } -void -br_trigger_sign (xlator_t *this, br_child_t *child, inode_t *linked_inode, - loc_t *loc, dict_t *xdata) -{ - fd_t *fd = NULL; - int32_t ret = -1; - pid_t pid = GF_CLIENT_PID_BITD; - - syncopctx_setfspid (&pid); - - fd = fd_create (linked_inode, 0); - if (!fd) { - gf_log (this->name, GF_LOG_ERROR, - "Failed to create fd [GFID %s]", - uuid_utoa (linked_inode->gfid)); - goto out; - } - - ret = syncop_open (child->xl, loc, O_RDWR, fd, NULL, NULL); - if (ret) { - br_log_object (this, "open", linked_inode->gfid, -ret); - fd_unref (fd); - fd = NULL; - } else { - fd_bind (fd); - } - - if (fd) { - br_send_dummy_write (this, fd, child, xdata); - syncop_close (fd); - } -out: - return; -} int32_t br_prepare_loc (xlator_t *this, br_child_t *child, loc_t *parent, @@ -1076,7 +1020,7 @@ bitd_oneshot_crawl (xlator_t *subvol, gf_log (this->name, GF_LOG_INFO, "Triggering signing for %s [GFID: %s | Brick: %s]", loc.path, uuid_utoa (linked_inode->gfid), child->brick_path); - br_trigger_sign (this, child, linked_inode, &loc, NULL); + br_trigger_sign (this, child, linked_inode, &loc, _gf_true); ret = 0; -- cgit