From: Bobi Jam Date: Wed, 29 Jun 2011 08:31:27 +0000 (+0800) Subject: LU-471 fix mkfs.lustre to avoid creating duplicate options X-Git-Tag: 2.0.66.0~16 X-Git-Url: https://git.whamcloud.com/?p=fs%2Flustre-release.git;a=commitdiff_plain;h=c0f12e41fbe766785ea0f6bb2db3a4801a4f3ff5 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 Change-Id: Ic946b4bc93b2941bd05761a6783f01cf94fb1431 Reviewed-on: http://review.whamcloud.com/1035 Tested-by: Hudson Reviewed-by: Andreas Dilger Tested-by: Maloo --- diff --git a/lustre/tests/conf-sanity.sh b/lustre/tests/conf-sanity.sh index fd30426..990b215 100644 --- 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 --- 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. */