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

[Keith Busch <keith.busch@intel.com>]

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.