[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Qemu-block] [PATCH] send readcapacity10 when readcapac
From: |
John Snow |
Subject: |
Re: [Qemu-devel] [Qemu-block] [PATCH] send readcapacity10 when readcapacity16 failed |
Date: |
Wed, 6 Jan 2016 12:57:01 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
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 <address@hidden
> <mailto:address@hidden>> 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 <address@hidden <mailto:address@hidden>>
> > ---
> > 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
>
>