14550 nvme_is_ignored_ns() needs less confusion

14709 nvmeadm list should be able to distinguish between inactive and ignored namespaces
Reviewed by: Andrew Giles <agiles@tintri.com>
Reviewed by: Guy Morrogh <gmorrogh@tintri.com>
Reviewed by: Robert Mustacchi <rm+illumos@fingolfin.org>
Approved by: Gordon Ross <gordon.w.ross@gmail.com>

6 files changed, 164 insertions, 38 deletions

diff --git a/usr/src/cmd/nvmeadm/nvmeadm.c b/usr/src/cmd/nvmeadm/nvmeadm.c
index 399631da88..18b1117a89 100644
--- a/usr/src/cmd/nvmeadm/nvmeadm.c
+++ b/usr/src/cmd/nvmeadm/nvmeadm.c
@@ -567,9 +567,10 @@ nvme_process(di_node_t node, di_minor_t minor, void *arg)
 	if (npa->npa_idns == NULL)
 		goto out;
 
+	npa->npa_dsk = NULL;
 	if (npa->npa_isns) {
-		npa->npa_ignored = nvme_is_ignored_ns(fd);
-		if (!npa->npa_ignored)
+		npa->npa_ns_state = nvme_namespace_state(fd);
+		if ((npa->npa_ns_state & NVME_NS_STATE_ATTACHED) != 0)
 			npa->npa_dsk = nvme_dskname(npa);
 	}
 
@@ -668,23 +669,38 @@ static int
 do_list_nsid(int fd, const nvme_process_arg_t *npa)
 {
 	_NOTE(ARGUNUSED(fd));
-	const uint_t format = npa->npa_idns->id_flbas.lba_format;
-	const uint_t bshift = npa->npa_idns->id_lbaf[format].lbaf_lbads;
+	char *dskname;
 
-	/*
-	 * Some devices have extra namespaces with illegal block sizes and
-	 * zero blocks. Don't list them when verbose operation isn't requested.
-	 */
-	if ((bshift < 9 || npa->npa_idns->id_nsize == 0) && verbose == 0)
+	if (!npa->npa_interactive &&
+	    (npa->npa_ns_state & NVME_NS_STATE_IGNORED) != 0 &&
+	    verbose == 0)
 		return (0);
 
-	if (npa->npa_ofmt == NULL) {
+	if (npa->npa_ofmt != NULL) {
+		ofmt_print(npa->npa_ofmt, (void *)npa);
+		return (0);
+	}
+
+	if (npa->npa_ns_state == NVME_NS_STATE_IGNORED) {
+		(void) printf("  %s/%s (unallocated)\n", npa->npa_name,
+		    di_minor_name(npa->npa_minor));
+	} else {
+		if ((npa->npa_ns_state & NVME_NS_STATE_ATTACHED) != 0) {
+			dskname = npa->npa_dsk;
+		} else if ((npa->npa_ns_state & NVME_NS_STATE_ACTIVE) != 0) {
+			if ((npa->npa_ns_state & NVME_NS_STATE_IGNORED) != 0) {
+				dskname = "ignored";
+			} else {
+				dskname = "unattached";
+			}
+		} else if ((npa->npa_ns_state & NVME_NS_STATE_ALLOCATED) != 0) {
+			dskname = "inactive";
+		} else {
+			dskname = "invalid state";
+		}
 		(void) printf("  %s/%s (%s): ", npa->npa_name,
-		    di_minor_name(npa->npa_minor),
-		    npa->npa_dsk != NULL ? npa->npa_dsk : "unattached");
+		    di_minor_name(npa->npa_minor), dskname);
 		nvme_print_nsid_summary(npa->npa_idns);
-	} else {
-		ofmt_print(npa->npa_ofmt, (void *)npa);
 	}
 
 	return (0);
@@ -868,6 +884,10 @@ do_get_logpage(int fd, const nvme_process_arg_t *npa)
 	else
 		errx(-1, "invalid log page: %s", npa->npa_argv[0]);
 
+	if (npa->npa_isns &&
+	    (npa->npa_ns_state & NVME_NS_STATE_ACTIVE) == 0)
+		errx(-1, "cannot get logpage: namespace is inactive");
+
 	ret = func(fd, npa);
 	return (ret);
 }
@@ -1092,6 +1112,10 @@ do_get_features(int fd, const nvme_process_arg_t *npa)
 	if (npa->npa_argc > 1)
 		errx(-1, "unexpected arguments");
 
+	if (npa->npa_isns &&
+	    (npa->npa_ns_state & NVME_NS_STATE_ACTIVE) == 0)
+		errx(-1, "cannot get feature: namespace is inactive");
+
 	/*
 	 * No feature list given, print all supported features.
 	 */
