[Qemu-devel] [PATCH 0/3] Chardev related fixes

[Qemu-devel] [PATCH 0/3] Chardev related fixes

[Lei Li]

This series tries to fix the consistency of char devices udp and 
ringbuf and a regression of chardev filename.

Lei Li (3):
  chardev: Make consistent with udp device for new qapi backend
  chardev: Make the name of ringbuf device consistent
  chardev: Get filename for new qapi backend

 qapi-schema.json |   10 +++++-----
 qemu-char.c      |   24 +++++++++++++-----------
 2 files changed, 18 insertions(+), 16 deletions(-)

[Qemu-devel] [PATCH 1/3] chardev: Make consistent with udp device for new qapi backend

[Lei Li]

When register and open a chardev udp, the backend name should be udp 
not dgram, and we do not have backend dgram in the chardev list. This
patch makes the new qapi udp backend consistent with the original
udp device.

Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
---
 qapi-schema.json |    8 ++++----
 qemu-char.c      |   10 +++++-----
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 199744a..9302e7d 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3210,7 +3210,7 @@
                                      '*telnet'  : 'bool' } }
 
 ##
-# @ChardevDgram:
+# @ChardevUdp:
 #
 # Configuration info for datagram socket chardevs.
 #
@@ -3219,8 +3219,8 @@
 #
 # Since: 1.5
 ##
-{ 'type': 'ChardevDgram', 'data': { 'remote' : 'SocketAddress',
-                                    '*local' : 'SocketAddress' } }
+{ 'type': 'ChardevUdp', 'data': { 'remote' : 'SocketAddress',
+                                  '*local' : 'SocketAddress' } }
 
 ##
 # @ChardevMux:
@@ -3310,7 +3310,7 @@
                                        'parallel': 'ChardevHostdev',
                                        'pipe'   : 'ChardevHostdev',
                                        'socket' : 'ChardevSocket',
-                                       'dgram'  : 'ChardevDgram',
+                                       'udp'    : 'ChardevUdp',
                                        'pty'    : 'ChardevDummy',
                                        'null'   : 'ChardevDummy',
                                        'mux'    : 'ChardevMux',
diff --git a/qemu-char.c b/qemu-char.c
index 30a2ddf..cff2896 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3698,12 +3698,12 @@ static CharDriverState *qmp_chardev_open_socket(ChardevSocket *sock,
                                    is_telnet, is_waitconnect, errp);
 }
 
-static CharDriverState *qmp_chardev_open_dgram(ChardevDgram *dgram,
-                                               Error **errp)
+static CharDriverState *qmp_chardev_open_udp(ChardevUdp *udp,
+                                             Error **errp)
 {
     int fd;
 
-    fd = socket_dgram(dgram->remote, dgram->local, errp);
+    fd = socket_dgram(udp->remote, udp->local, errp);
     if (error_is_set(errp)) {
         return NULL;
     }
@@ -3739,8 +3739,8 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
     case CHARDEV_BACKEND_KIND_SOCKET:
         chr = qmp_chardev_open_socket(backend->socket, errp);
         break;
-    case CHARDEV_BACKEND_KIND_DGRAM:
-        chr = qmp_chardev_open_dgram(backend->dgram, errp);
+    case CHARDEV_BACKEND_KIND_UDP:
+        chr = qmp_chardev_open_udp(backend->udp, errp);
         break;
 #ifdef HAVE_CHARDEV_TTY
     case CHARDEV_BACKEND_KIND_PTY:
-- 
1.7.7.6

[Qemu-devel] [PATCH 2/3] chardev: Make the name of ringbuf device consistent

[Lei Li]

Now we have ringbuf 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 'ringbuf'.

Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
---
 qapi-schema.json |    2 +-
 qemu-char.c      |   12 ++++++------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 9302e7d..61f6b34 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3321,7 +3321,7 @@
                                        'spicevmc' : 'ChardevSpiceChannel',
                                        'spiceport' : 'ChardevSpicePort',
                                        'vc'     : 'ChardevVC',
-                                       'memory' : 'ChardevRingbuf' } }
+                                       'ringbuf': 'ChardevRingbuf' } }
 
 ##
 # @ChardevReturn:
diff --git a/qemu-char.c b/qemu-char.c
index cff2896..7163bbf 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3195,12 +3195,12 @@ static void qemu_chr_parse_ringbuf(QemuOpts *opts, ChardevBackend *backend,
 {
     int val;
 
-    backend->memory = g_new0(ChardevRingbuf, 1);
+    backend->ringbuf = g_new0(ChardevRingbuf, 1);
 
     val = qemu_opt_get_number(opts, "size", 0);
     if (val != 0) {
-        backend->memory->has_size = true;
-        backend->memory->size = val;
+        backend->ringbuf->has_size = true;
+        backend->ringbuf->size = val;
     }
 }
 
@@ -3786,8 +3786,8 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
     case CHARDEV_BACKEND_KIND_VC:
         chr = vc_init(backend->vc);
         break;
-    case CHARDEV_BACKEND_KIND_MEMORY:
-        chr = qemu_chr_open_ringbuf(backend->memory, errp);
+    case CHARDEV_BACKEND_KIND_RINGBUF:
+        chr = qemu_chr_open_ringbuf(backend->ringbuf, errp);
         break;
     default:
         error_setg(errp, "unknown chardev backend (%d)", backend->kind);
@@ -3831,7 +3831,7 @@ static void register_types(void)
     register_char_driver_qapi("null", CHARDEV_BACKEND_KIND_NULL, NULL);
     register_char_driver("socket", qemu_chr_open_socket);
     register_char_driver("udp", qemu_chr_open_udp);
-    register_char_driver_qapi("memory", CHARDEV_BACKEND_KIND_MEMORY,
+    register_char_driver_qapi("ringbuf", CHARDEV_BACKEND_KIND_RINGBUF,
                               qemu_chr_parse_ringbuf);
     register_char_driver_qapi("file", CHARDEV_BACKEND_KIND_FILE,
                               qemu_chr_parse_file_out);
-- 
1.7.7.6

[Qemu-devel] [PATCH 3/3] chardev: Get filename for new qapi backend

[Lei Li]

This patch sets the filename when the new qapi backend
init from opts.

The previous patch and discussions as link below:

http://patchwork.ozlabs.org/patch/243896/

If anyone who have better idea to fix this please let
me know your suggestions.

Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
---
 qemu-char.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 7163bbf..477f211 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3276,6 +3276,7 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
         ChardevReturn *ret = NULL;
         const char *id = qemu_opts_id(opts);
         const char *bid = NULL;
+        char *filename = g_strdup(qemu_opt_get(opts, "backend"));
 
         if (qemu_opt_get_bool(opts, "mux", 0)) {
             bid = g_strdup_printf("%s-base", id);
@@ -3308,6 +3309,7 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
         }
 
         chr = qemu_chr_find(id);
+        chr->filename = filename;
 
     qapi_out:
         qapi_free_ChardevBackend(backend);
-- 
1.7.7.6

Re: [Qemu-devel] [PATCH for-1.5? 1/3] chardev: Make consistent with udp device for new qapi backend

[Paolo Bonzini]

Il 20/05/2013 08:51, Lei Li ha scritto:
> When register and open a chardev udp, the backend name should be udp 
> not dgram, and we do not have backend dgram in the chardev list. This
> patch makes the new qapi udp backend consistent with the original
> udp device.

This changes the QMP API, so it has to go into 1.5 or never.

Paolo

> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
> ---
>  qapi-schema.json |    8 ++++----
>  qemu-char.c      |   10 +++++-----
>  2 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 199744a..9302e7d 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3210,7 +3210,7 @@
>                                       '*telnet'  : 'bool' } }
>  
>  ##
> -# @ChardevDgram:
> +# @ChardevUdp:
>  #
>  # Configuration info for datagram socket chardevs.
>  #
> @@ -3219,8 +3219,8 @@
>  #
>  # Since: 1.5
>  ##
> -{ 'type': 'ChardevDgram', 'data': { 'remote' : 'SocketAddress',
> -                                    '*local' : 'SocketAddress' } }
> +{ 'type': 'ChardevUdp', 'data': { 'remote' : 'SocketAddress',
> +                                  '*local' : 'SocketAddress' } }
>  
>  ##
>  # @ChardevMux:
> @@ -3310,7 +3310,7 @@
>                                         'parallel': 'ChardevHostdev',
>                                         'pipe'   : 'ChardevHostdev',
>                                         'socket' : 'ChardevSocket',
> -                                       'dgram'  : 'ChardevDgram',
> +                                       'udp'    : 'ChardevUdp',
>                                         'pty'    : 'ChardevDummy',
>                                         'null'   : 'ChardevDummy',
>                                         'mux'    : 'ChardevMux',
> diff --git a/qemu-char.c b/qemu-char.c
> index 30a2ddf..cff2896 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -3698,12 +3698,12 @@ static CharDriverState *qmp_chardev_open_socket(ChardevSocket *sock,
>                                     is_telnet, is_waitconnect, errp);
>  }
>  
> -static CharDriverState *qmp_chardev_open_dgram(ChardevDgram *dgram,
> -                                               Error **errp)
> +static CharDriverState *qmp_chardev_open_udp(ChardevUdp *udp,
> +                                             Error **errp)
>  {
>      int fd;
>  
> -    fd = socket_dgram(dgram->remote, dgram->local, errp);
> +    fd = socket_dgram(udp->remote, udp->local, errp);
>      if (error_is_set(errp)) {
>          return NULL;
>      }
> @@ -3739,8 +3739,8 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
>      case CHARDEV_BACKEND_KIND_SOCKET:
>          chr = qmp_chardev_open_socket(backend->socket, errp);
>          break;
> -    case CHARDEV_BACKEND_KIND_DGRAM:
> -        chr = qmp_chardev_open_dgram(backend->dgram, errp);
> +    case CHARDEV_BACKEND_KIND_UDP:
> +        chr = qmp_chardev_open_udp(backend->udp, errp);
>          break;
>  #ifdef HAVE_CHARDEV_TTY
>      case CHARDEV_BACKEND_KIND_PTY:
> 

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

[Paolo Bonzini]

Il 20/05/2013 08:51, Lei Li ha scritto:
> Now we have ringbuf 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 'ringbuf'.
> 
> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
> ---
>  qapi-schema.json |    2 +-
>  qemu-char.c      |   12 ++++++------
>  2 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 9302e7d..61f6b34 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3321,7 +3321,7 @@
>                                         'spicevmc' : 'ChardevSpiceChannel',
>                                         'spiceport' : 'ChardevSpicePort',
>                                         'vc'     : 'ChardevVC',
> -                                       'memory' : 'ChardevRingbuf' } }
> +                                       'ringbuf': 'ChardevRingbuf' } }
>  
>  ##
>  # @ChardevReturn:
> diff --git a/qemu-char.c b/qemu-char.c
> index cff2896..7163bbf 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -3195,12 +3195,12 @@ static void qemu_chr_parse_ringbuf(QemuOpts *opts, ChardevBackend *backend,
>  {
>      int val;
>  
> -    backend->memory = g_new0(ChardevRingbuf, 1);
> +    backend->ringbuf = g_new0(ChardevRingbuf, 1);
>  
>      val = qemu_opt_get_number(opts, "size", 0);
>      if (val != 0) {
> -        backend->memory->has_size = true;
> -        backend->memory->size = val;
> +        backend->ringbuf->has_size = true;
> +        backend->ringbuf->size = val;
>      }
>  }
>  
> @@ -3786,8 +3786,8 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
>      case CHARDEV_BACKEND_KIND_VC:
>          chr = vc_init(backend->vc);
>          break;
> -    case CHARDEV_BACKEND_KIND_MEMORY:
> -        chr = qemu_chr_open_ringbuf(backend->memory, errp);
> +    case CHARDEV_BACKEND_KIND_RINGBUF:
> +        chr = qemu_chr_open_ringbuf(backend->ringbuf, errp);
>          break;
>      default:
>          error_setg(errp, "unknown chardev backend (%d)", backend->kind);
> @@ -3831,7 +3831,7 @@ static void register_types(void)
>      register_char_driver_qapi("null", CHARDEV_BACKEND_KIND_NULL, NULL);
>      register_char_driver("socket", qemu_chr_open_socket);
>      register_char_driver("udp", qemu_chr_open_udp);
> -    register_char_driver_qapi("memory", CHARDEV_BACKEND_KIND_MEMORY,
> +    register_char_driver_qapi("ringbuf", CHARDEV_BACKEND_KIND_RINGBUF,
>                                qemu_chr_parse_ringbuf);
>      register_char_driver_qapi("file", CHARDEV_BACKEND_KIND_FILE,
>                                qemu_chr_parse_file_out);
> 

Same here as for patch 1.

Paolo

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

[Paolo Bonzini]

Il 20/05/2013 12:43, Paolo Bonzini ha scritto:
> Il 20/05/2013 08:51, Lei Li ha scritto:
>> Now we have ringbuf 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 'ringbuf'.
>>
>> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
>> ---
>>  qapi-schema.json |    2 +-
>>  qemu-char.c      |   12 ++++++------
>>  2 files changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index 9302e7d..61f6b34 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -3321,7 +3321,7 @@
>>                                         'spicevmc' : 'ChardevSpiceChannel',
>>                                         'spiceport' : 'ChardevSpicePort',
>>                                         'vc'     : 'ChardevVC',
>> -                                       'memory' : 'ChardevRingbuf' } }
>> +                                       'ringbuf': 'ChardevRingbuf' } }
>>  
>>  ##
>>  # @ChardevReturn:
>> diff --git a/qemu-char.c b/qemu-char.c
>> index cff2896..7163bbf 100644
>> --- a/qemu-char.c
>> +++ b/qemu-char.c
>> @@ -3195,12 +3195,12 @@ static void qemu_chr_parse_ringbuf(QemuOpts *opts, ChardevBackend *backend,
>>  {
>>      int val;
>>  
>> -    backend->memory = g_new0(ChardevRingbuf, 1);
>> +    backend->ringbuf = g_new0(ChardevRingbuf, 1);
>>  
>>      val = qemu_opt_get_number(opts, "size", 0);
>>      if (val != 0) {
>> -        backend->memory->has_size = true;
>> -        backend->memory->size = val;
>> +        backend->ringbuf->has_size = true;
>> +        backend->ringbuf->size = val;
>>      }
>>  }
>>  
>> @@ -3786,8 +3786,8 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
>>      case CHARDEV_BACKEND_KIND_VC:
>>          chr = vc_init(backend->vc);
>>          break;
>> -    case CHARDEV_BACKEND_KIND_MEMORY:
>> -        chr = qemu_chr_open_ringbuf(backend->memory, errp);
>> +    case CHARDEV_BACKEND_KIND_RINGBUF:
>> +        chr = qemu_chr_open_ringbuf(backend->ringbuf, errp);
>>          break;
>>      default:
>>          error_setg(errp, "unknown chardev backend (%d)", backend->kind);
>> @@ -3831,7 +3831,7 @@ static void register_types(void)
>>      register_char_driver_qapi("null", CHARDEV_BACKEND_KIND_NULL, NULL);
>>      register_char_driver("socket", qemu_chr_open_socket);
>>      register_char_driver("udp", qemu_chr_open_udp);
>> -    register_char_driver_qapi("memory", CHARDEV_BACKEND_KIND_MEMORY,
>> +    register_char_driver_qapi("ringbuf", CHARDEV_BACKEND_KIND_RINGBUF,
>>                                qemu_chr_parse_ringbuf);
>>      register_char_driver_qapi("file", CHARDEV_BACKEND_KIND_FILE,
>>                                qemu_chr_parse_file_out);
>>
> 
> Same here as for patch 1.

Oh, actually this is different.  The only inconsistency is in the name
of the type, the enum is consistent with the -chardev option and it
cannot be renamed because it was in QEMU 1.4.

So we can change the type, but we can do that post 1.5.

Paolo

Re: [Qemu-devel] [PATCH 1/3] chardev: Make consistent with udp device for new qapi backend

[Eric Blake]

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

On 05/20/2013 12:51 AM, Lei Li wrote:
> When register and open a chardev udp, the backend name should be udp 
> not dgram, and we do not have backend dgram in the chardev list. This
> patch makes the new qapi udp backend consistent with the original
> udp device.
> 
> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
> ---
>  qapi-schema.json |    8 ++++----
>  qemu-char.c      |   10 +++++-----
>  2 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 199744a..9302e7d 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3210,7 +3210,7 @@
>                                       '*telnet'  : 'bool' } }
>  
>  ##
> -# @ChardevDgram:
> +# @ChardevUdp:

This rename is okay,

> @@ -3310,7 +3310,7 @@
>                                         'parallel': 'ChardevHostdev',
>                                         'pipe'   : 'ChardevHostdev',
>                                         'socket' : 'ChardevSocket',
> -                                       'dgram'  : 'ChardevDgram',
> +                                       'udp'    : 'ChardevUdp',

but this rename would be ABI-visible if it doesn't make the release.

-- 
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: 621 bytes --]

Re: [Qemu-devel] [for 1.5? Qemu-devel] [PATCH 2/3] chardev: Make the name of ringbuf device consistent

[Eric Blake]

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

On 05/20/2013 04:59 AM, Paolo Bonzini wrote:
> Il 20/05/2013 12:43, Paolo Bonzini ha scritto:
>> Il 20/05/2013 08:51, Lei Li ha scritto:
>>> Now we have ringbuf 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 'ringbuf'.
>>>
>>> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
>>> ---
>>>  qapi-schema.json |    2 +-
>>>  qemu-char.c      |   12 ++++++------
>>>  2 files changed, 7 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/qapi-schema.json b/qapi-schema.json
>>> index 9302e7d..61f6b34 100644
>>> --- a/qapi-schema.json
>>> +++ b/qapi-schema.json
>>> @@ -3321,7 +3321,7 @@
>>>                                         'spicevmc' : 'ChardevSpiceChannel',
>>>                                         'spiceport' : 'ChardevSpicePort',
>>>                                         'vc'     : 'ChardevVC',
>>> -                                       'memory' : 'ChardevRingbuf' } }
>>> +                                       'ringbuf': 'ChardevRingbuf' } }

This would be an ABI-visible change.

> Oh, actually this is different.  The only inconsistency is in the name
> of the type, the enum is consistent with the -chardev option and it
> cannot be renamed because it was in QEMU 1.4.
> 
> So we can change the type, but we can do that post 1.5.

Careful.  While the union existed in 1.4, it had fewer elements; the
'memory' element was added in commit 1da48c65, which means it has never
been released yet.  If you want to avoid an ABI change, then this commit
must go in NOW.

This also reiterates the question of how libvirt can know which members
of a union are present, since we have added members to the union that
were not available in 1.4 but still don't have a way to introspect which
chardevs can be added.

-- 
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: 621 bytes --]

Re: [Qemu-devel] [for 1.5? Qemu-devel] [PATCH 2/3] chardev: Make the name of ringbuf device consistent

[Paolo Bonzini]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Il 20/05/2013 17:05, Eric Blake ha scritto:
> On 05/20/2013 04:59 AM, Paolo Bonzini wrote:
>> Il 20/05/2013 12:43, Paolo Bonzini ha scritto:
>>> Il 20/05/2013 08:51, Lei Li ha scritto:
>>>> Now we have ringbuf 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
>>>> 'ringbuf'.
>>>> 
>>>> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com> --- 
>>>> qapi-schema.json |    2 +- qemu-char.c      |   12
>>>> ++++++------ 2 files changed, 7 insertions(+), 7
>>>> deletions(-)
>>>> 
>>>> diff --git a/qapi-schema.json b/qapi-schema.json index
>>>> 9302e7d..61f6b34 100644 --- a/qapi-schema.json +++
>>>> b/qapi-schema.json @@ -3321,7 +3321,7 @@ 'spicevmc' :
>>>> 'ChardevSpiceChannel', 'spiceport' : 'ChardevSpicePort', 'vc'
>>>> : 'ChardevVC', -
>>>> 'memory' : 'ChardevRingbuf' } } +
>>>> 'ringbuf': 'ChardevRingbuf' } }
> 
> This would be an ABI-visible change.
> 
>> Oh, actually this is different.  The only inconsistency is in the
>> name of the type, the enum is consistent with the -chardev option
>> and it cannot be renamed because it was in QEMU 1.4.
>> 
>> So we can change the type, but we can do that post 1.5.
> 
> Careful.  While the union existed in 1.4

