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

    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
    >
    >



    \
     
     \ /
      Last update: 2012-09-07 00:42    [W:2.913 / U:0.192 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site