ec: Fix incorrect management of healed bricks

The final lookup made to restore final file attributes after a self-heal
did clear the mask of bad bricks, causing that the final setattr won't
modify any brick at all. This caused that some attriutes, specially the
modification time of the file didn't get updated properly.

Now the mask of healed bricks is saved before doing the last lookup.
It's also used to correctly report the repaired bricks.

This is a backport of http://review.gluster.org/8905/

Change-Id: Ib94083c9e1b562515dfb54f9574120f1f031dccc
BUG: 1149725
Signed-off-by: Xavier Hernandez <xhernandez@datalab.es>
Reviewed-on: http://review.gluster.org/8906
Reviewed-by: Dan Lambright <dlambrig@redhat.com>
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Vijay Bellur <vbellur@redhat.com>

4 files changed, 47 insertions, 39 deletions

diff --git a/tests/basic/ec/self-heal.t b/tests/basic/ec/self-heal.t
index 99cfd9420aa..b7f09a56cd6 100644
--- a/tests/basic/ec/self-heal.t
+++ b/tests/basic/ec/self-heal.t
@@ -12,7 +12,7 @@ if [ ! -d $tmp ]; then
     exit 1
 fi
 
-TESTS_EXPECTED_IN_LOOP=85
+TESTS_EXPECTED_IN_LOOP=250
 
 TEST glusterd
 TEST pidof glusterd
@@ -34,13 +34,20 @@ done
 cd $M0
 TEST cp $tmp/test test
 TEST chmod 644 test
+TEST touch -d "@946681200" test
 EXPECT "-rw-r--r--" stat -c "%A" test
+EXPECT "946681200" stat -c "%Y" test
 
 for idx1 in {0..5}; do
     TEST chmod 666 ${brick[$idx1]}/test
+    TEST truncate -s 0 ${brick[$idx1]}/test
+    TEST setfattr -n user.test -v "test1" ${brick[$idx1]}/test
     sleep 1
     EXPECT "-rw-r--r--" stat -c "%A" test
-    EXPECT_WITHIN 5 "-rw-r--r--" stat -c "%A" ${brick[$idx1]}/test
+    EXPECT_WITHIN $HEAL_TIMEOUT "262144" stat -c "%s" ${brick[$idx1]}/test
+    EXPECT_WITHIN $HEAL_TIMEOUT "-rw-r--r--" stat -c "%A" ${brick[$idx1]}/test
+    EXPECT_WITHIN $HEAL_TIMEOUT "946681200" stat -c "%Y" ${brick[$idx1]}/test
+    TEST ! getfattr -n user.test ${brick[$idx1]}/test
 done
 
 for idx1 in {0..4}; do
@@ -48,32 +55,24 @@ for idx1 in {0..4}; do
         if [ $idx1 -ne $idx2 ]; then
             TEST chmod 666 ${brick[$idx1]}/test
             TEST chmod 600 ${brick[$idx2]}/test
+            TEST truncate -s 0 ${brick[$idx1]}/test
+            TEST truncate -s 2097152 ${brick[$idx2]}/test
+            TEST setfattr -n user.test -v "test1" ${brick[$idx1]}/test
+            TEST setfattr -n user.test -v "test2" ${brick[$idx2]}/test
             sleep 1
             EXPECT "-rw-r--r--" stat -c "%A" test
-            EXPECT_WITHIN 5 "-rw-r--r--" stat -c "%A" ${brick[$idx1]}/test
-            EXPECT_WITHIN 5 "-rw-r--r--" stat -c "%A" ${brick[$idx2]}/test
+            EXPECT_WITHIN $HEAL_TIMEOUT "262144" stat -c "%s" ${brick[$idx1]}/test
+            EXPECT_WITHIN $HEAL_TIMEOUT "262144" stat -c "%s" ${brick[$idx2]}/test
+            EXPECT_WITHIN $HEAL_TIMEOUT "-rw-r--r--" stat -c "%A" ${brick[$idx1]}/test
+            EXPECT_WITHIN $HEAL_TIMEOUT "-rw-r--r--" stat -c "%A" ${brick[$idx2]}/test
+            EXPECT_WITHIN $HEAL_TIMEOUT "946681200" stat -c "%Y" ${brick[$idx1]}/test
+            EXPECT_WITHIN $HEAL_TIMEOUT "946681200" stat -c "%Y" ${brick[$idx2]}/test
+            TEST ! getfattr -n user.test ${brick[$idx1]}/test
+            TEST ! getfattr -n user.test ${brick[$idx2]}/test
         fi
     done
 done
 