Sorry, I was referring to "-chardev memory", which exists in 1.4 and
cannot be renamed (which this patch does).

> , it had fewer elements; the 'memory' element was added in commit
> 1da48c65, which means it has never been released yet.  If you want
> to avoid an ABI change, then this commit must go in NOW.

We should not change the enum, only the name of the struct (and the
other way round: from ChardevRingbuf to ChardevMemory).  The enum and
- -chardev backend should be as consistent as possible.

> This also reiterates the question of how libvirt can know which
> members of a union are present, since we have added members to the
> union that were not available in 1.4 but still don't have a way to
> introspect which chardevs can be added.

Libvirt doesn't use most of the chardev types.  IIRC
 those that are supported were all in 1.4 (pty, sockets).

Paolo
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJRmj4tAAoJEBvWZb6bTYbyYqsP/jnBxMBMu/hk0JeDLB/WdCBy
5WLrDwMItBn7tfK4wFNqzMKgN+iWHKTKdDaspvQ6z/RaCKYO6hCtF67oe8jpE9/f
xogDYIyG4BOZTnRoI7Il5X/cyUtKduP/zvBaSoBjSCPw91oZYegQam3iYXJJxDrL
eiuRrVL14FOmy60xpqxCItC+0f4R7sff37PWCMAfMhOVzY4+I5r0nmLcjJpRznxX
PIoEGtArw3qE3SMD9gU0p7hFIKnpSn/1hOA7UC5ecA2t2OtDRABM2rXJ7q8JAPyE
2jK6OBgUKmHKTlrbu1ecsx+WnHbzHoU8xSAx6ojJVliX2o+A84rRQ8LF1I954++M
2pqRjnYO9pHxNVpLnVZyMssewGnxcNfqSIMn8YZqwax5jvgQIhZDcD3XTa9cfi8r
DABzKqx14GUTUYzCFgxEBi8s20oQB2dwNQuefhE/wE5RmjB83qvYWtZqaQqPFGmc
JwXUJSK5zRQ9u5wayolPNU2sdEKZfg2Aqfady2scMTncnM3V4nh/rrLC8h0pXbXG
VQaxVWmye/fb4vWoRvtq0+Wtltd8GcpFAoVwae56SGD5OSBS5dL574gkv+yht8pd
PpTdsYrIIPbl2dTIKMeuBXoTww+tfdLGORMoJcS2JpCWEvYfqAIt1mIfGxrpq14c
LvUwTnU1WRdHCqZh3sPU
=9Ari
-----END PGP SIGNATURE-----

