From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57675) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aGsKZ-00037a-Et for qemu-devel@nongnu.org; Wed, 06 Jan 2016 12:57:08 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aGsKY-0000vA-As for qemu-devel@nongnu.org; Wed, 06 Jan 2016 12:57:07 -0500 References: <1451359934-9236-1-git-send-email-lszhu@suse.com> <568C1C9A.7090802@redhat.com> From: John Snow Message-ID: <568D556D.2090408@redhat.com> Date: Wed, 6 Jan 2016 12:57:01 -0500 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [Qemu-block] [PATCH] send readcapacity10 when readcapacity16 failed List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Zhu Lingshan Cc: Paolo Bonzini , Peter Lieven , qemu-block@nongnu.org, ronnie sahlberg , qemu-devel On 01/05/2016 02:57 PM, ronnie sahlberg wrote: > MMC devices: > READ CAPACITY 10 support is mandatory. > No support for READ CAPACITY 16 > > SBC devices: > READ CAPACITY 10 is mandatory > READ CAPACITY 16 support is only required when you have thin > provisioning or protection information (or if the device is >2^32 blocks) > Almost all, but apparently not all, SBC devices support both. > > > For SBC devices you probably want to start with RC16 and only fallback > to RC10 if you get INVALID_OPCODE. > You start with RC16 since this is the way to discover if you have thin > provisioning or special protection information. > > For MMC devices you could try the "try RC16 first and fallback to RC10" > but as probably almost no MMC devices support RC16 it makes little sense > to do so. > > Ronnie: Thanks for the explanation! Zhu: In light of this, can the patch be reworked slightly to explicitly check *why* READCAPACITY16 failed and only attempt the READCAPACITY10 as a fallback if it receives INVALID_OPCODE? If it fails for any other reason it's probably best to report the error and let QEMU decide what to do about it. I suppose caching a flag that lets us know to go straight to READ_CAPACITY10 is not worthwhile because this command is not likely to be issued very often. Thanks, --js > > On Tue, Jan 5, 2016 at 11:42 AM, John Snow > wrote: > > > > On 12/28/2015 10:32 PM, Zhu Lingshan wrote: > > When play with Dell MD3000 target, for sure it > > is a TYPE_DISK, but readcapacity16 would fail. > > Then we find that readcapacity10 succeeded. It > > looks like the target just support readcapacity10 > > even through it is a TYPE_DISK or have some > > TYPE_ROM characteristics. > > > > This patch can give a chance to send > > readcapacity16 when readcapacity10 failed. > > This patch is not harmful to original pathes > > > > Signed-off-by: Zhu Lingshan > > > --- > > block/iscsi.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/block/iscsi.c b/block/iscsi.c > > index bd1f1bf..c8d167f 100644 > > --- a/block/iscsi.c > > +++ b/block/iscsi.c > > @@ -1243,8 +1243,9 @@ static void iscsi_readcapacity_sync(IscsiLun > *iscsilun, Error **errp) > > iscsilun->lbprz = !!rc16->lbprz; > > iscsilun->use_16_for_rw = (rc16->returned_lba > > 0xffffffff); > > } > > + break; > > } > > - break; > > + //fall through to try readcapacity10 instead > > case TYPE_ROM: > > task = iscsi_readcapacity10_sync(iscsilun->iscsi, > iscsilun->lun, 0, 0); > > if (task != NULL && task->status == SCSI_STATUS_GOOD) { > > > > For the uninitiated, why does readcapacity16 fail? > > My gut feeling is that this is a hack, because: > > - Either readcapacity16 should work, or > - We shouldn't be choosing 10/16 based on the target type to begin with > > but I don't know much about iSCSI, so maybe You, Paolo or Peter could > fill me in. > > --js > >