-TEST truncate -s 0 ${brick[0]}/test
-TEST truncate -s 2097152 ${brick[1]}/test
-TEST setfattr -n user.test -v "test1" ${brick[0]}/test
-TEST setfattr -n user.test -v "test2" ${brick[1]}/test
-TEST chmod 600 ${brick[0]}/test
-TEST chmod 666 ${brick[1]}/test
-sleep 1
-
-EXPECT "1048576" stat -c "%s" test
-TEST ! getfattr -n user.test test
-
-EXPECT_WITHIN 5 "262144" stat -c "%s" ${brick[0]}/test
-EXPECT_WITHIN 5 "262144" stat -c "%s" ${brick[1]}/test
-TEST ! getfattr -n user.test ${brick[0]}/test
-TEST ! getfattr -n user.test ${brick[1]}/test
-EXPECT "-rw-r--r--" stat -c "%A" ${brick[0]}/test
-EXPECT "-rw-r--r--" stat -c "%A" ${brick[1]}/test
-
 TEST kill_brick $V0 $H0 $B0/${V0}0
 TEST kill_brick $V0 $H0 $B0/${V0}1
 TEST cp $tmp/test test2
@@ -101,22 +100,22 @@ cd $M0
 
 EXPECT "1048576" stat -c "%s" test2
 EXPECT "-rwxrwxrwx" stat -c "%A" test2
-EXPECT_WITHIN 5 "262144" stat -c "%s" ${brick[0]}/test2
-EXPECT_WITHIN 5 "262144" stat -c "%s" ${brick[1]}/test2
-EXPECT "-rwxrwxrwx" stat -c "%A" ${brick[0]}/test2
-EXPECT "-rwxrwxrwx" stat -c "%A" ${brick[1]}/test2
+EXPECT_WITHIN $HEAL_TIMEOUT "262144" stat -c "%s" ${brick[0]}/test2
+EXPECT_WITHIN $HEAL_TIMEOUT "262144" stat -c "%s" ${brick[1]}/test2
+EXPECT_WITHIN $HEAL_TIMEOUT "-rwxrwxrwx" stat -c "%A" ${brick[0]}/test2
+EXPECT_WITHIN $HEAL_TIMEOUT "-rwxrwxrwx" stat -c "%A" ${brick[1]}/test2
 
 TEST ls -al dir1
-EXPECT_WITHIN 5 "1" eval "if [ -d ${brick[0]}/dir1 ]; then echo 1; fi"
-EXPECT_WITHIN 5 "1" eval "if [ -d ${brick[1]}/dir1 ]; then echo 1; fi"
+EXPECT_WITHIN $HEAL_TIMEOUT "1" eval "if [ -d ${brick[0]}/dir1 ]; then echo 1; fi"
+EXPECT_WITHIN $HEAL_TIMEOUT "1" eval "if [ -d ${brick[1]}/dir1 ]; then echo 1; fi"
 
 TEST [ -h test3 ]
-EXPECT_WITHIN 5 "1" eval "if [ -h ${brick[0]}/test3 ]; then echo 1; fi"
-EXPECT_WITHIN 5 "1" eval "if [ -h ${brick[1]}/test3 ]; then echo 1; fi"
+EXPECT_WITHIN $HEAL_TIMEOUT "1" eval "if [ -h ${brick[0]}/test3 ]; then echo 1; fi"
+EXPECT_WITHIN $HEAL_TIMEOUT "1" eval "if [ -h ${brick[1]}/test3 ]; then echo 1; fi"
 
 EXPECT "2" stat -c "%h" test4
-EXPECT_WITHIN 5 "3" stat -c "%h" ${brick[0]}/test4
-EXPECT_WITHIN 5 "3" stat -c "%h" ${brick[1]}/test4
+EXPECT_WITHIN $HEAL_TIMEOUT "3" stat -c "%h" ${brick[0]}/test4
+EXPECT_WITHIN $HEAL_TIMEOUT "3" stat -c "%h" ${brick[1]}/test4
 
 rm -rf $tmp
 
diff --git a/xlators/cluster/ec/src/ec-common.c b/xlators/cluster/ec/src/ec-common.c
index 4d7beee63b9..2f96ac10dbf 100644
--- a/xlators/cluster/ec/src/ec-common.c
+++ b/xlators/cluster/ec/src/ec-common.c
@@ -1166,7 +1166,13 @@ void ec_get_size_version(ec_fop_data_t * fop)
         goto out;
     }
 
-    ec_lookup(fop->frame, fop->xl, fop->mask, EC_MINIMUM_MIN,
+    /* For normal fops, ec_lookup() 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 always fail. Thus we
+     * always use the same minimum used for the main fop.
+     */
+    ec_lookup(fop->frame, fop->xl, fop->mask, fop->minimum,
               ec_get_size_version_set, NULL, &loc, xdata);
 
     fop->frame->root->uid = uid;
diff --git a/xlators/cluster/ec/src/ec-data.h b/xlators/cluster/ec/src/ec-data.h
index 49090933ade..d87cb26b52d 100644
--- a/xlators/cluster/ec/src/ec-data.h
+++ b/xlators/cluster/ec/src/ec-data.h
@@ -278,6 +278,7 @@ struct _ec_heal
     uintptr_t       good;
     uintptr_t       bad;
     uintptr_t       open;
+    uintptr_t       fixed;
     uint64_t        offset;
     uint64_t        size;
     uint64_t        version;
diff --git a/xlators/cluster/ec/src/ec-heal.c b/xlators/cluster/ec/src/ec-heal.c
index 54301d10df4..dc11e6d2e87 100644
--- a/xlators/cluster/ec/src/ec-heal.c
+++ b/xlators/cluster/ec/src/ec-heal.c
@@ -70,6 +70,7 @@ void ec_heal_lookup_resume(ec_fop_data_t * fop)
                 heal->raw_size = cbk->size;
                 heal->fop->pre_size = cbk->iatt[0].ia_size;
                 heal->fop->post_size = cbk->iatt[0].ia_size;
+                heal->fop->have_size = 1;
 
                 if (!ec_loc_prepare(heal->xl, &heal->loc, cbk->inode,
                                     &cbk->iatt[0]))
@@ -559,7 +560,7 @@ void ec_heal_inodelk(ec_heal_t * heal, int32_t type, int32_t use_fd,
     }
 }
 
-void ec_heal_lookup(ec_heal_t * heal)
+void ec_heal_lookup(ec_heal_t *heal, uintptr_t mask)
 {
     dict_t * xdata;
     int32_t error = ENOMEM;
@@ -574,7 +575,7 @@ void ec_heal_lookup(ec_heal_t * heal)
         goto out;
     }
 
-    ec_lookup(heal->fop->frame, heal->xl, heal->fop->mask, EC_MINIMUM_MIN,
+    ec_lookup(heal->fop->frame, heal->xl, mask, EC_MINIMUM_MIN,
               ec_heal_inode_lookup_cbk, heal, &heal->loc, xdata);
 
     error = 0;
@@ -1098,7 +1099,7 @@ void ec_heal_dispatch(ec_heal_t * heal)
     {
         cbk->uintptr[0] = heal->available;
         cbk->uintptr[1] = heal->good;
-        cbk->uintptr[2] = heal->bad;
+        cbk->uintptr[2] = heal->fixed;
 
         ec_combine(cbk, NULL);
 
@@ -1196,7 +1197,7 @@ int32_t ec_manager_heal(ec_fop_data_t * fop, int32_t state)
             return EC_STATE_HEAL_PRE_INODE_LOOKUP;
 
         case EC_STATE_HEAL_PRE_INODE_LOOKUP:
-            ec_heal_lookup(heal);
+            ec_heal_lookup(heal, heal->fop->mask);
 
             return EC_STATE_HEAL_XATTRIBUTES_REMOVE;
 
@@ -1278,12 +1279,13 @@ int32_t ec_manager_heal(ec_fop_data_t * fop, int32_t state)
             return EC_STATE_HEAL_POST_INODE_LOOKUP;
 
         case EC_STATE_HEAL_POST_INODE_LOOKUP:
-            ec_heal_lookup(heal);
+            heal->fixed = heal->bad;
+            ec_heal_lookup(heal, heal->good);
 
             return EC_STATE_HEAL_SETATTR;
 
         case EC_STATE_HEAL_SETATTR:
-            ec_setattr(heal->fop->frame, heal->xl, heal->bad, EC_MINIMUM_ONE,
+            ec_setattr(heal->fop->frame, heal->xl, heal->fixed, EC_MINIMUM_ONE,
                        ec_heal_setattr_cbk, heal, &heal->loc, &heal->iatt,
                        GF_SET_ATTR_MODE | GF_SET_ATTR_UID | GF_SET_ATTR_GID |
                        GF_SET_ATTR_ATIME | GF_SET_ATTR_MTIME, NULL);