cluster/ec: Fix tracking of good bricks

The bitmask of good and bad bricks was kept in the context of the
corresponding inode or fd. This was problematic when an external
process (another client or the self-heal process) did heal the
bricks but no one changed the bitmaks of other clients.

This patch removes the bitmask stored in the context and calculates
which bricks are healthy after locking them and doing the initial
xattrop. After that, it's updated using the result of each fop.

Change-Id: I225e31cd219a12af4ca58871d8a4bb6f742b223c
BUG: 1236065
Signed-off-by: Xavier Hernandez <xhernandez@datalab.es>
Reviewed-on: http://review.gluster.org/11844
Tested-by: NetBSD Build System <jenkins@build.gluster.org>
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Pranith Kumar Karampuri <pkarampu@redhat.com>

12 files changed, 217 insertions, 258 deletions

diff --git a/tests/bugs/disperse/bug-1236065.t b/tests/bugs/disperse/bug-1236065.t
new file mode 100644
index 00000000000..53dbcc57318
--- /dev/null
+++ b/tests/bugs/disperse/bug-1236065.t
@@ -0,0 +1,95 @@
+#!/bin/bash
+
+. $(dirname $0)/../../include.rc
+. $(dirname $0)/../../volume.rc
+
+cleanup
+
+ec_test_dir=$M0/test
+
+function ec_test_generate_src()
+{
+    mkdir -p $ec_test_dir
+    for i in `seq 0 19`; do
+        dd if=/dev/zero of=$ec_test_dir/$i.c bs=1024 count=2
+    done
+}
+
+function ec_test_make()
+{
+    for i in `ls *.c`; do
+        file=`basename $i`
+        filename=${file%.*}
+        cp $i $filename.o
+    done
+}
+
+## step 1
+TEST glusterd
+TEST pidof glusterd
+TEST $CLI volume create $V0 disperse 7 redundancy 3 $H0:$B0/${V0}{0..6}
+TEST $CLI volume start $V0
+TEST glusterfs --entry-timeout=0 --attribute-timeout=0 -s $H0 --volfile-id $V0 $M0
+EXPECT_WITHIN $CHILD_UP_TIMEOUT "7" ec_child_up_count $V0 0
+
+## step 2
+TEST ec_test_generate_src
+
+cd $ec_test_dir
+TEST ec_test_make
+
+## step 3
+TEST kill_brick $V0 $H0 $B0/${V0}0
+TEST kill_brick $V0 $H0 $B0/${V0}1
+EXPECT '5' online_brick_count
+
+TEST rm -f *.o
+TEST ec_test_make
+
+## step 4
+TEST $CLI volume start $V0 force
+EXPECT '7' online_brick_count
+
+# active heal
+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "[0-9][0-9]*" get_shd_process_pid
+TEST $CLI volume heal $V0 full
+EXPECT_WITHIN $HEAL_TIMEOUT "0" get_pending_heal_count $V0
+
+TEST rm -f *.o
+TEST ec_test_make
+
+## step 5
+TEST kill_brick $V0 $H0 $B0/${V0}2
+TEST kill_brick $V0 $H0 $B0/${V0}3
+EXPECT '5' online_brick_count
+
+TEST rm -f *.o
+TEST ec_test_make
+
+EXPECT '5' online_brick_count
+
+## step 6
+TEST $CLI volume start $V0 force
+EXPECT '7' online_brick_count
+
+# self-healing
+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "[0-9][0-9]*" get_shd_process_pid
+TEST $CLI volume heal $V0 full
+EXPECT_WITHIN $HEAL_TIMEOUT "0" get_pending_heal_count $V0
+
+TEST rm -f *.o
+TEST ec_test_make
+
+TEST pidof glusterd
+EXPECT "$V0" volinfo_field $V0 'Volume Name'
+EXPECT 'Started' volinfo_field $V0 'Status'
+EXPECT '7' online_brick_count
+
+## cleanup
+cd
+EXPECT_WITHIN $UMOUNT_TIMEOUT "Y" force_umount $M0
+TEST $CLI volume stop $V0
+TEST $CLI volume delete $V0
+TEST rm -rf $B0/*
+
+cleanup;
diff --git a/xlators/cluster/ec/src/ec-common.c b/xlators/cluster/ec/src/ec-common.c
index d1a02ce91ce..b39fcb55d4e 100644
--- a/xlators/cluster/ec/src/ec-common.c
+++ b/xlators/cluster/ec/src/ec-common.c
@@ -42,77 +42,6 @@ int32_t ec_child_next(ec_t * ec, ec_fop_data_t * fop, int32_t idx)
     return idx;
 }
 
-uintptr_t ec_inode_good(inode_t * inode, xlator_t * xl)
-{
-    ec_inode_t * ctx;
-    uintptr_t bad = 0;
-
-    ctx = ec_inode_get(inode, xl);
-    if (ctx != NULL)
-    {
-        bad = ctx->bad;
-    }
-
-    return ~bad;
-}
-
-uintptr_t ec_fd_good(fd_t * fd, xlator_t * xl)
-{
-    ec_fd_t * ctx;
-    uintptr_t bad = 0;
-
-    ctx = ec_fd_get(fd, xl);
-    if (ctx != NULL)
-    {
-        bad = ctx->bad;
-    }
-
-    return ~bad;
-}
-
-uintptr_t ec_update_inode(ec_fop_data_t * fop, inode_t * inode, uintptr_t good,
-                          uintptr_t bad)
-{
-    ec_inode_t * ctx = NULL;
-
-    if (inode != NULL)
-    {
-        LOCK(&inode->lock);
-
-        ctx = __ec_inode_get(inode, fop->xl);
-        if (ctx != NULL)
-        {
-            ctx->bad &= ~good;
-            bad |= ctx->bad;
-            ctx->bad = bad;
-        }
-
-        UNLOCK(&inode->lock);
-    }
-
-    return bad;
-}
-
-uintptr_t ec_update_fd(ec_fop_data_t * fop, fd_t * fd, uintptr_t good,
-                       uintptr_t bad)
-{
-    ec_fd_t * ctx = NULL;
-
-    LOCK(&fd->lock);
-
-    ctx = __ec_fd_get(fd, fop->xl);
-    if (ctx != NULL)
-    {
-        ctx->bad &= ~good;
-        bad |= ctx->bad;
-        ctx->bad = bad;
-    }
-
-    UNLOCK(&fd->lock);
-
-    return bad;
-}
-
 int32_t ec_heal_report(call_frame_t * frame, void * cookie, xlator_t * this,
                        int32_t op_ret, int32_t op_errno, uintptr_t mask,
                        uintptr_t good, uintptr_t bad, dict_t * xdata)
@@ -145,6 +74,10 @@ void ec_check_status(ec_fop_data_t * fop)
     ec_t * ec = fop->xl->private;
     int32_t partial = 0;
 
+    if (!ec_fop_needs_heal(fop)) {
+        return;
+    }
+
     if (fop->answer->op_ret >= 0) {
         if ((fop->id == GF_FOP_LOOKUP) ||
             (fop->id == GF_FOP_STAT) || (fop->id == GF_FOP_FSTAT)) {
@@ -154,16 +87,13 @@ void ec_check_status(ec_fop_data_t * fop)
         }
     }
 
-    if (!ec_fop_needs_heal(fop)) {
-        return;
-    }
-
     gf_msg (fop->xl->name, GF_LOG_WARNING, 0,
             EC_MSG_OP_FAIL_ON_SUBVOLS,
             "Operation failed on some "
             "subvolumes (up=%lX, mask=%lX, "
             "remaining=%lX, good=%lX, bad=%lX)",
-            ec->xl_up, fop->mask, fop->remaining, fop->good, fop->bad);
+            ec->xl_up, fop->mask, fop->remaining, fop->good,
+            ec->xl_up & ~(fop->remaining | fop->good));
 
     if (fop->use_fd)
     {
@@ -185,43 +115,31 @@ void ec_check_status(ec_fop_data_t * fop)
     }
 }
 
-void ec_update_bad(ec_fop_data_t * fop, uintptr_t good)
+void ec_update_good(ec_fop_data_t *fop, uintptr_t good)
 {
-    ec_t *ec = fop->xl->private;
-    uintptr_t bad;
-
-    /*Don't let fops that do dispatch_one() to update bad*/
-    if (fop->expected == 1)
-            return;
-
-    bad = ec->xl_up & ~(fop->remaining | good);
-    fop->bad |= bad;
-    fop->good |= good;
-
-    if (fop->parent == NULL)
-    {
-        if ((fop->flags & EC_FLAG_UPDATE_LOC_PARENT) != 0)
-        {
-            ec_update_inode(fop, fop->loc[0].parent, good, bad);
-        }
-        if ((fop->flags & EC_FLAG_UPDATE_LOC_INODE) != 0)
-        {
-            ec_update_inode(fop, fop->loc[0].inode, good, bad);
-        }
-        ec_update_inode(fop, fop->loc[1].inode, good, bad);
-        if ((fop->flags & EC_FLAG_UPDATE_FD_INODE) != 0)
-        {
-            ec_update_inode(fop, fop->fd->inode, good, bad);
-        }
-        if ((fop->flags & EC_FLAG_UPDATE_FD) != 0)
-        {
-            ec_update_fd(fop, fop->fd, good, bad);
-        }
+    fop->good = good;
 
+    /* Fops that are executed only on one brick do not have enough information
+     * to decide if healing is needed or not. */
+    if ((fop->expected != 1) && (fop->parent == NULL)) {
         ec_check_status(fop);
     }
 }
 
