[dm-devel] [PATCH v2 08/17] libmultipath: fix sgio_get_vpd looping

[Martin Wilck]

On Wed, 2020-02-05 at 12:58 -0600, Benjamin Marzinski wrote:
> If do_inq returns a page with a length that is less than maxlen, but
> larger than DEFAULT_SGIO_LEN, this function will loop forever. Also
> if do_inq returns with a length equal to or greater than maxlen,
> sgio_get_vpd will exit immediately, even if it hasn't read the entire
> page.  Fix these issues, modify the tests to verify the new behavior.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins at redhat.com>
> ---
>  libmultipath/discovery.c | 12 +++----
>  tests/vpd.c              | 77 ++++++++++++++++++++++++------------
> ----
>  2 files changed, 52 insertions(+), 37 deletions(-)
> 
> 
> diff --git a/tests/vpd.c b/tests/vpd.c
> index d9f80eaa..1e653ed4 100644
> --- a/tests/vpd.c
> +++ b/tests/vpd.c
> \
> @@ -518,10 +519,16 @@ static void test_vpd_eui_ ## len ## _ ##
> wlen(void **state)             \
> 	\
>  	n = create_vpd83(vt->vpdbuf, sizeof(vt->vpdbuf), test_id,	\
>  			 2, 0, len);					\
> +	if (sml) {							\
> +		/* overwrite the page side to DEFAULT_SGIO_LEN + 1 */	\
> +		put_unaligned_be16(255, vt->vpdbuf + 2);		\
> +		will_return(__wrap_ioctl, n);				\
> +		will_return(__wrap_ioctl, vt->vpdbuf);			\
> +	}								\
>  	will_return(__wrap_ioctl, n);					\
>  	will_return(__wrap_ioctl, vt->vpdbuf);				

Nitpick: 
1. "side" -> "size"
2. This looks like a missing "else" clause, even though it is not
(because get_vpd_sgio will make 2nd ioctl call). Perhaps add a comment
explaining that.

Anyway, that can be improved later, no need to resend the series again.
So:

Reviewed-by: Martin Wilck <mwilck at suse.com>

-- 
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