e3c68b
From 6565749c95e90f360a994bde1416cffd22cd8ce9 Mon Sep 17 00:00:00 2001
e3c68b
From: N Balachandran <nbalacha@redhat.com>
e3c68b
Date: Mon, 25 Mar 2019 15:56:56 +0530
e3c68b
Subject: [PATCH 126/141] cluster/dht: refactor dht lookup functions
e3c68b
e3c68b
Part 1:  refactor the dht_lookup_dir_cbk
e3c68b
and dht_selfheal_directory functions.
e3c68b
Added a simple dht selfheal directory test
e3c68b
e3c68b
upstream: https://review.gluster.org/#/c/glusterfs/+/22407/
e3c68b
> Change-Id: I1410c26359e3c14b396adbe751937a52bd2fcff9
e3c68b
> updates: bz#1590385
e3c68b
e3c68b
Change-Id: Idd0a7df7122d634c371ecf30c0dbb94dc6063416
e3c68b
BUG: 1703897
e3c68b
Signed-off-by: N Balachandran <nbalacha@redhat.com>
e3c68b
Reviewed-on: https://code.engineering.redhat.com/gerrit/169037
e3c68b
Tested-by: RHGS Build Bot <nigelb@redhat.com>
e3c68b
Reviewed-by: Susant Palai <spalai@redhat.com>
e3c68b
Reviewed-by: Atin Mukherjee <amukherj@redhat.com>
e3c68b
---
e3c68b
 tests/basic/distribute/dir-heal.t      | 145 +++++++++++++++++++++++++++
e3c68b
 xlators/cluster/dht/src/dht-common.c   | 178 +++++++++++++++------------------
e3c68b
 xlators/cluster/dht/src/dht-selfheal.c |  65 +++++++-----
e3c68b
 3 files changed, 264 insertions(+), 124 deletions(-)
e3c68b
 create mode 100644 tests/basic/distribute/dir-heal.t
