Re: scsi scan bug when peripheral qualifier of 3 is returned

[Patrick Mansfield <patmans@us.ibm.com>]

On Wed, Jan 04, 2006 at 12:28:03PM -0600, Mike Christie wrote:
> Patrick Mansfield wrote:
> >On Wed, Jan 04, 2006 at 01:11:06AM -0600, Mike Christie wrote:
> >
> >
> >>But for SCSI_SCAN_TARGET_PRESENT bflags is not set. Is the correct fix 
> >>to move where bflagsp gets set in scsi_probe_and_add_lun so that it gets 
> >>set for the SCSI_SCAN_TARGET_PRESENT case, or should __scsi_scan_target 
> >>be passing scsi_sequential_lun_scan and possibly scsi_report_lun_scan 
> >>some default bflags values?
> >
> >
> >It looks OK to me as-is, since bflags is also passed to and set in
> >scsi_probe_lun(), right?
> >
> 
> A blagfs variable gets set but it is not the same one passed into 
> scsi_probe_and_add_lun.

Oh ... good catch there.

> static int scsi_probe_and_add_lun(struct scsi_target *starget,
>                                   uint lun, int *bflagsp,
> 
> 
> {
>         struct scsi_device *sdev;
>         unsigned char *result;
>         int bflags,
> 
> 
> scsi_probe_and_add_lun gets a *bflagsp passed to it as a function arg, 
> but then also decalres a bflags variable itself. It then passes 
> scsi_probe_lun() the bflags it declared and does this
> 
>         res = scsi_add_lun(sdev, result, &bflags);
>         if (res == SCSI_SCAN_LUN_PRESENT) {
>                 if (bflags & BLIST_KEY) {
>                         sdev->lockable = 0;
>                         scsi_unlock_floptical(sdev, result);
>                 }
>                 if (bflagsp)
>                         *bflagsp = bflags;
>         }
> 
> so *bflagsp pointer only gets set if SCSI_SCAN_LUN_PRESENT was returned 
> by scsi_add_lun. For SCSI_SCAN_TARGET_PRESENT we do not even get to 
> scsi_add_lun, so for this case *bflagsp never gets set and 
> __scsi_scan_target gets zero. Previously, __scsi_scan_target would just 
> pass scsi_sequential_lun_scan the sparse blist flag, but now it passes zero.

So yes we should always set *bflagsp. i.e.:

--- linux-2.6.15/drivers/scsi/orig-scsi_scan.c	2006-01-02 21:52:12.000000000 -0800
+++ linux-2.6.15/drivers/scsi/scsi_scan.c	2006-01-04 10:58:36.000000000 -0800
@@ -891,13 +891,13 @@ static int scsi_probe_and_add_lun(struct
 	}
 
 	res = scsi_add_lun(sdev, result, &bflags);
+	if (bflagsp)
+		*bflagsp = bflags;
 	if (res == SCSI_SCAN_LUN_PRESENT) {
 		if (bflags & BLIST_KEY) {
 			sdev->lockable = 0;
 			scsi_unlock_floptical(sdev, result);
 		}
-		if (bflagsp)
-			*bflagsp = bflags;
 	}
 
  out_free_result:


Also ... it looks like we should just pass bflags not &bflags to
scsi_add_lun(), since scsi_add_lun() does not modifies bflags.

-- Patrick Mansfield