Re: [Qemu-devel] [PATCH] block: Don't lose FUA flag during ZERO_WRITE fallback

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2174 bytes --]

On 05/03/2016 01:55 AM, Kevin Wolf wrote:
>> And looking further, it looks like SCSI does NOT support FUA during
>> WRITESAME(10/16), only during WRITE(10/16).  Which means we probably
>> want to start having both .supported_write_flags AND
>> .supported_write_zero flags, so that at least the iscsi driver can
>> properly report that it does NOT natively support FUA on a write_zeroes
>> request.
> 
> Hm, not sure if it makes sense, but let me write that thought down:
> 
> You're going to convert .supported_write_flags to a function anyway.
> Instead of adding a second function, how about passing a set of flags
> there and the function returns a subset that it can support? For write
> zeroes with FUA you would pass in BDRV_REQ_ZERO_WRITE | BDRV_REQ_FUA and
> in this case the iscsi driver would return only BDRV_REQ_ZERO_WRITE.
> 
> If we ever decided to get rid of .bdrv_co_write_zeroes() and instead use
> .bdrv_co_pwritev() with BDRV_REQ_ZERO_WRITE, this would probably be the
> most consistent interface.

I'm currently leaning towards two flags rather than a function (the
flags are set during .bdrv_open(), and don't change during the life of
the BDS):

bs->supported_write_flags = BDRV_REQ_FUA;
bs->supported_zero_flags = BDRV_REQ_MAY_UNMAP; // | BDRV_REQ_FUA

and keeping the BDRV_REQ_ZERO_WRITE as a special case in the block layer
that chooses between .bdrv_co_write_zeroes() or .bdrv_driver_pwrite().
MAY_UNMAP makes no sense on a normal write, just as iscsi doesn't allow
FUA on a zero write, so having the two sets of valid flags according to
operations, as well as the two driver entries, seems to make the most
sense.  Then io.c, we ignore MAY_UNMAP where it is unsupported, and
emulate FUA on both write and write_zero as needed.

Of course, if we ever have a driver that can support everything through
a single entry, it could advertise supported_write_flags = BDRV_REQ_FUA
| BDRV_REQ_ZERO_WRITE | BDRV_REQ_MAY_UNMAP and provide only
.bdrv_driver_pwrite(), but I don't think that will be happening.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]