LU-471 fix mkfs.lustre to avoid creating duplicate options

Also make user specified options overwrite default internal default
options.

Signed-off-by: Bobi Jam <bobijam@whamcloud.com>
Change-Id: Ic946b4bc93b2941bd05761a6783f01cf94fb1431
Reviewed-on: http://review.whamcloud.com/1035
Tested-by: Hudson
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Tested-by: Maloo <whamcloud.maloo@gmail.com>

diff --git a/lustre/tests/conf-sanity.sh b/lustre/tests/conf-sanity.sh
index fd30426..990b215 100644 (file)
--- a/lustre/tests/conf-sanity.sh
+++ b/lustre/tests/conf-sanity.sh
@@ -2726,6 +2726,23 @@ test_59() {
 }
 run_test 59 "writeconf mount option"
 
+test_60() { # LU-471
+       add mds1 $MDS_MKFS_OPTS --mkfsoptions='\" -E stride=64 -O ^uninit_bg\"' --reformat $(mdsdevname 1)
+
+       dump=$(do_facet $SINGLEMDS dumpe2fs $(mdsdevname 1))
+       rc=${PIPESTATUS[0]}
+       [ $rc -eq 0 ] || error "dumpe2fs $(mdsdevname 1) failed"
+
+       # MDT default has dirdata feature
+       echo $dump | grep dirdata > /dev/null || error "dirdata is not set"
+       # we disable uninit_bg feature
+       echo $dump | grep uninit_bg > /dev/null && error "uninit_bg is set"
+       # we set stride extended options
+       echo $dump | grep stride > /dev/null || error "stride is not set"
+       reformat
+}
+run_test 60 "check mkfs.lustre --mkfsoptions -E -O options setting"
+
 if ! combined_mgs_mds ; then
        stop mgs
 fi

diff --git a/lustre/utils/mkfs_lustre.c b/lustre/utils/mkfs_lustre.c
index e49e9bd..4e0e57c 100644 (file)
--- a/lustre/utils/mkfs_lustre.c
+++ b/lustre/utils/mkfs_lustre.c
@@ -520,30 +520,68 @@ static int is_e2fsprogs_feature_supp(const char *feature)
         return ret;
 }
 
