Re: [Qemu-block] [PATCH v8 4/9] block: introduce BDRV_REQ_ALLOCATE flag

[Anton Nefedov]

On 16/3/2018 2:01 PM, Alberto Garcia wrote:
> On Mon 12 Mar 2018 11:16:53 AM CET, Anton Nefedov wrote:
> > The flag is supposed to indicate that the region of the disk image has
> > to be sufficiently allocated so it reads as zeroes.
> >
> > The call with the flag set must return -ENOTSUP if allocation cannot
> > be done efficiently.
> > This has to be made sure of by both
> >    - the drivers that support the flag
> >    - and the common block layer (so it will not fall back to any slowpath
> >      (like writing zero buffers) in case the driver does not support
> >      the flag).
> >
> > Signed-off-by: Anton Nefedov <address@hidden>
> > Reviewed-by: Eric Blake <address@hidden>
> > Reviewed-by: Alberto Garcia <address@hidden>
>
> Hmm... this is not the patch that I reviewed, please remove the R-b
> lines when you submit a modified patch.

Oops. sorry

> > +    /* The BDRV_REQ_ALLOCATE flag is used to indicate that the driver has to
> > +     * efficiently allocate the space so it reads as zeroes, or return an 
> > error.
> > +     * Must be used together with BDRV_REQ_ZERO_WRITE.
>
> I guess it's understandable from the context but it's not immediately
> obvious that you can set BDRV_REQ_ZERO_WRITE but not this one.
>
> Perhaps it could be rephrased as something like "If this is set then
> BDRV_REQ_ZERO_WRITE must also be set".

Sound much better, fixed

> > +     * Contradictory to BDRV_REQ_MAY_UNMAP so the two must not be used 
> > together.
>
> Or simply "This flag cannot be set together with BDRV_REQ_MAY_UNMAP".

Done

> > +     */
> > +    BDRV_REQ_ALLOCATE           = 0x40,
>
> Berto