14690 nvmeadm should refer to namespaces by their nsid

Reviewed by: Andrew Giles <agiles@tintri.com>
Reviewed by: Guy Morrogh <gmorrogh@tintri.com>
Reviewed by: Robert Mustacchi <rm@fingolfin.org>
Approved by: Dan McDonald <danmcd@mnx.io>

5 files changed, 114 insertions, 40 deletions

diff --git a/usr/src/cmd/nvmeadm/nvmeadm.c b/usr/src/cmd/nvmeadm/nvmeadm.c
index 74803baacb..6a80867ddd 100644
--- a/usr/src/cmd/nvmeadm/nvmeadm.c
+++ b/usr/src/cmd/nvmeadm/nvmeadm.c
@@ -109,7 +109,8 @@ struct nvmeadm_cmd {
 
 static void usage(const nvmeadm_cmd_t *);
 static void nvme_walk(nvme_process_arg_t *, di_node_t);
-static boolean_t nvme_match(nvme_process_arg_t *);
+static boolean_t nvme_match_ctrl(nvme_process_arg_t *);
+static boolean_t nvme_match_ns(nvme_process_arg_t *);
 
 static int nvme_process(di_node_t, di_minor_t, void *);
 
@@ -526,10 +527,9 @@ usage(const nvmeadm_cmd_t *cmd)
 }
 
 static boolean_t
-nvme_match(nvme_process_arg_t *npa)
+nvme_match_ctrl(nvme_process_arg_t *npa)
 {
 	char *name;
-	char *nsid = NULL;
 
 	if (npa->npa_name == NULL)
 		return (B_TRUE);
@@ -545,17 +545,66 @@ nvme_match(nvme_process_arg_t *npa)
 
 	free(name);
 
-	if (npa->npa_isns) {
-		if (npa->npa_nsid == NULL)
-			return (B_TRUE);
+	return (B_TRUE);
+}
 
-		nsid = di_minor_name(npa->npa_minor);
+static boolean_t
+nvme_match_ns(nvme_process_arg_t *npa)
+{
+	if (npa->npa_nsid == NULL)
+		return (B_TRUE);
 
-		if (nsid == NULL || strcmp(npa->npa_nsid, nsid) != 0)
-			return (B_FALSE);
+	if (strcasecmp(npa->npa_nsid, di_minor_name(npa->npa_minor)) ==
+	    0)
+		return (B_TRUE);
+
+	if (npa->npa_eui64 != NULL &&
+	    strcasecmp(npa->npa_nsid, npa->npa_eui64) == 0)
+		return (B_TRUE);
+
+	if (npa->npa_nguid != NULL &&
+	    strcasecmp(npa->npa_nsid, npa->npa_nguid) == 0)
+		return (B_TRUE);
+
+	return (B_FALSE);
+}
+
+char *
+nvme_nguid(const nvme_process_arg_t *npa)
+{
+	char *ret = NULL;
+
+	if (*(uint64_t *)npa->npa_idns->id_nguid != 0 ||
+	    *((uint64_t *)npa->npa_idns->id_nguid + 1) != 0) {
+		uint8_t *guid = npa->npa_idns->id_nguid;
+
+		(void) asprintf(&ret,
+		    "%0.2X%0.2X%0.2X%0.2X%0.2X%0.2X%0.2X%0.2X"
+		    "%0.2X%0.2X%0.2X%0.2X%0.2X%0.2X%0.2X%0.2X",
+		    guid[0], guid[1], guid[2], guid[3],
+		    guid[4], guid[5], guid[6], guid[7],
+		    guid[8], guid[9], guid[10], guid[11],
+		    guid[12], guid[13], guid[14], guid[15]);
 	}
 
-	return (B_TRUE);
+	return (ret);
+}
+
+char *
+nvme_eui64(const nvme_process_arg_t *npa)
+{
+	char *ret = NULL;
+
+	if (*(uint64_t *)npa->npa_idns->id_eui64 != 0) {
+		uint8_t *eui64 = npa->npa_idns->id_eui64;
+
+		(void) asprintf(&ret,
+		    "%0.2X%0.2X%0.2X%0.2X%0.2X%0.2X%0.2X%0.2X",
+		    eui64[0], eui64[1], eui64[2], eui64[3],
+		    eui64[4], eui64[5], eui64[6], eui64[7]);
+	}
+
+	return (ret);
 }
 
 char *
@@ -577,13 +626,30 @@ nvme_dskname(const nvme_process_arg_t *npa)
 		if (addr == NULL)
 			continue;
 
+		addr = strdup(addr);
+		if (addr == NULL)
+			goto fail;
+
 		if (addr[0] == 'w')
 			addr++;
 
-		if (strncasecmp(addr, di_minor_name(npa->npa_minor),
-		    strchrnul(addr, ',') - addr) != 0)
+		/* Chop off ,... from the bus address. */
+		*(strchrnul(addr, ',')) = '\0';
+
+		/*
+		 * If there's a EUI64, that's what was used for the bus address.
+		 * Otherwise it's just the numeric namespace id.
+		 */
+		if (npa->npa_eui64 != NULL &&
+		    strcasecmp(addr, npa->npa_eui64) == 0)
+			goto found;
+
+		if (strcasecmp(addr, di_minor_name(npa->npa_minor)) != 0) {
+			free(addr);
 			continue;
+		}
 
+found:
 		path = di_dim_path_dev(dim, di_driver_name(child),
 		    di_instance(child), "c");
 
@@ -604,6 +670,7 @@ nvme_dskname(const nvme_process_arg_t *npa)
 			goto fail;
 
 		free(path);
+		free(addr);
 		break;
 	}
 
