Whamcloud - gitweb
LU-10391 obdclass: truncate large uuids 67/55767/7
authorChris Horn <chris.horn@hpe.com>
Mon, 15 Jul 2024 18:33:41 +0000 (12:33 -0600)
committerOleg Drokin <green@whamcloud.com>
Fri, 16 Aug 2024 23:48:51 +0000 (23:48 +0000)
With large NIDs it is now possible for UUIDs, based on NID strings, to
exceed UUID_MAX length. To handle this, these large NID strings are
truncated.

Test-Parameters: trivial testlist=conf-sanity
Signed-off-by: Chris Horn <chris.horn@hpe.com>
Change-Id: Ic6b9310c9bf8a598e3cc87575c85f43ef4577345
Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/55767
Reviewed-by: Timothy Day <timday@amazon.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
lustre/ldlm/ldlm_lib.c
lustre/mgs/mgs_llog.c
lustre/obdclass/obd_mount.c

index 9a2391b..96f1d67 100644 (file)
@@ -368,8 +368,8 @@ int client_obd_setup(struct obd_device *obd, struct lustre_cfg *lcfg)
                RETURN(-EINVAL);
        }
 
-       if (LUSTRE_CFG_BUFLEN(lcfg, 1) > 37) {
-               CERROR("client UUID must be less than 38 characters\n");
+       if (LUSTRE_CFG_BUFLEN(lcfg, 1) > UUID_MAX) {
+               CERROR("client UUID must be %u characters or less\n", UUID_MAX);
                RETURN(-EINVAL);
        }
 
@@ -378,8 +378,8 @@ int client_obd_setup(struct obd_device *obd, struct lustre_cfg *lcfg)
                RETURN(-EINVAL);
        }
 
-       if (LUSTRE_CFG_BUFLEN(lcfg, 2) > 37) {
-               CERROR("target UUID must be less than 38 characters\n");
+       if (LUSTRE_CFG_BUFLEN(lcfg, 2) > UUID_MAX) {
+               CERROR("target UUID must be %u characters or less\n", UUID_MAX);
                RETURN(-EINVAL);
        }
 
index 3446d78..9dfb114 100644 (file)
@@ -132,11 +132,15 @@ fini:
 
 static inline int name_create(char **newname, char *prefix, char *suffix)
 {
+       size_t newname_len = strlen(prefix) + strlen(suffix) + 1;
+
        LASSERT(newname);
-       OBD_ALLOC(*newname, strlen(prefix) + strlen(suffix) + 1);
+
+       OBD_ALLOC(*newname, newname_len);
        if (!*newname)
                return -ENOMEM;
-       sprintf(*newname, "%s%s", prefix, suffix);
+
+       snprintf(*newname, newname_len, "%s%s", prefix, suffix);
        return 0;
 }
 
@@ -147,6 +151,34 @@ static inline void name_destroy(char **name)
        *name = NULL;
 }
 
