[dm-devel] [PATCH 2/4] libmultipath: fix priorities in parse_vpd_pg83

[Benjamin Marzinski]

On Fri, Mar 26, 2021 at 05:12:36PM +0000, Martin Wilck wrote:
> On Thu, 2021-03-25 at 19:52 -0500, Benjamin Marzinski wrote:
> > The priorities for the EUI-64 (0x02) and NAME (0x08) scsi identifiers
> > in
> > parse_vpd_pg83() don't match their priorities in 55-scsi-sg3_id.rules.
> > Switch them so that they match.
> 
> I think we should rather change the udev rules file, to be consistent
> with what the kernel does:
> 
> https://elixir.bootlin.com/linux/latest/A/ident/designator_prio

If we were going with the kernel as a standard, we should also change
scsi_id, since it doesn't match that the kernel either (or the priority
ordering in 55-scsi-sg3_id.rules for that matter).  The bigger issue
here is that distributions would need to make special plans to take this
change, because user's devices would change uuids, which could cause
problems on existing systems. It will definitely do so on systems using
multipath. Devices will suddenly change wwids, which will rename them.
The whole reason why Red Hat installs 55-scsi-sg3_id.rules as
61-scsi-sg3_id.rules is beause we didn't initally include it, and we
don't want go changing people's device UUIDs, so is has to go after
60-persistent-storage.rules, which sets ID_SERIAL if it's not already
set.

On the other hand, it's obviously safe to make our fallback method of
getting wwids return the same wwids as the primary method does.  Since
recheck_wwids relies on both methods giving the same wwid, I would argue
that it's a bug that they don't. If we don't add this fix, then we
should add back the code that disables recheck_wwids if multipathd
doesn't see that both methods return the same wwid at least once, so we
know that we can rely on parse_vpd_pg83.

Speaking of which, I was planning on doing more work to make our
fallback method of returning wwids work more like 55-scsi-sg3_id.rules
and 60-persistent-storage.rules. For instance, both of those will fall
back to using page 0x80, if setting ID_SERIAL from page 0x83 fails.

-Ben

> 
> Regards
> Martin
> 
> 
> 
> > 
> > Signed-off-by: Benjamin Marzinski <bmarzins at redhat.com>
> > ---
> >  libmultipath/discovery.c | 16 ++++++++--------
> >  1 file changed, 8 insertions(+), 8 deletions(-)
> > 
> > diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
> > index 5727f7a6..f8044141 100644
> > --- a/libmultipath/discovery.c
> > +++ b/libmultipath/discovery.c
> > @@ -1152,19 +1152,19 @@ parse_vpd_pg83(const unsigned char *in, size_t
> > in_len,
> >                                 vpd = d;
> >                         }
> >                         break;
> > -               case 0x8:
> > -                       /* SCSI Name: Prio 4 */
> > -                       if (memcmp(d + 4, "eui.", 4) &&
> > -                           memcmp(d + 4, "naa.", 4) &&
> > -                           memcmp(d + 4, "iqn.", 4))
> > -                               break;
> > +               case 0x2:
> > +                       /* EUI-64: Prio 4 */
> >                         if (prio < 4) {
> >                                 prio = 4;
> >                                 vpd = d;
> >                         }
> >                         break;
> > -               case 0x2:
> > -                       /* EUI-64: Prio 3 */
> > +               case 0x8:
> > +                       /* SCSI Name: Prio 3 */
> > +                       if (memcmp(d + 4, "eui.", 4) &&
> > +                           memcmp(d + 4, "naa.", 4) &&
> > +                           memcmp(d + 4, "iqn.", 4))
> > +                               break;
> >                         if (prio < 3) {
> >                                 prio = 3;
> >                                 vpd = d;
> 
> -- 
> Dr. Martin Wilck <mwilck at suse.com>, Tel. +49 (0)911 74053 2107
> SUSE Software Solutions Germany GmbH
> HRB 36809, AG Nürnberg GF: Felix Imendörffer
>