Re: [Qemu-devel] [PATCH] block/iscsi: use 16 byte CDBs only when necessary

[Peter Lieven]

Am 02.09.2014 um 17:28 schrieb ronnie sahlberg:
> That is one big request.  I assume the device reports "no limit" in
> the VPD page so we can not state it is the guest/application going
> beyond the allowed limit?

Yes, my storage reports no limit, but afaik there is no way to tell the
guest the limit at least with virtio-blk. Or can we?

If so we should define a sane limit in case the storage reports no limits.
It can't be our goal to allow the guest to issue a request in orders
of gigabytes. Which is possible. The xferlen in the iSCSI PDU is 32bit so
we have an actual limit of 2^32 - 1.

>
>
> I am not entirely sure what meaning the target assigns to Protocol
> Error means here.
> It could be that ~100M is way higher than MaxBurstLength ?  What is
> the MaxBurstLength that was reported by the server during login
> negotiation?
> If so, we should make libiscsi check the maxburstlength and fail the
> request early. We would still fail the I/O so it will not really solve
> anything much
> but at least we should not send the request to the server.

The "problem" is that we switched to read/write10. In this case
we can write 65535 sectors max. The procotol error refers to
the xferlen not matching the block count in the SCSI CDB in the
payload.

>
> Best would probably be to take the smallest of a non-zero
> Block-Limits.max_transfer_length and iscsi-MaxBurstLength/block-size
> and pass this back to the guest in the emulated Block-Limits-VPD.
> At least then you have tried to tell the guest "never do SCSI I/O
> bigger than this".

I would vote for the biggest number of sectors fitting in uint16_t. Which
is 32768. This is 16 MB with 512 byte sectors. We have already used
this for write zeroes and discard in case there is no limit specified.

>
> I.e. even if the target reports BlockLimits.MaxTransferLength == 0 ==
> no limit to QEMU, QEMU should probably take the iscsi transport limit
> into account and pass this to the guest
> by setting the emulated BlockLimits page it passes to scale to the
> maximum that MaxBurstLength allows.
>
>
> Then if BTRFS or SG_IO in the guest ignores the BlockLimits it is
> clearly a guest problem.
>
> (A different interpretation for ProtocolError could be the mismatch
> between the iscsi expected data transfer length and the scsi transfer
> length, but that should result in residuals, not protocol error.)
>
>
>
> Hypothetically there could be targets that support really huge
> MaxBurstLengths > 32MB. For those you probably want to switch to
> WRITE16 when the SCSI transfer length goes > 0xffff.
>
> - if (iscsilun->use_16_for_rw)  {
> + if (iscsilun->use_16_for_rw || num_sectors > 0xffff)  {

I already proposed this, but I think Michael voted that this
would cause unpredictable and hard to debug behaviour
for qemu.

[PATCH] block/iscsi: use 16 byte CDBs also for big requests
https://lists.nongnu.org/archive/html/qemu-devel/2014-06/msg03628.html

But maybe we have to do this if we cannot report the max
transfer length for virtio-blk or IDE. I will check what open-iscsi or
Linux does.

Peter