cluster/afr: Unwind with correct pre/post parent bufs

RCA:
In case of dir fops create, mknod, mkdir, link, symlink, rename
if the fop fails on read-child then unwinds are happening with
all-zero pre/post iatt-bufs. The bug occurs because the parent
bufs are not saved if the response is not from read-child.

Fix:
Save the pre/post-bufs for the first response. If the response
comes from read-child, overwrite whatever we have cached.

Tests:
Attached the mount process to gdb.
Tested that the unwinds happen with proper pre/post iatt bufs in
the following cases:
1) All success case
2) Failure on read-child
3) Failure on non-read-child
4) Failure on all children.

Tested soft-link self-heal to test the change made in that.
Tested errno ENOTEMPTY for rmdir, rename fops.

Change-Id: I82882423d2d766b4f4a3044203bcb5dbcaee1755
BUG: 845242
Signed-off-by: Pranith Kumar K <pranithk@gluster.com>
Reviewed-on: http://review.gluster.com/3775
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Anand Avati <avati@redhat.com>

3 files changed, 147 insertions, 403 deletions

diff --git a/xlators/cluster/afr/src/afr-dir-write.c b/xlators/cluster/afr/src/afr-dir-write.c
index fb4617d79..304103541 100644
--- a/xlators/cluster/afr/src/afr-dir-write.c
+++ b/xlators/cluster/afr/src/afr-dir-write.c
@@ -72,6 +72,66 @@ out:
         return ret;
 }
 
+void
+__dir_entry_fop_common_cbk (call_frame_t *frame, int child_index,
+                            xlator_t *this, int32_t op_ret,
+                            int32_t op_errno, inode_t *inode,
+                            struct iatt *buf, struct iatt *preparent,
+                            struct iatt *postparent, struct iatt *prenewparent,
+                            struct iatt *postnewparent)
+{
+        afr_local_t     *local          = NULL;
+
+        local = frame->local;
+
+        if (afr_fop_failed (op_ret, op_errno))
+                afr_transaction_fop_failed (frame, this, child_index);
+
+        if (op_ret > -1) {
+                local->op_ret = op_ret;
+
+                if ((local->success_count == 0) ||
+                    (child_index == local->read_child_index)) {
+                        local->cont.dir_fop.preparent      = *preparent;
+                        local->cont.dir_fop.postparent     = *postparent;
+                        if (buf)
+                                local->cont.dir_fop.buf            = *buf;
+                        if (prenewparent)
+                             local->cont.dir_fop.prenewparent  = *prenewparent;
+                        if (postnewparent)
+                             local->cont.dir_fop.postnewparent = *postnewparent;
+                }
+
+                local->cont.dir_fop.inode = inode;
+
+                local->fresh_children[local->success_count] = child_index;
+                local->success_count++;
+        }
+
+        local->op_errno = op_errno;
+}
+
+void
+afr_dir_fop_done (call_frame_t *frame, xlator_t *this)
+{
+        afr_local_t     *local          = NULL;
+        afr_private_t   *priv           = NULL;
+
+        local = frame->local;
+        priv  = this->private;
+
+        if (local->cont.dir_fop.inode == NULL)
+                goto done;
+        afr_set_read_ctx_from_policy (this, local->cont.dir_fop.inode,
+                                      local->fresh_children,
+                                      local->read_child_index,
+                                      priv->read_child,
+                                      local->cont.dir_fop.buf.ia_gfid);
+done:
+        local->transaction.unwind (frame, this);
+        local->transaction.resume (frame, this);
+}
+
 /* {{{ create */
 
 int
