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
-
: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
