| Message ID | 1369132079-11377-2-git-send-email-lilei@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On 05/21/2013 06:34 PM, Paolo Bonzini wrote: > Il 21/05/2013 12:27, Lei Li ha scritto: >> 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 <lilei@linux.vnet.ibm.com> > Acked-by: Paolo Bonzini <pbonzini@redhat.com> This might miss cc to all. :) > >> --- >> 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' } } >> >> ## >> # @ChardevBackend: >> @@ -3321,7 +3321,7 @@ >> 'spicevmc' : 'ChardevSpiceChannel', >> 'spiceport' : 'ChardevSpicePort', >> 'vc' : 'ChardevVC', >> - 'memory' : 'ChardevRingbuf' } } >> + 'memory' : 'ChardevMemory' } } >> >> ## >> # @ChardevReturn: >> diff --git a/qemu-char.c b/qemu-char.c >> index cff2896..ebeed04 100644 >> --- a/qemu-char.c >> +++ b/qemu-char.c >> @@ -2875,8 +2875,8 @@ static void ringbuf_chr_close(struct CharDriverState *chr) >> chr->opaque = NULL; >> } >> >> -static CharDriverState *qemu_chr_open_ringbuf(ChardevRingbuf *opts, >> - Error **errp) >> +static CharDriverState *qemu_chr_open_memory(ChardevMemory *opts, >> + Error **errp) >> { >> CharDriverState *chr; >> RingBufCharDriver *d; >> @@ -2888,7 +2888,7 @@ static CharDriverState *qemu_chr_open_ringbuf(ChardevRingbuf *opts, >> >> /* The size must be power of 2 */ >> if (d->size & (d->size - 1)) { >> - error_setg(errp, "size of ringbuf chardev must be power of two"); >> + error_setg(errp, "size of memory chardev must be power of two"); >> goto fail; >> } >> >> @@ -3190,12 +3190,12 @@ static void qemu_chr_parse_pipe(QemuOpts *opts, ChardevBackend *backend, >> backend->pipe->device = g_strdup(device); >> } >> >> -static void qemu_chr_parse_ringbuf(QemuOpts *opts, ChardevBackend *backend, >> - Error **errp) >> +static void qemu_chr_parse_memory(QemuOpts *opts, ChardevBackend *backend, >> + Error **errp) >> { >> int val; >> >> - backend->memory = g_new0(ChardevRingbuf, 1); >> + backend->memory = g_new0(ChardevMemory, 1); >> >> val = qemu_opt_get_number(opts, "size", 0); >> if (val != 0) { >> @@ -3787,7 +3787,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend, >> chr = vc_init(backend->vc); >> break; >> case CHARDEV_BACKEND_KIND_MEMORY: >> - chr = qemu_chr_open_ringbuf(backend->memory, errp); >> + chr = qemu_chr_open_memory(backend->memory, errp); >> break; >> default: >> error_setg(errp, "unknown chardev backend (%d)", backend->kind); >> @@ -3832,7 +3832,7 @@ static void register_types(void) >> 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, >> - qemu_chr_parse_ringbuf); >> + qemu_chr_parse_memory); >> register_char_driver_qapi("file", CHARDEV_BACKEND_KIND_FILE, >> qemu_chr_parse_file_out); >> register_char_driver_qapi("stdio", CHARDEV_BACKEND_KIND_STDIO, >> diff --git a/qemu-options.hx b/qemu-options.hx >> index fb62b75..fb3961d 100644 >> --- a/qemu-options.hx >> +++ 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" >> "-chardev file,id=id,path=path[,mux=on|off]\n" >> "-chardev pipe,id=id,path=path[,mux=on|off]\n" >> #ifdef _WIN32 >> @@ -1817,7 +1817,7 @@ Backend is one of: >> @option{udp}, >> @option{msmouse}, >> @option{vc}, >> -@option{ringbuf}, >> +@option{memory}, >> @option{file}, >> @option{pipe}, >> @option{console}, >> @@ -1926,7 +1926,7 @@ the console, in pixels. >> @option{cols} and @option{rows} specify that the console be sized to fit a text >> console with the given dimensions. >> >> -@item -chardev ringbuf ,id=@var{id} [,size=@var{size}] >> +@item -chardev memory ,id=@var{id} [,size=@var{size}] >> >> Create a ring buffer with fixed size @option{size}. >> @var{size} must be a power of two, and defaults to @code{64K}). >>
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 <lilei@linux.vnet.ibm.com> > --- > 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 <eblake@redhat.com> although I still think it might be worth considering option #3 in a respin or as a followup patch.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Il 21/05/2013 14:38, Eric Blake ha scritto: > 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 <lilei@linux.vnet.ibm.com> --- >> 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. This is just the help text. "qemu-system-x86_64 -chardev memory,id=foo" works here on 1.4.1. Paolo 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 <eblake@redhat.com> > > although I still think it might be worth considering option #3 in > a respin or as a followup patch. > -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.19 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJRm22XAAoJEBvWZb6bTYbyoA0P/0+aTY+L0f10PAS9HenLcMHN 4E4EDju2mhHSFDwiH05x8rax+/YNDPuGq96OD9Z2HJdtngDHUztmKmpB12kcmR72 skGgCafXatWk9In8DQKhKGFtHmdLStKg7DfDe8US8muxjEM+IXKFxynjkrycKPYn IttHBzeorZSHlLoAAz4Tlj096qt5oEvnIBYoJGIeMLCSbWDdSVk8FBvYPI2jfjkM HkudPq1C3Z0k1Xl9s4yUkk/5wztLVxaiO1Bh0JU50txUfQpFrFFHA9C0RU/XPja/ K2ZUQzqL7LpcOvMweV4oCOdOL8PXPPD0JimmrLRpT40YyrNTFLv+cwmhGv/9Vseo EpzWrSpYWHL/kmioxu7voHYuNqLNvizucboJz/pLs72dQubW+RzeRrieVHjJxUyK dnqFaspHOAeG9s5YKvwG1ObnORPWGdQvr035NPkkYgQX2R7ndEICuIKr5nqcdjXR HNZv59f/bxEEVRwx6wHrFsQ+6/417xIXwXJenZdZiZVikiCoNuaJekC+5fLYy1Q8 AamQq/xkuOthW8/s0Nykc3JqrVGZJ1RiYI7rDj6stMciFNlkWdTsYESNpi/Qq5Jb qTkIea2O0dEQ9nRt3/sOpOITBST4nIEdzWnvRqZXPEocwhqbNwH/C4qe65mJK+RE NTPjDtsKpDTODSAEl4tq =Gcyt -----END PGP SIGNATURE-----
On 05/21/2013 06:50 AM, Paolo Bonzini wrote: >>> - "-chardev ringbuf,id=id[,size=size]\n" + "-chardev >>> memory,id=id[,size=size]\n" > >> This change impacts the command line. > > This is just the help text. "qemu-system-x86_64 -chardev > memory,id=foo" works here on 1.4.1. Good to know. In that case: Reviewed-by: Eric Blake <eblake@redhat.com>
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' } } ## # @ChardevBackend: @@ -3321,7 +3321,7 @@ 'spicevmc' : 'ChardevSpiceChannel', 'spiceport' : 'ChardevSpicePort', 'vc' : 'ChardevVC', - 'memory' : 'ChardevRingbuf' } } + 'memory' : 'ChardevMemory' } } ## # @ChardevReturn: diff --git a/qemu-char.c b/qemu-char.c index cff2896..ebeed04 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -2875,8 +2875,8 @@ static void ringbuf_chr_close(struct CharDriverState *chr) chr->opaque = NULL; } -static CharDriverState *qemu_chr_open_ringbuf(ChardevRingbuf *opts, - Error **errp) +static CharDriverState *qemu_chr_open_memory(ChardevMemory *opts, + Error **errp) { CharDriverState *chr; RingBufCharDriver *d; @@ -2888,7 +2888,7 @@ static CharDriverState *qemu_chr_open_ringbuf(ChardevRingbuf *opts, /* The size must be power of 2 */ if (d->size & (d->size - 1)) { - error_setg(errp, "size of ringbuf chardev must be power of two"); + error_setg(errp, "size of memory chardev must be power of two"); goto fail; } @@ -3190,12 +3190,12 @@ static void qemu_chr_parse_pipe(QemuOpts *opts, ChardevBackend *backend, backend->pipe->device = g_strdup(device); } -static void qemu_chr_parse_ringbuf(QemuOpts *opts, ChardevBackend *backend, - Error **errp) +static void qemu_chr_parse_memory(QemuOpts *opts, ChardevBackend *backend, + Error **errp) { int val; - backend->memory = g_new0(ChardevRingbuf, 1); + backend->memory = g_new0(ChardevMemory, 1); val = qemu_opt_get_number(opts, "size", 0); if (val != 0) { @@ -3787,7 +3787,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend, chr = vc_init(backend->vc); break; case CHARDEV_BACKEND_KIND_MEMORY: - chr = qemu_chr_open_ringbuf(backend->memory, errp); + chr = qemu_chr_open_memory(backend->memory, errp); break; default: error_setg(errp, "unknown chardev backend (%d)", backend->kind); @@ -3832,7 +3832,7 @@ static void register_types(void) 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, - qemu_chr_parse_ringbuf); + qemu_chr_parse_memory); register_char_driver_qapi("file", CHARDEV_BACKEND_KIND_FILE, qemu_chr_parse_file_out); register_char_driver_qapi("stdio", CHARDEV_BACKEND_KIND_STDIO, diff --git a/qemu-options.hx b/qemu-options.hx index fb62b75..fb3961d 100644 --- a/qemu-options.hx +++ 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" "-chardev file,id=id,path=path[,mux=on|off]\n" "-chardev pipe,id=id,path=path[,mux=on|off]\n" #ifdef _WIN32 @@ -1817,7 +1817,7 @@ Backend is one of: @option{udp}, @option{msmouse}, @option{vc}, -@option{ringbuf}, +@option{memory}, @option{file}, @option{pipe}, @option{console}, @@ -1926,7 +1926,7 @@ the console, in pixels. @option{cols} and @option{rows} specify that the console be sized to fit a text console with the given dimensions. -@item -chardev ringbuf ,id=@var{id} [,size=@var{size}] +@item -chardev memory ,id=@var{id} [,size=@var{size}] Create a ring buffer with fixed size @option{size}. @var{size} must be a power of two, and defaults to @code{64K}).
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 <lilei@linux.vnet.ibm.com> --- qapi-schema.json | 6 +++--- qemu-char.c | 16 ++++++++-------- qemu-options.hx | 6 +++--- 3 files changed, 14 insertions(+), 14 deletions(-)