@@ -79,7 +139,6 @@ afr_create_unwind (call_frame_t *frame, xlator_t *this)
 {
         call_frame_t *main_frame = NULL;
         afr_local_t  *local = NULL;
-        struct iatt  *unwind_buf = NULL;
 
         local = frame->local;
 
@@ -93,18 +152,13 @@ afr_create_unwind (call_frame_t *frame, xlator_t *this)
         UNLOCK (&frame->lock);
 
         if (main_frame) {
-                if (local->cont.create.read_child_buf.ia_ino) {
-                        unwind_buf = &local->cont.create.read_child_buf;
-                } else {
-                        unwind_buf = &local->cont.create.buf;
-                }
-
                 AFR_STACK_UNWIND (create, main_frame,
                                   local->op_ret, local->op_errno,
                                   local->cont.create.fd,
-                                  local->cont.create.inode,
-                                  unwind_buf, &local->cont.create.preparent,
-                                  &local->cont.create.postparent,
+                                  local->cont.dir_fop.inode,
+                                  &local->cont.dir_fop.buf,
+                                  &local->cont.dir_fop.preparent,
+                                  &local->cont.dir_fop.postparent,
                                   local->xdata_rsp);
         }
 
@@ -120,29 +174,20 @@ afr_create_wind_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
                      dict_t *xdata)
 {
         afr_local_t     *local = NULL;
-        afr_private_t   *priv  = NULL;
         uint64_t        ctx = 0;
         afr_fd_ctx_t    *fd_ctx = NULL;
         int             ret = 0;
         int             call_count = -1;
         int             child_index = -1;
-        int32_t         *fresh_children = NULL;
 
         local = frame->local;
-        priv  = this->private;
 
         child_index = (long) cookie;
 
         LOCK (&frame->lock);
         {
-                if (afr_fop_failed (op_ret, op_errno))
-                        afr_transaction_fop_failed (frame, this, child_index);
-
                 if (op_ret > -1) {
-                        local->op_ret = op_ret;
-
                         ret = afr_fd_ctx_set (this, fd);
-
                         if (ret < 0) {
                                 gf_log (this->name, GF_LOG_ERROR,
                                         "could not set ctx on fd=%p", fd);
@@ -153,7 +198,6 @@ afr_create_wind_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
                         }
 
                         ret = fd_ctx_get (fd, this, &ctx);
-
                         if (ret < 0) {
                                 gf_log (this->name, GF_LOG_ERROR,
                                         "could not get fd ctx for fd=%p", fd);
@@ -168,25 +212,13 @@ afr_create_wind_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
                         fd_ctx->flags                  = local->cont.create.flags;
 
                         if (local->success_count == 0) {
-                                local->cont.create.buf        = *buf;
 				if (xdata)
 					local->xdata_rsp = dict_ref(xdata);
 			}
-
-                        if (child_index == local->read_child_index) {
-                                local->cont.create.read_child_buf = *buf;
-                                local->cont.create.preparent      = *preparent;
-                                local->cont.create.postparent     = *postparent;
-                        }
-
-                        local->cont.create.inode = inode;
-
-                        fresh_children = local->fresh_children;
-                        fresh_children[local->success_count] = child_index;
-                        local->success_count++;
                 }
-
-                local->op_errno = op_errno;
+                __dir_entry_fop_common_cbk (frame, child_index, this,
+                                            op_ret, op_errno, inode, buf,
+                                            preparent, postparent, NULL, NULL);
         }
 
 unlock:
@@ -194,19 +226,8 @@ unlock:
 
         call_count = afr_frame_return (frame);
 
-        if (call_count == 0) {
-                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);
-        }
+        if (call_count == 0)
+                afr_dir_fop_done (frame, this);
 
         return 0;
 }
