glusterd: Fixed glusterd_brick_create_path algo.

- check if any prefix of the brick path has "trusted.gfid"
  or "trusted.glusterfs.volume-id" set.
- set trusted.glusterfs.volume-id on the bricks as soon as
  its induction into the volume is settled. Earlier, the setting of
  "volume-id" used to happen during the first run of the brick process,
  leaving of window for bricks part of one volume to be (ab)used by another
  volume inadvertently.
- removed creation of brick directory (if missing), during start volume force.
  This is to avoid directory creation as part 'force'ful starting of volume
  and leave the responsibility with the user, who understands the
  'availability' of the export directory (brick) better.

Change-Id: I4237ec4ea7a4e38a7501027e7de7112edd67de8c
BUG: 812214
Signed-off-by: Krishnan Parthasarathi <kp@gluster.com>
Reviewed-on: http://review.gluster.com/3280
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Vijay Bellur <vijay@gluster.com>
Reviewed-on: http://review.gluster.com/3313

7 files changed, 172 insertions, 131 deletions

diff --git a/xlators/mgmt/glusterd/src/glusterd-brick-ops.c b/xlators/mgmt/glusterd/src/glusterd-brick-ops.c
index 72418684649..120fd5027e2 100644
--- a/xlators/mgmt/glusterd/src/glusterd-brick-ops.c
+++ b/xlators/mgmt/glusterd/src/glusterd-brick-ops.c
@@ -1164,7 +1164,7 @@ glusterd_op_stage_add_brick (dict_t *dict, char **op_errstr)
                         ret = glusterd_brick_create_path (brickinfo->hostname,
                                                           brickinfo->path,
                                                           volinfo->volume_id,
-                                                          0777, op_errstr);
+                                                          op_errstr);
                         if (ret)
                                 goto out;
                 }
diff --git a/xlators/mgmt/glusterd/src/glusterd-hooks.c b/xlators/mgmt/glusterd/src/glusterd-hooks.c
index 2afdb1625c6..ba428b0f359 100644
--- a/xlators/mgmt/glusterd/src/glusterd-hooks.c
+++ b/xlators/mgmt/glusterd/src/glusterd-hooks.c
@@ -39,8 +39,6 @@
 
 #include <fnmatch.h>
 
