Re: [PATCH v4 5/9] raw-format: Support BDRV_REQ_ZERO_WRITE for truncate

[Eric Blake <eblake@redhat.com>]

On 4/20/20 10:32 AM, Kevin Wolf wrote:

>>> @@ -445,6 +445,7 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags,
>>>        bs->supported_zero_flags = BDRV_REQ_WRITE_UNCHANGED |
>>>            ((BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP | BDRV_REQ_NO_FALLBACK) &
>>>                bs->file->bs->supported_zero_flags);
>>> +    bs->supported_truncate_flags = BDRV_REQ_ZERO_WRITE;
>>
>> Shouldn't this be:
>>
>> bs->supported_truncate_flags = (bs->file->bs->supported_truncate_flags &
>>                                  BDRV_REQ_ZERO_WRITE);
>>
>> rather than unconditionally advertising something that the underlying layer
>> may lack?
> 
> Maybe that makes more sense, yes.

If nothing else, it is more consistent with what we are doing for 
supported_zero_flags.  I also argue that having a reference to the 
passthrough is easier to grep for, if we ever add new flags in the 
future.  That is, while keeping passthrough as opt-in rather than blind 
copying or blind assignment is slightly more code, it is easier to maintain.

> 
> I think in practice it wouldn't make a difference because the nested
> bdrv_co_truncate() would still fail rather than silently ignoring the
> flag. It would behave the same as filter drivers, which also recursively
> call bdrv_co_truncate() without checking the flag first (which is, of
> course, because I don't want to modify each filter driver).

Probably true, but consistency and ease of maintenance are better than 
proving action at a distance :)

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org