-static void enable_default_ext4_features(struct mkfs_opts *mop)
+/**
+ * append_unique: append @key or @key=@val pair to @buf only if @key does not
+ *                exists
+ *      @buf: buffer to hold @key or @key=@val
+ *      @prefix: prefix string before @key
+ *      @key: key string
+ *      @val: value string if it's a @key=@val pair
+ */
+static void append_unique(char *buf, char *prefix, char *key, char *val,
+                          size_t maxbuflen)
 {
-        if (IS_OST(&mop->mo_ldd))
-                strscat(mop->mo_mkfsopts, " -O extents,uninit_bg",
-                        sizeof(mop->mo_mkfsopts));
-        else if (IS_MDT(&mop->mo_ldd))
-                strscat(mop->mo_mkfsopts, " -O dirdata,uninit_bg",
-                        sizeof(mop->mo_mkfsopts));
-        else
-                strscat(mop->mo_mkfsopts, " -O uninit_bg",
-                        sizeof(mop->mo_mkfsopts));
+        char *anchor, *end;
+        int  len;
+
+        if (key == NULL)
+                return;
+
+        anchor = end = strstr(buf, key);
+        /* try to find exact match string in @buf */
+        while (end && *end != '\0' && *end != ',' && *end != ' ' && *end != '=')
+                ++end;
+        len = end - anchor;
+        if (anchor == NULL || strlen(key) != len ||
+            strncmp(anchor, key, len) != 0) {
+                if (prefix != NULL)
+                        strscat(buf, prefix, maxbuflen);
+
+                strscat(buf, key, maxbuflen);
+                if (val != NULL) {
+                        strscat(buf, "=", maxbuflen);
+                        strscat(buf, val, maxbuflen);
+                }
+        }
+}
+
+static void enable_default_ext4_features(struct mkfs_opts *mop, char *anchor,
+                                         size_t maxbuflen, int user_spec)
+{
+        if (IS_OST(&mop->mo_ldd)) {
+                append_unique(anchor, user_spec ? "," : " -O ",
+                              "extents", NULL, sizeof(mop->mo_mkfsopts));
+                append_unique(anchor, ",", "uninit_bg", NULL, maxbuflen);
+        } else if (IS_MDT(&mop->mo_ldd)) {
+                append_unique(anchor, user_spec ? "," : " -O ",
+                              "dirdata", NULL, maxbuflen);
+                append_unique(anchor, ",", "uninit_bg", NULL, maxbuflen);
+        } else {
+                append_unique(anchor, user_spec ? "," : " -O ",
+                              "uninit_bg", NULL, maxbuflen);
+        }
 
         /* Multiple mount protection enabled only if failover node specified */
         if (failover) {
                 if (is_e2fsprogs_feature_supp("-O mmp") == 0)
-                        strscat(mop->mo_mkfsopts, ",mmp",
-                                sizeof(mop->mo_mkfsopts));
+                        append_unique(anchor, ",", "mmp", NULL, maxbuflen);
                 else
                         disp_old_e2fsprogs_msg("mmp", 1);
         }
 
         /* Allow more than 65000 subdirectories */
         if (is_e2fsprogs_feature_supp("-O dir_nlink") == 0)
-                strscat(mop->mo_mkfsopts,",dir_nlink",sizeof(mop->mo_mkfsopts));
+                append_unique(anchor, ",", "dir_nlink", NULL, maxbuflen);
 
 #ifdef HAVE_EXT4_LDISKFS
         /* The following options are only valid for ext4-based ldiskfs.
@@ -553,12 +591,12 @@ static void enable_default_ext4_features(struct mkfs_opts *mop)
 
         /* Allow files larger than 2TB.  Also needs LU-16, but not harmful. */
         if (is_e2fsprogs_feature_supp("-O huge_file") == 0)
-                strscat(mop->mo_mkfsopts,",huge_file",sizeof(mop->mo_mkfsopts));
+                append_unique(anchor, ",", "huge_file", NULL, maxbuflen);
 
         /* Enable large block addresses if the LUN is over 2^32 blocks. */
         if (mop->mo_device_sz / (L_BLOCK_SIZE >> 10) >= 0x100002000ULL &&
                     is_e2fsprogs_feature_supp("-O 64bit") == 0)
-                strscat(mop->mo_mkfsopts, ",64bit", sizeof(mop->mo_mkfsopts));
+                append_unique(anchor, ",", "64bit", NULL, maxbuflen);
 
         /* Cluster inode/block bitmaps and inode table for more efficient IO.
          * Align the flex groups on a 1MB boundary for better performance. */
@@ -566,27 +604,67 @@ static void enable_default_ext4_features(struct mkfs_opts *mop)
         if (is_e2fsprogs_feature_supp("-O flex_bg") == 0) {
                 char tmp_buf[64];
 
-                strscat(mop->mo_mkfsopts, ",flex_bg", sizeof(mop->mo_mkfsopts));
+                append_unique(anchor, ",", "flex_bg", NULL, maxbuflen);
 
                 if (IS_OST(&mop->mo_ldd)) {
                         snprintf(tmp_buf, sizeof(tmp_buf), " -G %u",
                                  (1 << 20) / L_BLOCK_SIZE);
-                        strscat(mop->mo_mkfsopts, tmp_buf,
-                                sizeof(mop->mo_mkfsopts));
+                        strscat(anchor, tmp_buf, maxbuflen);
                 }
         }
         /* Don't add any more "-O" options here, see last comment above */
 #endif
 }
 
