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

[Eric Blake]

On 05/02/2016 09:35 AM, Kevin Wolf wrote:
> Am 30.04.2016 um 23:48 hat Eric Blake geschrieben:
>> NBD has situations where it can support FUA but not ZERO_WRITE;
>> when that happens, the generic block layer fallback was losing
>> the FUA flag.  The problem of losing flags unrelated to
>> ZERO_WRITE has been latent in bdrv_co_do_write_zeroes() since
>> aa7bfbff, but back then, it did not matter because there was no
>> FUA flag.  But ever since 93f5e6d8 added bdrv_co_writev_flags(),
>> the loss of flags can impact correctness.
>>

>> +++ b/block/io.c
>> @@ -1213,7 +1213,8 @@ static int coroutine_fn 
>> bdrv_co_do_write_zeroes(BlockDriverState *bs,
>>              qemu_iovec_init_external(&qiov, &iov, 1);
>>
>>              ret = bdrv_driver_pwritev(bs, sector_num * BDRV_SECTOR_SIZE,
>> -                                      num * BDRV_SECTOR_SIZE, &qiov, 0);
>> +                                      num * BDRV_SECTOR_SIZE, &qiov,
>> +                                      flags & ~BDRV_REQ_ZERO_WRITE);
> 
> This is a good change, but it's only in the fallback code. If we succeed
> here:
> 
>     if (drv->bdrv_co_write_zeroes) {
>         ret = drv->bdrv_co_write_zeroes(bs, sector_num, num, flags);
>     }
> 
> then we still don't get the necessary flush unless the driver's
> .bdrv_co_write_zeroes() implementation explicitly handles FUA. As far as
> I know, we don't have any driver that implements FUA there.

NBD will, once we get to that part of my series.

But I see what you're saying: since we already emulate FUA for all
drivers where .supported_write_flags does not include BDRV_REQ_FUA
during bdrv_driver_pwritev(), we should also emulate it here for all the
same drivers (and any driver that DOES advertise BDRV_REQ_FUA as
supported as well as a write_zeroes callback should be fixed to honor
it).  I'll do that in v2, which I guess means I should post it at the
same time as my work for making .supported_write_flags a per-bds rather
than per-driver designation.

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

signature.asc
Description: OpenPGP digital signature