+void ec_lock_update_good(ec_lock_t *lock, ec_fop_data_t *fop)
+{
+    /* Fops that are executed only on one brick do not have enough information
+     * to update the global mask of good bricks. */
+    if (fop->expected == 1) {
+        return;
+    }
+
+    /* When updating the good mask of the lock, we only take into
+     * consideration those bits corresponding to the bricks where
+     * the fop has been executed. */
+    lock->good_mask &= ~fop->mask | fop->remaining;
+    lock->good_mask |= fop->good;
+}
 
 void __ec_fop_set_error(ec_fop_data_t * fop, int32_t error)
 {
@@ -410,12 +328,12 @@ void ec_complete(ec_fop_data_t * fop)
 
     UNLOCK(&fop->lock);
 
-    /* ec_update_bad() locks inode->lock. This may cause deadlocks with
-       fop->lock when used in another order. Since ec_update_bad() will not
+    /* ec_update_good() locks inode->lock. This may cause deadlocks with
+       fop->lock when used in another order. Since ec_update_good() will not
        be called more than once for each fop, it can be called from outside
        the fop->lock locked region. */
     if (update) {
-        ec_update_bad(fop, cbk->mask);
+        ec_update_good(fop, cbk->mask);
     }
 
     if (resume)
@@ -459,7 +377,6 @@ ec_internal_op (ec_fop_data_t *fop)
 int32_t ec_child_select(ec_fop_data_t * fop)
 {
     ec_t * ec = fop->xl->private;
-    uintptr_t mask = 0;
     int32_t first = 0, num = 0;
 
     ec_fop_cleanup(fop);
@@ -472,39 +389,15 @@ int32_t ec_child_select(ec_fop_data_t * fop)
             fop->mask &= (fop->parent->mask & ~fop->parent->healing);
     }
 
-    mask = ec->xl_up;
-    if (fop->parent == NULL)
-    {
-        if ((fop->flags & EC_FLAG_UPDATE_LOC_PARENT) && fop->loc[0].parent)
-            mask &= ec_inode_good(fop->loc[0].parent, fop->xl);
-
-        if ((fop->flags & EC_FLAG_UPDATE_LOC_INODE) && fop->loc[0].inode) {
-            mask &= ec_inode_good(fop->loc[0].inode, fop->xl);
-        }
-
-        if ((fop->flags & EC_FLAG_UPDATE_LOC_INODE) && fop->loc[1].inode) {
-            mask &= ec_inode_good(fop->loc[1].inode, fop->xl);
-        }
-
-        if (fop->fd) {
-            if ((fop->flags & EC_FLAG_UPDATE_FD_INODE) && fop->fd->inode) {
-                mask &= ec_inode_good(fop->fd->inode, fop->xl);
-            }
-            if (fop->flags & fop->flags & EC_FLAG_UPDATE_FD) {
-                    mask &= ec_fd_good(fop->fd, fop->xl);
-            }
-        }
-    }
-
-    if ((fop->mask & ~mask) != 0)
+    if ((fop->mask & ~ec->xl_up) != 0)
     {
         gf_msg (fop->xl->name, GF_LOG_WARNING, 0,
                 EC_MSG_OP_EXEC_UNAVAIL,
                 "Executing operation with "
                 "some subvolumes unavailable "
-                "(%lX)", fop->mask & ~mask);
+                "(%lX)", fop->mask & ~ec->xl_up);
 
-        fop->mask &= mask;
+        fop->mask &= ec->xl_up;
     }
 
     switch (fop->minimum)
