On Wed, Jan 22, 2020 at 12:52:10PM +0100, Martin Wilck wrote:
> On Wed, 2020-01-22 at 02:51 -0600, Benjamin Marzinski wrote:
> > On Fri, Jan 17, 2020 at 05:56:40PM +0100, Martin Wilck wrote:
> > > On Thu, 2020-01-16 at 20:18 -0600, Benjamin Marzinski wrote:
> > > BUT ok, that's overengineered with just one supported combination
> > > of
> > > page and vendor. I can understand that. But then, it seems also
> > > overengineered to carry around vpd_vendor_pg and vpd_vendor_id in
> > > the
> > > hwe.
> > >
> > > I'd suggest creating a hard-coded table with "vendor vpd schemes",
> > >
> > > struct {
> > > int pg;
> > > int vendor_id;
> > > const char *name;
> > > } vendor_vpd_schemes[] = {
> > > { 0xc0, VPD_VP_HP3PAR, "hp3par", },
> > > { 0, },
> > > };
> > >
> > > and in the hwentry, only carry around the index into this table,
> > > and look up the page and vendor ID there. Actually, with just that
> > > single user, we might as well not introduce a new device property
> > > at
> > > all, but simply use a separate hardcoded table with lookup values
> > > for
> > > vendor and product ID. I'm unsure if it's wise to make this
> > > configurable via multipath.conf.
> >
> > Sure. I can make the hwe change. The reason why I added this to
> > multipath.conf is that I'm not sure what devices actually support
> > these
> > vpd information pages. So, if we miss one, or a new device comes out
> > that isn't in the built-in config, the user can still get this info.
> >
>
> My main issue was that you have 2 fields in struct hwentry, and only
> one field in the hwtable. I think I made the point that I'd prefer to
> have only one everywhere, and use some sort of lookup table for the
> page and vendor ID parameters.
I meant I agreed with this approach.
> Another issue that I realized just now, the "vendor_id" is not
> standardized - it's just an internal identifier used by sg3_utils and
> has no specific meaning outside the sg3_utils code base. Even it's
> numeric value has changed between sg3_utils versions. I think we should
> cast it away, and simply stick with the symbolic identifier. Keeping
> that symbolic name in line with sg3_utils makes a lot of sense, of
> course.
Yeah, another reason I didn't go with vpd_vendor_pg and vpd_vendor_id
was that on earlier versions of sg_vpd, the vpd_vendor_id for
hp3par was different. We can easily remove it altogether.
-Ben
> Regards,
> Martin
>
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
