b=19689

i=adilger
i=bobijam
i=nathan
o=Jim Garlick

Change tunefs.lustre and mkfs.lustre --mountfsoptions so that
exactly the specified mount options are used.
Leaving off any "mandatory" mount options is an error.
Leaving off any default mount options causes a warning, but is allowed.
Change errors=remount-ro from mandatory to default.
Sanitize the mount string before storing it.
Update man pages accordingly.

This hopefully makes two awkward situations less so:
- making errors=panic the default (before we had to append errors=remount-ro
  and hope ldiskfs parsing caused the last option to override the first)
- setting other mount options on the OST's dropped mballoc,extents
  without warning.

diff --git a/lustre/doc/mkfs.lustre.8 b/lustre/doc/mkfs.lustre.8
index a891bf3..32a4098 100644 (file)
--- a/lustre/doc/mkfs.lustre.8
+++ b/lustre/doc/mkfs.lustre.8
@@ -59,7 +59,14 @@ Force a particular OST or MDT index
 Format options for the backing fs. For example, ext3 options could be set here.
 .TP
 .BI \--mountfsoptions= opts
 Format options for the backing fs. For example, ext3 options could be set here.
 .TP
 .BI \--mountfsoptions= opts

-Set permanent mount options, equivalent to setting in /etc/fstab
+Set the mount options that will be used when mounting the backing fs.
+WARNING: unlike earlier versions of \fBmkfs.lustre\fR, this version completely
+replaces the default mount options with those specified on the command line,
+issuing a warning on stderr if any of the default mount options are omitted.
+The defaults for \fIldiskfs\fR are
+OST: \fIerrors=remount-ro,mballoc,extents\fR;
+MGS/MDT: \fIerrors=remount-ro,iopen_nopriv,user_xattr\fR.
+\fBDO NOT\fR alter the default mount options unless you know what you are doing.

 .TP
 .BI \--mgsnode= nid,...  
 Set the NID(s) of the MGS node, required for all targets other than the MGS.
 .TP
 .BI \--mgsnode= nid,...  
 Set the NID(s) of the MGS node, required for all targets other than the MGS.

diff --git a/lustre/doc/tunefs.lustre.8 b/lustre/doc/tunefs.lustre.8
index ec1c46b..9993971 100644 (file)
--- a/lustre/doc/tunefs.lustre.8
+++ b/lustre/doc/tunefs.lustre.8
@@ -43,7 +43,14 @@ The Lustre filesystem this service will be part of.  Default is 'lustre'
 Force a particular OST or MDT index 
 .TP
 .BI \--mountfsoptions= opts
 Force a particular OST or MDT index 
 .TP
 .BI \--mountfsoptions= opts

-Set permanent mount options, equivalent to setting in /etc/fstab
+Set  the mount options that will be used when mounting the backing fs.
+WARNING: unlike earlier versions of \fBtunefs.lustre\fR,  this version
+completely replaces the existing mount options with those specified on
+the command line, issuing a warning  on  stderr  if any  of the default
+mount options are omitted.  The defaults for ldiskfs  are
+OST: \fIerrors=remount-ro,mballoc,extents\fR;
+MGS/MDT: \fIerrors=remount-ro,iopen_nopriv,user_xattr\fR.
+\fBDO NOT\fR alter the default mount options unless you know what you are doing.

 .TP
 .BI \--mgs
 Add a configuration management service to this target
 .TP
 .BI \--mgs
 Add a configuration management service to this target

diff --git a/lustre/tests/cfg/local.sh b/lustre/tests/cfg/local.sh
index 99c8950..d4c32b6 100644 (file)
--- a/lustre/tests/cfg/local.sh
+++ b/lustre/tests/cfg/local.sh
@@ -22,7 +22,7 @@ for num in $(seq $MDSCOUNT); do
 done
 MDSDEVBASE=${MDSDEVBASE:-$TMP/${FSNAME}-mdt}
 MDSSIZE=${MDSSIZE:-200000}
 done
 MDSDEVBASE=${MDSDEVBASE:-$TMP/${FSNAME}-mdt}
 MDSSIZE=${MDSSIZE:-200000}

-MDSOPT=${MDSOPT:-"--mountfsoptions=acl"}
+MDSOPT=${MDSOPT:-"--mountfsoptions=errors=remount-ro,iopen_nopriv,user_xattr,acl"}

 
 MGSDEV=${MGSDEV:-$MDSDEV1}
 MGSSIZE=${MGSSIZE:-$MDSSIZE}
 
 MGSDEV=${MGSDEV:-$MDSDEV1}
 MGSSIZE=${MGSSIZE:-$MDSSIZE}

diff --git a/lustre/utils/mkfs_lustre.c b/lustre/utils/mkfs_lustre.c
index 95f7260..59e68cc 100644 (file)
--- a/lustre/utils/mkfs_lustre.c
+++ b/lustre/utils/mkfs_lustre.c
@@ -62,6 +62,7 @@
 #include <string.h>
 #include <getopt.h>
 #include <limits.h>
 #include <string.h>
 #include <getopt.h>
 #include <limits.h>

