LU-2200 obdclass: be more careful processing server name

Because whole options line gets passed to exclude processing, don't
search from end of passed in argument to determine fsname at
beginning.

Re-enable conf-sanity/32 but do replace_nids on IB.

Test-Parameters: nettypes=o2ib testlist=conf-sanity
Signed-off-by: Nathaniel Clark <nathaniel.l.clark@intel.com>
Change-Id: Ibc43417cb71581198e3516c1e431c0d3f668f1e0
Reviewed-on: http://review.whamcloud.com/6197
Tested-by: Hudson
Reviewed-by: Andreas Dilger <andreas.dilger@intel.com>
Reviewed-by: Alex Zhuravlev <alexey.zhuravlev@intel.com>
Tested-by: Maloo <whamcloud.maloo@gmail.com>

diff --git a/lustre/mgs/mgs_llog.c b/lustre/mgs/mgs_llog.c
index 32a422a..d84d3fe 100644 (file)
--- a/lustre/mgs/mgs_llog.c
+++ b/lustre/mgs/mgs_llog.c
@@ -1207,26 +1207,24 @@ out_put:
  */
 static int mgs_parse_devname(char *devname, char *fsname, __u32 *index)
 {
-       char *ptr;
+       int rc;
        ENTRY;
 
        /* Extract fsname */
-       ptr = strrchr(devname, '-');
-
        if (fsname) {
-               if (!ptr) {
+               rc = server_name2fsname(devname, fsname, NULL);
+               if (rc < 0) {
                        CDEBUG(D_MGS, "Device name %s without fsname\n",
                               devname);
                        RETURN(-EINVAL);
                }
-               memset(fsname, 0, MTI_NAME_MAXLEN);
-               strncpy(fsname, devname, ptr - devname);
-               fsname[MTI_NAME_MAXLEN - 1] = 0;
        }
 
        if (index) {
-               if (server_name2index(ptr, index, NULL) < 0) {
-                       CDEBUG(D_MGS, "Device name with wrong index\n");
+               rc = server_name2index(devname, index, NULL);
+               if (rc < 0) {
+                       CDEBUG(D_MGS, "Device name %s with wrong index\n",
+                              devname);
                        RETURN(-EINVAL);
                }
        }

diff --git a/lustre/obdclass/obd_config.c b/lustre/obdclass/obd_config.c
index b0d416f..c663e5d 100644 (file)
--- a/lustre/obdclass/obd_config.c
+++ b/lustre/obdclass/obd_config.c
@@ -1181,11 +1181,11 @@ int class_process_config(struct lustre_cfg *lcfg)
                 GOTO(out, err = -EINVAL);
         }
 
-        switch(lcfg->lcfg_command) {
-        case LCFG_SETUP: {
-                err = class_setup(obd, lcfg);
-                GOTO(out, err);
-        }
+       switch(lcfg->lcfg_command) {
+       case LCFG_SETUP: {
+               err = class_setup(obd, lcfg);
+               GOTO(out, err);
+       }
         case LCFG_DETACH: {
                 err = class_detach(obd, lcfg);
                 GOTO(out, err = 0);
@@ -1463,10 +1463,13 @@ int class_config_llog_handler(const struct lu_env *env,
                }
 #endif
 
-                if ((clli->cfg_flags & CFG_F_EXCLUDE) &&
-                    (lcfg->lcfg_command == LCFG_LOV_ADD_OBD))
-                        /* Add inactive instead */
-                        lcfg->lcfg_command = LCFG_LOV_ADD_INA;
+               if (clli->cfg_flags & CFG_F_EXCLUDE) {
+                       CDEBUG(D_CONFIG, "cmd: %x marked EXCLUDED\n",
+                              lcfg->lcfg_command);
+                       if (lcfg->lcfg_command == LCFG_LOV_ADD_OBD)
+                               /* Add inactive instead */
+                               lcfg->lcfg_command = LCFG_LOV_ADD_INA;
+               }
 
                 lustre_cfg_bufs_init(&bufs, lcfg);
 
@@ -1532,7 +1535,7 @@ int class_config_llog_handler(const struct lu_env *env,
 
                 lcfg_new->lcfg_nal = 0; /* illegal value for obsolete field */
 
-                rc = class_process_config(lcfg_new);
+               rc = class_process_config(lcfg_new);
                 lustre_cfg_free(lcfg_new);
 
                 if (inst)

diff --git a/lustre/obdclass/obd_mount.c b/lustre/obdclass/obd_mount.c
index 9092837..425336c 100644 (file)
--- a/lustre/obdclass/obd_mount.c
+++ b/lustre/obdclass/obd_mount.c
@@ -649,6 +649,15 @@ int lustre_put_lsi(struct super_block *sb)
         RETURN(0);
 }
 
+/*** SERVER NAME ***
+ * <FSNAME><SEPERATOR><TYPE><INDEX>
+ * FSNAME is between 1 and 8 characters (inclusive).
+ *     Excluded characters are '/' and ':'
+ * SEPERATOR is either ':' or '-'
+ * TYPE: "OST", "MDT", etc.
+ * INDEX: Hex representation of the index
+ */
+
 /** Get the fsname ("lustre") from the server name ("lustre-OST003F").
  * @param [in] svname server name including type and index
  * @param [out] fsname Buffer to copy filesystem name prefix into.
@@ -658,22 +667,13 @@ int lustre_put_lsi(struct super_block *sb)
  */
 int server_name2fsname(const char *svname, char *fsname, const char **endptr)
 {
-       const char *dash = strrchr(svname, '-');
-       if (!dash) {
-               dash = strrchr(svname, ':');
-               if (!dash)
-                       return -EINVAL;
-       }
+       const char *dash;
 
-       /* interpret <fsname>-MDTXXXXX-mdc as mdt, the better way is to pass
-        * in the fsname, then determine the server index */
-       if (!strcmp(LUSTRE_MDC_NAME, dash + 1)) {
-               dash--;
-               for (; dash > svname && *dash != '-' && *dash != ':'; dash--)
-                       ;
-               if (dash == svname)
-                       return -EINVAL;
-       }
+       dash = svname + strnlen(svname, 8); /* max fsname length is 8 */
+       for (; dash > svname && *dash != '-' && *dash != ':'; dash--)
+               ;
+       if (dash == svname)
+               return -EINVAL;
 
        if (fsname != NULL) {
                strncpy(fsname, svname, dash - svname);
@@ -696,15 +696,15 @@ int server_name2svname(const char *label, char *svname, const char **endptr,
                       size_t svsize)
 {
        int rc;
-       const const char *dash;
+       const char *dash;
 
        /* We use server_name2fsname() just for parsing */
        rc = server_name2fsname(label, NULL, &dash);
        if (rc != 0)
                return rc;
 
-       if (*dash != '-')
-               return -1;
+       if (endptr != NULL)
+               *endptr = dash;
 
        if (strlcpy(svname, dash + 1, svsize) >= svsize)
                return -E2BIG;
@@ -729,9 +729,6 @@ int server_name2index(const char *svname, __u32 *idx, const char **endptr)
        if (rc != 0)
                return rc;
 
-       if (*dash != '-')
-               return -EINVAL;
-
        dash++;
 
        if (strncmp(dash, "MDT", 3) == 0)
@@ -743,11 +740,20 @@ int server_name2index(const char *svname, __u32 *idx, const char **endptr)
 
        dash += 3;
 
-       if (strcmp(dash, "all") == 0)
+       if (strncmp(dash, "all", 3) == 0) {
+               if (endptr != NULL)
+                       *endptr = dash + 3;
                return rc | LDD_F_SV_ALL;
+       }
 
        index = simple_strtoul(dash, (char **)endptr, 16);
-       *idx = index;
+       if (idx != NULL)
+               *idx = index;
+
+       /* Account for -mdc after index that is possible when specifying mdt */
+       if (endptr != NULL && strncmp(LUSTRE_MDC_NAME, *endptr + 1,
+                                     sizeof(LUSTRE_MDC_NAME)-1) == 0)
+               *endptr += sizeof(LUSTRE_MDC_NAME);
 
        return rc;
 }
@@ -838,55 +844,57 @@ int lustre_check_exclusion(struct super_block *sb, char *svname)
 static int lmd_make_exclusion(struct lustre_mount_data *lmd, const char *ptr)
 {
        const char *s1 = ptr, *s2;
-        __u32 index, *exclude_list;
-        int rc = 0, devmax;
-        ENTRY;
-
-        /* The shortest an ost name can be is 8 chars: -OST0000.
-           We don't actually know the fsname at this time, so in fact
-           a user could specify any fsname. */
-        devmax = strlen(ptr) / 8 + 1;
-
-        /* temp storage until we figure out how many we have */
-        OBD_ALLOC(exclude_list, sizeof(index) * devmax);
-        if (!exclude_list)
-                RETURN(-ENOMEM);
+       __u32 index, *exclude_list;
+       int rc = 0, devmax;
+       ENTRY;
+
+       /* The shortest an ost name can be is 8 chars: -OST0000.
+          We don't actually know the fsname at this time, so in fact
+          a user could specify any fsname. */
+       devmax = strlen(ptr) / 8 + 1;
+
+       /* temp storage until we figure out how many we have */
+       OBD_ALLOC(exclude_list, sizeof(index) * devmax);
+       if (!exclude_list)
+               RETURN(-ENOMEM);
 
-        /* we enter this fn pointing at the '=' */
-        while (*s1 && *s1 != ' ' && *s1 != ',') {
-                s1++;
-                rc = server_name2index(s1, &index, &s2);
-                if (rc < 0) {
-                        CERROR("Can't parse server name '%s'\n", s1);
-                        break;
-                }
-                if (rc == LDD_F_SV_TYPE_OST)
-                        exclude_list[lmd->lmd_exclude_count++] = index;
-                else
-                        CDEBUG(D_MOUNT, "ignoring exclude %.7s\n", s1);
-                s1 = s2;
-                /* now we are pointing at ':' (next exclude)
-                   or ',' (end of excludes) */
-                if (lmd->lmd_exclude_count >= devmax)
-                        break;
-        }
-        if (rc >= 0) /* non-err */
-                rc = 0;
-
-        if (lmd->lmd_exclude_count) {
-                /* permanent, freed in lustre_free_lsi */
-                OBD_ALLOC(lmd->lmd_exclude, sizeof(index) *
-                          lmd->lmd_exclude_count);
-                if (lmd->lmd_exclude) {
-                        memcpy(lmd->lmd_exclude, exclude_list,
-                               sizeof(index) * lmd->lmd_exclude_count);
-                } else {
-                        rc = -ENOMEM;
-                        lmd->lmd_exclude_count = 0;
-                }
-        }
-        OBD_FREE(exclude_list, sizeof(index) * devmax);
-        RETURN(rc);
+       /* we enter this fn pointing at the '=' */
+       while (*s1 && *s1 != ' ' && *s1 != ',') {
+               s1++;
+               rc = server_name2index(s1, &index, &s2);
+               if (rc < 0) {
+                       CERROR("Can't parse server name '%s': rc = %d\n",
+                              s1, rc);
+                       break;
+               }
+               if (rc == LDD_F_SV_TYPE_OST)
+                       exclude_list[lmd->lmd_exclude_count++] = index;
+               else
+                       CDEBUG(D_MOUNT, "ignoring exclude %.*s: type = %#x\n",
+                              (uint)(s2-s1), s1, rc);
+               s1 = s2;
+               /* now we are pointing at ':' (next exclude)
+                  or ',' (end of excludes) */
+               if (lmd->lmd_exclude_count >= devmax)
+                       break;
+       }
+       if (rc >= 0) /* non-err */
+               rc = 0;
+
+       if (lmd->lmd_exclude_count) {
+               /* permanent, freed in lustre_free_lsi */
+               OBD_ALLOC(lmd->lmd_exclude, sizeof(index) *
+                         lmd->lmd_exclude_count);
+               if (lmd->lmd_exclude) {
+                       memcpy(lmd->lmd_exclude, exclude_list,
+                              sizeof(index) * lmd->lmd_exclude_count);
+               } else {
+                       rc = -ENOMEM;
+                       lmd->lmd_exclude_count = 0;
+               }
+       }
+       OBD_FREE(exclude_list, sizeof(index) * devmax);
+       RETURN(rc);
 }
 
 static int lmd_parse_mgssec(struct lustre_mount_data *lmd, char *ptr)

diff --git a/lustre/tests/conf-sanity.sh b/lustre/tests/conf-sanity.sh
index 5924489..c14debe 100644 (file)
--- a/lustre/tests/conf-sanity.sh
+++ b/lustre/tests/conf-sanity.sh
@@ -1325,11 +1325,6 @@ t32_check() {
                exit 0
        fi
 
-       if [ -n "$($LCTL list_nids | grep -v '\(tcp\|lo\)[[:digit:]]*$')" ]; then
-               skip "LU-2200: Test cannot run over Infiniband"
-               exit 0
-       fi
-
        local IMGTYPE=$(facet_fstype $SINGLEMDS)
 
        tarballs=$($r find $RLUSTRE/tests -maxdepth 1 -name \'disk*-$IMGTYPE.tar.bz2\')
@@ -1550,6 +1545,20 @@ t32_test() {
                        }
                fi
        else
+               if [ -n "$($LCTL list_nids | grep -v '\(tcp\|lo\)[[:digit:]]*$')" ]; then
+                       [[ $(lustre_version_code mgs) -ge $(version_code 2.3.59) ]] ||
+                       { skip "LU-2200: Cannot run over Inifiniband w/o lctl replace_nids "
+                               "(Need MGS version at least 2.3.59)"; return 0; }
+
+                       local osthost=$(facet_active_host ost1)
+                       local ostnid=$(do_node $osthost $LCTL list_nids | head -1)
+
+                       $r mount -t lustre -o loop,nosvc $tmp/mdt $tmp/mnt/mdt
+                       $r lctl replace_nids $fsname-OST0000 $ostnid
+                       $r lctl replace_nids $fsname-MDT0000 $nid
+                       $r umount $tmp/mnt/mdt
+               fi
+
                mopts=loop,exclude=$fsname-OST0000
        fi
 
@@ -1760,8 +1769,8 @@ t32_test() {
                        # on an architecture with different number of bits per
                        # "long".
                        #
-                       if [ $(t32_bits_per_long $(uname -m)) !=                                                \
-                                $(t32_bits_per_long $img_arch) ]; then
+                       if [ $(t32_bits_per_long $(uname -m)) != \
+                               $(t32_bits_per_long $img_arch) ]; then
                                echo "Different number of bits per \"long\" from the disk image"
                                for list in list.orig list; do
                                        sed -i -e 's/^[0-9]\+[ \t]\+//' $tmp/$list
@@ -1834,7 +1843,7 @@ t32_test() {
                        error_noexit "tunefs.lustre before remounting the MDT"
                        return 1
                }
-               $r mount -t lustre -o loop,exclude=$fsname-OST0000 $tmp/mdt                     \
+               $r mount -t lustre -o loop,exclude=$fsname-OST0000 $tmp/mdt \
                                 $tmp/mnt/mdt || {
                        error_noexit "Remounting the MDT"
                        return 1
@@ -1850,7 +1859,7 @@ test_32a() {
 
        t32_check
        for tarball in $tarballs; do
-               t32_test $tarball || rc=$?
+               t32_test $tarball || let "rc += $?"
        done
        return $rc
 }
@@ -1863,7 +1872,7 @@ test_32b() {
 
        t32_check
        for tarball in $tarballs; do
-               t32_test $tarball writeconf || rc=$?
+               t32_test $tarball writeconf || let "rc += $?"
        done
        return $rc
 }