@@ -614,7 +507,6 @@ void ec_dispatch_start(ec_fop_data_t * fop)
 {
     fop->answer = NULL;
     fop->good = 0;
-    fop->bad = 0;
 
     INIT_LIST_HEAD(&fop->cbk_list);
 
@@ -1053,6 +945,8 @@ unlock:
     UNLOCK(&lock->loc.inode->lock);
 out:
     if (op_errno == 0) {
+        /* We don't allow the main fop to be executed on bricks that have not
+         * succeeded the initial xattrop. */
         parent->mask &= fop->good;
 
         /*As of now only data healing marks bricks as healing*/
@@ -1135,7 +1029,7 @@ void ec_get_size_version(ec_lock_link_t *link)
     /* For normal fops, ec_[f]xattrop() must succeed on at least
      * EC_MINIMUM_MIN bricks, however when this is called as part of a
      * self-heal operation the mask of target bricks (fop->mask) could
-     * contain less than EC_MINIMUM_MIN bricks, causing the lookup to
+     * contain less than EC_MINIMUM_MIN bricks, causing the xattrop to
      * always fail. Thus we always use the same minimum used for the main
      * fop.
      */
@@ -1607,11 +1501,13 @@ int32_t ec_update_size_version_done(call_frame_t * frame, void * cookie,
                EC_MSG_SIZE_VERS_UPDATE_FAIL,
                "Failed to update version and size");
     } else {
-        fop->parent->mask &= fop->good;
+        fop->parent->good &= fop->good;
         link = fop->data;
         lock = link->lock;
         ctx = lock->ctx;
 
+        ec_lock_update_good(lock, fop);
+
         if (ec_dict_del_array(xattr, EC_XATTR_VERSION, ctx->post_version,
                               EC_VERSION_SIZE) == 0) {
             ctx->pre_version[0] = ctx->post_version[0];
@@ -1710,11 +1606,11 @@ ec_update_size_version(ec_lock_link_t *link, uint64_t *version,
     fop->frame->root->gid = 0;
 
     if (link->lock->fd == NULL) {
-            ec_xattrop(fop->frame, fop->xl, fop->mask, EC_MINIMUM_MIN,
+            ec_xattrop(fop->frame, fop->xl, fop->good, EC_MINIMUM_MIN,
                        ec_update_size_version_done, link, &link->lock->loc,
                        GF_XATTROP_ADD_ARRAY64, dict, NULL);
     } else {
-            ec_fxattrop(fop->frame, fop->xl, fop->mask, EC_MINIMUM_MIN,
+            ec_fxattrop(fop->frame, fop->xl, fop->good, EC_MINIMUM_MIN,
                        ec_update_size_version_done, link, link->lock->fd,
                        GF_XATTROP_ADD_ARRAY64, dict, NULL);
     }
@@ -1906,6 +1802,13 @@ void ec_flush_size_version(ec_fop_data_t * fop)
 {
     GF_ASSERT(fop->lock_count == 1);
 
+    /* In normal circumstances, ec_update_info() is called after having
+     * executed a normal fop, and it uses fop->good to update only those bricks
+     * that succeeded. In this case we haven't executed any fop, so fop->good
+     * is 0. We use the current good mask of the lock itself to send the
+     * updates.*/
+    fop->good = fop->locks[0].lock->good_mask;
+
     ec_update_info(&fop->locks[0]);
 }
 
@@ -1956,19 +1859,19 @@ void ec_lock_reuse(ec_fop_data_t *fop)
         if ((fop->error == 0) && (cbk != NULL) && (cbk->op_ret >= 0)) {
             if (link->update[0]) {
                 ctx->post_version[0]++;
-                if (ec->node_mask & ~fop->mask) {
+                if (ec->node_mask & ~fop->good) {
                     ctx->dirty[0]++;
                 }
             }
             if (link->update[1]) {
                 ctx->post_version[1]++;
-                if (ec->node_mask & ~fop->mask) {
+                if (ec->node_mask & ~fop->good) {
                     ctx->dirty[1]++;
                 }
             }
         }
 
-        lock->good_mask &= fop->mask;
+        ec_lock_update_good(lock, fop);
 
         link = NULL;
         if (!list_empty(&lock->waiting))
diff --git a/xlators/cluster/ec/src/ec-common.h b/xlators/cluster/ec/src/ec-common.h
index 39787126700..036da091f43 100644
--- a/xlators/cluster/ec/src/ec-common.h
+++ b/xlators/cluster/ec/src/ec-common.h
@@ -27,11 +27,6 @@ typedef enum {
 
 #define EC_CONFIG_ALGORITHM 0
 
-#define EC_FLAG_UPDATE_LOC_PARENT 0x0001
-#define EC_FLAG_UPDATE_LOC_INODE  0x0002
-#define EC_FLAG_UPDATE_FD         0x0004
-#define EC_FLAG_UPDATE_FD_INODE   0x0008
-
 #define EC_SELFHEAL_BIT 62
 
 #define EC_MINIMUM_ONE   -1
@@ -78,11 +73,11 @@ typedef enum {
 gf_boolean_t ec_dispatch_one_retry (ec_fop_data_t *fop, ec_cbk_data_t **cbk);
 int32_t ec_dispatch_next(ec_fop_data_t * fop, int32_t idx);
 
-void ec_complete(ec_fop_data_t * fop);
+void ec_complete(ec_fop_data_t *fop);
 
-void ec_update_bad(ec_fop_data_t * fop, uintptr_t good);
+void ec_update_good(ec_fop_data_t *fop, uintptr_t good);
 
-void ec_fop_set_error(ec_fop_data_t * fop, int32_t error);
+void ec_fop_set_error(ec_fop_data_t *fop, int32_t error);
 
 ec_cbk_data_t *
 ec_fop_prepare_answer(ec_fop_data_t *fop, gf_boolean_t ro);
diff --git a/xlators/cluster/ec/src/ec-data.h b/xlators/cluster/ec/src/ec-data.h
index 18da06cea04..285f71e702d 100644
--- a/xlators/cluster/ec/src/ec-data.h
+++ b/xlators/cluster/ec/src/ec-data.h
@@ -58,7 +58,6 @@ struct _ec_config
 
 struct _ec_fd
 {
-    uintptr_t bad;
     loc_t     loc;
     uintptr_t open;
     int32_t   flags;
@@ -66,7 +65,6 @@ struct _ec_fd
 
 struct _ec_inode
 {
-    uintptr_t         bad;
     ec_lock_t        *inode_lock;
     gf_boolean_t      have_info;
     gf_boolean_t      have_config;
@@ -206,7 +204,6 @@ struct _ec_fop_data
     uintptr_t          remaining;
     uintptr_t          received; /* Mask of responses */
     uintptr_t          good;
-    uintptr_t          bad;
 
     ec_wind_f          wind;
     ec_handler_f       handler;
diff --git a/xlators/cluster/ec/src/ec-dir-read.c b/xlators/cluster/ec/src/ec-dir-read.c
index 24de9e70e73..03bb60cc7b7 100644
--- a/xlators/cluster/ec/src/ec-dir-read.c
+++ b/xlators/cluster/ec/src/ec-dir-read.c
@@ -212,9 +212,9 @@ void ec_opendir(call_frame_t * frame, xlator_t * this, uintptr_t target,
     GF_VALIDATE_OR_GOTO(this->name, frame, out);
     GF_VALIDATE_OR_GOTO(this->name, this->private, out);
 
-    fop = ec_fop_data_allocate(frame, this, GF_FOP_OPENDIR, EC_FLAG_UPDATE_FD,
-                               target, minimum, ec_wind_opendir,
-                               ec_manager_opendir, callback, data);
+    fop = ec_fop_data_allocate(frame, this, GF_FOP_OPENDIR, 0, target, minimum,
+                               ec_wind_opendir, ec_manager_opendir, callback,
+                               data);
     if (fop == NULL) {
         goto out;
     }
diff --git a/xlators/cluster/ec/src/ec-dir-write.c b/xlators/cluster/ec/src/ec-dir-write.c
index 94a1ea2a587..3ae17e565ed 100644
--- a/xlators/cluster/ec/src/ec-dir-write.c
+++ b/xlators/cluster/ec/src/ec-dir-write.c
@@ -277,9 +277,7 @@ void ec_create(call_frame_t * frame, xlator_t * this, uintptr_t target,
     GF_VALIDATE_OR_GOTO(this->name, frame, out);
     GF_VALIDATE_OR_GOTO(this->name, this->private, out);
 
-    fop = ec_fop_data_allocate(frame, this, GF_FOP_CREATE,
-                               EC_FLAG_UPDATE_LOC_PARENT |
-                               EC_FLAG_UPDATE_FD_INODE, target, minimum,
+    fop = ec_fop_data_allocate(frame, this, GF_FOP_CREATE, 0, target, minimum,
                                ec_wind_create, ec_manager_create, callback,
                                data);
     if (fop == NULL) {
@@ -631,8 +629,7 @@ void ec_mkdir(call_frame_t * frame, xlator_t * this, uintptr_t target,
     GF_VALIDATE_OR_GOTO(this->name, frame, out);
     GF_VALIDATE_OR_GOTO(this->name, this->private, out);
 
-    fop = ec_fop_data_allocate(frame, this, GF_FOP_MKDIR,
-                               EC_FLAG_UPDATE_LOC_PARENT, target, minimum,
+    fop = ec_fop_data_allocate(frame, this, GF_FOP_MKDIR, 0, target, minimum,
                                ec_wind_mkdir, ec_manager_mkdir, callback,
                                data);
     if (fop == NULL) {
@@ -836,8 +833,7 @@ void ec_mknod(call_frame_t * frame, xlator_t * this, uintptr_t target,
     GF_VALIDATE_OR_GOTO(this->name, frame, out);
     GF_VALIDATE_OR_GOTO(this->name, this->private, out);
 
-    fop = ec_fop_data_allocate(frame, this, GF_FOP_MKNOD,
-                               EC_FLAG_UPDATE_LOC_PARENT, target, minimum,
+    fop = ec_fop_data_allocate(frame, this, GF_FOP_MKNOD, 0, target, minimum,
                                ec_wind_mknod, ec_manager_mknod, callback,
                                data);
     if (fop == NULL) {
@@ -998,8 +994,7 @@ void ec_rename(call_frame_t * frame, xlator_t * this, uintptr_t target,
     GF_VALIDATE_OR_GOTO(this->name, frame, out);
     GF_VALIDATE_OR_GOTO(this->name, this->private, out);
 
-    fop = ec_fop_data_allocate(frame, this, GF_FOP_RENAME,
-                               EC_FLAG_UPDATE_LOC_PARENT, target, minimum,
+    fop = ec_fop_data_allocate(frame, this, GF_FOP_RENAME, 0, target, minimum,
                                ec_wind_rename, ec_manager_rename, callback,
                                data);
     if (fop == NULL) {
@@ -1150,8 +1145,7 @@ void ec_rmdir(call_frame_t * frame, xlator_t * this, uintptr_t target,
     GF_VALIDATE_OR_GOTO(this->name, frame, out);
     GF_VALIDATE_OR_GOTO(this->name, this->private, out);
 
-    fop = ec_fop_data_allocate(frame, this, GF_FOP_RMDIR,
-                               EC_FLAG_UPDATE_LOC_PARENT, target, minimum,
+    fop = ec_fop_data_allocate(frame, this, GF_FOP_RMDIR, 0, target, minimum,
                                ec_wind_rmdir, ec_manager_rmdir, callback,
                                data);
     if (fop == NULL) {
@@ -1308,8 +1302,7 @@ void ec_symlink(call_frame_t * frame, xlator_t * this, uintptr_t target,
     GF_VALIDATE_OR_GOTO(this->name, frame, out);
     GF_VALIDATE_OR_GOTO(this->name, this->private, out);
 
-    fop = ec_fop_data_allocate(frame, this, GF_FOP_SYMLINK,
-                               EC_FLAG_UPDATE_LOC_PARENT, target, minimum,
+    fop = ec_fop_data_allocate(frame, this, GF_FOP_SYMLINK, 0, target, minimum,
                                ec_wind_symlink, ec_manager_symlink, callback,
                                data);
     if (fop == NULL) {
@@ -1464,8 +1457,7 @@ void ec_unlink(call_frame_t * frame, xlator_t * this, uintptr_t target,
     GF_VALIDATE_OR_GOTO(this->name, frame, out);
     GF_VALIDATE_OR_GOTO(this->name, this->private, out);
 
-    fop = ec_fop_data_allocate(frame, this, GF_FOP_UNLINK,
-                               EC_FLAG_UPDATE_LOC_PARENT, target, minimum,
+    fop = ec_fop_data_allocate(frame, this, GF_FOP_UNLINK, 0, target, minimum,
                                ec_wind_unlink, ec_manager_unlink, callback,
                                data);
     if (fop == NULL) {
diff --git a/xlators/cluster/ec/src/ec-generic.c b/xlators/cluster/ec/src/ec-generic.c
index c5ea11b1210..47118faa917 100644
--- a/xlators/cluster/ec/src/ec-generic.c
+++ b/xlators/cluster/ec/src/ec-generic.c
@@ -168,9 +168,9 @@ void ec_flush(call_frame_t * frame, xlator_t * this, uintptr_t target,
     GF_VALIDATE_OR_GOTO(this->name, frame, out);
     GF_VALIDATE_OR_GOTO(this->name, this->private, out);
 
-    fop = ec_fop_data_allocate(frame, this, GF_FOP_FLUSH, EC_FLAG_UPDATE_FD,
-                               target, minimum, ec_wind_flush,
-                               ec_manager_flush, callback, data);
+    fop = ec_fop_data_allocate(frame, this, GF_FOP_FLUSH, 0, target, minimum,
+                               ec_wind_flush, ec_manager_flush, callback,
+                               data);
     if (fop == NULL) {
         goto out;
     }
@@ -393,9 +393,9 @@ void ec_fsync(call_frame_t * frame, xlator_t * this, uintptr_t target,
     GF_VALIDATE_OR_GOTO(this->name, frame, out);
     GF_VALIDATE_OR_GOTO(this->name, this->private, out);
 
-    fop = ec_fop_data_allocate(frame, this, GF_FOP_FSYNC, EC_FLAG_UPDATE_FD,
-                               target, minimum, ec_wind_fsync,
-                               ec_manager_fsync, callback, data);
+    fop = ec_fop_data_allocate(frame, this, GF_FOP_FSYNC, 0, target, minimum,
+                               ec_wind_fsync, ec_manager_fsync, callback,
+                               data);
     if (fop == NULL) {
         goto out;
     }
@@ -584,9 +584,9 @@ void ec_fsyncdir(call_frame_t * frame, xlator_t * this, uintptr_t target,
     GF_VALIDATE_OR_GOTO(this->name, frame, out);
     GF_VALIDATE_OR_GOTO(this->name, this->private, out);
 
-    fop = ec_fop_data_allocate(frame, this, GF_FOP_FSYNCDIR, EC_FLAG_UPDATE_FD,
-                               target, minimum, ec_wind_fsyncdir,
-                               ec_manager_fsyncdir, callback, data);
+    fop = ec_fop_data_allocate(frame, this, GF_FOP_FSYNCDIR, 0, target,
+                               minimum, ec_wind_fsyncdir, ec_manager_fsyncdir,
+                               callback, data);
     if (fop == NULL) {
         goto out;
     }
@@ -1326,8 +1326,7 @@ void ec_xattrop(call_frame_t * frame, xlator_t * this, uintptr_t target,
     GF_VALIDATE_OR_GOTO(this->name, frame, out);
     GF_VALIDATE_OR_GOTO(this->name, this->private, out);
 
-    fop = ec_fop_data_allocate(frame, this, GF_FOP_XATTROP,
-                               EC_FLAG_UPDATE_LOC_INODE, target, minimum,
+    fop = ec_fop_data_allocate(frame, this, GF_FOP_XATTROP, 0, target, minimum,
                                ec_wind_xattrop, ec_manager_xattrop, callback,
                                data);
     if (fop == NULL) {
@@ -1399,10 +1398,9 @@ void ec_fxattrop(call_frame_t * frame, xlator_t * this, uintptr_t target,
     GF_VALIDATE_OR_GOTO(this->name, frame, out);
     GF_VALIDATE_OR_GOTO(this->name, this->private, out);
 
-    fop = ec_fop_data_allocate(frame, this, GF_FOP_FXATTROP,
-                               EC_FLAG_UPDATE_FD_INODE, target, minimum,
-                               ec_wind_fxattrop, ec_manager_xattrop, callback,
-                               data);
+    fop = ec_fop_data_allocate(frame, this, GF_FOP_FXATTROP, 0, target,
+                               minimum, ec_wind_fxattrop, ec_manager_xattrop,
+                               callback, data);
     if (fop == NULL) {
         goto out;
     }
diff --git a/xlators/cluster/ec/src/ec-heal.c b/xlators/cluster/ec/src/ec-heal.c
index f025cab9dfb..d095aceec00 100644
--- a/xlators/cluster/ec/src/ec-heal.c
+++ b/xlators/cluster/ec/src/ec-heal.c
@@ -292,8 +292,8 @@ void ec_fheal(call_frame_t * frame, xlator_t * this, uintptr_t target,
 
     if (ctx != NULL)
     {
-        gf_msg_trace ("ec", 0, "FHEAL ctx: flags=%X, open=%lX, bad=%lX",
-               ctx->flags, ctx->open, ctx->bad);
+        gf_msg_trace ("ec", 0, "FHEAL ctx: flags=%X, open=%lX", ctx->flags,
+                      ctx->open);
         ec_heal(frame, this, target, minimum, func, data, &ctx->loc, partial,
                 xdata);
     }
@@ -1751,8 +1751,7 @@ ec_heal_block (call_frame_t *frame, xlator_t *this, uintptr_t target,
     VALIDATE_OR_GOTO(this, out);
     GF_VALIDATE_OR_GOTO(this->name, this->private, out);
 
-    fop = ec_fop_data_allocate (frame, this, EC_FOP_HEAL,
-                                EC_FLAG_UPDATE_LOC_INODE, target, minimum,
+    fop = ec_fop_data_allocate (frame, this, EC_FOP_HEAL, 0, target, minimum,
                                 NULL, ec_manager_heal_block, callback,
                                 heal);
     if (fop == NULL)
@@ -2444,8 +2443,7 @@ ec_heal (call_frame_t *frame, xlator_t *this, uintptr_t target,
 
     if (frame && frame->local)
             goto fail;
-    fop = ec_fop_data_allocate (frame, this, EC_FOP_HEAL,
-                                EC_FLAG_UPDATE_LOC_INODE, target, minimum,
+    fop = ec_fop_data_allocate (frame, this, EC_FOP_HEAL, 0, target, minimum,
                                 NULL, NULL, callback, data);
 
     err = ENOMEM;
diff --git a/xlators/cluster/ec/src/ec-helpers.c b/xlators/cluster/ec/src/ec-helpers.c
index b959c0bc2cc..01e5e56a9b6 100644
--- a/xlators/cluster/ec/src/ec-helpers.c
+++ b/xlators/cluster/ec/src/ec-helpers.c
@@ -88,7 +88,7 @@ void ec_trace(const char * event, ec_fop_data_t * fop, const char * fmt, ...)
            fop->expected, fop->error, fop->state,
            ec_bin(str1, sizeof(str1), fop->mask, ec->nodes),
            ec_bin(str2, sizeof(str2), fop->remaining, ec->nodes),
-           ec_bin(str3, sizeof(str3), fop->bad, ec->nodes), msg);
+           ec_bin(str3, sizeof(str3), fop->good, ec->nodes), msg);
 
     if (ret >= 0)
     {
diff --git a/xlators/cluster/ec/src/ec-inode-read.c b/xlators/cluster/ec/src/ec-inode-read.c
index d9a33c878b2..8b76cc58abb 100644
--- a/xlators/cluster/ec/src/ec-inode-read.c
+++ b/xlators/cluster/ec/src/ec-inode-read.c
@@ -468,10 +468,9 @@ ec_getxattr (call_frame_t *frame, xlator_t *this, uintptr_t target,
         return;
     }
 
-    fop = ec_fop_data_allocate(frame, this, GF_FOP_GETXATTR,
-                               EC_FLAG_UPDATE_LOC_INODE, target, minimum,
-                               ec_wind_getxattr, ec_manager_getxattr, callback,
-                               data);
+    fop = ec_fop_data_allocate(frame, this, GF_FOP_GETXATTR, 0, target,
+                               minimum, ec_wind_getxattr, ec_manager_getxattr,
+                               callback, data);
     if (fop == NULL) {
         goto out;
     }
@@ -608,9 +607,8 @@ ec_fgetxattr (call_frame_t *frame, xlator_t *this, uintptr_t target,
     GF_VALIDATE_OR_GOTO(this->name, frame, out);
     GF_VALIDATE_OR_GOTO(this->name, this->private, out);
 
-    fop = ec_fop_data_allocate(frame, this, GF_FOP_FGETXATTR,
-                               EC_FLAG_UPDATE_FD_INODE, target, minimum,
-                               ec_wind_fgetxattr, ec_manager_getxattr,
+    fop = ec_fop_data_allocate(frame, this, GF_FOP_FGETXATTR, 0, target,
+                               minimum, ec_wind_fgetxattr, ec_manager_getxattr,
                                callback, data);
     if (fop == NULL) {
         goto out;
@@ -898,9 +896,8 @@ void ec_open(call_frame_t * frame, xlator_t * this, uintptr_t target,
     GF_VALIDATE_OR_GOTO(this->name, frame, out);
     GF_VALIDATE_OR_GOTO(this->name, this->private, out);
 
-    fop = ec_fop_data_allocate(frame, this, GF_FOP_OPEN, EC_FLAG_UPDATE_FD,
-                               target, minimum, ec_wind_open, ec_manager_open,
-                               callback, data);
+    fop = ec_fop_data_allocate(frame, this, GF_FOP_OPEN, 0, target, minimum,
+                               ec_wind_open, ec_manager_open, callback, data);
     if (fop == NULL) {
         goto out;
     }
@@ -1453,9 +1450,9 @@ void ec_readv(call_frame_t * frame, xlator_t * this, uintptr_t target,
     GF_VALIDATE_OR_GOTO(this->name, frame, out);
     GF_VALIDATE_OR_GOTO(this->name, this->private, out);
 
-    fop = ec_fop_data_allocate(frame, this, GF_FOP_READ, EC_FLAG_UPDATE_FD,
-                               target, minimum, ec_wind_readv,
-                               ec_manager_readv, callback, data);
+    fop = ec_fop_data_allocate(frame, this, GF_FOP_READ, 0, target, minimum,
+                               ec_wind_readv, ec_manager_readv, callback,
+                               data);
     if (fop == NULL) {
         goto out;
     }
@@ -1699,8 +1696,7 @@ void ec_stat(call_frame_t * frame, xlator_t * this, uintptr_t target,
     GF_VALIDATE_OR_GOTO(this->name, frame, out);
     GF_VALIDATE_OR_GOTO(this->name, this->private, out);
 
-    fop = ec_fop_data_allocate(frame, this, GF_FOP_STAT,
-                               EC_FLAG_UPDATE_LOC_INODE, target, minimum,
+    fop = ec_fop_data_allocate(frame, this, GF_FOP_STAT, 0, target, minimum,
                                ec_wind_stat, ec_manager_stat, callback, data);
     if (fop == NULL) {
         goto out;
@@ -1814,8 +1810,7 @@ void ec_fstat(call_frame_t * frame, xlator_t * this, uintptr_t target,
     GF_VALIDATE_OR_GOTO(this->name, frame, out);
     GF_VALIDATE_OR_GOTO(this->name, this->private, out);
 
-    fop = ec_fop_data_allocate(frame, this, GF_FOP_FSTAT,
-                               EC_FLAG_UPDATE_FD_INODE, target, minimum,
+    fop = ec_fop_data_allocate(frame, this, GF_FOP_FSTAT, 0, target, minimum,
                                ec_wind_fstat, ec_manager_stat, callback, data);
     if (fop == NULL) {
         goto out;
diff --git a/xlators/cluster/ec/src/ec-inode-write.c b/xlators/cluster/ec/src/ec-inode-write.c
index 31ab53b86c7..e43313012b3 100644
--- a/xlators/cluster/ec/src/ec-inode-write.c
+++ b/xlators/cluster/ec/src/ec-inode-write.c
@@ -204,9 +204,8 @@ ec_removexattr (call_frame_t *frame, xlator_t *this, uintptr_t target,
     GF_VALIDATE_OR_GOTO (this->name, frame, out);
     GF_VALIDATE_OR_GOTO (this->name, this->private, out);
 
-    fop = ec_fop_data_allocate(frame, this, GF_FOP_REMOVEXATTR,
-                               EC_FLAG_UPDATE_LOC_INODE, target, minimum,
-                               ec_wind_removexattr, ec_manager_xattr,
+    fop = ec_fop_data_allocate(frame, this, GF_FOP_REMOVEXATTR, 0, target,
+                               minimum, ec_wind_removexattr, ec_manager_xattr,
                                callback, data);
     if (fop == NULL) {
         goto out;
@@ -287,9 +286,8 @@ ec_fremovexattr (call_frame_t *frame, xlator_t *this, uintptr_t target,
     GF_VALIDATE_OR_GOTO (this->name, frame, out);
     GF_VALIDATE_OR_GOTO (this->name, this->private, out);
 
-    fop = ec_fop_data_allocate(frame, this, GF_FOP_FREMOVEXATTR,
-                               EC_FLAG_UPDATE_FD_INODE, target, minimum,
-                               ec_wind_fremovexattr, ec_manager_xattr,
+    fop = ec_fop_data_allocate(frame, this, GF_FOP_FREMOVEXATTR, 0, target,
+                               minimum, ec_wind_fremovexattr, ec_manager_xattr,
                                callback, data);
     if (fop == NULL) {
         goto out;
@@ -492,8 +490,7 @@ void ec_setattr(call_frame_t * frame, xlator_t * this, uintptr_t target,
     GF_VALIDATE_OR_GOTO(this->name, frame, out);
     GF_VALIDATE_OR_GOTO(this->name, this->private, out);
 
-    fop = ec_fop_data_allocate(frame, this, GF_FOP_SETATTR,
-                               EC_FLAG_UPDATE_LOC_INODE, target, minimum,
+    fop = ec_fop_data_allocate(frame, this, GF_FOP_SETATTR, 0, target, minimum,
                                ec_wind_setattr, ec_manager_setattr, callback,
                                data);
     if (fop == NULL) {
@@ -570,10 +567,9 @@ void ec_fsetattr(call_frame_t * frame, xlator_t * this, uintptr_t target,
     GF_VALIDATE_OR_GOTO(this->name, frame, out);
     GF_VALIDATE_OR_GOTO(this->name, this->private, out);
 
-    fop = ec_fop_data_allocate(frame, this, GF_FOP_FSETATTR,
-                               EC_FLAG_UPDATE_FD_INODE, target, minimum,
-                               ec_wind_fsetattr, ec_manager_setattr, callback,
-                               data);
+    fop = ec_fop_data_allocate(frame, this, GF_FOP_FSETATTR, 0, target,
+                               minimum, ec_wind_fsetattr, ec_manager_setattr,
+                               callback, data);
     if (fop == NULL) {
         goto out;
     }
@@ -651,10 +647,9 @@ ec_setxattr (call_frame_t *frame, xlator_t *this, uintptr_t target,
     GF_VALIDATE_OR_GOTO(this->name, frame, out);
     GF_VALIDATE_OR_GOTO(this->name, this->private, out);
 
-    fop = ec_fop_data_allocate(frame, this, GF_FOP_SETXATTR,
-                               EC_FLAG_UPDATE_LOC_INODE, target, minimum,
-                               ec_wind_setxattr, ec_manager_xattr, callback,
-                               data);
+    fop = ec_fop_data_allocate(frame, this, GF_FOP_SETXATTR, 0, target,
+                               minimum, ec_wind_setxattr, ec_manager_xattr,
+                               callback, data);
     if (fop == NULL) {
         goto out;
     }
@@ -777,9 +772,8 @@ ec_fsetxattr (call_frame_t *frame, xlator_t *this, uintptr_t target,
     GF_VALIDATE_OR_GOTO(this->name, frame, out);
     GF_VALIDATE_OR_GOTO(this->name, this->private, out);
 
-    fop = ec_fop_data_allocate(frame, this, GF_FOP_FSETXATTR,
-                               EC_FLAG_UPDATE_FD_INODE, target, minimum,
-                               ec_wind_fsetxattr, ec_manager_xattr,
+    fop = ec_fop_data_allocate(frame, this, GF_FOP_FSETXATTR, 0, target,
+                               minimum, ec_wind_fsetxattr, ec_manager_xattr,
                                callback, data);
     if (fop == NULL) {
         goto out;
@@ -1078,10 +1072,9 @@ void ec_truncate(call_frame_t * frame, xlator_t * this, uintptr_t target,
     GF_VALIDATE_OR_GOTO(this->name, frame, out);
     GF_VALIDATE_OR_GOTO(this->name, this->private, out);
 
-    fop = ec_fop_data_allocate(frame, this, GF_FOP_TRUNCATE,
-                               EC_FLAG_UPDATE_LOC_INODE, target, minimum,
-                               ec_wind_truncate, ec_manager_truncate, callback,
-                               data);
+    fop = ec_fop_data_allocate(frame, this, GF_FOP_TRUNCATE, 0, target,
+                               minimum, ec_wind_truncate, ec_manager_truncate,
+                               callback, data);
     if (fop == NULL) {
         goto out;
     }
@@ -1153,9 +1146,8 @@ void ec_ftruncate(call_frame_t * frame, xlator_t * this, uintptr_t target,
     GF_VALIDATE_OR_GOTO(this->name, frame, out);
     GF_VALIDATE_OR_GOTO(this->name, this->private, out);
 
-    fop = ec_fop_data_allocate(frame, this, GF_FOP_FTRUNCATE,
-                               EC_FLAG_UPDATE_FD_INODE, target, minimum,
-                               ec_wind_ftruncate, ec_manager_truncate,
+    fop = ec_fop_data_allocate(frame, this, GF_FOP_FTRUNCATE, 0, target,
+                               minimum, ec_wind_ftruncate, ec_manager_truncate,
                                callback, data);
     if (fop == NULL) {
         goto out;
@@ -1580,8 +1572,7 @@ void ec_writev(call_frame_t * frame, xlator_t * this, uintptr_t target,
     GF_VALIDATE_OR_GOTO(this->name, frame, out);
     GF_VALIDATE_OR_GOTO(this->name, this->private, out);
 
-    fop = ec_fop_data_allocate(frame, this, GF_FOP_WRITE,
-                               EC_FLAG_UPDATE_FD_INODE, target, minimum,
+    fop = ec_fop_data_allocate(frame, this, GF_FOP_WRITE, 0, target, minimum,
                                ec_wind_writev, ec_manager_writev, callback,
                                data);
     if (fop == NULL) {
diff --git a/xlators/cluster/ec/src/ec-locks.c b/xlators/cluster/ec/src/ec-locks.c
index 00bfc0d2d99..7902a99ba84 100644
--- a/xlators/cluster/ec/src/ec-locks.c
+++ b/xlators/cluster/ec/src/ec-locks.c
@@ -58,7 +58,7 @@ int32_t ec_lock_check(ec_fop_data_t *fop, uintptr_t *mask)
                     fop->answer = cbk;
                 }
 
-                ec_update_bad(fop, locked);
+                ec_update_good(fop, locked);
 
                 error = 0;
             } else {
@@ -306,8 +306,7 @@ void ec_entrylk(call_frame_t * frame, xlator_t * this, uintptr_t target,
     GF_VALIDATE_OR_GOTO(this->name, frame, out);
     GF_VALIDATE_OR_GOTO(this->name, this->private, out);
 
-    fop = ec_fop_data_allocate(frame, this, GF_FOP_ENTRYLK,
-                               EC_FLAG_UPDATE_LOC_INODE, target, minimum,
+    fop = ec_fop_data_allocate(frame, this, GF_FOP_ENTRYLK, 0, target, minimum,
                                ec_wind_entrylk, ec_manager_entrylk, callback,
                                data);
     if (fop == NULL) {
@@ -442,10 +441,9 @@ void ec_fentrylk(call_frame_t * frame, xlator_t * this, uintptr_t target,
     GF_VALIDATE_OR_GOTO(this->name, frame, out);
     GF_VALIDATE_OR_GOTO(this->name, this->private, out);
 
-    fop = ec_fop_data_allocate(frame, this, GF_FOP_FENTRYLK,
-                               EC_FLAG_UPDATE_FD_INODE, target, minimum,
-                               ec_wind_fentrylk, ec_manager_entrylk, callback,
-                               data);
+    fop = ec_fop_data_allocate(frame, this, GF_FOP_FENTRYLK, 0, target,
+                               minimum, ec_wind_fentrylk, ec_manager_entrylk,
+                               callback, data);
     if (fop == NULL) {
         goto out;
     }
@@ -709,8 +707,7 @@ void ec_inodelk(call_frame_t * frame, xlator_t * this, uintptr_t target,
     GF_VALIDATE_OR_GOTO(this->name, frame, out);
     GF_VALIDATE_OR_GOTO(this->name, this->private, out);
 
-    fop = ec_fop_data_allocate(frame, this, GF_FOP_INODELK,
-                               EC_FLAG_UPDATE_LOC_INODE, target, minimum,
+    fop = ec_fop_data_allocate(frame, this, GF_FOP_INODELK, 0, target, minimum,
                                ec_wind_inodelk, ec_manager_inodelk, callback,
                                data);
     if (fop == NULL) {
@@ -846,10 +843,9 @@ void ec_finodelk(call_frame_t * frame, xlator_t * this, uintptr_t target,
     GF_VALIDATE_OR_GOTO(this->name, frame, out);
     GF_VALIDATE_OR_GOTO(this->name, this->private, out);
 
-    fop = ec_fop_data_allocate(frame, this, GF_FOP_FINODELK,
-                               EC_FLAG_UPDATE_FD_INODE, target, minimum,
-                               ec_wind_finodelk, ec_manager_inodelk, callback,
-                               data);
+    fop = ec_fop_data_allocate(frame, this, GF_FOP_FINODELK, 0, target,
+                               minimum, ec_wind_finodelk, ec_manager_inodelk,
+                               callback, data);
     if (fop == NULL) {
         goto out;
     }
@@ -1117,9 +1113,8 @@ void ec_lk(call_frame_t * frame, xlator_t * this, uintptr_t target,
     GF_VALIDATE_OR_GOTO(this->name, frame, out);
     GF_VALIDATE_OR_GOTO(this->name, this->private, out);
 
-    fop = ec_fop_data_allocate(frame, this, GF_FOP_LK, EC_FLAG_UPDATE_FD_INODE,
-                               target, minimum, ec_wind_lk, ec_manager_lk,
-                               callback, data);
+    fop = ec_fop_data_allocate(frame, this, GF_FOP_LK, 0, target, minimum,
+                               ec_wind_lk, ec_manager_lk, callback, data);
     if (fop == NULL) {
         goto out;
     }