[dm-devel] [PATCH v2 8/9] libmultipath: check_rdac(): pre-check in hwtable

[Martin Wilck]

On Mon, 2019-03-18 at 08:10 +0100, Hannes Reinecke wrote:
> On 3/15/19 6:19 PM, Martin Wilck wrote:
> > Currently check_rdac() always runs an SG_IO for VPD 0xc9 to check
> > if the storage supports RDAC. This is an extra IO, and may cause
> > annoying error messages on the storage side for non-RDAC arrays.
> > Do the RDAC override only for arrays that have legacy configuration
> > to use the rdac checker.
> > 
> > Cc: Steve.Schremmer at netapp.com
> > Cc: NetApp RDAC team <ng-eseries-upstream-maintainers at netapp.com>
> > Cc: Hannes Reinecke <hare at suse.com>
> > Signed-off-by: Martin Wilck <mwilck at suse.com>
> > ---
> >   libmultipath/propsel.c | 8 ++++++++
> >   1 file changed, 8 insertions(+)
> > 
> > diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
> > index 27474f05..8c08a5cc 100644
> > --- a/libmultipath/propsel.c
> > +++ b/libmultipath/propsel.c
> > @@ -470,9 +470,17 @@ check_rdac(struct path * pp)
> >   {
> >   	int len;
> >   	char buff[44];
> > +	const char *checker_name = NULL;
> > +	/* dummy, for do_set_from_hwe */
> > +	const char *origin __attribute__((unused));
> >   
> >   	if (pp->bus != SYSFS_BUS_SCSI)
> >   		return 0;
> > +	/* Avoid ioctl if this is likely not an RDAC array */
> > +	do_set_from_hwe(checker_name, pp, checker_name, NULL);
> > +out:    /* for do_set_from_hwe */
> > +	if (!checker_name || strcmp(checker_name, RDAC))
> > +		return 0;
> >   	len = get_vpd_sgio(pp->fd, 0xC9, buff, 44);
> >   	if (len <= 0)
> >   		return 0;
> > 
> *origin? dummy for do_set_from_hwe()?
> What funky interface is that?
> 
> Please redesign do_set_from_hwe() so as _not_ to require 'magic'
> variables.

It's the truly "funky" way the macros are written in dict.c :-/
(Admittedly I wrote that one myself, but I was following the style of
the file).

But you're right, I don't like this "dummy" stuff either. As this patch
set is meant as a bug fix and not a major redesign of dict.c, I guess
I'll rather not use the do_set_from_hwe() macro there. And put the
redesign on my to-do list.

Thanks,
Martin

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