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

[Eric Blake]

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