Re: [Qemu-block] [Qemu-devel] [PATCH 1/3] qapi/block-core: update documentation of preallocation parameter

[Stefano Garzarella]

On Thu, May 23, 2019 at 06:33:06PM +0200, Markus Armbruster wrote:
> Stefano Garzarella <address@hidden> writes:
> 
> > Add default and available values in the documentation block of
> > each block device or protocol that supports the 'preallocation'
> > parameter during the image creation.
> >
> > Suggested-by: Markus Armbruster <address@hidden>
> > Signed-off-by: Stefano Garzarella <address@hidden>
> > ---
> >  qapi/block-core.json | 20 ++++++++++++++------
> >  1 file changed, 14 insertions(+), 6 deletions(-)
> >
> > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > index 7ccbfff9d0..6aab0ebfe3 100644
> > --- a/qapi/block-core.json
> > +++ b/qapi/block-core.json
> > @@ -4103,7 +4103,10 @@
> >  #
> >  # @filename         Filename for the new image file
> >  # @size             Size of the virtual disk in bytes
> > -# @preallocation    Preallocation mode for the new image (default: off)
> > +# @preallocation    Preallocation mode for the new image (default: off;
> > +#                   allowed values for file-posix: off,
> > +#                   falloc (if defined CONFIG_POSIX_FALLOCATE), full;
> > +#                   allowed values for file-win32: off)
> 
> "file-win32" isn't a QAPI thing, it's the stem of the source file where
> the driver consuming this is defined.  Not ideal
> 
> Perhaps: off, falloc (if defined CONFIG_POSIX_FALLOCATE), full (if
> defined CONFIG_POSIX).
> 

Yes, it's better :) I'll fix it in the v2.

> >  # @nocow            Turn off copy-on-write (valid only on btrfs; default: 
> > off)
> >  #
> >  # Since: 2.12
> > @@ -4121,7 +4124,10 @@
> >  #
> >  # @location         Where to store the new image file
> >  # @size             Size of the virtual disk in bytes
> > -# @preallocation    Preallocation mode for the new image (default: off)
> > +# @preallocation    Preallocation mode for the new image (default: off;
> > +#                   allowed values: off,
> > +#                   falloc (if defined CONFIG_GLUSTERFS_FALLOCATE),
> > +#                   full (if defined CONFIG_GLUSTERFS_ZEROFILL))
> >  #
> >  # Since: 2.12
> >  ##
> > @@ -4225,7 +4231,8 @@
> >  # @backing-fmt      Name of the block driver to use for the backing file
> >  # @encrypt          Encryption options if the image should be encrypted
> >  # @cluster-size     qcow2 cluster size in bytes (default: 65536)
> > -# @preallocation    Preallocation mode for the new image (default: off)
> > +# @preallocation    Preallocation mode for the new image (default: off;
> > +#                   allowed values: off, falloc, full, metadata)
> >  # @lazy-refcounts   True if refcounts may be updated lazily (default: off)
> >  # @refcount-bits    Width of reference counts in bits (default: 16)
> >  #
> > @@ -4408,7 +4415,8 @@
> >  # @location         Where to store the new image file
> >  # @size             Size of the virtual disk in bytes
> >  # @backing-file     File name of a base image
> > -# @preallocation    Preallocation mode (allowed values: off, full)
> > +# @preallocation    Preallocation mode for the new image (default: off;
> > +#                   allowed values: off, full)
> >  # @redundancy       Redundancy of the image
> >  # @object-size      Object size of the image
> >  #
> > @@ -4443,8 +4451,8 @@
> >  #
> >  # @file             Node to create the image format on
> >  # @size             Size of the virtual disk in bytes
> > -# @preallocation    Preallocation mode for the new image (allowed values: 
> > off,
> > -#                   metadata; default: off)
> > +# @preallocation    Preallocation mode for the new image (default: off;
> > +#                   allowed values: off, metadata)
> >  #
> >  # Since: 2.12
> >  ##
> 
> Preferably with file-win32 rephrased:
> Reviewed-by: Markus Armbruster <address@hidden>

Thanks,
Stefano