@@ -352,7 +373,6 @@ afr_mknod_unwind (call_frame_t *frame, xlator_t *this)
 {
         call_frame_t *main_frame = NULL;
         afr_local_t  *local = NULL;
-        struct iatt *unwind_buf = NULL;
 
         local = frame->local;
 
@@ -366,17 +386,12 @@ afr_mknod_unwind (call_frame_t *frame, xlator_t *this)
         UNLOCK (&frame->lock);
 
         if (main_frame) {
-                if (local->cont.mknod.read_child_buf.ia_ino) {
-                        unwind_buf = &local->cont.mknod.read_child_buf;
-                } else {
-                        unwind_buf = &local->cont.mknod.buf;
-                }
-
                 AFR_STACK_UNWIND (mknod, main_frame,
                                   local->op_ret, local->op_errno,
-                                  local->cont.mknod.inode,
-                                  unwind_buf, &local->cont.mknod.preparent,
-                                  &local->cont.mknod.postparent,
+                                  local->cont.dir_fop.inode,
+                                  &local->cont.dir_fop.buf,
+                                  &local->cont.dir_fop.preparent,
+                                  &local->cont.dir_fop.postparent,
                                   NULL);
         }
 
@@ -390,60 +405,23 @@ afr_mknod_wind_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
                     struct iatt *buf, struct iatt *preparent,
                     struct iatt *postparent, dict_t *xdata)
 {
-        afr_local_t     *local          = NULL;
-        afr_private_t   *priv           = NULL;
         int             call_count      = -1;
         int             child_index     = -1;
-        int32_t         *fresh_children = NULL;
-
-        local = frame->local;
-        priv = this->private;
 
         child_index = (long) cookie;
 
         LOCK (&frame->lock);
         {
-                if (afr_fop_failed (op_ret, op_errno))
-                        afr_transaction_fop_failed (frame, this, child_index);
-
-                if (op_ret > -1) {
-                        local->op_ret = op_ret;
-
-                        if (local->success_count == 0)
-                                local->cont.mknod.buf   = *buf;
-
-                        if (child_index == local->read_child_index) {
-                                local->cont.mknod.read_child_buf = *buf;
-                                local->cont.mknod.preparent      = *preparent;
-                                local->cont.mknod.postparent     = *postparent;
-                        }
-
-                        local->cont.mknod.inode = inode;
-
-                        fresh_children = local->fresh_children;
-                        fresh_children[local->success_count] = child_index;
-                        local->success_count++;
-                }
-
-                local->op_errno = op_errno;
+                __dir_entry_fop_common_cbk (frame, child_index, this,
+                                            op_ret, op_errno, inode, buf,
+                                            preparent, postparent, NULL, NULL);
         }
         UNLOCK (&frame->lock);
 
         call_count = afr_frame_return (frame);
 
-        if (call_count == 0) {
-                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);
-        }
+        if (call_count == 0)
+                afr_dir_fop_done (frame, this);
 
         return 0;
 }
@@ -584,7 +562,6 @@ afr_mkdir_unwind (call_frame_t *frame, xlator_t *this)
 {
         call_frame_t *main_frame = NULL;
         afr_local_t  *local = NULL;
-        struct iatt *unwind_buf = NULL;
 
         local = frame->local;
 
@@ -598,17 +575,12 @@ afr_mkdir_unwind (call_frame_t *frame, xlator_t *this)
         UNLOCK (&frame->lock);
 
         if (main_frame) {
-                if (local->cont.mkdir.read_child_buf.ia_ino) {
-                        unwind_buf = &local->cont.mkdir.read_child_buf;
-                } else {
-                        unwind_buf = &local->cont.mkdir.buf;
-                }
-
                 AFR_STACK_UNWIND (mkdir, main_frame,
                                   local->op_ret, local->op_errno,
-                                  local->cont.mkdir.inode,
-                                  unwind_buf, &local->cont.mkdir.preparent,
-                                  &local->cont.mkdir.postparent,
+                                  local->cont.dir_fop.inode,
+                                  &local->cont.dir_fop.buf,
+                                  &local->cont.dir_fop.preparent,
+                                  &local->cont.dir_fop.postparent,
                                   NULL);
         }
 
@@ -622,60 +594,23 @@ afr_mkdir_wind_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
                     struct iatt *buf, struct iatt *preparent,
                     struct iatt *postparent, dict_t *xdata)
 {
-        afr_local_t     *local          = NULL;
-        afr_private_t   *priv           = NULL;
         int             call_count      = -1;
         int             child_index     = -1;
-        int32_t         *fresh_children = NULL;
-
-        local = frame->local;
-        priv = this->private;
 
         child_index = (long) cookie;
 
         LOCK (&frame->lock);
         {
-                if (afr_fop_failed (op_ret, op_errno))
-                        afr_transaction_fop_failed (frame, this, child_index);
-
-                if (op_ret > -1) {
-                        local->op_ret           = op_ret;
-
-                        if (local->success_count == 0)
-                                local->cont.mkdir.buf   = *buf;
-
-                        if (child_index == local->read_child_index) {
-                                local->cont.mkdir.read_child_buf = *buf;
-                                local->cont.mkdir.preparent      = *preparent;
-                                local->cont.mkdir.postparent     = *postparent;
-                        }
-
-                        local->cont.mkdir.inode = inode;
-
-                        fresh_children = local->fresh_children;
-                        fresh_children[local->success_count] = child_index;
-                        local->success_count++;
-                }
-
-                local->op_errno         = op_errno;
+                __dir_entry_fop_common_cbk (frame, child_index, this,
+                                            op_ret, op_errno, inode, buf,
+                                            preparent, postparent, NULL, NULL);
         }
         UNLOCK (&frame->lock);
 
         call_count = afr_frame_return (frame);
 
-        if (call_count == 0) {
-                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);
-        }
+        if (call_count == 0)
+                afr_dir_fop_done (frame, this);
 
         return 0;
 }