Re: [Qemu-devel] [for 1.5? Qemu-devel] [PATCH 2/3] chardev: Make the name of ringbuf device consistent

[Lei Li]

On 05/20/2013 11:15 PM, Paolo Bonzini wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Il 20/05/2013 17:05, Eric Blake ha scritto:
>> On 05/20/2013 04:59 AM, Paolo Bonzini wrote:
>>> Il 20/05/2013 12:43, Paolo Bonzini ha scritto:
>>>> Il 20/05/2013 08:51, Lei Li ha scritto:
>>>>> Now we have ringbuf 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
>>>>> 'ringbuf'.
>>>>>
>>>>> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com> ---
>>>>> qapi-schema.json |    2 +- qemu-char.c      |   12
>>>>> ++++++------ 2 files changed, 7 insertions(+), 7
>>>>> deletions(-)
>>>>>
>>>>> diff --git a/qapi-schema.json b/qapi-schema.json index
>>>>> 9302e7d..61f6b34 100644 --- a/qapi-schema.json +++
>>>>> b/qapi-schema.json @@ -3321,7 +3321,7 @@ 'spicevmc' :
>>>>> 'ChardevSpiceChannel', 'spiceport' : 'ChardevSpicePort', 'vc'
>>>>> : 'ChardevVC', -
>>>>> 'memory' : 'ChardevRingbuf' } } +
>>>>> 'ringbuf': 'ChardevRingbuf' } }
>> This would be an ABI-visible change.
>>
>>> Oh, actually this is different.  The only inconsistency is in the
>>> name of the type, the enum is consistent with the -chardev option
>>> and it cannot be renamed because it was in QEMU 1.4.
>>>
>>> So we can change the type, but we can do that post 1.5.
>> Careful.  While the union existed in 1.4
> Sorry, I was referring to "-chardev memory", which exists in 1.4 and
> cannot be renamed (which this patch does).
>
>> , it had fewer elements; the 'memory' element was added in commit
>> 1da48c65, which means it has never been released yet.  If you want
>> to avoid an ABI change, then this commit must go in NOW.
> We should not change the enum, only the name of the struct (and the
> other way round: from ChardevRingbuf to ChardevMemory).  The enum and
> - -chardev backend should be as consistent as possible.

