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