@@ -817,7 +752,6 @@ afr_link_unwind (call_frame_t *frame, xlator_t *this)
 {
         call_frame_t *main_frame = NULL;
         afr_local_t  *local = NULL;
-        struct iatt *unwind_buf = NULL;
 
         local = frame->local;
 
@@ -831,17 +765,12 @@ afr_link_unwind (call_frame_t *frame, xlator_t *this)
         UNLOCK (&frame->lock);
 
         if (main_frame) {
-                if (local->cont.link.read_child_buf.ia_ino) {
-                        unwind_buf = &local->cont.link.read_child_buf;
-                } else {
-                        unwind_buf = &local->cont.link.buf;
-                }
-
                 AFR_STACK_UNWIND (link, main_frame,
                                   local->op_ret, local->op_errno,
-                                  local->cont.link.inode,
-                                  unwind_buf, &local->cont.link.preparent,
-                                  &local->cont.link.postparent,
+                                  local->cont.dir_fop.inode,
+                                  &local->cont.dir_fop.buf,
+                                  &local->cont.dir_fop.preparent,
+                                  &local->cont.dir_fop.postparent,
                                   NULL);
         }
 
@@ -855,60 +784,23 @@ afr_link_wind_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
                    struct iatt *buf, struct iatt *preparent,
                    struct iatt *postparent, dict_t *xdata)
 {
-        afr_local_t     *local          = NULL;
-        afr_private_t   *priv           = NULL;
         int             call_count      = -1;
         int             child_index     = -1;
-        int32_t         *fresh_children = NULL;
-
-        local = frame->local;
-        priv = this->private;
 
         child_index = (long) cookie;
 
         LOCK (&frame->lock);
         {
-                if (afr_fop_failed (op_ret, op_errno))
-                        afr_transaction_fop_failed (frame, this, child_index);
-
-                if (op_ret > -1) {
-                        local->op_ret   = op_ret;
-
-                        if (local->success_count == 0) {
-                                local->cont.link.buf        = *buf;
-                        }
-
-                        if (child_index == local->read_child_index) {
-                                local->cont.link.read_child_buf = *buf;
-                                local->cont.link.preparent      = *preparent;
-                                local->cont.link.postparent     = *postparent;
-                        }
-
-                        fresh_children = local->fresh_children;
-                        fresh_children[local->success_count] = child_index;
-                        local->cont.link.inode = inode;
-                        local->success_count++;
-                }
-
-                local->op_errno = op_errno;
+                __dir_entry_fop_common_cbk (frame, child_index, this,
+                                            op_ret, op_errno, inode, buf,
+                                            preparent, postparent, NULL, NULL);
         }
         UNLOCK (&frame->lock);
 
         call_count = afr_frame_return (frame);
 
-        if (call_count == 0) {
-                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);
-        }
+        if (call_count == 0)
+                afr_dir_fop_done (frame, this);
 
         return 0;
 }