-extern int mkdir_if_missing (char *dir);
-
 #define EMPTY ""
 char glusterd_hook_dirnames[GD_OP_MAX][256] =
 {
@@ -95,7 +93,7 @@ glusterd_hooks_create_hooks_directory (char *basedir)
         priv = THIS->private;
 
         snprintf (path, sizeof (path), "%s/hooks", basedir);
-        ret = mkdir_if_missing (path);
+        ret = mkdir_if_missing (path, NULL);
         if (ret) {
                 gf_log (THIS->name, GF_LOG_CRITICAL, "Unable to create %s due"
                          "to %s", path, strerror (errno));
@@ -103,7 +101,7 @@ glusterd_hooks_create_hooks_directory (char *basedir)
         }
 
         GLUSTERD_GET_HOOKS_DIR (version_dir, GLUSTERD_HOOK_VER, priv);
-        ret = mkdir_if_missing (version_dir);
+        ret = mkdir_if_missing (version_dir, NULL);
         if (ret) {
                 gf_log (THIS->name, GF_LOG_CRITICAL, "Unable to create %s due "
                         "to %s", version_dir, strerror (errno));
@@ -117,7 +115,7 @@ glusterd_hooks_create_hooks_directory (char *basedir)
 
                 snprintf (path, sizeof (path), "%s/%s", version_dir,
                           cmd_subdir);
-                ret = mkdir_if_missing (path);
+                ret = mkdir_if_missing (path, NULL);
                 if (ret) {
                         gf_log (THIS->name, GF_LOG_CRITICAL,
                                 "Unable to create %s due to %s",
@@ -129,7 +127,7 @@ glusterd_hooks_create_hooks_directory (char *basedir)
                      type++) {
                         snprintf (path, sizeof (path), "%s/%s/%s",
                                   version_dir, cmd_subdir, type_subdir[type]);
-                        ret = mkdir_if_missing (path);
+                        ret = mkdir_if_missing (path, NULL);
                         if (ret) {
                                 gf_log (THIS->name, GF_LOG_CRITICAL,
                                         "Unable to create %s due to %s",
diff --git a/xlators/mgmt/glusterd/src/glusterd-replace-brick.c b/xlators/mgmt/glusterd/src/glusterd-replace-brick.c
index fdfa9c020e1..58829832683 100644
--- a/xlators/mgmt/glusterd/src/glusterd-replace-brick.c
+++ b/xlators/mgmt/glusterd/src/glusterd-replace-brick.c
@@ -321,8 +321,7 @@ glusterd_op_stage_replace_brick (dict_t *dict, char **op_errstr,
                 break;
 
         case GF_REPLACE_OP_ABORT:
-                if ((!glusterd_is_rb_paused (volinfo)) &&
-                     (!glusterd_is_rb_started (volinfo))) {
+                if (!glusterd_is_rb_ongoing (volinfo)) {
                         gf_log ("", GF_LOG_ERROR, "Replace brick is not"
                                 " started or paused for volume ");
                         ret = -1;
@@ -331,7 +330,7 @@ glusterd_op_stage_replace_brick (dict_t *dict, char **op_errstr,
                 break;
 
         case GF_REPLACE_OP_COMMIT:
-                if (!glusterd_is_rb_started (volinfo)) {
+                if (!glusterd_is_rb_ongoing (volinfo)) {
                         gf_log ("", GF_LOG_ERROR, "Replace brick is not "
                                 "started for volume ");
                         ret = -1;
@@ -445,7 +444,7 @@ glusterd_op_stage_replace_brick (dict_t *dict, char **op_errstr,
         if (ret)
                 goto out;
 
-       if ((volinfo->rb_status ==GF_RB_STATUS_NONE) &&
+       if (!glusterd_is_rb_ongoing (volinfo) &&
             (replace_op == GF_REPLACE_OP_START ||
              replace_op == GF_REPLACE_OP_COMMIT_FORCE)) {
 
@@ -463,13 +462,15 @@ glusterd_op_stage_replace_brick (dict_t *dict, char **op_errstr,
                 goto out;
        }
 
-        if (!glusterd_is_local_addr (host)) {
+        if (!glusterd_is_rb_ongoing (volinfo)) {
                 ret = glusterd_brick_create_path (host, path,
-                                                  volinfo->volume_id, 0777,
+                                                  volinfo->volume_id,
                                                   op_errstr);
                 if (ret)
                         goto out;
-        } else {
+        }
+
+        if (glusterd_is_local_addr (host)) {
                 ret = glusterd_friend_find (NULL, host, &peerinfo);
                 if (ret) {
                         snprintf (msg, sizeof (msg), "%s, is not a friend",
diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c
index 8e28a7a1b3c..f04792c7b9a 100644
--- a/xlators/mgmt/glusterd/src/glusterd-utils.c
+++ b/xlators/mgmt/glusterd/src/glusterd-utils.c
@@ -4367,115 +4367,158 @@ glusterd_rb_check_bricks (glusterd_volinfo_t *volinfo,
         return 0;
 }
 
-int
-glusterd_brick_create_path (char *host, char *path, uuid_t uuid, mode_t mode,
-                            char **op_errstr)
+/*path needs to be absolute; works only on gfid, volume-id*/
+static int
+glusterd_is_uuid_present (char *path, char *xattr, gf_boolean_t *present)
 {
-        int     ret = -1;
-        char    msg[2048] = {0};
-        struct  stat st_buf = {0};
-        uuid_t  gfid = {0,};
-        uuid_t  old_uuid = {0,};
-
-        ret = stat (path, &st_buf);
-        if ((!ret) && (!S_ISDIR (st_buf.st_mode))) {
-                snprintf (msg, sizeof (msg), "brick %s:%s, "
-                          "path %s is not a directory", host, path, path);
-                gf_log ("", GF_LOG_ERROR, "%s", msg);
-                ret = -1;
+        GF_ASSERT (path);
+        GF_ASSERT (xattr);
+        GF_ASSERT (present);
+
+        int     ret      = -1;
+        uuid_t  uid     = {0,};
+
+        if (!path || !xattr || !present)
                 goto out;
-        } else if (!ret) {
-                goto check_xattr;
+
+        ret = sys_lgetxattr (path, xattr, &uid, 16);
+        if (ret < 0 && errno != ENODATA) {
+                goto out;
+
+        } else if (ret >= 0) {
+                *present = _gf_true;
+
         } else {
-                ret = mkdir (path, mode);
-                if (ret) {
-                        snprintf (msg, sizeof (msg), "brick: %s:%s, path "
-                                  "creation failed, reason: %s",
-                                  host, path, strerror(errno));
-                        gf_log ("glusterd", GF_LOG_ERROR, "%s", msg);
+                *present = _gf_false;
+        }
+
+        ret = 0;
+out:
+        return ret;
+}
+
+/*path needs to be absolute*/
+static int
+glusterd_is_path_in_use (char *path, gf_boolean_t *in_use, char **op_errstr)
+{
+        int             i               = 0;
+        int             ret             = -1;
+        gf_boolean_t    used            = _gf_false;
+        char            dir[PATH_MAX]   = {0,};
+        char            *curdir         = NULL;
+        char            msg[2048]       = {0};
+        char            *keys[3]         = {GFID_XATTR_KEY,
+                                            GF_XATTR_VOL_ID_KEY,
+                                            NULL};
+
+        GF_ASSERT (path);
+        if (!path)
+                goto out;
+
+        strcpy (dir, path);
+        curdir = dir;
+        do {
+                for (i = 0; !used && keys[i]; i++) {
+                        ret = glusterd_is_uuid_present (curdir, keys[i], &used);
+                        if (ret)
+                                goto out;
+                }
+
+                if (used)
+                        break;
+
+                curdir = dirname (dir);
+                if (!strcmp (curdir, "."))
                         goto out;
-                } else {
-                        goto check_xattr;
+
+
+        } while (strcmp (curdir, "/"));
+
+        if (!strcmp (curdir, "/")) {
+                for (i = 0; !used && keys[i]; i++) {
+                        ret = glusterd_is_uuid_present (curdir, keys[i], &used);
+                        if (ret)
+                                goto out;
                 }
         }
 
-/* To check if filesystem is read-only
-   and if it supports extended attributes */
-check_xattr:
+        ret = 0;
+        *in_use = used;
+out:
+        if (ret) {
+                snprintf (msg, sizeof (msg), "Failed to get extended "
+                          "attribute %s, reason: %s", keys[i],
+                          strerror (errno));
+        }
+
+        if (*in_use) {
+                snprintf (msg, sizeof (msg), "%s or a prefix of it is "
+                          "already part of a volume", path);
+        }
+
+        if (strlen (msg)) {
+                gf_log (THIS->name, GF_LOG_ERROR, "%s", msg);
+                *op_errstr = gf_strdup (msg);
+        }
+
+        return ret;
+}
+
+int
+glusterd_brick_create_path (char *host, char *path, uuid_t uuid,
+                            char **op_errstr)
+{
+        int             ret             = -1;
+        char            msg[2048]       = {0,};
+        gf_boolean_t    in_use          = _gf_false;
+        gf_boolean_t    created         = _gf_true;
+
+        ret = mkdir_if_missing (path, &created);
+        if (ret)
+                goto out;
+
+        /* Check for xattr support in backend fs */
         ret = sys_lsetxattr (path, "trusted.glusterfs.test",
                              "working", 8, 0);
         if (ret) {
-                snprintf (msg, sizeof (msg), "glusterfs is not"
+                snprintf (msg, sizeof (msg), "Glusterfs is not"
                           " supported on brick: %s:%s.\nSetting"
                           " extended attributes failed, reason:"
                           " %s.", host, path, strerror(errno));
-                gf_log ("glusterd", GF_LOG_ERROR, "%s", msg);
                 goto out;
+
         } else {
-                /* Remove xattr *cannot* fail after setting it succeeded */
                 sys_lremovexattr (path, "trusted.glusterfs.test");
+
         }
 
-        /* Now check if the export directory has some other 'gfid',
-           other than that of root '/' */
-        ret = sys_lgetxattr (path, "trusted.gfid", gfid, 16);
-        if (ret == 16) {
-                if (!__is_root_gfid (gfid)) {
-                        gf_log (THIS->name, GF_LOG_WARNING,
-                                "%s: gfid (%s) is not that of glusterfs '/' ",
-                                path, uuid_utoa (gfid));
-                        snprintf (msg, sizeof (msg),
-                                  "'%s:%s' gfid (%s) is not that of "
-                                  "glusterfs '/' ", host, path, uuid_utoa (gfid));
-                        ret = -1;
-                        goto out;
-                }
-        } else if (ret != -1) {
-                /* Wrong 'gfid' is set, it should be error */
-                ret = -1;
-                snprintf (msg, sizeof (msg), "'%s:%s' has wrong entry"
-                          "for 'gfid'.", host, path);
-                goto out;
-        } else if ((ret == -1) && (errno != ENODATA)) {
-                /* Wrong 'gfid' is set, it should be error */
-                snprintf (msg, sizeof (msg), "'%s:%s' has failed to fetch "
-                          "'gfid' (%s)", host, path, strerror (errno));
+        ret = glusterd_is_path_in_use (path, &in_use, op_errstr);
+        if (ret)
                 goto out;
-        }
 
-        ret = 0;
-        if (!uuid)
-                goto out;
-
-        /* This 'key' is set when the volume is started for the first time */
-        ret = sys_lgetxattr (path, "trusted.glusterfs.volume-id",
-                             old_uuid, 16);
-        if ((ret >= 0 && ret != 16) || ((ret == 16) &&
-                                        uuid_compare (old_uuid, uuid))) {
-                snprintf (msg, sizeof (msg), "'%s:%s' has been part of "
-                          "a volume with id %s. Please re-create the brick "
-                          "directory.", host, path, uuid_utoa (old_uuid));
-                gf_log (THIS->name, GF_LOG_WARNING, "%s", msg);
+        if (in_use) {
                 ret = -1;
                 goto out;
+        }
 
-        } else if ((ret == -1) && (errno != ENODATA)) {
-                snprintf (msg, sizeof (msg), "'%s:%s' : failed to fetch "
-                          "'volume-id' (%s)", host, path, strerror (errno));
-                gf_log (THIS->name, GF_LOG_WARNING, "%s", msg);
+        ret = sys_lsetxattr (path, GF_XATTR_VOL_ID_KEY, uuid, 16,
+                             XATTR_CREATE);
+        if (ret) {
+                snprintf (msg, sizeof (msg), "Failed to set extended "
+                          "attributes %s, reason: %s",
+                          GF_XATTR_VOL_ID_KEY, strerror (errno));
+                if (created)
+                        rmdir (path);
                 goto out;
-
         }
-        /* if 'ret == -1' then 'volume-id' not set, seems to be a fresh
-           directory */
 
         ret = 0;
 out:
-        if (msg[0] != '\0')
+        if (strlen (msg))
                 *op_errstr = gf_strdup (msg);
 
-        gf_log ("", GF_LOG_DEBUG, "returning %d", ret);
         return ret;
+
 }
 
 int
@@ -4816,19 +4859,35 @@ glusterd_delete_all_bricks (glusterd_volinfo_t* volinfo)
         return ret;
 }
 
+/* @new should be used by caller only if ret is zero.
+ * caller should set @new to 'true' by default.*/
 int
-mkdir_if_missing (char *path)
+mkdir_if_missing (char *path, gf_boolean_t *new)
 {
         struct stat st = {0,};
         int        ret = 0;
+        gf_boolean_t created = _gf_true;
 
         ret = mkdir (path, 0777);
-        if (!ret || errno == EEXIST)
-                ret = stat (path, &st);
-        if (ret == -1 || !S_ISDIR (st.st_mode))
+        if (ret && errno != EEXIST)
+                goto out;
+
+        if (ret && errno == EEXIST)
+                created = _gf_false;
+
+        ret = stat (path, &st);
+        if (ret == -1 || !S_ISDIR (st.st_mode)) {
+                ret = -1;
+                goto out;
+        }
+
+        if (new)
+                *new = created;
+
+out:
+        if (ret)
                 gf_log ("", GF_LOG_WARNING, "Failed to create the"
                         " directory %s", path);
-
         return ret;
 }
 
@@ -4859,7 +4918,7 @@ glusterd_start_gsync (glusterd_volinfo_t *master_vol, char *slave,
                 goto out;
 
         snprintf (buf, PATH_MAX, "%s/"GEOREP"/%s", priv->workdir, master_vol->volname);
-        ret = mkdir_if_missing (buf);
+        ret = mkdir_if_missing (buf, NULL);
         if (ret) {
                 errcode = -1;
                 goto out;
@@ -4867,7 +4926,7 @@ glusterd_start_gsync (glusterd_volinfo_t *master_vol, char *slave,
 
         snprintf (buf, PATH_MAX, DEFAULT_LOG_FILE_DIRECTORY"/"GEOREP"/%s",
                   master_vol->volname);
-        ret = mkdir_if_missing (buf);
+        ret = mkdir_if_missing (buf, NULL);
         if (ret) {
                 errcode = -1;
                 goto out;
diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.h b/xlators/mgmt/glusterd/src/glusterd-utils.h
index 7b71f05a459..d149cb9d0dd 100644
--- a/xlators/mgmt/glusterd/src/glusterd-utils.h
+++ b/xlators/mgmt/glusterd/src/glusterd-utils.h
@@ -57,6 +57,15 @@ typedef struct glusterd_voldict_ctx_ {
         char    *val_name;
 } glusterd_voldict_ctx_t;
 
+/* Moved the definition from gluster-utils.c avoiding
+ * extern'ing in multiple places.
+ * (Indeed, XXX: we'd rather need a general
+ * "mkdir -p" like routine in libglusterfs)
+*/
+
+int
+mkdir_if_missing (char *path, gf_boolean_t *new);
+
 int
 glusterd_compare_lines (const void *a, const void *b);
 
@@ -319,7 +328,7 @@ glusterd_rb_check_bricks (glusterd_volinfo_t *volinfo,
                           glusterd_brickinfo_t *dst_brick);
 
 int
-glusterd_brick_create_path (char *host, char *path, uuid_t uuid, mode_t mode,
+glusterd_brick_create_path (char *host, char *path, uuid_t uuid,
                             char **op_errstr);
 int
 glusterd_sm_tr_log_transition_add (glusterd_sm_tr_log_t *log,
diff --git a/xlators/mgmt/glusterd/src/glusterd-volume-ops.c b/xlators/mgmt/glusterd/src/glusterd-volume-ops.c
index 4ee9582fd4f..9ee28733a4f 100644
--- a/xlators/mgmt/glusterd/src/glusterd-volume-ops.c
+++ b/xlators/mgmt/glusterd/src/glusterd-volume-ops.c
@@ -718,7 +718,7 @@ glusterd_op_stage_create_volume (dict_t *dict, char **op_errstr)
                         ret = glusterd_brick_create_path (brick_info->hostname,
                                                           brick_info->path,
                                                           volume_uuid,
-                                                          0777, op_errstr);
+                                                          op_errstr);
                         if (ret)
                                 goto out;
                         brick_list = tmpptr;
@@ -843,21 +843,6 @@ glusterd_op_stage_start_volume (dict_t *dict, char **op_errstr)
                         goto out;
                 }
 
-                /* we should not be creating the directory if 'force' option
-                   is not given. This may lead to issues where the actual data
-                   disk is not mounted after a machine reboot, but because
-                   'glusterd' restarts the processes, the export directories
-                   can be automatically created and brick would start */
-                if ((flags & GF_CLI_FLAG_OP_FORCE) &&
-                    !uuid_compare (brickinfo->uuid, priv->uuid)) {
-                        ret = glusterd_brick_create_path (brickinfo->hostname,
-                                                          brickinfo->path,
-                                                          volinfo->volume_id,
-                                                          0777, op_errstr);
-                        if (ret)
-                                goto out;
-                }
-
                 if (!(flags & GF_CLI_FLAG_OP_FORCE)) {
                         if (glusterd_is_volume_started (volinfo)) {
                                 snprintf (msg, sizeof (msg), "Volume %s already"
diff --git a/xlators/mgmt/glusterd/src/glusterd.c b/xlators/mgmt/glusterd/src/glusterd.c
index 6acf3daffce..61d77043950 100644
--- a/xlators/mgmt/glusterd/src/glusterd.c
+++ b/xlators/mgmt/glusterd/src/glusterd.c
@@ -246,18 +246,6 @@ out:
         return ret;
 }
 
-/* defined in glusterd-utils.c -- no
- * glusterd header where it would be
- * appropriate to put to, and too
- * accidental routine to place in
- * libglusterfs.
- *
- * (Indeed, XXX: we'd rather need a general
- * "mkdir -p" like routine in
- * libglusterfs)
- */
-extern int mkdir_if_missing (char *path);
-
 #if SYNCDAEMON_COMPILE
 static int
 glusterd_check_gsync_present (int *valid_state)
@@ -356,7 +344,7 @@ glusterd_crt_georep_folders (char *georepdir, glusterd_conf_t *conf)
         }
 
         snprintf (georepdir, PATH_MAX, "%s/"GEOREP, conf->workdir);
-        ret = mkdir_if_missing (georepdir);
+        ret = mkdir_if_missing (georepdir, NULL);
         if (-1 == ret) {
                 gf_log ("glusterd", GF_LOG_CRITICAL,
                         "Unable to create "GEOREP" directory %s",
@@ -371,7 +359,7 @@ glusterd_crt_georep_folders (char *georepdir, glusterd_conf_t *conf)
                         georepdir);
                 goto out;
         }
-        ret = mkdir_if_missing (DEFAULT_LOG_FILE_DIRECTORY"/"GEOREP);
+        ret = mkdir_if_missing (DEFAULT_LOG_FILE_DIRECTORY"/"GEOREP, NULL);
         if (-1 == ret) {
                 gf_log ("glusterd", GF_LOG_CRITICAL,
                         "Unable to create "GEOREP" log directory");
@@ -385,7 +373,8 @@ glusterd_crt_georep_folders (char *georepdir, glusterd_conf_t *conf)
                         georepdir);
                 goto out;
         }
-        ret = mkdir_if_missing (DEFAULT_LOG_FILE_DIRECTORY"/"GEOREP"-slaves");
+        ret = mkdir_if_missing (DEFAULT_LOG_FILE_DIRECTORY"/"GEOREP"-slaves",
+                                NULL);
         if (-1 == ret) {
                 gf_log ("glusterd", GF_LOG_CRITICAL,
                         "Unable to create "GEOREP" slave log directory");