From b06a77f3446d622f8159f08508f4d5063ec50339 Mon Sep 17 00:00:00 2001 From: Poornima G Date: Thu, 19 May 2016 05:04:13 -0400 Subject: leases: Fix the recall code path 1. Replace frame->op usage with frame->root->op, as frame->op is not filled with appropriate value in all cases 2. Add few log messages 3. Fix boolean assignment Change-Id: I340f2200c1fcc4f4ce5a139b0fd22508cb8ac1e3 BUG: 1319992 Signed-off-by: Poornima G Reviewed-on: http://review.gluster.org/14434 Smoke: Gluster Build System NetBSD-regression: NetBSD Build System Reviewed-by: Raghavendra Talur CentOS-regression: Gluster Build System Reviewed-by: Pranith Kumar Karampuri --- xlators/features/leases/src/leases-internal.c | 16 ++++++++------ xlators/features/leases/src/leases.c | 32 +++++++++++++-------------- xlators/features/leases/src/leases.h | 16 +++++++++----- 3 files changed, 36 insertions(+), 28 deletions(-) (limited to 'xlators/features') diff --git a/xlators/features/leases/src/leases-internal.c b/xlators/features/leases/src/leases-internal.c index 24e8389cbc6..6884b581273 100644 --- a/xlators/features/leases/src/leases-internal.c +++ b/xlators/features/leases/src/leases-internal.c @@ -1033,7 +1033,8 @@ __check_lease_conflict (call_frame_t *frame, lease_inode_ctx_t *lease_ctx, /* If the fop is rename or unlink conflict the lease even if its * from the same client?? */ - if ((frame->op == GF_FOP_RENAME) || (frame->op == GF_FOP_UNLINK)) { + if ((frame->root->op == GF_FOP_RENAME) || + (frame->root->op == GF_FOP_UNLINK)) { conflicts = _gf_true; goto recall; } @@ -1096,15 +1097,15 @@ check_lease_conflict (call_frame_t *frame, inode_t *inode, goto out; } - is_blocking_fop = fop_flags & BLOCKING_FOP; - is_write_fop = fop_flags & DATA_MODIFY_FOP; + is_blocking_fop = ((fop_flags & BLOCKING_FOP) != 0); + is_write_fop = ((fop_flags & DATA_MODIFY_FOP) != 0); pthread_mutex_lock (&lease_ctx->lock); { if (lease_ctx->lease_type == NONE) { gf_msg_debug (frame->this->name, 0, - "No leases found continuing with the" - " fop:%s", gf_fop_list[frame->op]); + "No leases found continuing with the" + " fop:%s", gf_fop_list[frame->root->op]); ret = WIND_FOP; goto unlock; } @@ -1115,14 +1116,15 @@ check_lease_conflict (call_frame_t *frame, inode_t *inode, gf_msg_debug (frame->this->name, 0, "Fop: %s " "conflicting existing " "lease: %d, blocking the" - "fop", gf_fop_list[frame->op], + "fop", gf_fop_list[frame->root->op], lease_ctx->lease_type); ret = BLOCK_FOP; } else { gf_msg_debug (frame->this->name, 0, "Fop: %s " "conflicting existing " "lease: %d, sending " - "EAGAIN", gf_fop_list[frame->op], + "EAGAIN", + gf_fop_list[frame->root->op], lease_ctx->lease_type); errno = EAGAIN; ret = -1; diff --git a/xlators/features/leases/src/leases.c b/xlators/features/leases/src/leases.c index 4e5ceab3ffa..3e0460000d7 100644 --- a/xlators/features/leases/src/leases.c +++ b/xlators/features/leases/src/leases.c @@ -45,7 +45,7 @@ leases_open (call_frame_t *frame, xlator_t *this, loc_t *loc, int32_t flags, goto err; } - GET_FLAGS (frame->op, flags); + GET_FLAGS (frame->root->op, flags); GET_LEASE_ID (xdata, lease_id, frame->root->client->client_uid); if (lease_id != NULL) memcpy (fd_ctx->lease_id, lease_id, LEASE_ID_SIZE); @@ -108,7 +108,7 @@ leases_writev (call_frame_t *frame, xlator_t *this, fd_t *fd, EXIT_IF_LEASES_OFF (this, out); GET_LEASE_ID (xdata, lease_id, frame->root->client->client_uid); - GET_FLAGS (frame->op, fd->flags); + GET_FLAGS (frame->root->op, fd->flags); ret = check_lease_conflict (frame, fd->inode, lease_id, fop_flags); if (ret < 0) @@ -161,7 +161,7 @@ leases_readv (call_frame_t *frame, xlator_t *this, EXIT_IF_LEASES_OFF (this, out); GET_LEASE_ID (xdata, lease_id, frame->root->client->client_uid); - GET_FLAGS (frame->op, fd->flags); + GET_FLAGS (frame->root->op, fd->flags); ret = check_lease_conflict (frame, fd->inode, lease_id, fop_flags); if (ret < 0) @@ -293,7 +293,7 @@ leases_truncate (call_frame_t *frame, xlator_t *this, loc_t *loc, off_t offset, EXIT_IF_LEASES_OFF (this, out); GET_LEASE_ID (xdata, lease_id, frame->root->client->client_uid); - GET_FLAGS (frame->op, 0); + GET_FLAGS (frame->root->op, 0); ret = check_lease_conflict (frame, loc->inode, lease_id, fop_flags); if (ret < 0) @@ -343,7 +343,7 @@ leases_setattr (call_frame_t *frame, xlator_t *this, loc_t *loc, EXIT_IF_LEASES_OFF (this, out); GET_LEASE_ID (xdata, lease_id, frame->root->client->client_uid); - GET_FLAGS (frame->op, 0); + GET_FLAGS (frame->root->op, 0); ret = check_lease_conflict (frame, loc->inode, lease_id, fop_flags); if (ret < 0) @@ -397,7 +397,7 @@ leases_rename (call_frame_t *frame, xlator_t *this, /* should the lease be also checked for newloc */ GET_LEASE_ID (xdata, lease_id, frame->root->client->client_uid); - GET_FLAGS (frame->op, 0); + GET_FLAGS (frame->root->op, 0); ret = check_lease_conflict (frame, oldloc->inode, lease_id, fop_flags); if (ret < 0) @@ -448,7 +448,7 @@ leases_unlink (call_frame_t *frame, xlator_t *this, loc_t *loc, int xflag, EXIT_IF_LEASES_OFF (this, out); GET_LEASE_ID (xdata, lease_id, frame->root->client->client_uid); - GET_FLAGS (frame->op, 0); + GET_FLAGS (frame->root->op, 0); ret = check_lease_conflict (frame, loc->inode, lease_id, fop_flags); if (ret < 0) @@ -498,7 +498,7 @@ leases_link (call_frame_t *frame, xlator_t *this, loc_t *oldloc, EXIT_IF_LEASES_OFF (this, out); GET_LEASE_ID (xdata, lease_id, frame->root->client->client_uid); - GET_FLAGS (frame->op, 0); + GET_FLAGS (frame->root->op, 0); ret = check_lease_conflict (frame, oldloc->inode, lease_id, fop_flags); if (ret < 0) @@ -550,7 +550,7 @@ leases_create (call_frame_t *frame, xlator_t *this, EXIT_IF_LEASES_OFF (this, out); GET_LEASE_ID (xdata, lease_id, frame->root->client->client_uid); - GET_FLAGS (frame->op, flags); + GET_FLAGS (frame->root->op, flags); ret = check_lease_conflict (frame, fd->inode, lease_id, fop_flags); if (ret < 0) @@ -601,7 +601,7 @@ leases_fsync (call_frame_t *frame, xlator_t *this, fd_t *fd, EXIT_IF_LEASES_OFF (this, out); GET_LEASE_ID (xdata, lease_id, frame->root->client->client_uid); - GET_FLAGS (frame->op, fd->flags); + GET_FLAGS (frame->root->op, fd->flags); ret = check_lease_conflict (frame, fd->inode, lease_id, fop_flags); if (ret < 0) @@ -649,7 +649,7 @@ leases_ftruncate (call_frame_t *frame, xlator_t *this, EXIT_IF_LEASES_OFF (this, out); GET_LEASE_ID (xdata, lease_id, frame->root->client->client_uid); - GET_FLAGS (frame->op, 0); /* TODO:fd->flags?*/ + GET_FLAGS (frame->root->op, 0); /* TODO:fd->flags?*/ ret = check_lease_conflict (frame, fd->inode, lease_id, fop_flags); if (ret < 0) @@ -699,7 +699,7 @@ leases_fsetattr (call_frame_t *frame, xlator_t *this, fd_t *fd, EXIT_IF_LEASES_OFF (this, out); GET_LEASE_ID (xdata, lease_id, frame->root->client->client_uid); - GET_FLAGS (frame->op, fd->flags); + GET_FLAGS (frame->root->op, fd->flags); ret = check_lease_conflict (frame, fd->inode, lease_id, fop_flags); if (ret < 0) @@ -750,7 +750,7 @@ leases_fallocate (call_frame_t *frame, xlator_t *this, fd_t *fd, EXIT_IF_LEASES_OFF (this, out); GET_LEASE_ID (xdata, lease_id, frame->root->client->client_uid); - GET_FLAGS (frame->op, fd->flags); + GET_FLAGS (frame->root->op, fd->flags); ret = check_lease_conflict (frame, fd->inode, lease_id, fop_flags); if (ret < 0) @@ -801,7 +801,7 @@ leases_discard (call_frame_t *frame, xlator_t *this, fd_t *fd, EXIT_IF_LEASES_OFF (this, out); GET_LEASE_ID (xdata, lease_id, frame->root->client->client_uid); - GET_FLAGS (frame->op, fd->flags); + GET_FLAGS (frame->root->op, fd->flags); ret = check_lease_conflict (frame, fd->inode, lease_id, fop_flags); if (ret < 0) @@ -852,7 +852,7 @@ leases_zerofill (call_frame_t *frame, xlator_t *this, fd_t *fd, EXIT_IF_LEASES_OFF (this, out); GET_LEASE_ID (xdata, lease_id, frame->root->client->client_uid); - GET_FLAGS (frame->op, fd->flags); + GET_FLAGS (frame->root->op, fd->flags); ret = check_lease_conflict (frame, fd->inode, lease_id, fop_flags); if (ret < 0) @@ -901,7 +901,7 @@ leases_flush (call_frame_t *frame, xlator_t *this, EXIT_IF_LEASES_OFF (this, out); GET_LEASE_ID (xdata, lease_id, frame->root->client->client_uid); - GET_FLAGS (frame->op, fd->flags); + GET_FLAGS (frame->root->op, fd->flags); ret = check_lease_conflict (frame, fd->inode, lease_id, fop_flags); if (ret < 0) diff --git a/xlators/features/leases/src/leases.h b/xlators/features/leases/src/leases.h index 77818b2053f..df5e8beb85c 100644 --- a/xlators/features/leases/src/leases.h +++ b/xlators/features/leases/src/leases.h @@ -57,7 +57,7 @@ #define GET_FLAGS(fop, fd_flags) \ do { \ - if (fd_flags & (O_WRONLY | O_RDWR) && fop == GF_FOP_OPEN) \ + if ((fd_flags & (O_WRONLY | O_RDWR)) && fop == GF_FOP_OPEN) \ fop_flags = DATA_MODIFY_FOP; \ \ if (fop == GF_FOP_UNLINK || fop == GF_FOP_RENAME || \ @@ -69,7 +69,7 @@ do { \ fop == GF_FOP_LINK) \ fop_flags = DATA_MODIFY_FOP; \ \ - if (fd_flags & (O_NONBLOCK | O_NDELAY)) \ + if (!(fd_flags & (O_NONBLOCK | O_NDELAY))) \ fop_flags |= BLOCKING_FOP; \ \ } while (0) \ @@ -99,13 +99,19 @@ do { \ __stub = fop_##fop_name##_stub (frame, default_##fop_name##_resume, \ params); \ if (!__stub) { \ + gf_msg (this->name, GF_LOG_WARNING, ENOMEM, \ + LEASE_MSG_NO_MEM, \ + "Unable to create stub"); \ ret = -ENOMEM; \ goto __out; \ } \ \ blk_fop = GF_CALLOC (1, sizeof (*blk_fop), \ gf_leases_mt_fop_stub_t); \ - if (blk_fop) { \ + if (!blk_fop) { \ + gf_msg (this->name, GF_LOG_WARNING, ENOMEM, \ + LEASE_MSG_NO_MEM, \ + "Unable to create lease fop stub"); \ ret = -ENOMEM; \ goto __out; \ } \ @@ -119,6 +125,7 @@ do { \ goto __out; \ } \ \ + blk_fop->stub = __stub; \ pthread_mutex_lock (&lease_ctx->lock); \ { \ /*TODO: If the lease is unlocked btw check lease conflict and \ @@ -132,8 +139,7 @@ __out: \ if (ret < 0) { \ gf_msg (this->name, GF_LOG_WARNING, ENOMEM, LEASE_MSG_NO_MEM, \ "Unable to create stub for blocking the fop:%s (%s)", \ - gf_fop_list[frame->op], strerror(ENOMEM)); \ - default_##fop_name##_failure_cbk (frame, -__ret); \ + gf_fop_list[frame->root->op], strerror(ENOMEM)); \ if (__stub != NULL) { \ call_stub_destroy (__stub); \ } \ -- cgit