@@ -1044,7 +936,6 @@ afr_symlink_unwind (call_frame_t *frame, xlator_t *this)
 {
         call_frame_t *main_frame = NULL;
         afr_local_t  *local = NULL;
-        struct iatt *unwind_buf = NULL;
 
         local = frame->local;
 
@@ -1058,17 +949,12 @@ afr_symlink_unwind (call_frame_t *frame, xlator_t *this)
         UNLOCK (&frame->lock);
 
         if (main_frame) {
-                if (local->cont.symlink.read_child_buf.ia_ino) {
-                        unwind_buf = &local->cont.symlink.read_child_buf;
-                } else {
-                        unwind_buf = &local->cont.symlink.buf;
-                }
-
                 AFR_STACK_UNWIND (symlink, main_frame,
                                   local->op_ret, local->op_errno,
-                                  local->cont.symlink.inode,
-                                  unwind_buf, &local->cont.symlink.preparent,
-                                  &local->cont.symlink.postparent,
+                                  local->cont.dir_fop.inode,
+                                  &local->cont.dir_fop.buf,
+                                  &local->cont.dir_fop.preparent,
+                                  &local->cont.dir_fop.postparent,
                                   NULL);
         }
 
@@ -1082,60 +968,23 @@ afr_symlink_wind_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
                       struct iatt *buf, struct iatt *preparent,
                       struct iatt *postparent, dict_t *xdata)
 {
-        afr_local_t     *local          = NULL;
-        afr_private_t   *priv           = NULL;
         int             call_count      = -1;
         int             child_index     = -1;
-        int32_t         *fresh_children = NULL;
-
-        local = frame->local;
-        priv = this->private;
 
         child_index = (long) cookie;
 
         LOCK (&frame->lock);
         {
-                if (afr_fop_failed (op_ret, op_errno))
-                        afr_transaction_fop_failed (frame, this, child_index);
-
-                if (op_ret > -1) {
-                        local->op_ret   = op_ret;
-
-                        if (local->success_count == 0)
-                                local->cont.symlink.buf        = *buf;
-
-                        if (child_index == local->read_child_index) {
-                                local->cont.symlink.read_child_buf = *buf;
-                                local->cont.symlink.preparent      = *preparent;
-                                local->cont.symlink.postparent     = *postparent;
-                        }
-
-                        local->cont.symlink.inode    = inode;
-
-                        fresh_children = local->fresh_children;
-                        fresh_children[local->success_count] = child_index;
-                        local->success_count++;
-                }
-
-                local->op_errno = op_errno;
+                __dir_entry_fop_common_cbk (frame, child_index, this,
+                                            op_ret, op_errno, inode, buf,
+                                            preparent, postparent, NULL, NULL);
         }
         UNLOCK (&frame->lock);
 
         call_count = afr_frame_return (frame);
 
-        if (call_count == 0) {
-                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);
-        }
+        if (call_count == 0)
+                afr_dir_fop_done (frame, this);
 
         return 0;
 }
@@ -1276,7 +1125,6 @@ afr_rename_unwind (call_frame_t *frame, xlator_t *this)
 {
         call_frame_t *main_frame = NULL;
         afr_local_t  *local = NULL;
-        struct iatt *unwind_buf = NULL;
 
         local = frame->local;
 
@@ -1290,19 +1138,13 @@ afr_rename_unwind (call_frame_t *frame, xlator_t *this)
         UNLOCK (&frame->lock);
 
         if (main_frame) {
-                if (local->cont.rename.read_child_buf.ia_ino) {
-                        unwind_buf = &local->cont.rename.read_child_buf;
-                } else {
-                        unwind_buf = &local->cont.rename.buf;
-                }
-
                 AFR_STACK_UNWIND (rename, main_frame,
                                   local->op_ret, local->op_errno,
-                                  unwind_buf,
-                                  &local->cont.rename.preoldparent,
-                                  &local->cont.rename.postoldparent,
-                                  &local->cont.rename.prenewparent,
-                                  &local->cont.rename.postnewparent,
+                                  &local->cont.dir_fop.buf,
+                                  &local->cont.dir_fop.preparent,
+                                  &local->cont.dir_fop.postparent,
+                                  &local->cont.dir_fop.prenewparent,
+                                  &local->cont.dir_fop.postnewparent,
                                   NULL);
         }
 
@@ -1329,38 +1171,21 @@ afr_rename_wind_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
         {
                 if (afr_fop_failed (op_ret, op_errno) && op_errno != ENOTEMPTY)
                         afr_transaction_fop_failed (frame, this, child_index);
+                local->op_errno = op_errno;
 
-                if (op_ret != -1) {
-                        if (local->success_count == 0) {
-                                local->op_ret = op_ret;
-
-                                if (buf) {
-                                        local->cont.rename.buf = *buf;
-                                }
-
-                                local->success_count++;
-                        }
-
-                        if (child_index == local->read_child_index) {
-                                local->cont.rename.read_child_buf = *buf;
-
-                                local->cont.rename.preoldparent  = *preoldparent;
-                                local->cont.rename.postoldparent = *postoldparent;
-                                local->cont.rename.prenewparent  = *prenewparent;
-                                local->cont.rename.postnewparent = *postnewparent;
-                        }
-                }
+                if (op_ret > -1)
+                        __dir_entry_fop_common_cbk (frame, child_index, this,
+                                                   op_ret, op_errno, NULL, buf,
+                                                   preoldparent, postoldparent,
+                                                   prenewparent, postnewparent);
 
-                local->op_errno = op_errno;
         }
         UNLOCK (&frame->lock);
 
         call_count = afr_frame_return (frame);
 
-        if (call_count == 0) {
-                local->transaction.unwind (frame, this);
-                local->transaction.resume (frame, this);
-        }
+        if (call_count == 0)
+                afr_dir_fop_done (frame, this);
 
         return 0;
 }
