Re: [PATCH 4/7] nvme: implement multipath access to nvme subsystems

Ahh, I incorporated non-multipath disks into the mix and observing some
trouble. Details below:

On Thu, Nov 09, 2017 at 06:44:47PM +0100, Christoph Hellwig wrote:

+#ifdef CONFIG_NVME_MULTIPATH
+	if (ns->head->disk) {
+		sprintf(disk_name, "nvme%dc%dn%d", ctrl->subsys->instance,
+				ctrl->cntlid, ns->head->instance);
+		flags = GENHD_FL_HIDDEN;
+	} else
+#endif
+		sprintf(disk_name, "nvme%dn%d", ctrl->instance, ns->head->instance);

...
  

+int nvme_mpath_alloc_disk(struct nvme_ctrl *ctrl, struct nvme_ns_head *head)
+{
+	struct request_queue *q;
+	bool vwc = false;
+
+	bio_list_init(&head->requeue_list);
+	spin_lock_init(&head->requeue_lock);
+	INIT_WORK(&head->requeue_work, nvme_requeue_work);
+
+	/*
+	 * Add a multipath node if the subsystems supports multiple controllers.
+	 * We also do this for private namespaces as the namespace sharing data could
+	 * change after a rescan.
+	 */
+	if (!(ctrl->subsys->cmic & (1 << 1)) || !multipath)
+		return 0;

...

+	sprintf(head->disk->disk_name, "nvme%dn%d",
+			ctrl->subsys->instance, head->instance);

If we've CMIC capabilities, we'll use the subsys->instance; if we don't
have CMIC, we use the ctrl->instance. 

Since the two instances are independent of each other, they can create
duplicate names.

To fix, I think we'll need to always use the subsys instance for
consistency if CONFIG_NVME_MULTIPATH=y.

---

@@ -2837,8 +2826,10 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid)
 				ctrl->cntlid, ns->head->instance);
 		flags = GENHD_FL_HIDDEN;
 	} else
+		sprintf(disk_name, "nvme%dn%d", ctrl->subsys->instance, ns->head->instance);
+#else
+	sprintf(disk_name, "nvme%dn%d", ctrl->instance, ns->head->instance);
 #endif
-		sprintf(disk_name, "nvme%dn%d", ctrl->instance, ns->head->instance);
 
 	if ((ctrl->quirks & NVME_QUIRK_LIGHTNVM) && id->vs[0] == 0x1) {
 		if (nvme_nvm_register(ns, disk_name, node)) {
--

This may confuse some people since the block device's name will not always
match up to the controller's chardev name, but I don't see another way
to do it.