e3c68b
e3c68b
diff --git a/tests/basic/distribute/dir-heal.t b/tests/basic/distribute/dir-heal.t
e3c68b
new file mode 100644
e3c68b
index 0000000..851f765
e3c68b
--- /dev/null
e3c68b
+++ b/tests/basic/distribute/dir-heal.t
e3c68b
@@ -0,0 +1,145 @@
e3c68b
+#!/bin/bash
e3c68b
+
e3c68b
+. $(dirname $0)/../../include.rc
e3c68b
+. $(dirname $0)/../../volume.rc
e3c68b
+. $(dirname $0)/../../nfs.rc
e3c68b
+. $(dirname $0)/../../common-utils.rc
e3c68b
+
e3c68b
+# Test 1 overview:
e3c68b
+# ----------------
e3c68b
+#
e3c68b
+# 1. Kill one brick of the volume.
e3c68b
+# 2. Create directories and change directory properties.
e3c68b
+# 3. Bring up the brick and access the directory
e3c68b
+# 4. Check the permissions and xattrs on the backend
e3c68b
+
e3c68b
+cleanup
e3c68b
+
e3c68b
+TEST glusterd
e3c68b
+TEST pidof glusterd
e3c68b
+
e3c68b
+TEST $CLI volume create $V0 $H0:$B0/$V0-{1..3}
e3c68b
+TEST $CLI volume start $V0
e3c68b
+
e3c68b
+# We want the lookup to reach DHT
e3c68b
+TEST $CLI volume set $V0 performance.stat-prefetch off
e3c68b
+
e3c68b
+# Mount using FUSE , kill a brick and create directories
e3c68b
+TEST glusterfs --entry-timeout=0 --attribute-timeout=0 -s $H0 --volfile-id $V0 $M0
e3c68b
+
e3c68b
+ls $M0/
e3c68b
+cd $M0
e3c68b
+
e3c68b
+TEST kill_brick $V0 $H0 $B0/$V0-1
e3c68b
+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "0" brick_up_status $V0 $H0 $B0/$V0-1
e3c68b
+
e3c68b
+TEST mkdir dir{1..4}
e3c68b
+
e3c68b
+# No change for dir1
e3c68b
+# Change permissions for dir2
e3c68b
+# Set xattr on dir3
e3c68b
+# Change permissions and set xattr on dir4
e3c68b
+
e3c68b
+TEST chmod 777 $M0/dir2
e3c68b
+
e3c68b
+TEST setfattr -n "user.test" -v "test" $M0/dir3
e3c68b
+
e3c68b
+TEST chmod 777 $M0/dir4
e3c68b
+TEST setfattr -n "user.test" -v "test" $M0/dir4
e3c68b
+
e3c68b
+
e3c68b
+# Start all bricks
e3c68b
+
e3c68b
+TEST $CLI volume start $V0 force
e3c68b
+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" brick_up_status $V0 $H0 $B0/$V0-1
e3c68b
+
e3c68b
+#$CLI volume status
e3c68b
+
e3c68b
+# It takes a while for the client to reconnect to the brick
e3c68b
+sleep 5
e3c68b
+
e3c68b
+stat $M0/dir* > /dev/null
e3c68b
+
e3c68b
+# Check that directories have been created on the brick that was killed
e3c68b
+
e3c68b
+TEST ls $B0/$V0-1/dir1
e3c68b
+
e3c68b
+TEST ls $B0/$V0-1/dir2
e3c68b
+EXPECT "777" stat -c "%a" $B0/$V0-1/dir2
e3c68b
+
e3c68b
+TEST ls $B0/$V0-1/dir3
e3c68b
+EXPECT "test" getfattr -n "user.test" --absolute-names --only-values $B0/$V0-1/dir3
e3c68b
+
e3c68b
+
e3c68b
+TEST ls $B0/$V0-1/dir4
e3c68b
+EXPECT "777" stat -c "%a" $B0/$V0-1/dir4
e3c68b
+EXPECT "test" getfattr -n "user.test" --absolute-names --only-values $B0/$V0-1/dir4
e3c68b
+
e3c68b
+
e3c68b
+TEST rm -rf $M0/*
e3c68b
+
e3c68b
+cd
e3c68b
+
e3c68b
+EXPECT_WITHIN $UMOUNT_TIMEOUT "Y" force_umount $M0
e3c68b
+
e3c68b
+
e3c68b
+# Test 2 overview:
e3c68b
+# ----------------
e3c68b
+# 1. Create directories with all bricks up.
e3c68b
+# 2. Kill a brick and change directory properties and set user xattr.
e3c68b
+# 2. Bring up the brick and access the directory
e3c68b
+# 3. Check the permissions and xattrs on the backend
e3c68b
+
e3c68b
+
e3c68b
+TEST glusterfs --entry-timeout=0 --attribute-timeout=0 -s $H0 --volfile-id $V0 $M0
e3c68b
+
e3c68b
+ls $M0/
e3c68b
+cd $M0
e3c68b
+TEST mkdir dir{1..4}
e3c68b
+
e3c68b
+TEST kill_brick $V0 $H0 $B0/$V0-1
e3c68b
+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "0" brick_up_status $V0 $H0 $B0/$V0-1
e3c68b
+
e3c68b
+# No change for dir1
e3c68b
+# Change permissions for dir2
e3c68b
+# Set xattr on dir3
e3c68b
+# Change permissions and set xattr on dir4
e3c68b
+
e3c68b
+TEST chmod 777 $M0/dir2
e3c68b
+
e3c68b
+TEST setfattr -n "user.test" -v "test" $M0/dir3
e3c68b
+
e3c68b
+TEST chmod 777 $M0/dir4
e3c68b
+TEST setfattr -n "user.test" -v "test" $M0/dir4
e3c68b
+
e3c68b
+
e3c68b
+# Start all bricks
e3c68b
+
e3c68b
+TEST $CLI volume start $V0 force
e3c68b
+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" brick_up_status $V0 $H0 $B0/$V0-1
e3c68b
+
e3c68b
+#$CLI volume status
e3c68b
+
e3c68b
+# It takes a while for the client to reconnect to the brick
e3c68b
+sleep 5
e3c68b
+
e3c68b
+stat $M0/dir* > /dev/null
e3c68b
+
e3c68b
+# Check directories on the brick that was killed
e3c68b
+
e3c68b
+TEST ls $B0/$V0-1/dir2
e3c68b
+EXPECT "777" stat -c "%a" $B0/$V0-1/dir2
e3c68b
+
e3c68b
+TEST ls $B0/$V0-1/dir3
e3c68b
+EXPECT "test" getfattr -n "user.test" --absolute-names --only-values $B0/$V0-1/dir3
e3c68b
+
e3c68b
+
e3c68b
+TEST ls $B0/$V0-1/dir4
e3c68b
+EXPECT "777" stat -c "%a" $B0/$V0-1/dir4
e3c68b
+EXPECT "test" getfattr -n "user.test" --absolute-names --only-values $B0/$V0-1/dir4
e3c68b
+cd
e3c68b
+
e3c68b
+
e3c68b
+# Cleanup
e3c68b
+cleanup
e3c68b
+
e3c68b
diff --git a/xlators/cluster/dht/src/dht-common.c b/xlators/cluster/dht/src/dht-common.c
e3c68b
index 2a68193..d3e900c 100644
e3c68b
--- a/xlators/cluster/dht/src/dht-common.c
e3c68b
+++ b/xlators/cluster/dht/src/dht-common.c
e3c68b
@@ -801,9 +801,8 @@ dht_common_mark_mdsxattr(call_frame_t *frame, int *errst,
e3c68b
     call_frame_t *xattr_frame = NULL;
e3c68b
     gf_boolean_t vol_down = _gf_false;
e3c68b
 
e3c68b
-    this = frame->this;
e3c68b
-
e3c68b
     GF_VALIDATE_OR_GOTO("dht", frame, out);
e3c68b
+    this = frame->this;
e3c68b
     GF_VALIDATE_OR_GOTO("dht", this, out);
e3c68b
     GF_VALIDATE_OR_GOTO(this->name, frame->local, out);
e3c68b
     GF_VALIDATE_OR_GOTO(this->name, this->private, out);
e3c68b
@@ -812,6 +811,7 @@ dht_common_mark_mdsxattr(call_frame_t *frame, int *errst,
e3c68b
     conf = this->private;
e3c68b
     layout = local->selfheal.layout;
e3c68b
     local->mds_heal_fresh_lookup = mark_during_fresh_lookup;
e3c68b
+
e3c68b
     gf_uuid_unparse(local->gfid, gfid_local);
e3c68b
 
e3c68b
     /* Code to update hashed subvol consider as a mds subvol
e3c68b
@@ -1240,6 +1240,31 @@ out:
e3c68b
 }
e3c68b
 
e3c68b
 int
e3c68b
+dht_needs_selfheal(call_frame_t *frame, xlator_t *this)
e3c68b
+{
e3c68b
+    dht_local_t *local = NULL;
e3c68b
+    dht_layout_t *layout = NULL;
e3c68b
+    int needs_selfheal = 0;
e3c68b
+    int ret = 0;
e3c68b
+
e3c68b
+    local = frame->local;
e3c68b
+    layout = local->layout;
e3c68b
+
e3c68b
+    if (local->need_attrheal || local->need_xattr_heal ||
e3c68b
+        local->need_selfheal) {
e3c68b
+        needs_selfheal = 1;
e3c68b
+    }
e3c68b
+
e3c68b
+    ret = dht_layout_normalize(this, &local->loc, layout);
e3c68b
+
e3c68b
+    if (ret != 0) {
e3c68b
+        gf_msg_debug(this->name, 0, "fixing assignment on %s", local->loc.path);
e3c68b
+        needs_selfheal = 1;
e3c68b
+    }
e3c68b
+    return needs_selfheal;
e3c68b
+}
e3c68b
+
e3c68b
+int
e3c68b
 dht_lookup_dir_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
e3c68b
                    int op_ret, int op_errno, inode_t *inode, struct iatt *stbuf,
e3c68b
                    dict_t *xattr, struct iatt *postparent)
e3c68b
@@ -1256,8 +1281,6 @@ dht_lookup_dir_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
e3c68b
     char gfid_local[GF_UUID_BUF_SIZE] = {0};
e3c68b
     char gfid_node[GF_UUID_BUF_SIZE] = {0};
e3c68b
     int32_t mds_xattr_val[1] = {0};
e3c68b
-    call_frame_t *copy = NULL;
e3c68b
-    dht_local_t *copy_local = NULL;
e3c68b
 
e3c68b
     GF_VALIDATE_OR_GOTO("dht", frame, out);
e3c68b
     GF_VALIDATE_OR_GOTO("dht", this, out);
e3c68b
@@ -1270,7 +1293,11 @@ dht_lookup_dir_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
e3c68b
     conf = this->private;
e3c68b
 
e3c68b
     layout = local->layout;
e3c68b
+    gf_msg_debug(this->name, op_errno,
e3c68b
+                 "%s: lookup on %s returned with op_ret = %d, op_errno = %d",
e3c68b
+                 local->loc.path, prev->name, op_ret, op_errno);
e3c68b
 
e3c68b
+    /* The first successful lookup*/
e3c68b
     if (!op_ret && gf_uuid_is_null(local->gfid)) {
e3c68b
         memcpy(local->gfid, stbuf->ia_gfid, 16);
e3c68b
     }