@@ -1509,8 +1334,8 @@ afr_unlink_unwind (call_frame_t *frame, xlator_t *this)
         if (main_frame) {
                 AFR_STACK_UNWIND (unlink, main_frame,
                                   local->op_ret, local->op_errno,
-                                  &local->cont.unlink.preparent,
-                                  &local->cont.unlink.postparent,
+                                  &local->cont.dir_fop.preparent,
+                                  &local->cont.dir_fop.postparent,
                                   NULL);
         }
 
@@ -1534,36 +1359,15 @@ afr_unlink_wind_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
                 if (child_index == local->read_child_index) {
                         local->read_child_returned = _gf_true;
                 }
-
-                if (afr_fop_failed (op_ret, op_errno))
-                        afr_transaction_fop_failed (frame, this, child_index);
-
-                if (op_ret != -1) {
-                        if (local->success_count == 0) {
-                                local->op_ret   = op_ret;
-                                local->cont.unlink.preparent  = *preparent;
-                                local->cont.unlink.postparent = *postparent;
-                        }
-
-                        if (child_index == local->read_child_index) {
-                                local->cont.unlink.preparent  = *preparent;
-                                local->cont.unlink.postparent = *postparent;
-                        }
-
-                        local->success_count++;
-                }
-
-                local->op_errno = op_errno;
+                __dir_entry_fop_common_cbk (frame, child_index, this,
+                                            op_ret, op_errno, NULL, NULL,
+                                            preparent, postparent, NULL, NULL);
         }
         UNLOCK (&frame->lock);
 
         call_count = afr_frame_return (frame);
-
-        if (call_count == 0) {
-                local->transaction.unwind (frame, this);
-
-                local->transaction.resume (frame, this);
-        }
+        if (call_count == 0)
+                afr_dir_fop_done (frame, this);
 
         return 0;
 }
@@ -1709,8 +1513,8 @@ afr_rmdir_unwind (call_frame_t *frame, xlator_t *this)
         if (main_frame) {
                 AFR_STACK_UNWIND (rmdir, main_frame,
                                   local->op_ret, local->op_errno,
-                                  &local->cont.rmdir.preparent,
-                                  &local->cont.rmdir.postparent,
+                                  &local->cont.dir_fop.preparent,
+                                  &local->cont.dir_fop.postparent,
                                   NULL);
         }
 
@@ -1735,36 +1539,21 @@ afr_rmdir_wind_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
                 if (child_index == read_child) {
                         local->read_child_returned = _gf_true;
                 }
-
                 if (afr_fop_failed (op_ret, op_errno) && (op_errno != ENOTEMPTY))
                         afr_transaction_fop_failed (frame, this, child_index);
-
-                if (op_ret != -1) {
-                        if (local->success_count == 0) {
-                                local->op_ret = op_ret;
-                                local->cont.rmdir.preparent  = *preparent;
-                                local->cont.rmdir.postparent = *postparent;
-
-                        }
-
-                        if (child_index == read_child) {
-                                local->cont.rmdir.preparent  = *preparent;
-                                local->cont.rmdir.postparent = *postparent;
-                        }
-
-                        local->success_count++;
-                }
-
                 local->op_errno = op_errno;
+                if (op_ret > -1)
+                        __dir_entry_fop_common_cbk (frame, child_index, this,
+                                                   op_ret, op_errno, NULL, NULL,
+                                                   preparent, postparent, NULL,
+                                                   NULL);
+
         }
         UNLOCK (&frame->lock);
 
         call_count = afr_frame_return (frame);
