On Thu, 2012-09-06 at 22:51 +0200, Paolo Bonzini wrote: > Il 06/09/2012 20:58, Nicholas A. Bellinger ha scritto: > > On Thu, 2012-09-06 at 17:13 +0200, Paolo Bonzini wrote: > >> The purpose of this function is to clear a LUN set in the CDB, in case > >> the initiator talking to us is speaking an old standards version. > >> However, as things stand, pscsi_clear_cdb_lun has two problems. It > >> will "deceive" the guest by clearing the LUN bits on initial > >> commands (INQUIRY, TEST UNIT READY, etc.); but then it will let the > >> LUN bits through on reads, which will likely fail due to protection not > >> being enabled. Second, not all commands are properly filtered, in > >> particular WRITE's WRPROTECT bits are cleared. > >> > >> This should be done by the fabric module rather than by PSCSI, if it > >> knows such initiators may be lying around _and_ it can assume that its > >> initiators do not really care about protection information. Nuke it > >> from PSCSI. > >> > >> Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx> > >> --- > > > > NAK. This code was originally added to prevent certain pSCSI HBAs from > > going bonkers when they got a legacy SCSI LUN ID encoded within the CDB > > (eg: the fabric LUN ID) that's different from the physical SCSI LUN ID > > on an Parallel SCSI bus. > > I understood, but what's good in making INQUIRY work, if the HBA will > equally go bonkers on the first actual I/O (READ/WRITE/VERIFY are all > affected)? > The original purpose of pscsi_clear_cdb_lun() is to clear byte 1 bit 0-5 for all incoming CDBs in SCSI-2 that did encode LUN ID into the CDB as a work-around for (some) Parallel SCSI hardware that depends on this value to function. As modern SCSI-3 + SAS fabrics, et al have moved away from doing this completely, thinking about this some more I would be OK with disabling this by default for modern SCSI HW if the pSCSI device is reporting >= SCSI-3 from se_subsystem_api->get_device_rev(). > This is something the LLDs or the fabric module should be doing. But I > guess it could be an attribute too. > I think it's safe to disable this for modern SCSI hw, but lets keep it around for pSCSI backend devices that are reporting SCSI-2 support and below. Care to respin..? ;) Thanks Paolo! -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
