distribute: handle 'fix.layout' key in setxattr() instead of getxattr()

as 'fix.layout' command does changes to directory layout, its not a
'read/get' type of operation, and hence as per the symantics, it suits
setxattr() better

also fix a memory leak in getxattr(), where 'local' was allocated twice
in few cases

Signed-off-by: Amar Tumballi <amar@gluster.com>
Signed-off-by: Anand Avati <avati@gluster.com>

BUG: 3075 (the 'fix.layout' command should happen through 'setxattr', not getxattr().)
URL: http://bugs.gluster.com/cgi-bin/bugzilla3/show_bug.cgi?id=3075

2 files changed, 50 insertions, 73 deletions

diff --git a/xlators/cluster/dht/src/dht-common.c b/xlators/cluster/dht/src/dht-common.c
index 6bdc9d30139..18bb65d3098 100644
--- a/xlators/cluster/dht/src/dht-common.c
+++ b/xlators/cluster/dht/src/dht-common.c
@@ -1659,15 +1659,6 @@ err:
 }
 
 
-int
-dht_fix_layout_cbk (call_frame_t *frame, void *cookie,
-                    xlator_t *this, int32_t op_ret, int32_t op_errno)
-{
-        DHT_STACK_UNWIND (getxattr, frame, -1, ENODATA, NULL);
-
-        return 0;
-}
-
 static void
 fill_layout_info (dht_layout_t *layout, char *buf)
 {
@@ -1845,7 +1836,6 @@ dht_getxattr (call_frame_t *frame, xlator_t *this,
         xlator_t     **sub_volumes  = NULL;
         int           op_errno      = -1;
         int           ret           = 0;
-        int           flag          = 0;
         int           i             = 0;
         int           cnt           = 0;
 
@@ -1872,30 +1862,23 @@ dht_getxattr (call_frame_t *frame, xlator_t *this,
                 goto err;
         }
 
+        ret = loc_dup (loc, &local->loc);
+        if (ret == -1) {
+                op_errno = ENOMEM;
+                goto err;
+        }
+        local->layout = layout;
+
         if (key && (strcmp (key, GF_XATTR_PATHINFO_KEY) == 0)) {
                 hashed_subvol = dht_subvol_get_hashed (this, loc);
                 cached_subvol = dht_subvol_get_cached (this, loc->inode);
 
-                local = dht_local_init (frame);
-                if (!local) {
-                        op_errno = ENOMEM;
-
-                        goto err;
-                }
-
-                ret = loc_dup (loc, &local->loc);
-                if (ret == -1) {
-                        op_errno = ENOMEM;
-
-                        goto err;
-                }
                 local->key = gf_strdup (key);
                 if (!local->key) {
                         op_errno = ENOMEM;
 
                         goto err;
                 }
-                local->layout = layout;
 
                 local->call_cnt = 1;
                 if (hashed_subvol != cached_subvol) {
@@ -1924,40 +1907,6 @@ dht_getxattr (call_frame_t *frame, xlator_t *this,
                 op_errno = ENODATA;
                 goto err;
         }
-        if (key && (strcmp (key, GF_XATTR_FIX_LAYOUT_KEY) == 0)) {
-                for (i = 0; i < layout->cnt; i++) {
-                        if (layout->list[i].start == layout->list[i].stop) {
-                                flag = 1;
-                                break;
-                        }
-                }
-                if ((layout->cnt < conf->subvolume_cnt) || flag) {
-                        gf_log (this->name, GF_LOG_INFO,
-                                "expanding layout of %s from %d to %d",
-                                loc->path, layout->cnt, conf->subvolume_cnt);
-                        local = dht_local_init (frame);
-                        if (!local) {
-                                op_errno = ENOMEM;
-
-                                goto err;
-                        }
-
-                        ret = loc_dup (loc, &local->loc);
-                        if (ret == -1) {
-                                op_errno = ENOMEM;
-
-                                goto err;
-                        }
-                        local->layout = layout;
-                        //layout = dht_layout_new (this, conf->subvolume_cnt);
-
-                        dht_selfheal_new_directory (frame, dht_fix_layout_cbk,
-                                                    layout);
-                        return 0;
-                }
-                op_errno = ENODATA;
-                goto err;
-        }
 
         if (key && (!strcmp (GF_XATTR_MARKER_KEY, key))
             && (-1 == frame->root->pid)) {
@@ -2007,22 +1956,13 @@ dht_getxattr (call_frame_t *frame, xlator_t *this,
                 }
         }
 
-        ret = loc_dup (loc, &local->loc);
-        if (ret == -1) {
-                op_errno = ENOMEM;
-
-                goto err;
-        }
-
         if (key) {
                 local->key = gf_strdup (key);
                 if (!local->key) {
                         op_errno = ENOMEM;
-
                         goto err;
                 }
         }
-        local->layout = layout;
 
         if (loc->inode-> ia_type == IA_IFDIR) {
                 cnt = local->call_cnt = layout->cnt;
@@ -2088,6 +2028,15 @@ err:
 }
 
 
+static int
+dht_fix_layout_cbk (call_frame_t *frame, void *cookie,
+                    xlator_t *this, int32_t op_ret, int32_t op_errno)
+{
+        DHT_STACK_UNWIND (setxattr, frame, op_ret, op_errno);
+
+        return 0;
+}
+
 int
 dht_setxattr (call_frame_t *frame, xlator_t *this,
               loc_t *loc, dict_t *xattr, int flags)
@@ -2098,6 +2047,9 @@ dht_setxattr (call_frame_t *frame, xlator_t *this,
         dht_layout_t *layout   = NULL;
         int           i        = 0;
         int           op_errno = EINVAL;
+        int           flag     = 0;
+        int           ret      = -1;
+        data_t       *tmp      = NULL;
 
         VALIDATE_OR_GOTO (frame, err);
         VALIDATE_OR_GOTO (this, err);
@@ -2128,6 +2080,33 @@ dht_setxattr (call_frame_t *frame, xlator_t *this,
                 goto err;
         }
 
+        tmp = dict_get (xattr, GF_XATTR_FIX_LAYOUT_KEY);
+        if (tmp) {
+                for (i = 0; i < layout->cnt; i++) {
+                        if (layout->list[i].start == layout->list[i].stop) {
+                                flag = 1;
+                                break;
+                        }
+                }
+                if ((layout->cnt < conf->subvolume_cnt) || flag) {
+                        gf_log (this->name, GF_LOG_INFO,
+                                "expanding layout of %s from %d to %d",
+                                loc->path, layout->cnt, conf->subvolume_cnt);
+
+                        ret = loc_dup (loc, &local->loc);
+                        if (ret == -1) {
+                                op_errno = ENOMEM;
+                                goto err;
+                        }
+
+                        dht_selfheal_new_directory (frame, dht_fix_layout_cbk,
+                                                    layout);
+                        return 0;
+                }
+                op_errno = ENOTSUP;
+                goto err;
+        }
+
         local->call_cnt = layout->cnt;
 
         for (i = 0; i < layout->cnt; i++) {
diff --git a/xlators/mgmt/glusterd/src/glusterd-rebalance.c b/xlators/mgmt/glusterd/src/glusterd-rebalance.c
index 0a072e7741a..7f4a53a1702 100644
--- a/xlators/mgmt/glusterd/src/glusterd-rebalance.c
+++ b/xlators/mgmt/glusterd/src/glusterd-rebalance.c
@@ -304,7 +304,6 @@ int
 gf_glusterd_rebalance_fix_layout (glusterd_volinfo_t *volinfo, const char *dir)
 {
         int            ret             = -1;
-        char           value[128]      = {0,};
         char           full_path[1024] = {0,};
         struct stat    stbuf           = {0,};
         DIR           *fd              = NULL;
@@ -332,8 +331,8 @@ gf_glusterd_rebalance_fix_layout (glusterd_volinfo_t *volinfo, const char *dir)
 
                 if (S_ISDIR (stbuf.st_mode)) {
                         /* Fix the layout of the directory */
-                        sys_lgetxattr (full_path, "trusted.distribute.fix.layout",
-                                       &value, 128);
+                        sys_lsetxattr (full_path, "trusted.distribute.fix.layout",
+                                       "yes", 3, 0);
 
                         volinfo->defrag->total_files += 1;
 
@@ -366,7 +365,6 @@ glusterd_defrag_start (void *data)
         glusterd_defrag_info_t *defrag  = NULL;
         int                     ret     = -1;
         struct stat             stbuf   = {0,};
-        char                    value[128] = {0,};
 
         defrag = volinfo->defrag;
         if (!defrag)
@@ -388,8 +386,8 @@ glusterd_defrag_start (void *data)
         }
 
         /* Fix the root ('/') first */
-        sys_lgetxattr (defrag->mount, "trusted.distribute.fix.layout",
-                       &value, 128);
+        sys_lsetxattr (defrag->mount, "trusted.distribute.fix.layout",
+                       "yes", 3, 0);
 
         if ((defrag->cmd == GF_DEFRAG_CMD_START) ||
             (defrag->cmd == GF_DEFRAG_CMD_START_LAYOUT_FIX)) {