Re: [Qemu-devel] [PATCH 1/2] chardev: Make the name of memory device consistent

[Eric Blake]

On 05/21/2013 04:27 AM, Lei Li wrote:
> Now we have memory char device, but the backend name of it
> is a little confusion. We actually register it by 'memory', but
> the description in qemu-option, the name of open functions
> and the new api backend called it 'ringbuf'. It should keep
> consistent. This patch named it all to 'memory'.
> 
> Signed-off-by: Lei Li <address@hidden>
> ---
>  qapi-schema.json |    6 +++---
>  qemu-char.c      |   16 ++++++++--------
>  qemu-options.hx  |    6 +++---
>  3 files changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 9302e7d..664b31f 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3286,7 +3286,7 @@
>                                   '*rows'   : 'int' } }
>  
>  ##
> -# @ChardevRingbuf:
> +# @ChardevMemory:
>  #
>  # Configuration info for memory chardevs
>  #
> @@ -3294,7 +3294,7 @@
>  #
>  # Since: 1.5
>  ##
> -{ 'type': 'ChardevRingbuf', 'data': { '*size'  : 'int' } }
> +{ 'type': 'ChardevMemory', 'data': { '*size'  : 'int' } }

Since we don't have introspection (yet), this change is fine.

> +++ b/qemu-options.hx
> @@ -1779,7 +1779,7 @@ DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
>      "-chardev msmouse,id=id[,mux=on|off]\n"
>      "-chardev 
> vc,id=id[[,width=width][,height=height]][[,cols=cols][,rows=rows]]\n"
>      "         [,mux=on|off]\n"
> -    "-chardev ringbuf,id=id[,size=size]\n"
> +    "-chardev memory,id=id[,size=size]\n"

This change impacts the command line.  Have you tested whether
'query-command-line-options' exposes the difference?  Libvirt does not
(yet) expose a ringbuf/memory device.  Assuming we add support in the
future, we have several possibilities:

1. we have some way to detect the command line naming, and can thus
support the device for both qemu 1.5 and 1.6
2. we can't detect the command line naming, so we can only use the
device in 1.6 or newer even though it was technically available in 1.5
3. qemu continues to support ringbuf on the command line as a (possibly
undocumented) alias forever, so that libvirt would ALWAYS use ringbuf
regardless of qemu version

My preference would be #1 or #3; but I could live with #2 as a last resort.

As written, I don't know if your patch is option #1 or #2.  I don't know
whether query-command-line-options can expose the difference, or whether
it can be enhanced to expose the difference in time for 1.0.6 (where if
the enhancement is not present, we assume 1.5 behavior of the old name).
 If you can prove we can introspect the change in the command line (ie.
prove this is option #1), then you can add:
Reviewed-by: Eric Blake <address@hidden>

although I still think it might be worth considering option #3 in a
respin or as a followup patch.

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

signature.asc
Description: OpenPGP digital signature