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 <[email protected]>
> ---
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.
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 linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