+static inline int niduuid_create(char **niduuid, char *nidstr)
+{
+       size_t niduuid_len = strlen(nidstr) + 1;
+
+       LASSERT(niduuid);
+
+       /* Large NIDs may be longer than UUID_MAX. In this case we skip bytes at
+        * the start of the string because the bytes at the end of the NID
+        * should be more unique
+        */
+       if (niduuid_len > UUID_MAX) {
+               nidstr += niduuid_len - UUID_MAX;
+               niduuid_len = strlen(nidstr) + 1;
+       }
+
+       OBD_ALLOC(*niduuid, niduuid_len);
+       if (!*niduuid)
+               return -ENOMEM;
+
+       snprintf(*niduuid, niduuid_len, "%s", nidstr);
+       return 0;
+}
+
+static inline void niduuid_destroy(char **niduuid)
+{
+       name_destroy(niduuid);
+}
+
 static inline int name_create_osp(char **ospname, char **devtype, char *tgtname,
                                  int index)
 {
@@ -1220,8 +1252,8 @@ static int process_command(const struct lu_env *env, struct lustre_cfg *lcfg,
                ptr = mrd->target.mti_params;
                while (class_parse_nid(ptr, &nid, &ptr) == 0) {
                        if (!mrd->nodeuuid) {
-                               rc = name_create(&mrd->nodeuuid,
-                                                libcfs_nidstr(&nid), "");
+                               rc = niduuid_create(&mrd->nodeuuid,
+                                                   libcfs_nidstr(&nid));
                                if (rc) {
                                        CERROR("Can't create uuid for "
                                                "nid  %s, device %s\n",
@@ -1256,7 +1288,7 @@ static int process_command(const struct lu_env *env, struct lustre_cfg *lcfg,
                               libcfs_nidstr(&nid),
                               mrd->nodeuuid ? mrd->nodeuuid : "NULL",
                               mrd->target.mti_svname);
-                       name_destroy(&mrd->nodeuuid);
+                       niduuid_destroy(&mrd->nodeuuid);
                        return -ENXIO;
                } else {
                        mrd->state = REPLACE_SETUP;
@@ -1279,7 +1311,7 @@ static int process_command(const struct lu_env *env, struct lustre_cfg *lcfg,
                                  /* s4 is not changed */
                                  lustre_cfg_string(lcfg, 4));
 
-               name_destroy(&mrd->nodeuuid);
+               niduuid_destroy(&mrd->nodeuuid);
                if (rc)
                        return rc;
 
@@ -1287,9 +1319,8 @@ static int process_command(const struct lu_env *env, struct lustre_cfg *lcfg,
                        ptr = mrd->failover;
                        while (class_parse_nid(ptr, &nid, &ptr) == 0) {
                                if (mrd->nodeuuid == NULL) {
-                                       rc =  name_create(&mrd->nodeuuid,
-                                                         libcfs_nidstr(&nid),
-                                                         "");
+                                       rc = niduuid_create(&mrd->nodeuuid,
+                                                          libcfs_nidstr(&nid));
                                        if (rc)
                                                return rc;
                                }
@@ -1303,7 +1334,7 @@ static int process_command(const struct lu_env *env, struct lustre_cfg *lcfg,
                                                mrd->target.mti_svname,
                                                libcfs_nidstr(&nid),
                                                mrd->nodeuuid, rc);
-                                       name_destroy(&mrd->nodeuuid);
+                                       niduuid_destroy(&mrd->nodeuuid);
                                        return rc;
                                }
                                if (*ptr == ':') {
@@ -1311,7 +1342,7 @@ static int process_command(const struct lu_env *env, struct lustre_cfg *lcfg,
                                                mrd->temp_llh,
                                                lustre_cfg_string(lcfg, 0),
                                                mrd->nodeuuid);
-                                       name_destroy(&mrd->nodeuuid);
+                                       niduuid_destroy(&mrd->nodeuuid);
                                        if (rc)
                                                return rc;
                                }
@@ -1320,7 +1351,7 @@ static int process_command(const struct lu_env *env, struct lustre_cfg *lcfg,
                                rc = record_add_conn(env, mrd->temp_llh,
                                                     lustre_cfg_string(lcfg, 0),
                                                     mrd->nodeuuid);
-                               name_destroy(&mrd->nodeuuid);
+                               niduuid_destroy(&mrd->nodeuuid);
                                if (rc)
                                        return rc;
                        }
@@ -2834,7 +2865,7 @@ static int mgs_write_log_failnids(const struct lu_env *env,
                                /* We don't know the failover node name,
                                 * so just use the first nid as the uuid */
                                libcfs_nidstr_r(&nid, nidstr, sizeof(nidstr));
-                               rc = name_create(&failnodeuuid, nidstr, "");
+                               rc = niduuid_create(&failnodeuuid, nidstr);
                                if (rc != 0)
                                        return rc;
                        }
@@ -2850,13 +2881,13 @@ static int mgs_write_log_failnids(const struct lu_env *env,
                        if (*ptr == ':') {
                                rc = record_add_conn(env, llh, cliname,
                                                     failnodeuuid);
-                               name_destroy(&failnodeuuid);
+                               niduuid_destroy(&failnodeuuid);
                                failnodeuuid = NULL;
                        }
                }
                if (failnodeuuid) {
                        rc = record_add_conn(env, llh, cliname, failnodeuuid);
-                       name_destroy(&failnodeuuid);
+                       niduuid_destroy(&failnodeuuid);
                        failnodeuuid = NULL;
                }
        }
@@ -2895,7 +2926,7 @@ static int mgs_write_log_mdc_to_lmv(const struct lu_env *env,
                nidstr = mti->mti_nidlist[0];
        }
 
-       rc = name_create(&nodeuuid, nidstr, "");
+       rc = niduuid_create(&nodeuuid, nidstr);
        if (rc)
                RETURN(rc);
        rc = name_create(&mdcname, mti->mti_svname, "-mdc");
@@ -2963,7 +2994,7 @@ out_free:
        name_destroy(&lmvuuid);
        name_destroy(&mdcuuid);
        name_destroy(&mdcname);
-       name_destroy(&nodeuuid);
+       niduuid_destroy(&nodeuuid);
        RETURN(rc);
 }
 
@@ -3025,7 +3056,7 @@ static int mgs_write_log_osp_to_mdt(const struct lu_env *env,
                nidstr = mti->mti_nidlist[0];
        }
 
-       rc = name_create(&nodeuuid, nidstr, "");
+       rc = niduuid_create(&nodeuuid, nidstr);
        if (rc)
                GOTO(out_destory, rc);
 
@@ -3108,7 +3139,7 @@ out_destory:
        name_destroy(&lovuuid);
        name_destroy(&lovname);
        name_destroy(&ospname);
-       name_destroy(&nodeuuid);
+       niduuid_destroy(&nodeuuid);
        name_destroy(&mdtname);
        RETURN(rc);
 }
@@ -3331,7 +3362,7 @@ static int mgs_write_log_osc_to_lov(const struct lu_env *env,
                nidstr = mti->mti_nidlist[0];
        }
 
-       rc = name_create(&nodeuuid, nidstr, "");
+       rc = niduuid_create(&nodeuuid, nidstr);
        if (rc)
                RETURN(rc);
        rc = name_create(&svname, mti->mti_svname, "-osc");
@@ -3425,7 +3456,7 @@ out_free:
        name_destroy(&oscuuid);
        name_destroy(&oscname);
        name_destroy(&svname);
-       name_destroy(&nodeuuid);
+       niduuid_destroy(&nodeuuid);
        RETURN(rc);
 }
 
index 5c0d945..9f53c90 100644 (file)
@@ -220,6 +220,46 @@ SERVER_ONLY_EXPORT_SYMBOL(lustre_start_simple);
 
 static DEFINE_MUTEX(mgc_start_lock);
 
+/* 9 for '_%x' (INT_MAX as hex is 8 chars - '7FFFFFFF') and 1 for '\0' */
+#define NIDUUID_SUFFIX_MAX_LEN 10
+static inline int mgc_niduuid_create(char **niduuid, char *nidstr)
+{
+       size_t niduuid_len = strlen(nidstr) + strlen(LUSTRE_MGC_OBDNAME) +
+                            NIDUUID_SUFFIX_MAX_LEN;
+
+       LASSERT(niduuid);
+
+       /* See comment in niduuid_create() */
+       if (niduuid_len > UUID_MAX) {
+               nidstr += niduuid_len - UUID_MAX;
+               niduuid_len = strlen(LUSTRE_MGC_OBDNAME) +
+                             strlen(nidstr) + NIDUUID_SUFFIX_MAX_LEN;
+       }
+
+       OBD_ALLOC(*niduuid, niduuid_len);
+       if (!*niduuid)
+               return -ENOMEM;
+
+       snprintf(*niduuid, niduuid_len, "%s%s", LUSTRE_MGC_OBDNAME, nidstr);
+       return 0;
+}
+
+static inline void mgc_niduuid_destroy(char **niduuid)
+{
+       if (*niduuid) {
+               char *tmp = strchr(*niduuid, '_');
+
+               /* If the "_%x" suffix hasn't been added yet then the size
+                * calculation below should still be correct
+                */
+               if (tmp)
+                       *tmp = '\0';
+
+               OBD_FREE(*niduuid, strlen(*niduuid) + NIDUUID_SUFFIX_MAX_LEN);
+       }
+       *niduuid = NULL;
+}
+
 /**
  * Set up a MGC OBD to process startup logs
  *
@@ -239,7 +279,7 @@ int lustre_start_mgc(struct super_block *sb)
        char nidstr[LNET_NIDSTR_SIZE];
        char *mgcname = NULL, *niduuid = NULL, *mgssec = NULL;
        bool large_nids = false;
-       char *ptr;
+       char *ptr, *niduuid_suffix;
        int rc = 0, i = 0, j;
        size_t len;
 
@@ -287,9 +327,10 @@ int lustre_start_mgc(struct super_block *sb)
        libcfs_nidstr_r(&nid, nidstr, sizeof(nidstr));
        len = strlen(LUSTRE_MGC_OBDNAME) + strlen(nidstr) + 1;
        OBD_ALLOC(mgcname, len);
-       OBD_ALLOC(niduuid, len + 2);
-       if (mgcname == NULL || niduuid == NULL)
+       rc = mgc_niduuid_create(&niduuid, nidstr);
+       if (rc || mgcname == NULL)
                GOTO(out_free, rc = -ENOMEM);
+
        snprintf(mgcname, len, "%s%s", LUSTRE_MGC_OBDNAME, nidstr);
 
        mgssec = lsi->lsi_lmd->lmd_mgssec ? lsi->lsi_lmd->lmd_mgssec : "";
@@ -366,7 +407,8 @@ int lustre_start_mgc(struct super_block *sb)
 
        /* Add the primary NIDs for the MGS */
        i = 0;
-       snprintf(niduuid, len + 2, "%s_%x", mgcname, i);
+       niduuid_suffix = niduuid + strlen(niduuid);
+       snprintf(niduuid_suffix, NIDUUID_SUFFIX_MAX_LEN, "_%x", i);
        if (IS_SERVER(lsi)) {
                ptr = lsi->lsi_lmd->lmd_mgs;
                CDEBUG(D_MOUNT, "mgs NIDs %s.\n", ptr);
@@ -444,7 +486,7 @@ int lustre_start_mgc(struct super_block *sb)
        while (ptr && ((*ptr == ':' ||
               class_find_param(ptr, PARAM_MGSNODE, &ptr) == 0))) {
                /* New failover node */
-               sprintf(niduuid, "%s_%x", mgcname, i);
+               snprintf(niduuid_suffix, NIDUUID_SUFFIX_MAX_LEN, "_%x", i);
                j = 0;
                while (class_parse_nid_quiet(ptr, &nid, &ptr) == 0) {
                        if (!nid_is_nid4(&nid))
@@ -532,8 +574,8 @@ out_free:
                OBD_FREE_PTR(data);
        if (mgcname)
                OBD_FREE(mgcname, len);
-       if (niduuid)
-               OBD_FREE(niduuid, len + 2);
+       mgc_niduuid_destroy(&niduuid);
+
        RETURN(rc);
 }
 EXPORT_SYMBOL(lustre_start_mgc);
@@ -542,7 +584,8 @@ SERVER_ONLY int lustre_stop_mgc(struct super_block *sb)
 {
        struct lustre_sb_info *lsi = s2lsi(sb);
        struct obd_device *obd;
-       char niduuid[MAX_OBD_NAME + 6], *ptr = NULL;
+       char *niduuid = NULL, *niduuid_suffix;
+       char nidstr[LNET_NIDSTR_SIZE];
        int i, rc = 0;
 
        ENTRY;
@@ -554,6 +597,16 @@ SERVER_ONLY int lustre_stop_mgc(struct super_block *sb)
                RETURN(-ENOENT);
        lsi->lsi_mgc = NULL;
 
+       /* Reconstruct the NID uuid from the obd_name */
+       strscpy(nidstr, &obd->obd_name[0] + strlen(LUSTRE_MGC_OBDNAME),
+               sizeof(nidstr));
+
+       rc = mgc_niduuid_create(&niduuid, nidstr);
+       if (rc)
+               RETURN(-ENOMEM);
+
+       niduuid_suffix = niduuid + strlen(niduuid);
+
        mutex_lock(&mgc_start_lock);
        LASSERT(atomic_read(&obd->u.cli.cl_mgc_refcount) > 0);
        if (!atomic_dec_and_test(&obd->u.cli.cl_mgc_refcount)) {
@@ -582,19 +635,12 @@ SERVER_ONLY int lustre_stop_mgc(struct super_block *sb)
                        CDEBUG(D_MOUNT, "disconnect failed %d\n", rc);
        }
 
-       /*
-        * Cache the obdname for cleaning the nid uuids, which are
-        * obdname_XX before calling class_manual_cleanup
-        */
-       strcpy(niduuid, obd->obd_name);
-       ptr = niduuid + strlen(niduuid);
-
        rc = class_manual_cleanup(obd);
        if (rc)
                GOTO(out, rc);
 
        for (i = 0; i < lsi->lsi_lmd->lmd_mgs_failnodes; i++) {
-               sprintf(ptr, "_%x", i);
+               snprintf(niduuid_suffix, NIDUUID_SUFFIX_MAX_LEN, "_%x", i);
                rc = do_lcfg(LUSTRE_MGC_OBDNAME, 0, LCFG_DEL_UUID,
                             niduuid, NULL, NULL, NULL);
                if (rc)
@@ -604,6 +650,9 @@ SERVER_ONLY int lustre_stop_mgc(struct super_block *sb)
 out:
        /* class_import_put will get rid of the additional connections */
        mutex_unlock(&mgc_start_lock);
+
+       mgc_niduuid_destroy(&niduuid);
+
        RETURN(rc);
 }
 SERVER_ONLY_EXPORT_SYMBOL(lustre_stop_mgc);