| Message ID | 1369032665-18159-3-git-send-email-lilei@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
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
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
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.
-----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-----
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----- >
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);
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(-)