lkml.org 
[lkml]   [2012]   [Sep]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 3.6 1/2] target: remove pscsi_clear_cdb_lun
    From
    Date
    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@redhat.com>
    > ---

    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




    \
     
     \ /
      Last update: 2012-09-06 21:43    [W:2.947 / U:0.144 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site