Re: [PATCH] scsi: disable VPD page check on error

["Ewan D. Milne" <emilne@redhat.com>]

On Mon, 2016-06-13 at 14:48 +0200, Hannes Reinecke wrote:
> If we encounter an error during VPD page scanning we should be
> setting the 'skip_vpd_pages' bit to avoid further accesses.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> ---
>  drivers/scsi/scsi.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
> index 1deb6ad..0359864 100644
> --- a/drivers/scsi/scsi.c
> +++ b/drivers/scsi/scsi.c
> @@ -796,6 +796,7 @@ retry_pg0:
>  	result = scsi_vpd_inquiry(sdev, vpd_buf, 0, vpd_len);
>  	if (result < 0) {
>  		kfree(vpd_buf);
> +		sdev->skip_vpd_pages = 1;
>  		return;
>  	}
>  	if (result > vpd_len) {
> @@ -822,6 +823,7 @@ retry_pg80:
>  		result = scsi_vpd_inquiry(sdev, vpd_buf, 0x80, vpd_len);
>  		if (result < 0) {
>  			kfree(vpd_buf);
> +			sdev->skip_vpd_pages = 1;
>  			return;
>  		}
>  		if (result > vpd_len) {
> @@ -851,6 +853,7 @@ retry_pg83:
>  		result = scsi_vpd_inquiry(sdev, vpd_buf, 0x83, vpd_len);
>  		if (result < 0) {
>  			kfree(vpd_buf);
> +			sdev->skip_vpd_pages = 1;
>  			return;
>  		}
>  		if (result > vpd_len) {

So, this changes scsi_attach_vpd() but not scsi_get_vpd_page() ?

This particular implementation worries me.  If we get an error
performing a VPD inquiry, we will never, ever, attempt one again?  What
happens if a path is down at the time?  The idea behind getting updated
VPD info was that it might change, so if that does happen we don't want
to stop updating after an isolated error.

I think what we want to do is check if the VPD inquiry is supported on
the *initial* inquiry, and if that fails then suppress further updates.

-Ewan