Re: [PATCH v2 10/17] libmultipath: add code to get vendor specific vpd data

[Benjamin Marzinski <bmarzins@redhat.com>]

On Mon, Feb 10, 2020 at 02:49:38PM +0000, Martin Wilck wrote:
> On Wed, 2020-02-05 at 12:58 -0600, Benjamin Marzinski wrote:
> > This adds the wildcard 'g' for multipath and path formatted printing,
> > which returns extra data from a device's vendor specific vpd
> > page.  The
> > specific vendor vpd page to use, and the vendor/product id to decode
> > it
> > can be set in the hwentry with vpd_vendor_pg and vpd_vendor_id. It
> > can
> > be configured in the devices section of multipath.conf with the
> > vpd_vendor parameter. Currently, the only devices that use this are
> > HPE
> > 3PAR arrays, to return the Volume Name.
> > 
> > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> > ---
> >  libmultipath/config.c      |  2 ++
> >  libmultipath/config.h      |  1 +
> >  libmultipath/dict.c        | 38 ++++++++++++++++++++++++++++++++++++
> >  libmultipath/discovery.c   | 40
> > +++++++++++++++++++++++++++++++++++++-
> >  libmultipath/hwtable.c     |  1 +
> >  libmultipath/print.c       | 25 ++++++++++++++++++++++++
> >  libmultipath/propsel.c     | 18 +++++++++++++++++
> >  libmultipath/propsel.h     |  1 +
> >  libmultipath/structs.h     | 15 ++++++++++++++
> >  multipath/multipath.conf.5 |  8 ++++++++
> >  10 files changed, 148 insertions(+), 1 deletion(-)
> > 
> > diff --git a/libmultipath/structs.h b/libmultipath/structs.h
> > index 1c32a799..6c03ee5d 100644
> > --- a/libmultipath/structs.h
> > +++ b/libmultipath/structs.h
> > @@ -21,6 +21,7 @@
> >  #define HOST_NAME_LEN		16
> >  #define SLOT_NAME_SIZE		40
> >  #define PRKEY_SIZE		19
> > +#define VPD_DATA_SIZE		128
> >  
> >  #define SCSI_VENDOR_SIZE	9
> >  #define SCSI_PRODUCT_SIZE	17
> > @@ -221,6 +222,18 @@ enum all_tg_pt_states {
> >  	ALL_TG_PT_ON = YNU_YES,
> >  };
> >  
> > +enum vpd_vendor_ids {
> > +	VPD_VP_UNDEF,
> > +	VPD_VP_HP3PAR,
> > +	VPD_VP_ARRAY_SIZE, /* This must remain the last entry */
> > +};
> > +
> > +struct vpd_vendor_page {
> > +	int pg;
> > +	const char *name;
> > +};
> > +extern struct vpd_vendor_page vpd_vendor_pages[VPD_VP_ARRAY_SIZE];
> > +
> >  struct sg_id {
> >  	int host_no;
> >  	int channel;
> > @@ -255,6 +268,7 @@ struct path {
> >  	char rev[PATH_REV_SIZE];
> >  	char serial[SERIAL_SIZE];
> >  	char tgt_node_name[NODE_NAME_SIZE];
> > +	char vpd_data[VPD_DATA_SIZE];
> 
> 
> Hm, 128 more bytes per path for a rarely-used property... do we need to
> store this in "struct path"? Can't we simply fetch that information
> when someone requests it with the %g format wildcard? It doesn't matter
> much today as "struct path" is really thick already, but I have hopes
> to make it a bit leaner some day.

Well, the thought was that this information (which should help map the
path to a physical array) would most often be wanted when things were
going badly, and you might not be able to access the device.

-Ben

> Regards
> Martin
> 
> -- 
> Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
> SUSE  Software Solutions Germany GmbH
> HRB 36809, AG Nürnberg GF: Felix
> Imendörffer
>