Re: [Qemu-devel] [PATCH 3/5] block: make 'top' argument to block-commit optional

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2176 bytes --]

On 05/14/2014 09:20 PM, Jeff Cody wrote:
> Now that active layer block-commit is supported, the 'top' argument
> no longer needs to be mandatory.
> 
> Change it optional, with the default being the active layer in the
> device chain.

Good, this is an API-compatible change (doesn't break old clients that
always provide 'top').  I'm definitely in favor of it.

> 
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
>  blockdev.c             |  3 ++-
>  qapi-schema.json       |  7 ++++---
>  qmp-commands.hx        |  5 +++--
>  tests/qemu-iotests/040 | 28 ++++++++++++++++++----------
>  4 files changed, 27 insertions(+), 16 deletions(-)
> 

> +++ b/qapi-schema.json
> @@ -2099,8 +2099,9 @@
>  # @base:   #optional The file name of the backing image to write data into.
>  #                    If not specified, this is the deepest backing image
>  #
> -# @top:              The file name of the backing image within the image chain,
> -#                    which contains the topmost data to be committed down.
> +# @top:    #optional The file name of the backing image within the image chain,
> +#                    which contains the topmost data to be committed down. If
> +#                    not specified, this is the active layer.

We have not done the conversion from mandatory to optional very often; I
wonder if it is worth an annotation of when the conversion happened (so
someone reading just this version of the schema but planning on
targeting older qemu knows to always supply the argument).  Something like:

@top: since 1.3, #optional since 2.1; The file name...

But that feels rather long.  It's not a show-stopper to me, particularly
since we still don't have any automated conversion from these comments
into any other documentation formats.  That is, if we decide it is worth
adding hints about when something changed from mandatory to optional, it
would be its own patch and cover the work we've done on other recent
changes like block_passwd.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]