[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1346988173.4162.581.camel@haakon2.linux-iscsi.org>
Date: Thu, 06 Sep 2012 20:22:53 -0700
From: "Nicholas A. Bellinger" <nab@...ux-iscsi.org>
To: Paolo Bonzini <pbonzini@...hat.com>
Cc: linux-kernel@...r.kernel.org, target-devel@...r.kernel.org
Subject: Re: [PATCH 3.6 1/2] target: remove pscsi_clear_cdb_lun
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@...hat.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)?
>
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 linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists