cluster/dht: log error only if layout healing is required

selfhealing of directory is invoked on two conditions:
1. no layout on disk or layout has some anomalies (holes/overlaps)
2. mds xattr is not set on the directory

When dht_selfheal_directory is called with a correct layout just to
set mds xattr, we see error msgs complaining about "not able to form
layout on directory", which is misleading as the layout is
correct. So, log this msg only if layout has anomalies.

Change-Id: I4af25246fc3a2450c2426e9902d1a5b372eab125
updates: bz#1543279
BUG: 1543279
Signed-off-by: Raghavendra G <rgowdapp@redhat.com>

2 files changed, 12 insertions, 188 deletions

diff --git a/xlators/cluster/dht/src/dht-common.h b/xlators/cluster/dht/src/dht-common.h
index 87f60cac3d9..96f7726bc04 100644
--- a/xlators/cluster/dht/src/dht-common.h
+++ b/xlators/cluster/dht/src/dht-common.h
@@ -308,6 +308,7 @@ struct dht_local {
                 uint32_t                overlaps_cnt;
                 uint32_t                down;
                 uint32_t                misc;
+                uint32_t                missing_cnt;
                 dht_selfheal_dir_cbk_t  dir_cbk;
                 dht_selfheal_layout_t   healer;
                 dht_need_heal_t         should_heal;
diff --git a/xlators/cluster/dht/src/dht-selfheal.c b/xlators/cluster/dht/src/dht-selfheal.c
index 49540a04c9e..6425bdc506e 100644
--- a/xlators/cluster/dht/src/dht-selfheal.c
+++ b/xlators/cluster/dht/src/dht-selfheal.c
@@ -364,7 +364,8 @@ dht_should_heal_layout (call_frame_t *frame, dht_layout_t **heal,
         ret = dht_layout_anomalies (frame->this, &local->loc, *ondisk,
                                     &local->selfheal.hole_cnt,
                                     &local->selfheal.overlaps_cnt,
-                                    NULL, &local->selfheal.down,
+                                    &local->selfheal.missing_cnt,
+                                    &local->selfheal.down,
                                     &local->selfheal.misc, NULL);
 
         if (ret < 0)
@@ -1022,115 +1023,6 @@ dht_layout_index_from_conf (dht_layout_t *layout, xlator_t *xlator)
         return i;
 }
 
-
-static int
-dht_selfheal_dir_xattr_for_nameless_lookup (call_frame_t *frame, loc_t *loc,
-                                            dht_layout_t  *layout)
-{
-        dht_local_t     *local = NULL;
-        int             missing_xattr = 0;
-        int             i = 0;
-        xlator_t        *this = NULL;
-        dht_conf_t      *conf = NULL;
-        dht_layout_t    *dummy = NULL;
-        int             j = 0;
-
-        local = frame->local;
-        this = frame->this;
-        conf = this->private;
-
-        for (i = 0; i < layout->cnt; i++) {
-                if (layout->list[i].err != -1 || !layout->list[i].stop) {
-                        /* err != -1 would mean xattr present on the directory
-                           or the directory is non existent.
-                           !layout->list[i].stop would mean layout absent
-                        */
-
-                        continue;
-                }
-                missing_xattr++;
-        }
-
-        /* Also account for subvolumes with no-layout. Used for zero'ing out
-           the layouts and for setting quota key's if present */
-
-        /* Send  where either the subvol is not part of layout,
-         * or it is part of the layout but error is non-zero but error
-         * is not equal to -1 or ENOENT.
-         */
-
-        for (i = 0; i < conf->subvolume_cnt; i++) {
-                if (dht_is_subvol_part_of_layout (layout, conf->subvolumes[i])
-                    == _gf_false) {
-                        missing_xattr++;
-                        continue;
-                }
-
-                j = dht_layout_index_from_conf (layout, conf->subvolumes[i]);
-
-                if ((j != -1) && (layout->list[j].err != -1) &&
-                   (layout->list[j].err != 0) &&
-                   (layout->list[j].err != ENOENT)) {
-                        missing_xattr++;
-                }
-
-        }
-
-
-        gf_msg_trace (this->name, 0,
-                      "%d subvolumes missing xattr for %s",
-                      missing_xattr, loc->path);
-
-        if (missing_xattr == 0) {
-                dht_selfheal_dir_finish (frame, this, 0, 1);
-                return 0;
-        }
-
-        local->call_cnt = missing_xattr;
-
-        if (gf_log_get_loglevel () >= GF_LOG_DEBUG)
-                dht_log_new_layout_for_dir_selfheal (this, loc, layout);
-
-        for (i = 0; i < layout->cnt; i++) {
-                if (layout->list[i].err != -1 || !layout->list[i].stop)
-                        continue;
-
-                dht_selfheal_dir_xattr_persubvol (frame, loc, layout, i, NULL);
-
-                if (--missing_xattr == 0)
-                        break;
-        }
-
-        dummy = dht_layout_new (this, 1);
-        if (!dummy)
-                goto out;
-
-        for (i = 0; i < conf->subvolume_cnt && missing_xattr; i++) {
-              if (dht_is_subvol_part_of_layout (layout, conf->subvolumes[i])
-                  == _gf_false) {
-                        dht_selfheal_dir_xattr_persubvol (frame, loc, dummy, 0,
-                                                          conf->subvolumes[i]);
-                        missing_xattr--;
-                        continue;
-              }
-
-                j = dht_layout_index_from_conf (layout, conf->subvolumes[i]);
-
-                if ((j != -1) && (layout->list[j].err != -1) &&
-                    (layout->list[j].err != ENOENT) &&
-                    (layout->list[j].err != 0)) {
-                        dht_selfheal_dir_xattr_persubvol (frame, loc, dummy, 0,
-                                                          conf->subvolumes[i]);
-                        missing_xattr--;
-                }
-        }
-
-        dht_layout_unref (this, dummy);
-out:
-        return 0;
-
-}
-
 int
 dht_selfheal_dir_setattr_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
                               int op_ret, int op_errno, struct iatt *statpre,
@@ -2303,7 +2195,8 @@ dht_selfheal_directory (call_frame_t *frame, dht_selfheal_dir_cbk_t dir_cbk,
         dht_layout_anomalies (this, loc, layout,
                               &local->selfheal.hole_cnt,
                               &local->selfheal.overlaps_cnt,
-                              NULL, &local->selfheal.down,
+                              &local->selfheal.missing_cnt,
+                              &local->selfheal.down,
                               &local->selfheal.misc, NULL);
 
         down     = local->selfheal.down;
@@ -2332,14 +2225,14 @@ dht_selfheal_directory (call_frame_t *frame, dht_selfheal_dir_cbk_t dir_cbk,
 
         dht_layout_sort_volname (layout);
         local->heal_layout = _gf_true;
-        ret = dht_selfheal_dir_getafix (frame, loc, layout);
 
-        if (ret == -1) {
-                gf_msg (this->name, GF_LOG_INFO, 0,
-                        DHT_MSG_DIR_SELFHEAL_FAILED,
-                        "Directory selfheal failed: "
-                        "Unable to form layout for directory %s",
-                        loc->path);
+        /* Ignore return value as it can be inferred from result of
+         * dht_layout_anomalies
+         */
+        dht_selfheal_dir_getafix (frame, loc, layout);
+
+        if (!(local->selfheal.hole_cnt || local->selfheal.overlaps_cnt ||
+              local->selfheal.missing_cnt)) {
                 local->heal_layout = _gf_false;
         }
 
@@ -2359,76 +2252,6 @@ sorry_no_fix:
 }
 
 int
-dht_selfheal_directory_for_nameless_lookup (call_frame_t *frame,
-                                            dht_selfheal_dir_cbk_t dir_cbk,
-                                            loc_t *loc, dht_layout_t *layout)
-{
-        dht_local_t     *local  = NULL;
-        uint32_t        down    = 0;
-        uint32_t        misc    = 0;
-        int             ret     = 0;
-        xlator_t        *this   = NULL;
-
-        local = frame->local;
-        this = frame->this;
-        dht_layout_anomalies (this, loc, layout,
-                              &local->selfheal.hole_cnt,
-                              &local->selfheal.overlaps_cnt,
-                              NULL, &local->selfheal.down,
-                              &local->selfheal.misc, NULL);
-
-        down     = local->selfheal.down;
-        misc     = local->selfheal.misc;
-
-        local->selfheal.dir_cbk = dir_cbk;
-        local->selfheal.layout = dht_layout_ref (this, layout);
-
-        if (down) {
-                gf_msg (this->name, GF_LOG_WARNING, 0,
-                        DHT_MSG_SUBVOL_DOWN_ERROR,
-                        "%d subvolumes down -- not fixing", down);
-                ret = 0;
-                goto sorry_no_fix;
-        }
-
-        if (misc) {
-                gf_msg (this->name, GF_LOG_WARNING, 0,
-                        DHT_MSG_SUBVOL_ERROR,
-                        "%d subvolumes have unrecoverable errors", misc);
-                ret = 0;
-                goto sorry_no_fix;
-        }
-
-        dht_layout_sort_volname (layout);
-        ret = dht_selfheal_dir_getafix (frame, loc, layout);
-
-        if (ret == -1) {
-                gf_msg (this->name, GF_LOG_WARNING, 0,
-                        DHT_MSG_LAYOUT_FORM_FAILED,
-                        "not able to form layout for the directory");
-                goto sorry_no_fix;
-        }
-
-        ret = dht_selfheal_layout_lock (frame, layout, _gf_false,
-                                     dht_selfheal_dir_xattr_for_nameless_lookup,
-                                        dht_should_heal_layout);
-
-        if (ret < 0) {
-                goto sorry_no_fix;
-        }
-
-        return 0;
-
-sorry_no_fix:
-        /* TODO: need to put appropriate local->op_errno */
-        dht_selfheal_dir_finish (frame, this, ret, 1);
-
-        return 0;
-
-
-}
-
-int
 dht_selfheal_restore (call_frame_t *frame, dht_selfheal_dir_cbk_t dir_cbk,
                       loc_t *loc, dht_layout_t *layout)
 {