@@ -613,6 +680,7 @@ nvme_dskname(const nvme_process_arg_t *npa)
 
 fail:
 	free(path);
+	free(addr);
 	err(-1, "nvme_dskname");
 }
 
@@ -625,18 +693,12 @@ nvme_process(di_node_t node, di_minor_t minor, void *arg)
 	npa->npa_node = node;
 	npa->npa_minor = minor;
 
-	if (!nvme_match(npa))
+	if (!nvme_match_ctrl(npa))
 		return (DI_WALK_CONTINUE);
 
 	if ((fd = nvme_open(minor, npa->npa_excl)) < 0)
 		return (DI_WALK_CONTINUE);
 
-	npa->npa_found++;
-
-	npa->npa_path = di_devfs_path(node);
-	if (npa->npa_path == NULL)
-		goto out;
-
 	npa->npa_version = nvme_version(fd);
 	if (npa->npa_version == NULL)
 		goto out;
@@ -660,27 +722,44 @@ nvme_process(di_node_t node, di_minor_t minor, void *arg)
 	if (npa->npa_idns == NULL)
 		goto out;
 
+	npa->npa_eui64 = NULL;
+	npa->npa_nguid = NULL;
 	npa->npa_dsk = NULL;
+
 	if (npa->npa_isns) {
 		npa->npa_ns_state = nvme_namespace_state(fd);
-		if ((npa->npa_ns_state & NVME_NS_STATE_ATTACHED) != 0)
+
+		if ((npa->npa_ns_state & NVME_NS_STATE_ACTIVE) != 0) {
+			npa->npa_eui64 = nvme_eui64(npa);
+			npa->npa_nguid = nvme_nguid(npa);
+		}
+
+		if ((npa->npa_ns_state & NVME_NS_STATE_ATTACHED) != 0) {
 			npa->npa_dsk = nvme_dskname(npa);
+		}
+
+		if (!nvme_match_ns(npa))
+			goto out;
 	}
 
+	npa->npa_found++;
 
 	exitcode += npa->npa_cmd->c_func(fd, npa);
 
 out:
-	di_devfs_path_free(npa->npa_path);
 	free(npa->npa_version);
 	free(npa->npa_idctl);
 	free(npa->npa_idns);
 	free(npa->npa_dsk);
+	free(npa->npa_eui64);
+	free(npa->npa_nguid);
 
 	npa->npa_version = NULL;
 	npa->npa_idctl = NULL;
 	npa->npa_idns = NULL;
 	npa->npa_dsk = NULL;
+	npa->npa_eui64 = NULL;
+	npa->npa_nguid = NULL;
 
 	nvme_close(fd);
 