+#include <ctype.h>

 
 #ifdef __linux__
 /* libcfs.h is not really needed here, but on SLES10/PPC, fs.h includes idr.h which
 
 #ifdef __linux__
 /* libcfs.h is not really needed here, but on SLES10/PPC, fs.h includes idr.h which


@@ -1403,6 +1404,76 @@ int parse_opts(int argc, char *const argv[], struct mkfs_opts *mop,
         return 0;
 }
 
         return 0;
 }
 

+/* Search for opt in mntlist, returning true if found.
+ */
+static int in_mntlist(char *opt, char *mntlist)
+{
+        char *ml, *mlp, *item, *ctx;
+
+        if (!(ml = strdup(mntlist))) {
+                fprintf(stderr, "%s: out of memory\n", progname);
+                exit(1);
+        }
+        mlp = ml;
+        while ((item = strtok_r(mlp, ",", &ctx))) {
+                if (!strcmp(opt, item))
+                        break;
+                mlp = NULL;
+        }
+        free(ml);
+        return (item != NULL);
+}
+
+/* Issue a message on stderr for every item in wanted_mountopts that is not
+ * present in mountopts.  The justwarn boolean toggles between error and
+ * warning message.  Return an error count.
+ */
+static int check_mountfsoptions(char *mountopts, char *wanted_mountopts,
+                                int justwarn)
+{
+        char *ml, *mlp, *item, *ctx;
+        int errors = 0;
+
+        if (!(ml = strdup(wanted_mountopts))) {
+                fprintf(stderr, "%s: out of memory\n", progname);
+                exit(1);
+        }
+        mlp = ml;
+        while ((item = strtok_r(mlp, ",", &ctx))) {
+                if (!in_mntlist(item, mountopts)) {
+                        fprintf(stderr, "%s: %s mount option `%s' is missing\n",
+                                progname, justwarn ? "Warning: default"
+                                : "Error: mandatory", item);
+                        errors++;
+                }
+                mlp = NULL;
+        }
+        free(ml);
+        return errors;
+}
+
+/* Trim embedded white space, leading and trailing commas from string s.
+ */
+static void trim_mountfsoptions(char *s)
+{
+        char *p;
+
+        for (p = s; *p; ) {
+                if (isspace(*p)) {
+                        memmove(p, p + 1, strlen(p + 1) + 1);
+                        continue;
+                }
+                p++;
+        }
+
+        while (s[0] == ',')
+                memmove(&s[0], &s[1], strlen(&s[1]) + 1);
+
+        p = s + strlen(s) - 1;
+        while (p >= s && *p == ',')
+                *p-- = '\0';
+}
+

 int main(int argc, char *const argv[])
 {
         struct mkfs_opts mop;
 int main(int argc, char *const argv[])
 {
         struct mkfs_opts mop;


@@ -1517,7 +1588,8 @@ int main(int argc, char *const argv[])
         case LDD_MT_EXT3:
         case LDD_MT_LDISKFS:
         case LDD_MT_LDISKFS2: {
         case LDD_MT_EXT3:
         case LDD_MT_LDISKFS:
         case LDD_MT_LDISKFS2: {

-                sprintf(always_mountopts, "errors=remount-ro");
+                strscat(default_mountopts, ",errors=remount-ro",
+                        sizeof(default_mountopts));

                 if (IS_MDT(ldd) || IS_MGS(ldd))
                         strscat(always_mountopts, ",iopen_nopriv,user_xattr",
                                 sizeof(always_mountopts));
                 if (IS_MDT(ldd) || IS_MGS(ldd))
                         strscat(always_mountopts, ",iopen_nopriv,user_xattr",
                                 sizeof(always_mountopts));


@@ -1537,7 +1609,7 @@ int main(int argc, char *const argv[])
         }
         case LDD_MT_SMFS: {
                 mop.mo_flags |= MO_IS_LOOP;
         }
         case LDD_MT_SMFS: {
                 mop.mo_flags |= MO_IS_LOOP;

-                sprintf(always_mountopts, "type=ext3,dev=%s",
+                sprintf(always_mountopts, ",type=ext3,dev=%s",

                         mop.mo_device);
                 break;
         }
                         mop.mo_device);
                 break;
         }


@@ -1552,10 +1624,13 @@ int main(int argc, char *const argv[])
         }
 
         if (mountopts) {
         }
 
         if (mountopts) {

-                /* If user specifies mount opts, don't use defaults,
-                   but always use always_mountopts */
-                sprintf(ldd->ldd_mount_opts, "%s,%s",
-                        always_mountopts, mountopts);
+                trim_mountfsoptions(mountopts);
+                (void)check_mountfsoptions(mountopts, default_mountopts, 1);
+                if (check_mountfsoptions(mountopts, always_mountopts, 0)) {
+                        ret = EINVAL;
+                        goto out;
+                }
+                sprintf(ldd->ldd_mount_opts, "%s", mountopts);

         } else {
 #ifdef TUNEFS
                 if (ldd->ldd_mount_opts[0] == 0)
         } else {
 #ifdef TUNEFS
                 if (ldd->ldd_mount_opts[0] == 0)


@@ -1564,6 +1639,7 @@ int main(int argc, char *const argv[])
                 {
                         sprintf(ldd->ldd_mount_opts, "%s%s",
                                 always_mountopts, default_mountopts);
                 {
                         sprintf(ldd->ldd_mount_opts, "%s%s",
                                 always_mountopts, default_mountopts);

+                        trim_mountfsoptions(ldd->ldd_mount_opts);

                 }
         }
 
                 }
         }