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)?
This is something the LLDs or the fabric module should be doing. But I
guess it could be an attribute too.
Paolo
> If there are more special cases where the legacy SCSI LUN ID bit-range
> needs to be cleared than please add those missing bits, but I don't
> think it's yet safe to remove this completely for pSCSI w/ older LLDs.
>
>> drivers/target/target_core_pscsi.c | 26 --------------------------
>> 1 files changed, 0 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/target/target_core_pscsi.c b/drivers/target/target_core_pscsi.c
>> index 5f7151d..a0fb2c3 100644
>> --- a/drivers/target/target_core_pscsi.c
>> +++ b/drivers/target/target_core_pscsi.c
>> @@ -1031,30 +1031,6 @@ fail:
>> return -ENOMEM;
>> }
>>
>> -/*
>> - * Clear a lun set in the cdb if the initiator talking to use spoke
>> - * and old standards version, as we can't assume the underlying device
>> - * won't choke up on it.
>> - */
>> -static inline void pscsi_clear_cdb_lun(unsigned char *cdb)
>> -{
>> - switch (cdb[0]) {
>> - case READ_10: /* SBC - RDProtect */
>> - case READ_12: /* SBC - RDProtect */
>> - case READ_16: /* SBC - RDProtect */
>> - case SEND_DIAGNOSTIC: /* SPC - SELF-TEST Code */
>> - case VERIFY: /* SBC - VRProtect */
>> - case VERIFY_16: /* SBC - VRProtect */
>> - case WRITE_VERIFY: /* SBC - VRProtect */
>> - case WRITE_VERIFY_12: /* SBC - VRProtect */
>> - case MAINTENANCE_IN: /* SPC - Parameter Data Format for SA RTPG */
>> - break;
>> - default:
>> - cdb[1] &= 0x1f; /* clear logical unit number */
>> - break;
>> - }
>> -}
>> -
>> static int pscsi_parse_cdb(struct se_cmd *cmd)
>> {
>> unsigned char *cdb = cmd->t_task_cdb;
>> @@ -1067,8 +1043,6 @@ static int pscsi_parse_cdb(struct se_cmd *cmd)
>> return -EINVAL;
>> }
>>
>> - pscsi_clear_cdb_lun(cdb);
>> -
>> /*
>> * For REPORT LUNS we always need to emulate the response, for everything
>> * else the default for pSCSI is to pass the command to the underlying
>
>
--
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