e3c68b
@@ -1298,13 +1325,10 @@ dht_lookup_dir_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
e3c68b
 
e3c68b
         if (op_ret == -1) {
e3c68b
             local->op_errno = op_errno;
e3c68b
-            gf_msg_debug(this->name, op_errno,
e3c68b
-                         "%s: lookup on %s returned error", local->loc.path,
e3c68b
-                         prev->name);
e3c68b
 
e3c68b
             /* The GFID is missing on this subvol. Force a heal. */
e3c68b
             if (op_errno == ENODATA) {
e3c68b
-                local->need_selfheal = 1;
e3c68b
+                local->need_lookup_everywhere = 1;
e3c68b
             }
e3c68b
             goto unlock;
e3c68b
         }
e3c68b
@@ -1312,12 +1336,11 @@ dht_lookup_dir_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
e3c68b
         is_dir = check_is_dir(inode, stbuf, xattr);
e3c68b
         if (!is_dir) {
e3c68b
             gf_msg_debug(this->name, 0,
e3c68b
-                         "lookup of %s on %s returned non"
e3c68b
-                         "dir 0%o"
e3c68b
+                         "%s: lookup on %s returned non dir 0%o"
e3c68b
                          "calling lookup_everywhere",
e3c68b
                          local->loc.path, prev->name, stbuf->ia_type);
e3c68b
 
e3c68b
-            local->need_selfheal = 1;
e3c68b
+            local->need_lookup_everywhere = 1;
e3c68b
             goto unlock;
e3c68b
         }
