diff options
author | Pranith Kumar K <pranithk@gluster.com> | 2012-07-25 20:46:53 +0530 |
---|---|---|
committer | Anand Avati <avati@redhat.com> | 2012-07-31 23:47:23 -0700 |
commit | 9fcc3f4dede2829d457b6e1c76f53c25ba790988 (patch) | |
tree | 7bbc0175b0fe44eedad871d2e39fd7843cfd3db2 /xlators/cluster | |
parent | b18c913f58aadcadb830b59d8ed8979d18c0e0d0 (diff) |
cluster/afr: Handle failures in fop_cbk gracefully
RCA:
Afr crashes when a last fop response fails and
'fop output' arguments are NULL. Afr does not handle
these gracefully.
Fix:
Changed the fops to not access the 'fop output' arguments
in case of failures.
Tests:
Changed afr wind_cbk code to fail the last response by setting
op_ret as -1 and op_errno as ENOMEM and setting all other output
variables as NULL to test the change. Removed the code to verify
success cases. No crashes or errors seen.
Change-Id: Iad9bc54db093a162f85bfb8dbeeda5b95acd21d8
BUG: 844689
Signed-off-by: Pranith Kumar K <pranithk@gluster.com>
Reviewed-on: http://review.gluster.com/3760
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Amar Tumballi <amarts@redhat.com>
Reviewed-by: Anand Avati <avati@redhat.com>
Diffstat (limited to 'xlators/cluster')
-rw-r--r-- | xlators/cluster/afr/src/afr-dir-write.c | 77 |
1 files changed, 46 insertions, 31 deletions
diff --git a/xlators/cluster/afr/src/afr-dir-write.c b/xlators/cluster/afr/src/afr-dir-write.c index 4d86fa480..fb4617d79 100644 --- a/xlators/cluster/afr/src/afr-dir-write.c +++ b/xlators/cluster/afr/src/afr-dir-write.c @@ -138,7 +138,7 @@ afr_create_wind_cbk (call_frame_t *frame, void *cookie, xlator_t *this, if (afr_fop_failed (op_ret, op_errno)) afr_transaction_fop_failed (frame, this, child_index); - if (op_ret != -1) { + if (op_ret > -1) { local->op_ret = op_ret; ret = afr_fd_ctx_set (this, fd); @@ -195,11 +195,14 @@ unlock: call_count = afr_frame_return (frame); if (call_count == 0) { - afr_set_read_ctx_from_policy (this, inode, - local->fresh_children, - local->read_child_index, - priv->read_child, - local->cont.create.buf.ia_gfid); + if (local->op_ret > -1) { + afr_set_read_ctx_from_policy (this, + local->cont.create.inode, + local->fresh_children, + local->read_child_index, + priv->read_child, + local->cont.create.buf.ia_gfid); + } local->transaction.unwind (frame, this); local->transaction.resume (frame, this); @@ -403,7 +406,7 @@ afr_mknod_wind_cbk (call_frame_t *frame, void *cookie, xlator_t *this, if (afr_fop_failed (op_ret, op_errno)) afr_transaction_fop_failed (frame, this, child_index); - if (op_ret != -1) { + if (op_ret > -1) { local->op_ret = op_ret; if (local->success_count == 0) @@ -429,11 +432,14 @@ afr_mknod_wind_cbk (call_frame_t *frame, void *cookie, xlator_t *this, call_count = afr_frame_return (frame); if (call_count == 0) { - afr_set_read_ctx_from_policy (this, inode, - local->fresh_children, - local->read_child_index, - priv->read_child, - local->cont.mknod.buf.ia_gfid); + if (local->op_ret > -1) { + afr_set_read_ctx_from_policy (this, + local->cont.mknod.inode, + local->fresh_children, + local->read_child_index, + priv->read_child, + local->cont.mknod.buf.ia_gfid); + } local->transaction.unwind (frame, this); local->transaction.resume (frame, this); @@ -632,7 +638,7 @@ afr_mkdir_wind_cbk (call_frame_t *frame, void *cookie, xlator_t *this, if (afr_fop_failed (op_ret, op_errno)) afr_transaction_fop_failed (frame, this, child_index); - if (op_ret != -1) { + if (op_ret > -1) { local->op_ret = op_ret; if (local->success_count == 0) @@ -658,11 +664,14 @@ afr_mkdir_wind_cbk (call_frame_t *frame, void *cookie, xlator_t *this, call_count = afr_frame_return (frame); if (call_count == 0) { - afr_set_read_ctx_from_policy (this, inode, - local->fresh_children, - local->read_child_index, - priv->read_child, - local->cont.mkdir.buf.ia_gfid); + if (local->op_ret > -1) { + afr_set_read_ctx_from_policy (this, + local->cont.mkdir.inode, + local->fresh_children, + local->read_child_index, + priv->read_child, + local->cont.mkdir.buf.ia_gfid); + } local->transaction.unwind (frame, this); local->transaction.resume (frame, this); @@ -862,7 +871,7 @@ afr_link_wind_cbk (call_frame_t *frame, void *cookie, xlator_t *this, if (afr_fop_failed (op_ret, op_errno)) afr_transaction_fop_failed (frame, this, child_index); - if (op_ret != -1) { + if (op_ret > -1) { local->op_ret = op_ret; if (local->success_count == 0) { @@ -877,10 +886,10 @@ afr_link_wind_cbk (call_frame_t *frame, void *cookie, xlator_t *this, fresh_children = local->fresh_children; fresh_children[local->success_count] = child_index; + local->cont.link.inode = inode; local->success_count++; } - local->cont.link.inode = inode; local->op_errno = op_errno; } UNLOCK (&frame->lock); @@ -888,11 +897,14 @@ afr_link_wind_cbk (call_frame_t *frame, void *cookie, xlator_t *this, call_count = afr_frame_return (frame); if (call_count == 0) { - afr_set_read_ctx_from_policy (this, inode, - local->fresh_children, - local->read_child_index, - priv->read_child, - local->cont.link.buf.ia_gfid); + if (local->op_ret > -1) { + afr_set_read_ctx_from_policy (this, + local->cont.link.inode, + local->fresh_children, + local->read_child_index, + priv->read_child, + local->cont.link.buf.ia_gfid); + } local->transaction.unwind (frame, this); local->transaction.resume (frame, this); @@ -1086,7 +1098,7 @@ afr_symlink_wind_cbk (call_frame_t *frame, void *cookie, xlator_t *this, if (afr_fop_failed (op_ret, op_errno)) afr_transaction_fop_failed (frame, this, child_index); - if (op_ret != -1) { + if (op_ret > -1) { local->op_ret = op_ret; if (local->success_count == 0) @@ -1112,11 +1124,14 @@ afr_symlink_wind_cbk (call_frame_t *frame, void *cookie, xlator_t *this, call_count = afr_frame_return (frame); if (call_count == 0) { - afr_set_read_ctx_from_policy (this, inode, - local->fresh_children, - local->read_child_index, - priv->read_child, - local->cont.symlink.buf.ia_gfid); + if (local->op_ret > -1) { + afr_set_read_ctx_from_policy (this, + local->cont.symlink.inode, + local->fresh_children, + local->read_child_index, + priv->read_child, + local->cont.symlink.buf.ia_gfid); + } local->transaction.unwind (frame, this); local->transaction.resume (frame, this); |