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>

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 4d86fa4805e..fb4617d79f9 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);