diff --git a/usr/src/cmd/nvmeadm/nvmeadm.h b/usr/src/cmd/nvmeadm/nvmeadm.h
index 97f3e6a677..400c685ef7 100644
--- a/usr/src/cmd/nvmeadm/nvmeadm.h
+++ b/usr/src/cmd/nvmeadm/nvmeadm.h
@@ -40,6 +40,8 @@ struct nvme_process_arg {
 	char **npa_argv;
 	char *npa_name;
 	char *npa_nsid;
+	char *npa_eui64;
+	char *npa_nguid;
 	int npa_found;
 	boolean_t npa_excl;
 	boolean_t npa_isns;
@@ -48,7 +50,6 @@ struct nvme_process_arg {
 	const nvmeadm_cmd_t *npa_cmd;
 	di_node_t npa_node;
 	di_minor_t npa_minor;
-	char *npa_path;
 	char *npa_dsk;
 	uint32_t npa_ns_state;
 	nvme_identify_ctrl_t *npa_idctl;
diff --git a/usr/src/man/man8/nvmeadm.8 b/usr/src/man/man8/nvmeadm.8
index 04508b2c22..90ac4251ac 100644
--- a/usr/src/man/man8/nvmeadm.8
+++ b/usr/src/man/man8/nvmeadm.8
@@ -139,9 +139,11 @@ The controller name consists of the driver name
 followed by an instance number.
 A namespace is specified by appending a single
 .Qq /
-followed by the namespace ID to the controller name.
-The namespace ID is the EUI64 of the namespace, or a positive non-zero
-decimal number if the namespace doesn't have an EUI64.
+to the controller name, followed by either the namespace ID or the namespace
+EUI64 or NGUID as reported by the
+.Cm identify
+command.
+The namespace ID is a positive non-zero decimal number.
 For commands that don't change the device state multiple controllers
 and namespaces can be specified as a comma-separated list.
 .Pp
diff --git a/usr/src/uts/common/io/nvme/nvme.c b/usr/src/uts/common/io/nvme/nvme.c
index d76fa48d1b..2aa87600f4 100644
--- a/usr/src/uts/common/io/nvme/nvme.c
+++ b/usr/src/uts/common/io/nvme/nvme.c
@@ -2013,8 +2013,9 @@ nvme_async_event_task(void *arg)
 					break;
 
 				dev_err(nvme->n_dip, CE_NOTE,
-				    "!namespace %u (%s) has changed.", nsid,
-				    NVME_NSID2NS(nvme, nsid)->ns_name);
+				    "!namespace nvme%d/%u has changed.",
+				    ddi_get_instance(nvme->n_dip), nsid);
+
 
 				if (nvme_init_ns(nvme, nsid) != DDI_SUCCESS)
 					continue;
@@ -3016,19 +3017,10 @@ nvme_init_ns(nvme_t *nvme, int nsid)
 		bcopy(idns->id_nguid, ns->ns_nguid, sizeof (ns->ns_nguid));
 
 	/*LINTED: E_BAD_PTR_CAST_ALIGN*/
-	if (*(uint64_t *)ns->ns_eui64 != 0) {
-		uint8_t *eui64 = ns->ns_eui64;
-
-		(void) snprintf(ns->ns_name, sizeof (ns->ns_name),
-		    "%02x%02x%02x%02x%02x%02x%02x%02x",
-		    eui64[0], eui64[1], eui64[2], eui64[3],
-		    eui64[4], eui64[5], eui64[6], eui64[7]);
-	} else {
-		(void) snprintf(ns->ns_name, sizeof (ns->ns_name), "%d",
-		    ns->ns_id);
-
+	if (*(uint64_t *)ns->ns_eui64 == 0)
 		nvme_prepare_devid(nvme, ns->ns_id);
-	}
+
+	(void) snprintf(ns->ns_name, sizeof (ns->ns_name), "%u", ns->ns_id);
 
 	/*
 	 * Find the LBA format with no metadata and the best relative
diff --git a/usr/src/uts/common/io/nvme/nvme_var.h b/usr/src/uts/common/io/nvme/nvme_var.h
index dde0af58fb..ca3e7ef1a4 100644
--- a/usr/src/uts/common/io/nvme/nvme_var.h
+++ b/usr/src/uts/common/io/nvme/nvme_var.h
@@ -279,7 +279,7 @@ struct nvme_namespace {
 	nvme_t *ns_nvme;
 	uint8_t ns_eui64[8];
 	uint8_t	ns_nguid[16];
-	char	ns_name[17];
+	char	ns_name[11];
 
 	bd_handle_t ns_bd_hdl;