-
-        if (call_count == 0) {
-                local->transaction.unwind (frame, this);
-                local->transaction.resume (frame, this);
-        }
+        if (call_count == 0)
+                afr_dir_fop_done (frame, this);
 
         return 0;
 }
diff --git a/xlators/cluster/afr/src/afr-self-heal-entry.c b/xlators/cluster/afr/src/afr-self-heal-entry.c
index 14ce3aa9f..e7f74c752 100644
--- a/xlators/cluster/afr/src/afr-self-heal-entry.c
+++ b/xlators/cluster/afr/src/afr-self-heal-entry.c
@@ -1357,7 +1357,7 @@ afr_sh_entry_impunge_symlink (call_frame_t *impunge_frame, xlator_t *this,
         priv = this->private;
         impunge_local = impunge_frame->local;
 
-        buf = &impunge_local->cont.symlink.buf;
+        buf = &impunge_local->cont.dir_fop.buf;
 
         dict = dict_new ();
         if (!dict) {
@@ -1617,7 +1617,7 @@ afr_sh_entry_impunge_readlink (call_frame_t *impunge_frame, xlator_t *this,
         impunge_local = impunge_frame->local;
         impunge_sh = &impunge_local->self_heal;
         active_src = impunge_sh->active_source;
-        impunge_local->cont.symlink.buf = *stbuf;
+        impunge_local->cont.dir_fop.buf = *stbuf;
 
         STACK_WIND_COOKIE (impunge_frame, afr_sh_entry_impunge_readlink_cbk,
                            (void *) (long) child_index,
diff --git a/xlators/cluster/afr/src/afr.h b/xlators/cluster/afr/src/afr.h
index 5ed478c4a..9ae154a3d 100644
--- a/xlators/cluster/afr/src/afr.h
+++ b/xlators/cluster/afr/src/afr.h
@@ -587,85 +587,40 @@ typedef struct _afr_local {
                 /* dir write */
 
                 struct {
-                        fd_t *fd;
-                        dict_t *params;
-                        int32_t flags;
-                        mode_t mode;
                         inode_t *inode;
                         struct iatt buf;
                         struct iatt preparent;
                         struct iatt postparent;
-                        struct iatt read_child_buf;
+                        struct iatt prenewparent;
+                        struct iatt postnewparent;
+                } dir_fop; //common structure for all dir fops
+
+                struct {
+                        fd_t *fd;
+                        dict_t *params;
+                        int32_t flags;
+                        mode_t mode;
                 } create;
 
                 struct {
                         dev_t dev;
                         mode_t mode;
                         dict_t *params;
-                        inode_t *inode;
-                        struct iatt buf;
-                        struct iatt preparent;
-                        struct iatt postparent;
-                        struct iatt read_child_buf;
                 } mknod;
 
                 struct {
                         int32_t mode;
                         dict_t *params;
-                        inode_t *inode;
-                        struct iatt buf;
-                        struct iatt read_child_buf;
-                        struct iatt preparent;
-                        struct iatt postparent;
                 } mkdir;
 
                 struct {
-                        int32_t op_ret;
-                        int32_t op_errno;
-                        struct iatt preparent;
-                        struct iatt postparent;
-                } unlink;
-
-                struct {
-                        int   flags;
-                        int32_t op_ret;
-                        int32_t op_errno;
-                        struct iatt preparent;
-                        struct iatt postparent;
+                        int flags;
                 } rmdir;
 
                 struct {
-                        struct iatt buf;
-                        struct iatt read_child_buf;
-                        struct iatt preoldparent;
-                        struct iatt prenewparent;
-                        struct iatt postoldparent;
-                        struct iatt postnewparent;
-                } rename;
-
-                struct {
-                        inode_t *inode;
-                        struct iatt buf;
-                        struct iatt read_child_buf;
-                        struct iatt preparent;
-                        struct iatt postparent;
-                } link;
-
-                struct {
-                        inode_t *inode;
                         dict_t *params;
-                        struct iatt buf;
-                        struct iatt read_child_buf;
                         char *linkpath;
-                        struct iatt preparent;
-                        struct iatt postparent;
                 } symlink;
-
-                struct {
-                        int32_t flags;
-                        dir_entry_t *entries;
-                        int32_t count;
-                } setdents;
         } cont;
 
         struct {