@@ -1167,6 +1191,12 @@ do_format_common(int fd, const nvme_process_arg_t *npa, unsigned long lbaf,
 	nvme_process_arg_t ns_npa = { 0 };
 	nvmeadm_cmd_t cmd = { 0 };
 
+	if (npa->npa_isns &&
+	    (npa->npa_ns_state & NVME_NS_STATE_ACTIVE) == 0) {
+		errx(-1, "cannot %s: namespace is inactive",
+		    npa->npa_cmd->c_name);
+	}
+
 	cmd = *(npa->npa_cmd);
 	cmd.c_func = do_attach_detach;
 	cmd.c_name = "detach";
@@ -1316,7 +1346,8 @@ do_attach_detach(int fd, const nvme_process_arg_t *npa)
 	 * that are ignored by the driver, thereby avoiding printing pointless
 	 * error messages.
 	 */
-	if (!npa->npa_interactive && npa->npa_ignored)
+	if (!npa->npa_interactive &&
+	    (npa->npa_ns_state & NVME_NS_STATE_IGNORED))
 		return (0);
 
 	if ((c_name[0] == 'd' ? nvme_detach : nvme_attach)(fd)
diff --git a/usr/src/cmd/nvmeadm/nvmeadm.h b/usr/src/cmd/nvmeadm/nvmeadm.h
index ab4d4e4113..0be877305f 100644
--- a/usr/src/cmd/nvmeadm/nvmeadm.h
+++ b/usr/src/cmd/nvmeadm/nvmeadm.h
@@ -43,7 +43,6 @@ struct nvme_process_arg {
 	int npa_found;
 	boolean_t npa_excl;
 	boolean_t npa_isns;
-	boolean_t npa_ignored;
 	boolean_t npa_interactive;
 	uint32_t npa_cmdflags;
 	const nvmeadm_cmd_t *npa_cmd;
@@ -51,6 +50,7 @@ struct nvme_process_arg {
 	di_minor_t npa_minor;
 	char *npa_path;
 	char *npa_dsk;
+	uint32_t npa_ns_state;
 	nvme_identify_ctrl_t *npa_idctl;
 	nvme_identify_nsid_t *npa_idns;
 	nvme_version_t *npa_version;
@@ -118,7 +118,7 @@ extern boolean_t nvme_detach(int);
 extern boolean_t nvme_attach(int);
 extern boolean_t nvme_firmware_load(int, void *, size_t, offset_t, uint16_t *);
 extern boolean_t nvme_firmware_commit(int, int, int, uint16_t *);
-extern boolean_t nvme_is_ignored_ns(int);
+extern boolean_t nvme_namespace_state(int);
 
 /*
  * ofmt related
diff --git a/usr/src/cmd/nvmeadm/nvmeadm_dev.c b/usr/src/cmd/nvmeadm/nvmeadm_dev.c
index ce86a8d164..a86224c973 100644
--- a/usr/src/cmd/nvmeadm/nvmeadm_dev.c
+++ b/usr/src/cmd/nvmeadm/nvmeadm_dev.c
@@ -222,22 +222,23 @@ nvme_firmware_commit(int fd, int slot, int action, uint16_t *sc)
 	return (rv);
 }
 
-boolean_t
-nvme_is_ignored_ns(int fd)
+uint32_t
+nvme_namespace_state(int fd)
 {
-	boolean_t ret;
 	uint64_t res = 0;
 
 	/*
-	 * The ioctl shouldn't fail. If it does, we treat it the same as if the
-	 * namespace was ignored.
+	 * Ask the driver for the namespace state.
 	 */
-	ret = nvme_ioctl(fd, NVME_IOC_IS_IGNORED_NS, NULL, NULL, 0, &res);
-
-	if (ret)
-		ret = (res == 0) ? B_FALSE : B_TRUE;
+	if (nvme_ioctl(fd, NVME_IOC_NS_STATE, NULL, NULL, 0, &res)) {
+		return (res);
+	}
 
-	return (ret);
+	/*
+	 * We're only here if the ioctl failed, which it really shouldnt. If so,
+	 * we treat this the same as if the namespace was ignored.
+	 */
+	return (NVME_NS_STATE_IGNORED);
 }
 
 int
diff --git a/usr/src/uts/common/io/nvme/nvme.c b/usr/src/uts/common/io/nvme/nvme.c
index e65a7a4139..a59564544f 100644
--- a/usr/src/uts/common/io/nvme/nvme.c
+++ b/usr/src/uts/common/io/nvme/nvme.c
@@ -2923,6 +2923,44 @@ nvme_prepare_devid(nvme_t *nvme, uint32_t nsid)
 	    nvme->n_idctl->id_vid, model, serial, nsid);
 }
 
+static boolean_t
+nvme_allocated_ns(nvme_namespace_t *ns)
+{
+	nvme_t *nvme = ns->ns_nvme;
+
+	ASSERT(MUTEX_HELD(&nvme->n_mgmt_mutex));
+
+	/*
+	 * Since we don't know any better, we assume all namespaces to be
+	 * allocated.
+	 */
+	return (B_TRUE);
+}
+
+static boolean_t
+nvme_active_ns(nvme_namespace_t *ns)
+{
+	nvme_t *nvme = ns->ns_nvme;
+	boolean_t ret = B_FALSE;
+	uint64_t *ptr;
+
+	ASSERT(MUTEX_HELD(&nvme->n_mgmt_mutex));
+
+	/*
+	 * Check whether the IDENTIFY NAMESPACE data is zero-filled.
+	 */
+	for (ptr = (uint64_t *)ns->ns_idns;
+	    ptr != (uint64_t *)(ns->ns_idns + 1);
+	    ptr++) {
+		if (*ptr != 0) {
+			ret = B_TRUE;
+			break;
+		}
+	}
+
+	return (ret);
+}
+
 static int
 nvme_init_ns(nvme_t *nvme, int nsid)
 {
@@ -2946,6 +2984,12 @@ nvme_init_ns(nvme_t *nvme, int nsid)
 
 	ns->ns_idns = idns;
 	ns->ns_id = nsid;
+
+	was_ignored = ns->ns_ignore;
+
+	ns->ns_allocated = nvme_allocated_ns(ns);
+	ns->ns_active = nvme_active_ns(ns);
+
 	ns->ns_block_count = idns->id_nsize;
 	ns->ns_block_size =
 	    1 << idns->id_lbaf[idns->id_flbas.lba_format].lbaf_lbads;
@@ -3410,7 +3454,7 @@ nvme_init(nvme_t *nvme)
 	nvme->n_progress_supported = B_TRUE;
 
 	/*
-	 * Identify Namespaces
+	 * Get number of supported namespaces and allocate namespace array.
 	 */
 	nvme->n_namespace_count = nvme->n_idctl->id_nn;
 
@@ -4884,6 +4928,9 @@ nvme_ioctl_get_logpage(nvme_t *nvme, int nsid, nvme_ioctl_t *nioc,
 	if ((mode & FREAD) == 0)
 		return (EPERM);
 
+	if (nsid > 0 && !NVME_NSID2NS(nvme, nsid)->ns_active)
+		return (EINVAL);
+
 	switch (nioc->n_arg) {
 	case NVME_LOGPAGE_ERROR:
 		if (nsid != 0)
@@ -4949,6 +4996,9 @@ nvme_ioctl_get_features(nvme_t *nvme, int nsid, nvme_ioctl_t *nioc,
 	if ((mode & FREAD) == 0)
 		return (EPERM);
 
+	if (nsid > 0 && !NVME_NSID2NS(nvme, nsid)->ns_active)
+		return (EINVAL);
+
 	if ((nioc->n_arg >> 32) > 0xff)
 		return (EINVAL);
 
@@ -5099,8 +5149,12 @@ nvme_ioctl_format(nvme_t *nvme, int nsid, nvme_ioctl_t *nioc, int mode,
 	if (nm->nm_oexcl != curthread)
 		return (EACCES);
 
-	if (nsid != 0 && NVME_NSID2NS(nvme, nsid)->ns_attached)
-		return (EBUSY);
+	if (nsid != 0) {
+		if (NVME_NSID2NS(nvme, nsid)->ns_attached)
+			return (EBUSY);
+		else if (!NVME_NSID2NS(nvme, nsid)->ns_active)
+			return (EINVAL);
+	}
 
 	frmt.r = nioc->n_arg & 0xffffffff;
 
@@ -5597,10 +5651,11 @@ out:
 }
 
 static int
-nvme_ioctl_is_ignored_ns(nvme_t *nvme, int nsid, nvme_ioctl_t *nioc, int mode,
+nvme_ioctl_ns_state(nvme_t *nvme, int nsid, nvme_ioctl_t *nioc, int mode,
     cred_t *cred_p)
 {
 	_NOTE(ARGUNUSED(cred_p));
+	nvme_namespace_t *ns = NVME_NSID2NS(nvme, nsid);
 
 	if ((mode & FREAD) == 0)
 		return (EPERM);
@@ -5608,10 +5663,23 @@ nvme_ioctl_is_ignored_ns(nvme_t *nvme, int nsid, nvme_ioctl_t *nioc, int mode,
 	if (nsid == 0)
 		return (EINVAL);
 
-	if (NVME_NSID2NS(nvme, nsid)->ns_ignore)
-		nioc->n_arg = 1;
-	else
-		nioc->n_arg = 0;
+	nioc->n_arg = 0;
+
+	mutex_enter(&nvme->n_mgmt_mutex);
+
+	if (ns->ns_allocated)
+		nioc->n_arg |= NVME_NS_STATE_ALLOCATED;
+
+	if (ns->ns_active)
+		nioc->n_arg |= NVME_NS_STATE_ACTIVE;
+
+	if (ns->ns_attached)
+		nioc->n_arg |= NVME_NS_STATE_ATTACHED;
+
+	if (ns->ns_ignore)
+		nioc->n_arg |= NVME_NS_STATE_IGNORED;
+
+	mutex_exit(&nvme->n_mgmt_mutex);
 
 	return (0);
 }
@@ -5644,7 +5712,7 @@ nvme_ioctl(dev_t dev, int cmd, intptr_t arg, int mode, cred_t *cred_p,
 		nvme_ioctl_firmware_download,
 		nvme_ioctl_firmware_commit,
 		nvme_ioctl_passthru,
-		nvme_ioctl_is_ignored_ns
+		nvme_ioctl_ns_state
 	};
 
 	if (nvme == NULL)
diff --git a/usr/src/uts/common/io/nvme/nvme_var.h b/usr/src/uts/common/io/nvme/nvme_var.h
index 0266f193dc..10cc529fd9 100644
--- a/usr/src/uts/common/io/nvme/nvme_var.h
+++ b/usr/src/uts/common/io/nvme/nvme_var.h
@@ -287,6 +287,8 @@ struct nvme_namespace {
 	size_t ns_block_size;
 	size_t ns_best_block_size;
 
+	boolean_t ns_allocated;
+	boolean_t ns_active;
 	boolean_t ns_ignore;
 	boolean_t ns_attached;
 
diff --git a/usr/src/uts/common/sys/nvme.h b/usr/src/uts/common/sys/nvme.h
index 9e154131b1..04f39057e2 100644
--- a/usr/src/uts/common/sys/nvme.h
+++ b/usr/src/uts/common/sys/nvme.h
@@ -54,8 +54,8 @@ extern "C" {
 #define	NVME_IOC_FIRMWARE_DOWNLOAD	(NVME_IOC | 11)
 #define	NVME_IOC_FIRMWARE_COMMIT	(NVME_IOC | 12)
 #define	NVME_IOC_PASSTHRU		(NVME_IOC | 13)
-#define	NVME_IOC_IS_IGNORED_NS		(NVME_IOC | 14)
-#define	NVME_IOC_MAX			NVME_IOC_IS_IGNORED_NS
+#define	NVME_IOC_NS_STATE		(NVME_IOC | 14)
+#define	NVME_IOC_MAX			NVME_IOC_NS_STATE
 
 #define	IS_NVME_IOC(x)			((x) > NVME_IOC && (x) <= NVME_IOC_MAX)
 #define	NVME_IOC_CMD(x)			((x) & 0xff)
@@ -1029,6 +1029,30 @@ typedef struct {
 } nvme_passthru_cmd32_t;
 #endif
 
+/*
+ * NVME namespace state flags for NVME_IOC_NS_STATE ioctl
+ *
+ * The values are defined entirely by the driver. Some states correspond to
+ * namespace states described by the NVMe specification r1.3 section 6.1, others
+ * are specific to the implementation of this driver.
+ *
+ * The states are as follows:
+ * - ALLOCATED: the namespace exists in the controller as per the NVMe spec
+ * - ACTIVE: the namespace exists and is attached to this controller as per the
+ *   NVMe spec. Any namespace that is ACTIVE is also ALLOCATED. This must not be
+ *   confused with the ATTACHED state.
+ * - ATTACHED: the driver has attached a blkdev(4D) instance to this namespace.
+ *   This state can be changed by userspace with the ioctls NVME_IOC_ATTACH and
+ *   NVME_IOC_DETACH. A namespace can only be ATTACHED when it is not IGNORED.
+ * - IGNORED: the driver ignores this namespace, it never attaches a blkdev(4D).
+ *   Namespaces are IGNORED when they are not ACTIVE, or if they are ACTIVE but
+ *   have certain properties that the driver cannot handle.
+ */
+#define	NVME_NS_STATE_ALLOCATED		0x1
+#define	NVME_NS_STATE_ACTIVE		0x2
+#define	NVME_NS_STATE_ATTACHED		0x4
+#define	NVME_NS_STATE_IGNORED		0x8
+
 #ifdef __cplusplus
 }
 #endif