Re: [PATCH 10/14] libmultipath: hwtable: multibus for NetApp NVMe-FC

[Martin Wilck <mwilck@suse.com>]

On Sat, 2018-01-13 at 00:42 +0100, Xose Vazquez Perez wrote:
> On 01/12/2018 11:07 PM, Martin Wilck wrote:
> 
> > Use multibus policy for NetApp NVMe-FC namespace controllers.
> > 
> > Signed-off-by: Martin Wilck <mwilck@suse.com>
> > ---
> >  libmultipath/hwtable.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> > 
> > diff --git a/libmultipath/hwtable.c b/libmultipath/hwtable.c
> > index 40c4724fcd1b..bae280c835e6 100644
> > --- a/libmultipath/hwtable.c
> > +++ b/libmultipath/hwtable.c
> > @@ -1142,6 +1142,19 @@ static struct hwentry default_hw[] = {
> >  		.checker_name  = NONE,
> >  		.retain_hwhandler = RETAIN_HWHANDLER_OFF,
> >  	},
> > +	/*
> > +	 * NetApp NVMe-FC namespace devices: MULTIBUS preferred
> > +	 *
> > +	 * The table is searched backwards, so place this after
> > generic NVMe
> 
> Too tricky.

Sorry? This is just utilizing the logic we've implemented in find_hwe()
to let user-specified entries override built-in ones. Nothing "tricky"
about it. Please read the code. I just added that comment there because
the way libmultipath handles this (generic entries first, specific
later) is a bit unusual - casual readers might think the ordering is
wrong, which it is not.

> 
> > +	 */
> > +	{
> > +		.vendor	       = "NVME",
> > +		.product       = "(NetApp |)ONTAP Controller)",
> 
>                                           ^^ Is this correct?

What do you mean? It's correct posix regexp syntax. And AFAIK it's also
the correct expression to identify the controllers in question.

> > +		.uid_attribute = "ID_WWN",
> > +		.checker_name  = NONE,
> > +		.pgpolicy      = MULTIBUS,
> > +		.retain_hwhandler = RETAIN_HWHANDLER_OFF,
> > +	},
> 
> Place it inside NetApp vendor entry, and remove:
>         /*
>          * Generic NVMe devices
>          */
>         {
>                 .vendor        = "NVME",
>                 .product       = ".*",
>                 .uid_attribute = "ID_WWN",
>                 .checker_name  = NONE,
>                 .retain_hwhandler = RETAIN_HWHANDLER_OFF,
>         },

That won't work. The NetApp vendor entry uses '.vendor = "NETAPP"',
while this device has '.vendor = "NVME"'. 

And sorry again, the generic entry needs to stay in place. All NVMe
controllers we know expect to be configured with the default "failover"
policy. The "ONTAP controller" is the first exception to this rule.
More may be added later. As explained earlier, because of the backward-
search logic in find_hwe(), specific entries need to be listed *after*
generic entries.

If it's important for you to have this Netapp specific entry in the
section commented "NetApp": that can be done, if we move the generic
NVMe entry to the top of hwtable.c. I personally don't care. I 

Martin


-- 
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel