From mboxrd@z Thu Jan  1 00:00:00 1970
From: hare@suse.de (Hannes Reinecke)
Date: Thu, 18 Feb 2016 16:39:45 +0100
Subject: NVMe wwid revisited
In-Reply-To: <20160218151010.GA4901@localhost.localdomain>
References: <56C5C15C.8030903@suse.de> <20160218132856.GA5150@lst.de>
 <20160218151010.GA4901@localhost.localdomain>
Message-ID: <56C5E5C1.1070707@suse.de>

On 02/18/2016 04:10 PM, Keith Busch wrote:
> On Thu, Feb 18, 2016@02:28:56PM +0100, Christoph Hellwig wrote:
>> On Thu, Feb 18, 2016@02:04:28PM +0100, Hannes Reinecke wrote:
>>> Which, of course, raises the question: What _is_ the correct
>>> identification?
>>> Implement it according to the translation spec, leaving out the 'T10
>>> Vendor Identification' field?
>>> Or implement it with the correct designator type?
>>
>> Leave it as-is, given a certain Linux distributor started using it for
>> their udev rules and we'd otherwise break their setup.
>
> We mentioned exposing a single sysfs handle that provides the unique so
> the user doesn't have to check/concat multiple files or use the deprecated
> NVMe SG_IO interface.
>
> Does the below patch look alright? It uses either 64 or 128 bit EUI if
> provided, or concats other parts to make a unique (and long!) identifier
> if the namespace doesn't support its own unique id.
>
> Here's a couple examples with a 1.0 controller and a 1.2 controller
> (I don't have a 1.1 controller; no EUI-64 example):
>
>    1.0:
>      # cat /sys/block/nvme0n1/wwid
>      nvme.8086-43564654343032333030324b38303043474e-494e54454c205353445045444d443830304734-00000001
>
>    1.2:
>      # cat /sys/block/nvme1n2/wwid
>      eui.5cd2e400000000000000000000000100000002
>
> ---
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index face31f..93ec2e0 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -872,6 +876,7 @@ int nvme_init_identify(struct nvme_ctrl *ctrl)
>   		return -EIO;
>   	}
>
> +	ctrl->vid = le16_to_cpu(id->vid);
>   	ctrl->oncs = le16_to_cpup(&id->oncs);
>   	atomic_set(&ctrl->abort_limit, id->acl + 1);
>   	ctrl->vwc = id->vwc;
> @@ -1008,6 +1013,30 @@ static ssize_t nvme_sysfs_reset(struct device *dev,
>   }
>   static DEVICE_ATTR(reset_controller, S_IWUSR, NULL, nvme_sysfs_reset);
>
> +static ssize_t wwid_show(struct device *dev, struct device_attribute *attr,
> +								char *buf)
> +{
> +	struct nvme_ns *ns = dev_to_disk(dev)->private_data;
> +	struct nvme_ctrl *ctrl = ns->ctrl;
> +	int serial_len = sizeof(ctrl->serial);
> +	int model_len = sizeof(ctrl->model);
> +
> +	if (memchr_inv(ns->uuid, 0, sizeof(ns->uuid)))
> +		return sprintf(buf, "eui.%16phN\n", ns->uuid);
> +
> +	if (memchr_inv(ns->eui, 0, sizeof(ns->eui)))
> +		return sprintf(buf, "eui.%8phN\n", ns->eui);
> +
> +	while (ctrl->serial[serial_len - 1] == ' ')
> +		serial_len--;
> +	while (ctrl->model[model_len - 1] == ' ')
> +		model_len--;
> +
> +	return sprintf(buf, "nvme.%04x-%*phN-%*phN-%08x\n", ctrl->vid,
> +		serial_len, ctrl->serial, model_len, ctrl->model, ns->ns_id);
> +}
> +static DEVICE_ATTR(wwid, S_IRUGO, wwid_show, NULL);
> +
>   static ssize_t uuid_show(struct device *dev, struct device_attribute *attr,
>   								char *buf)
>   {
> @@ -1033,6 +1062,7 @@ static ssize_t nsid_show(struct device *dev, struct device_attribute *attr,
>   static DEVICE_ATTR(nsid, S_IRUGO, nsid_show, NULL);
>
>   static struct attribute *nvme_ns_attrs[] = {
> +	&dev_attr_wwid.attr,
>   	&dev_attr_uuid.attr,
>   	&dev_attr_eui.attr,
>   	&dev_attr_nsid.attr,
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index dcdc2d7..b50e591 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -84,6 +84,7 @@ struct nvme_ctrl {
>   	u32 max_hw_sectors;
>   	u32 stripe_size;
>   	u16 oncs;
> +	u16 vid;
>   	atomic_t abort_limit;
>   	u8 event_limit;
>   	u8 vwc;
> ---
>
Yeah, that looks good.

Reviewed-by: Hannes Reinecke <hare at suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare at suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: J. Hawn, J. Guild, F. Imend?rffer, HRB 16746 (AG N?rnberg)