[Qemu-devel] [PATCH 0/3] Update documentation and help related to the preallocation parameter

[Qemu-devel] [PATCH 0/3] Update documentation and help related to the preallocation parameter

[Stefano Garzarella]

Following Markus' advice, I updated the documentation of preallocation
parameter in qapi/block-core.json adding default and allowed values (patch 1).
I also updated the help related to BLOCK_OPT_PREALLOC in the QemuOptsList of
file-posix (patch 2) and gluster (patch 3).

Stefano Garzarella (3):
  qapi/block-core: update documentation of preallocation parameter
  block/file-posix: update .help of BLOCK_OPT_PREALLOC option
  block/gluster: update .help of BLOCK_OPT_PREALLOC option

 qapi/block-core.json | 20 ++++++++++++++------
 block/file-posix.c   |  6 +++++-
 block/gluster.c      |  9 ++++++++-
 3 files changed, 27 insertions(+), 8 deletions(-)

-- 
2.20.1

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

[Stefano Garzarella]

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 <armbru@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 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)
 # @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
 ##
-- 
2.20.1

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

[Markus Armbruster]

Stefano Garzarella <sgarzare@redhat.com> 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 <armbru@redhat.com>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>  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).

>  # @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 <armbru@redhat.com>

Re: [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 <sgarzare@redhat.com> 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 <armbru@redhat.com>
> > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> > ---
> >  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 <armbru@redhat.com>

Thanks,
Stefano

[Qemu-devel] [PATCH 2/3] block/file-posix: update .help of BLOCK_OPT_PREALLOC option

[Stefano Garzarella]

Show 'falloc' among the allowed values of 'preallocation'
option, only when it is supported (if defined CONFIG_POSIX_FALLOCATE)

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 block/file-posix.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index d018429672..9632e3a87b 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2751,7 +2751,11 @@ static QemuOptsList raw_create_opts = {
         {
             .name = BLOCK_OPT_PREALLOC,
             .type = QEMU_OPT_STRING,
-            .help = "Preallocation mode (allowed values: off, falloc, full)"
+            .help = "Preallocation mode (allowed values: off"
+#ifdef CONFIG_POSIX_FALLOCATE
+                    ", falloc"
+#endif
+                    ", full)"
         },
         { /* end of list */ }
     }
-- 
2.20.1

Re: [Qemu-devel] [PATCH 2/3] block/file-posix: update .help of BLOCK_OPT_PREALLOC option

[Markus Armbruster]

Stefano Garzarella <sgarzare@redhat.com> writes:

> Show 'falloc' among the allowed values of 'preallocation'
> option, only when it is supported (if defined CONFIG_POSIX_FALLOCATE)
>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>  block/file-posix.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/block/file-posix.c b/block/file-posix.c
> index d018429672..9632e3a87b 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -2751,7 +2751,11 @@ static QemuOptsList raw_create_opts = {
>          {
>              .name = BLOCK_OPT_PREALLOC,
>              .type = QEMU_OPT_STRING,
> -            .help = "Preallocation mode (allowed values: off, falloc, full)"
> +            .help = "Preallocation mode (allowed values: off"
> +#ifdef CONFIG_POSIX_FALLOCATE
> +                    ", falloc"
> +#endif
> +                    ", full)"
>          },
>          { /* end of list */ }
>      }

Reviewed-by: Markus Armbruster <armbru@redhat.com>

[Qemu-devel] [PATCH 3/3] block/gluster: update .help of BLOCK_OPT_PREALLOC option

[Stefano Garzarella]

Show 'falloc' and 'full' among the allowed values of
'preallocation' option, only when they are supported
('falloc' is support if defined CONFIG_GLUSTERFS_FALLOCATE,
'full' is support if defined CONFIG_GLUSTERFS_ZEROFILL)

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 block/gluster.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/block/gluster.c b/block/gluster.c
index e664ca4462..682fe49912 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -97,7 +97,14 @@ static QemuOptsList qemu_gluster_create_opts = {
         {
             .name = BLOCK_OPT_PREALLOC,
             .type = QEMU_OPT_STRING,
-            .help = "Preallocation mode (allowed values: off, full)"
+            .help = "Preallocation mode (allowed values: off"
+#ifdef CONFIG_GLUSTERFS_FALLOCATE
+                    ", falloc"
+#endif
+#ifdef CONFIG_GLUSTERFS_ZEROFILL
+                    ", full"
+#endif
+                    ")"
         },
         {
             .name = GLUSTER_OPT_DEBUG,
-- 
2.20.1

Re: [Qemu-devel] [PATCH 3/3] block/gluster: update .help of BLOCK_OPT_PREALLOC option

[Markus Armbruster]

Stefano Garzarella <sgarzare@redhat.com> writes:

> Show 'falloc' and 'full' among the allowed values of
> 'preallocation' option, only when they are supported
> ('falloc' is support if defined CONFIG_GLUSTERFS_FALLOCATE,
> 'full' is support if defined CONFIG_GLUSTERFS_ZEROFILL)

You could mention that 'falloc' is missing before the patch.

> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>  block/gluster.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/block/gluster.c b/block/gluster.c
> index e664ca4462..682fe49912 100644
> --- a/block/gluster.c
> +++ b/block/gluster.c
> @@ -97,7 +97,14 @@ static QemuOptsList qemu_gluster_create_opts = {
>          {
>              .name = BLOCK_OPT_PREALLOC,
>              .type = QEMU_OPT_STRING,
> -            .help = "Preallocation mode (allowed values: off, full)"
> +            .help = "Preallocation mode (allowed values: off"
> +#ifdef CONFIG_GLUSTERFS_FALLOCATE
> +                    ", falloc"
> +#endif
> +#ifdef CONFIG_GLUSTERFS_ZEROFILL
> +                    ", full"
> +#endif
> +                    ")"
>          },
>          {
>              .name = GLUSTER_OPT_DEBUG,

Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [Qemu-devel] [PATCH 3/3] block/gluster: update .help of BLOCK_OPT_PREALLOC option

[Stefano Garzarella]

On Thu, May 23, 2019 at 06:35:18PM +0200, Markus Armbruster wrote:
> Stefano Garzarella <sgarzare@redhat.com> writes:
> 
> > Show 'falloc' and 'full' among the allowed values of
> > 'preallocation' option, only when they are supported
> > ('falloc' is support if defined CONFIG_GLUSTERFS_FALLOCATE,
> > 'full' is support if defined CONFIG_GLUSTERFS_ZEROFILL)
> 
> You could mention that 'falloc' is missing before the patch.
> 

I'll fix the commit message.

> > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> > ---
> >  block/gluster.c | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/block/gluster.c b/block/gluster.c
> > index e664ca4462..682fe49912 100644
> > --- a/block/gluster.c
> > +++ b/block/gluster.c
> > @@ -97,7 +97,14 @@ static QemuOptsList qemu_gluster_create_opts = {
> >          {
> >              .name = BLOCK_OPT_PREALLOC,
> >              .type = QEMU_OPT_STRING,
> > -            .help = "Preallocation mode (allowed values: off, full)"
> > +            .help = "Preallocation mode (allowed values: off"
> > +#ifdef CONFIG_GLUSTERFS_FALLOCATE
> > +                    ", falloc"
> > +#endif
> > +#ifdef CONFIG_GLUSTERFS_ZEROFILL
> > +                    ", full"
> > +#endif
> > +                    ")"
> >          },
> >          {
> >              .name = GLUSTER_OPT_DEBUG,
> 
> Reviewed-by: Markus Armbruster <armbru@redhat.com>

Thanks,
Stefano