+/**
+ * moveopts_to_end: find the option string, move remaining strings to
+ *                  where option string starts, and append the option
+ *                  string at the end
+ *      @start: where the option string starts before the move
+ *      RETURN: where the option string starts after the move
+ */
+static char *moveopts_to_end(char *start)
+{
+        char save[512];
+        char *end, *idx;
+
+        /* skip whitespace before options */
+        end = start + 2;
+        while (*end == ' ')
+                ++end;
+
+        /* find end of option characters */
+        while (*end != ' ' && *end != '\0')
+                ++end;
+
+        /* save options */
+        strncpy(save, start, end - start);
+        save[end - start] = '\0';
+
+        /* move remaining options up front */
+        if (*end)
+                memmove(start, end, strlen(end));
+        *(start + strlen(end)) = '\0';
+
+        /* append the specified options */
+        if (*(start + strlen(start) - 1) != ' ')
+                strcat(start, " ");
+        idx = start + strlen(start);
+        strcat(start, save);
+
+        return idx;
+}
+
 /* Build fs according to type */
 int make_lustre_backfs(struct mkfs_opts *mop)
 {
         __u64 device_sz = mop->mo_device_sz, block_count = 0;
         char mkfs_cmd[PATH_MAX];
         char buf[64];
+        char *start;
         char *dev;
-        int ret = 0;
+        int ret = 0, ext_opts = 0;
+        size_t maxbuflen;
 
         if (!(mop->mo_flags & MO_IS_LOOP)) {
                 mop->mo_device_sz = get_device_size(mop->mo_device);
@@ -720,8 +798,43 @@ int make_lustre_backfs(struct mkfs_opts *mop)
                                 sizeof(mop->mo_mkfsopts));
                 }
 
-                if (strstr(mop->mo_mkfsopts, "-O") == NULL)
-                        enable_default_ext4_features(mop);
+                /* start handle -O mkfs options */
+                if ((start = strstr(mop->mo_mkfsopts, "-O")) != NULL) {
+                        if (strstr(start + 2, "-O") != NULL) {
+                                fprintf(stderr,
+                                        "%s: don't specify multiple -O options\n",
+                                        progname);
+                                return EINVAL;
+                        }
+                        start = moveopts_to_end(start);
+                        maxbuflen = sizeof(mop->mo_mkfsopts) -
+                                    (start - mop->mo_mkfsopts) - strlen(start);
+                        enable_default_ext4_features(mop, start, maxbuflen, 1);
+                } else {
+                        start = mop->mo_mkfsopts + strlen(mop->mo_mkfsopts),
+                        maxbuflen = sizeof(mop->mo_mkfsopts) -
+                                    strlen(mop->mo_mkfsopts);
+                        enable_default_ext4_features(mop, start, maxbuflen, 0);
+                }
+                /* end handle -O mkfs options */
+
+                /* start handle -E mkfs options */
+                if ((start = strstr(mop->mo_mkfsopts, "-E")) != NULL) {
+                        if (strstr(start + 2, "-E") != NULL) {
+                                fprintf(stderr,
+                                        "%s: don't specify multiple -E options\n",
+                                        progname);
+                                return EINVAL;
+                        }
+                        start = moveopts_to_end(start);
+                        maxbuflen = sizeof(mop->mo_mkfsopts) -
+                                    (start - mop->mo_mkfsopts) - strlen(start);
+                        ext_opts = 1;
+                } else {
+                        start = mop->mo_mkfsopts + strlen(mop->mo_mkfsopts);
+                        maxbuflen = sizeof(mop->mo_mkfsopts) -
+                                    strlen(mop->mo_mkfsopts);
+                }
 
                 /* In order to align the filesystem metadata on 1MB boundaries,
                  * give a resize value that will reserve a power-of-two group
@@ -735,17 +848,17 @@ int make_lustre_backfs(struct mkfs_opts *mop)
                         unsigned resize_blks;
 
                         resize_blks = (1ULL<<32) - desc_per_block*group_blocks;
-                        snprintf(buf, sizeof(buf)," -E resize=%u,",resize_blks);
-                } else {
-                        strncpy(buf, " -E ", sizeof(buf));
+                        snprintf(buf, sizeof(buf), "%u", resize_blks);
+                        append_unique(start, ext_opts ? "," : " -E ",
+                                      "resize", buf, maxbuflen);
+                        ext_opts = 1;
                 }
 
                 /* Avoid zeroing out the full journal - speeds up mkfs */
                 if (is_e2fsprogs_feature_supp("-E lazy_journal_init") == 0)
-                        strscat(buf, "lazy_journal_init,", sizeof(buf));
-
-                if (strlen(buf) > strlen(" -E "))
-                        strscat(mop->mo_mkfsopts, buf,sizeof(mop->mo_mkfsopts));
+                        append_unique(start, ext_opts ? "," : " -E ",
+                                      "lazy_journal_init", NULL, maxbuflen);
+                /* end handle -E mkfs options */
 
                 /* Allow reformat of full devices (as opposed to
                    partitions.)  We already checked for mounted dev. */