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 <js...@redhat.com > <mailto:js...@redhat.com>> 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 <ls...@suse.com <mailto:ls...@suse.com>> > > --- > > 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 > >
