Patchwork [2/3] chardev: Make the name of ringbuf device consistent

login
register
mail settings
Submitter Lei Li
Date May 20, 2013, 6:51 a.m.
Message ID <1369032665-18159-3-git-send-email-lilei@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/244848/
State New
Headers show

Comments

Lei Li - May 20, 2013, 6:51 a.m.
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(-)
Paolo Bonzini - May 20, 2013, 10:43 a.m.
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
Paolo Bonzini - May 20, 2013, 10:59 a.m.
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
Eric Blake - May 20, 2013, 3:05 p.m.
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.
Paolo Bonzini - May 20, 2013, 3:15 p.m.
-----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-----
Lei Li - May 21, 2013, 10:14 a.m.
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-----
>

Patch

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);