e3c68b
 
e3c68b
@@ -1328,14 +1351,8 @@ dht_lookup_dir_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
e3c68b
             dht_aggregate_xattr(local->xattr, xattr);
e3c68b
         }
e3c68b
 
e3c68b
-        if (dict_get(xattr, conf->mds_xattr_key)) {
e3c68b
-            local->mds_subvol = prev;
e3c68b
-            local->mds_stbuf.ia_gid = stbuf->ia_gid;
e3c68b
-            local->mds_stbuf.ia_uid = stbuf->ia_uid;
e3c68b
-            local->mds_stbuf.ia_prot = stbuf->ia_prot;
e3c68b
-        }
e3c68b
-
e3c68b
         if (local->stbuf.ia_type != IA_INVAL) {
e3c68b
+            /* This is not the first subvol to respond */
e3c68b
             if (!__is_root_gfid(stbuf->ia_gfid) &&
e3c68b
                 ((local->stbuf.ia_gid != stbuf->ia_gid) ||
e3c68b
                  (local->stbuf.ia_uid != stbuf->ia_uid) ||
e3c68b
@@ -1348,65 +1365,64 @@ dht_lookup_dir_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
e3c68b
         if (local->inode == NULL)
e3c68b
             local->inode = inode_ref(inode);
e3c68b
 
e3c68b
+        /* This could be a problem */
e3c68b
         dht_iatt_merge(this, &local->stbuf, stbuf);
e3c68b
         dht_iatt_merge(this, &local->postparent, postparent);
e3c68b
 
e3c68b
         if (!dict_get(xattr, conf->mds_xattr_key)) {
e3c68b
             gf_msg_debug(this->name, 0,
e3c68b
-                         "Internal xattr %s is not present "
e3c68b
-                         " on path %s gfid is %s ",
e3c68b
-                         conf->mds_xattr_key, local->loc.path, gfid_local);
e3c68b
+                         "%s: mds xattr %s is not present "
e3c68b
+                         "on %s(gfid = %s)",
e3c68b
+                         local->loc.path, conf->mds_xattr_key, prev->name,
e3c68b
+                         gfid_local);
e3c68b
             goto unlock;
e3c68b
-        } else {
e3c68b
-            /* Save mds subvol on inode ctx */
e3c68b
-            ret = dht_inode_ctx_mdsvol_set(local->inode, this, prev);
e3c68b
-            if (ret) {
e3c68b
-                gf_msg(this->name, GF_LOG_ERROR, 0,
e3c68b
-                       DHT_MSG_SET_INODE_CTX_FAILED,
e3c68b
-                       "Failed to set hashed subvol for %s vol is %s",
e3c68b
-                       local->loc.path, prev->name);
e3c68b
-            }
e3c68b
+        }
e3c68b
+
e3c68b
+        local->mds_subvol = prev;
e3c68b
+        local->mds_stbuf = *stbuf;
e3c68b
+
e3c68b
+        /* Save mds subvol on inode ctx */
e3c68b
+
e3c68b
+        ret = dht_inode_ctx_mdsvol_set(local->inode, this, prev);
e3c68b
+        if (ret) {
e3c68b
+            gf_msg(this->name, GF_LOG_ERROR, 0, DHT_MSG_SET_INODE_CTX_FAILED,
e3c68b
+                   "%s: Failed to set mds (%s)", local->loc.path, prev->name);
e3c68b
         }
e3c68b
         check_mds = dht_dict_get_array(xattr, conf->mds_xattr_key,
e3c68b
                                        mds_xattr_val, 1, &errst);
e3c68b
         if ((check_mds < 0) && !errst) {
e3c68b
             local->mds_xattr = dict_ref(xattr);
e3c68b
             gf_msg_debug(this->name, 0,
e3c68b
-                         "Value of %s is not zero on hashed subvol "
e3c68b
-                         "so xattr needs to be heal on non hashed"
e3c68b
-                         " path is %s and vol name is %s "
e3c68b
-                         " gfid is %s",
e3c68b
-                         conf->mds_xattr_key, local->loc.path, prev->name,
e3c68b
+                         "%s: %s is not zero on %s. Xattrs need to be healed."
e3c68b
+                         "(gfid = %s)",
e3c68b
+                         local->loc.path, conf->mds_xattr_key, prev->name,
e3c68b
                          gfid_local);
e3c68b
             local->need_xattr_heal = 1;
e3c68b
-            local->mds_subvol = prev;
e3c68b
         }
e3c68b
     }
e3c68b
+
e3c68b
 unlock:
e3c68b
     UNLOCK(&frame->lock);
e3c68b
 
e3c68b
     this_call_cnt = dht_frame_return(frame);
e3c68b
 
e3c68b
     if (is_last_call(this_call_cnt)) {
e3c68b
+        /* If the mds subvol is not set correctly*/
e3c68b
+        if (!__is_root_gfid(local->gfid) &&
e3c68b
+            (!dict_get(local->xattr, conf->mds_xattr_key))) {
e3c68b
+            local->need_selfheal = 1;
e3c68b
+        }
e3c68b
+
e3c68b
         /* No need to call xattr heal code if volume count is 1
e3c68b
          */
e3c68b
-        if (conf->subvolume_cnt == 1)
e3c68b
+        if (conf->subvolume_cnt == 1) {
e3c68b
             local->need_xattr_heal = 0;
e3c68b
-
e3c68b
-        /* Code to update all extended attributed from hashed subvol
e3c68b
-           to local->xattr
e3c68b
-        */
e3c68b
-        if (local->need_xattr_heal && (local->mds_xattr)) {
e3c68b
-            dht_dir_set_heal_xattr(this, local, local->xattr, local->mds_xattr,
e3c68b
-                                   NULL, NULL);
e3c68b
-            dict_unref(local->mds_xattr);
e3c68b
-            local->mds_xattr = NULL;
e3c68b
         }
e3c68b
 
e3c68b
-        if (local->need_selfheal) {
e3c68b
-            local->need_selfheal = 0;
e3c68b
+        if (local->need_selfheal || local->need_lookup_everywhere) {
e3c68b
             /* Set the gfid-req so posix will set the GFID*/
e3c68b
             if (!gf_uuid_is_null(local->gfid)) {
e3c68b
+                /* Ok, this should _never_ happen */
e3c68b
                 ret = dict_set_static_bin(local->xattr_req, "gfid-req",
e3c68b
                                           local->gfid, 16);
e3c68b
             } else {
e3c68b
@@ -1414,73 +1430,36 @@ unlock:
e3c68b
                     ret = dict_set_static_bin(local->xattr_req, "gfid-req",
e3c68b
                                               local->gfid_req, 16);
e3c68b
             }
e3c68b
+        }
e3c68b
+
e3c68b
+        if (local->need_lookup_everywhere) {
e3c68b
+            local->need_lookup_everywhere = 0;
e3c68b
             dht_lookup_everywhere(frame, this, &local->loc);
e3c68b
             return 0;
e3c68b
         }
e3c68b
 
e3c68b
         if (local->op_ret == 0) {
e3c68b
-            ret = dht_layout_normalize(this, &local->loc, layout);
e3c68b
-
e3c68b
-            if (ret != 0) {
e3c68b
-                gf_msg_debug(this->name, 0, "fixing assignment on %s",
e3c68b
-                             local->loc.path);
e3c68b
+            if (dht_needs_selfheal(frame, this)) {
e3c68b
                 goto selfheal;
e3c68b
             }
e3c68b
 
e3c68b
             dht_layout_set(this, local->inode, layout);
e3c68b
-            if (!dict_get(local->xattr, conf->mds_xattr_key) ||
e3c68b
-                local->need_xattr_heal)
e3c68b
-                goto selfheal;
e3c68b
-        }
e3c68b
-
e3c68b
-        if (local->inode) {
e3c68b
-            dht_inode_ctx_time_update(local->inode, this, &local->stbuf, 1);
e3c68b
-        }
e3c68b
-
e3c68b
-        if (local->loc.parent) {
e3c68b
-            dht_inode_ctx_time_update(local->loc.parent, this,
e3c68b
-                                      &local->postparent, 1);
e3c68b
-        }
e3c68b
-
e3c68b
-        if (local->need_attrheal) {
e3c68b
-            local->need_attrheal = 0;
e3c68b
-            if (!__is_root_gfid(inode->gfid)) {
e3c68b
-                local->stbuf.ia_gid = local->mds_stbuf.ia_gid;
e3c68b
-                local->stbuf.ia_uid = local->mds_stbuf.ia_uid;
e3c68b
-                local->stbuf.ia_prot = local->mds_stbuf.ia_prot;
e3c68b
+            if (local->inode) {
e3c68b
+                dht_inode_ctx_time_update(local->inode, this, &local->stbuf, 1);
e3c68b
             }
e3c68b
-            copy = create_frame(this, this->ctx->pool);
e3c68b
-            if (copy) {
e3c68b
-                copy_local = dht_local_init(copy, &local->loc, NULL, 0);
e3c68b
-                if (!copy_local) {
e3c68b
-                    DHT_STACK_DESTROY(copy);
e3c68b
-                    goto skip_attr_heal;
e3c68b
-                }
e3c68b
-                copy_local->stbuf = local->stbuf;
e3c68b
-                gf_uuid_copy(copy_local->loc.gfid, local->stbuf.ia_gfid);
e3c68b
-                copy_local->mds_stbuf = local->mds_stbuf;
e3c68b
-                copy_local->mds_subvol = local->mds_subvol;
e3c68b
-                copy->local = copy_local;
e3c68b
-                FRAME_SU_DO(copy, dht_local_t);
e3c68b
-                ret = synctask_new(this->ctx->env, dht_dir_attr_heal,
e3c68b
-                                   dht_dir_attr_heal_done, copy, copy);
e3c68b
-                if (ret) {
e3c68b
-                    gf_msg(this->name, GF_LOG_ERROR, ENOMEM,
e3c68b
-                           DHT_MSG_DIR_ATTR_HEAL_FAILED,
e3c68b
-                           "Synctask creation failed to heal attr "
e3c68b
-                           "for path %s gfid %s ",
e3c68b
-                           local->loc.path, local->gfid);
e3c68b
-                    DHT_STACK_DESTROY(copy);
e3c68b
-                }
e3c68b
+
e3c68b
+            if (local->loc.parent) {
e3c68b
+                dht_inode_ctx_time_update(local->loc.parent, this,
e3c68b
+                                          &local->postparent, 1);
e3c68b
             }
e3c68b
         }
e3c68b
 
e3c68b
-    skip_attr_heal:
e3c68b
         DHT_STRIP_PHASE1_FLAGS(&local->stbuf);
e3c68b
         dht_set_fixed_dir_stat(&local->postparent);
e3c68b
         /* Delete mds xattr at the time of STACK UNWIND */
e3c68b
         if (local->xattr)
e3c68b
             GF_REMOVE_INTERNAL_XATTR(conf->mds_xattr_key, local->xattr);
e3c68b
+
e3c68b
         DHT_STACK_UNWIND(lookup, frame, local->op_ret, local->op_errno,
e3c68b
                          local->inode, &local->stbuf, local->xattr,
e3c68b
                          &local->postparent);
e3c68b
@@ -5444,9 +5423,8 @@ dht_dir_common_set_remove_xattr(call_frame_t *frame, xlator_t *this, loc_t *loc,
e3c68b
         } else {
e3c68b
             gf_msg(this->name, GF_LOG_ERROR, 0,
e3c68b
                    DHT_MSG_HASHED_SUBVOL_GET_FAILED,
e3c68b
-                   "Failed to get mds subvol for path %s"
e3c68b
-                   "gfid is %s ",
e3c68b
-                   loc->path, gfid_local);
e3c68b
+                   "%s: Failed to get mds subvol. (gfid is %s)", loc->path,
e3c68b
+                   gfid_local);
e3c68b
         }
e3c68b
         (*op_errno) = ENOENT;
e3c68b
         goto err;
e3c68b
diff --git a/xlators/cluster/dht/src/dht-selfheal.c b/xlators/cluster/dht/src/dht-selfheal.c
e3c68b
index bd1b7ea..5420fca 100644
e3c68b
--- a/xlators/cluster/dht/src/dht-selfheal.c
e3c68b
+++ b/xlators/cluster/dht/src/dht-selfheal.c
e3c68b
@@ -1033,18 +1033,27 @@ dht_selfheal_dir_setattr(call_frame_t *frame, loc_t *loc, struct iatt *stbuf,
e3c68b
     int missing_attr = 0;
e3c68b
     int i = 0, ret = -1;
e3c68b
     dht_local_t *local = NULL;
e3c68b
+    dht_conf_t *conf = NULL;
e3c68b
     xlator_t *this = NULL;
e3c68b
     int cnt = 0;
e3c68b
 
e3c68b
     local = frame->local;
e3c68b
     this = frame->this;
e3c68b
+    conf = this->private;
e3c68b
+
e3c68b
+    /* We need to heal the attrs if:
e3c68b
+     * 1. Any directories were missing - the newly created dirs will need
e3c68b
+     *    to have the correct attrs set
e3c68b
+     * 2. An existing dir does not have the correct permissions -they may
e3c68b
+     *    have been changed when a brick was down.
e3c68b
+     */
e3c68b
 
e3c68b
     for (i = 0; i < layout->cnt; i++) {
e3c68b
         if (layout->list[i].err == -1)
e3c68b
             missing_attr++;
e3c68b
     }
e3c68b
 
e3c68b
-    if (missing_attr == 0) {
e3c68b
+    if ((missing_attr == 0) && (local->need_attrheal == 0)) {
e3c68b
         if (!local->heal_layout) {
e3c68b
             gf_msg_trace(this->name, 0, "Skip heal layout for %s gfid = %s ",
e3c68b
                          loc->path, uuid_utoa(loc->gfid));
e3c68b
@@ -1062,19 +1071,12 @@ dht_selfheal_dir_setattr(call_frame_t *frame, loc_t *loc, struct iatt *stbuf,
e3c68b
         return 0;
e3c68b
     }
e3c68b
 
e3c68b
-    local->call_cnt = missing_attr;
e3c68b
-    cnt = layout->cnt;
e3c68b
+    cnt = local->call_cnt = conf->subvolume_cnt;
e3c68b
 
e3c68b
     for (i = 0; i < cnt; i++) {
e3c68b
-        if (layout->list[i].err == -1) {
e3c68b
-            gf_msg_trace(this->name, 0, "%s: setattr on subvol %s, gfid = %s",
e3c68b
-                         loc->path, layout->list[i].xlator->name,
e3c68b
-                         uuid_utoa(loc->gfid));
e3c68b
-
e3c68b
-            STACK_WIND(
e3c68b
-                frame, dht_selfheal_dir_setattr_cbk, layout->list[i].xlator,
e3c68b
-                layout->list[i].xlator->fops->setattr, loc, stbuf, valid, NULL);
e3c68b
-        }
e3c68b
+        STACK_WIND(frame, dht_selfheal_dir_setattr_cbk, layout->list[i].xlator,
e3c68b
+                   layout->list[i].xlator->fops->setattr, loc, stbuf, valid,
e3c68b
+                   NULL);
e3c68b
     }
e3c68b
 
e3c68b
     return 0;
e3c68b
@@ -1492,6 +1494,9 @@ dht_selfheal_dir_mkdir(call_frame_t *frame, loc_t *loc, dht_layout_t *layout,
e3c68b
     }
e3c68b
 
e3c68b
     if (missing_dirs == 0) {
e3c68b
+        /* We don't need to create any directories. Proceed to heal the
e3c68b
+         * attrs and xattrs
e3c68b
+         */
e3c68b
         if (!__is_root_gfid(local->stbuf.ia_gfid)) {
e3c68b
             if (local->need_xattr_heal) {
e3c68b
                 local->need_xattr_heal = 0;
e3c68b
@@ -1499,8 +1504,8 @@ dht_selfheal_dir_mkdir(call_frame_t *frame, loc_t *loc, dht_layout_t *layout,
e3c68b
                 if (ret)
e3c68b
                     gf_msg(this->name, GF_LOG_ERROR, ret,
e3c68b
                            DHT_MSG_DIR_XATTR_HEAL_FAILED,
e3c68b
-                           "xattr heal failed for "
e3c68b
-                           "directory  %s gfid %s ",
e3c68b
+                           "%s:xattr heal failed for "
e3c68b
+                           "directory (gfid = %s)",
e3c68b
                            local->loc.path, local->gfid);
e3c68b
             } else {
e3c68b
                 if (!gf_uuid_is_null(local->gfid))
e3c68b
@@ -1512,8 +1517,8 @@ dht_selfheal_dir_mkdir(call_frame_t *frame, loc_t *loc, dht_layout_t *layout,
e3c68b
 
e3c68b
                 gf_msg(this->name, GF_LOG_INFO, 0,
e3c68b
                        DHT_MSG_DIR_XATTR_HEAL_FAILED,
e3c68b
-                       "Failed to set mds xattr "
e3c68b
-                       "for directory  %s gfid %s ",
e3c68b
+                       "%s: Failed to set mds xattr "
e3c68b
+                       "for directory (gfid = %s)",
e3c68b
                        local->loc.path, local->gfid);
e3c68b
             }
e3c68b
         }
e3c68b
@@ -2085,10 +2090,10 @@ dht_selfheal_directory(call_frame_t *frame, dht_selfheal_dir_cbk_t dir_cbk,
e3c68b
                        loc_t *loc, dht_layout_t *layout)
e3c68b
 {
e3c68b
     dht_local_t *local = NULL;
e3c68b
+    xlator_t *this = NULL;
e3c68b
     uint32_t down = 0;
e3c68b
     uint32_t misc = 0;
e3c68b
     int ret = 0;
e3c68b
-    xlator_t *this = NULL;
e3c68b
     char pgfid[GF_UUID_BUF_SIZE] = {0};
e3c68b
     char gfid[GF_UUID_BUF_SIZE] = {0};
e3c68b
     inode_t *linked_inode = NULL, *inode = NULL;
e3c68b
@@ -2099,6 +2104,11 @@ dht_selfheal_directory(call_frame_t *frame, dht_selfheal_dir_cbk_t dir_cbk,
e3c68b
     local->selfheal.dir_cbk = dir_cbk;
e3c68b
     local->selfheal.layout = dht_layout_ref(this, layout);
e3c68b
 
e3c68b
+    if (local->need_attrheal && !IA_ISINVAL(local->mds_stbuf.ia_type)) {
e3c68b
+        /*Use the one in the mds_stbuf*/
e3c68b
+        local->stbuf = local->mds_stbuf;
e3c68b
+    }
e3c68b
+
e3c68b
     if (!__is_root_gfid(local->stbuf.ia_gfid)) {
e3c68b
         gf_uuid_unparse(local->stbuf.ia_gfid, gfid);
e3c68b
         gf_uuid_unparse(loc->parent->gfid, pgfid);
e3c68b
@@ -2118,6 +2128,13 @@ dht_selfheal_directory(call_frame_t *frame, dht_selfheal_dir_cbk_t dir_cbk,
e3c68b
         inode_unref(inode);
e3c68b
     }
e3c68b
 
e3c68b
+    if (local->need_xattr_heal && (local->mds_xattr)) {
e3c68b
+        dht_dir_set_heal_xattr(this, local, local->xattr, local->mds_xattr,
e3c68b
+                               NULL, NULL);
e3c68b
+        dict_unref(local->mds_xattr);
e3c68b
+        local->mds_xattr = NULL;
e3c68b
+    }
e3c68b
+
e3c68b
     dht_layout_anomalies(this, loc, layout, &local->selfheal.hole_cnt,
e3c68b
                          &local->selfheal.overlaps_cnt,
e3c68b
                          &local->selfheal.missing_cnt, &local->selfheal.down,
e3c68b
@@ -2128,18 +2145,18 @@ dht_selfheal_directory(call_frame_t *frame, dht_selfheal_dir_cbk_t dir_cbk,
e3c68b
 
e3c68b
     if (down) {
e3c68b
         gf_msg(this->name, GF_LOG_WARNING, 0, DHT_MSG_DIR_SELFHEAL_FAILED,
e3c68b
-               "Directory selfheal failed: %d subvolumes down."
e3c68b
-               "Not fixing. path = %s, gfid = %s",
e3c68b
-               down, loc->path, gfid);
e3c68b
+               "%s: Directory selfheal failed: %d subvolumes down."
e3c68b
+               "Not fixing. gfid = %s",
e3c68b
+               loc->path, down, gfid);
e3c68b
         ret = 0;
e3c68b
         goto sorry_no_fix;
e3c68b
     }
e3c68b
 
e3c68b
     if (misc) {
e3c68b
         gf_msg(this->name, GF_LOG_WARNING, 0, DHT_MSG_DIR_SELFHEAL_FAILED,
e3c68b
-               "Directory selfheal failed : %d subvolumes "
e3c68b
-               "have unrecoverable errors. path = %s, gfid = %s",
e3c68b
-               misc, loc->path, gfid);
e3c68b
+               "%s: Directory selfheal failed : %d subvolumes "
e3c68b
+               "have unrecoverable errors. gfid = %s",
e3c68b
+               loc->path, misc, gfid);
e3c68b
 
e3c68b
         ret = 0;
e3c68b
         goto sorry_no_fix;
e3c68b
@@ -2369,13 +2386,13 @@ dht_dir_attr_heal(void *data)
e3c68b
 
e3c68b
     frame = data;
e3c68b
     local = frame->local;
e3c68b
-    mds_subvol = local->mds_subvol;
e3c68b
     this = frame->this;
e3c68b
     GF_VALIDATE_OR_GOTO("dht", this, out);
e3c68b
     GF_VALIDATE_OR_GOTO("dht", local, out);
e3c68b
     conf = this->private;
e3c68b
     GF_VALIDATE_OR_GOTO("dht", conf, out);
e3c68b
 
e3c68b
+    mds_subvol = local->mds_subvol;
e3c68b
     call_cnt = conf->subvolume_cnt;
e3c68b
 
e3c68b
     if (!__is_root_gfid(local->stbuf.ia_gfid) && (!mds_subvol)) {
e3c68b
-- 
e3c68b
1.8.3.1
e3c68b