Sure, patches based on this will be send out soon.
Thanks!

>
>> This also reiterates the question of how libvirt can know which
>> members of a union are present, since we have added members to the
>> union that were not available in 1.4 but still don't have a way to
>> introspect which chardevs can be added.
> Libvirt doesn't use most of the chardev types.  IIRC
>   those that are supported were all in 1.4 (pty, sockets).
>
> Paolo
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2.0.19 (GNU/Linux)
> Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
>
> iQIcBAEBAgAGBQJRmj4tAAoJEBvWZb6bTYbyYqsP/jnBxMBMu/hk0JeDLB/WdCBy
> 5WLrDwMItBn7tfK4wFNqzMKgN+iWHKTKdDaspvQ6z/RaCKYO6hCtF67oe8jpE9/f
> xogDYIyG4BOZTnRoI7Il5X/cyUtKduP/zvBaSoBjSCPw91oZYegQam3iYXJJxDrL
> eiuRrVL14FOmy60xpqxCItC+0f4R7sff37PWCMAfMhOVzY4+I5r0nmLcjJpRznxX
> PIoEGtArw3qE3SMD9gU0p7hFIKnpSn/1hOA7UC5ecA2t2OtDRABM2rXJ7q8JAPyE
> 2jK6OBgUKmHKTlrbu1ecsx+WnHbzHoU8xSAx6ojJVliX2o+A84rRQ8LF1I954++M
> 2pqRjnYO9pHxNVpLnVZyMssewGnxcNfqSIMn8YZqwax5jvgQIhZDcD3XTa9cfi8r
> DABzKqx14GUTUYzCFgxEBi8s20oQB2dwNQuefhE/wE5RmjB83qvYWtZqaQqPFGmc
> JwXUJSK5zRQ9u5wayolPNU2sdEKZfg2Aqfady2scMTncnM3V4nh/rrLC8h0pXbXG
> VQaxVWmye/fb4vWoRvtq0+Wtltd8GcpFAoVwae56SGD5OSBS5dL574gkv+yht8pd
> PpTdsYrIIPbl2dTIKMeuBXoTww+tfdLGORMoJcS2JpCWEvYfqAIt1mIfGxrpq14c
> LvUwTnU1WRdHCqZh3sPU
> =9Ari
> -----END PGP SIGNATURE-----
>


-- 
Lei