Re: [Qemu-devel] [PATCH v2 19/41] char: remove class kind field

[Eric Blake]

On 01/30/2017 07:39 AM, Marc-André Lureau wrote:
> The class kind is necessary to lookup the chardev name in
> qmp_chardev_add() after calling qemu_chr_new_from_opts() and to set
> the appropriate ChardevBackend (mainly to free the right
> fields).
> 
> qemu_chr_new_from_opts() can be changed to use a non-qmp function
> using the chardev class typename. Introduce qemu_chardev_add() to be
> called from qemu_chr_new_from_opts() and remove the class chardev kind
> field. Set the backend->type in the parse callback (when non-common
> fields are added).
> 
> Signed-off-by: Marc-André Lureau 
> ---
>  include/sysemu/char.h |  1 -
>  backends/baum.c   |  1 -
>  backends/msmouse.c|  1 -
>  backends/testdev.c|  1 -
>  qemu-char.c   | 99 
> +--
>  spice-qemu-char.c |  4 +--
>  ui/console.c  |  2 +-
>  ui/gtk.c  |  1 -
>  8 files changed, 51 insertions(+), 59 deletions(-)
> 

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 19/41] char: remove class kind field

[Paolo Bonzini]

On 30/01/2017 08:39, Marc-André Lureau wrote:
> The class kind is necessary to lookup the chardev name in
> qmp_chardev_add() after calling qemu_chr_new_from_opts() and to set
> the appropriate ChardevBackend (mainly to free the right
> fields).
> 
> qemu_chr_new_from_opts() can be changed to use a non-qmp function
> using the chardev class typename. Introduce qemu_chardev_add() to be
> called from qemu_chr_new_from_opts() and remove the class chardev kind
> field. Set the backend->type in the parse callback (when non-common
> fields are added).
> 
> Signed-off-by: Marc-André Lureau 
> ---
>  include/sysemu/char.h |  1 -
>  backends/baum.c   |  1 -
>  backends/msmouse.c|  1 -
>  backends/testdev.c|  1 -
>  qemu-char.c   | 99 
> +--
>  spice-qemu-char.c |  4 +--
>  ui/console.c  |  2 +-
>  ui/gtk.c  |  1 -
>  8 files changed, 51 insertions(+), 59 deletions(-)

I am not sure about this patch.  Why not remove backend->type
altogether, and instead look at ChardevClass with object_dynamic_cast?

Paolo

Re: [Qemu-devel] [PATCH v2 19/41] char: remove class kind field

[Marc-André Lureau]

Hi

On Tue, Jan 31, 2017 at 12:08 AM Paolo Bonzini  wrote:

>
>
> On 30/01/2017 08:39, Marc-André Lureau wrote:
> > The class kind is necessary to lookup the chardev name in
> > qmp_chardev_add() after calling qemu_chr_new_from_opts() and to set
> > the appropriate ChardevBackend (mainly to free the right
> > fields).
> >
> > qemu_chr_new_from_opts() can be changed to use a non-qmp function
> > using the chardev class typename. Introduce qemu_chardev_add() to be
> > called from qemu_chr_new_from_opts() and remove the class chardev kind
> > field. Set the backend->type in the parse callback (when non-common
> > fields are added).
> >
> > Signed-off-by: Marc-André Lureau 
> > ---
> >  include/sysemu/char.h |  1 -
> >  backends/baum.c   |  1 -
> >  backends/msmouse.c|  1 -
> >  backends/testdev.c|  1 -
> >  qemu-char.c   | 99
> +--
> >  spice-qemu-char.c |  4 +--
> >  ui/console.c  |  2 +-
> >  ui/gtk.c  |  1 -
> >  8 files changed, 51 insertions(+), 59 deletions(-)
>
> I am not sure about this patch.  Why not remove backend->type
> altogether, and instead look at ChardevClass with object_dynamic_cast?
>
>
For the reason I gave in the patch summary: qapi_free_ChardevBackend()
dispatch based on ChardevBackendKind enum, not on Chardev object type.

-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v2 19/41] char: remove class kind field

[Marc-André Lureau]

Hi

On Tue, Jan 31, 2017 at 1:08 PM Marc-André Lureau <
marcandre.lur...@gmail.com> wrote:

> Hi
>
> On Tue, Jan 31, 2017 at 12:08 AM Paolo Bonzini 
> wrote:
>
>
>
> On 30/01/2017 08:39, Marc-André Lureau wrote:
> > The class kind is necessary to lookup the chardev name in
> > qmp_chardev_add() after calling qemu_chr_new_from_opts() and to set
> > the appropriate ChardevBackend (mainly to free the right
> > fields).
> >
> > qemu_chr_new_from_opts() can be changed to use a non-qmp function
> > using the chardev class typename. Introduce qemu_chardev_add() to be
> > called from qemu_chr_new_from_opts() and remove the class chardev kind
> > field. Set the backend->type in the parse callback (when non-common
> > fields are added).
> >
> > Signed-off-by: Marc-André Lureau 
> > ---
> >  include/sysemu/char.h |  1 -
> >  backends/baum.c   |  1 -
> >  backends/msmouse.c|  1 -
> >  backends/testdev.c|  1 -
> >  qemu-char.c   | 99
> +--
> >  spice-qemu-char.c |  4 +--
> >  ui/console.c  |  2 +-
> >  ui/gtk.c  |  1 -
> >  8 files changed, 51 insertions(+), 59 deletions(-)
>
> I am not sure about this patch.  Why not remove backend->type
> altogether, and instead look at ChardevClass with object_dynamic_cast?
>
>
> For the reason I gave in the patch summary: qapi_free_ChardevBackend()
> dispatch based on ChardevBackendKind enum, not on Chardev object type.
>

Unless I missed something and you object, are you ok for me to send the
pull request for this series? thanks

-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v2 19/41] char: remove class kind field

[Paolo Bonzini]

On 31/01/2017 06:23, Marc-André Lureau wrote:
> 
> Unless I missed something and you object, are you ok for me to send the
> pull request for this series? thanks

Yes, go ahead.

Paolo

Re: [Qemu-devel] [PATCH v2 19/41] char: remove class kind field

[Paolo Bonzini]

On 31/01/2017 04:08, Marc-André Lureau wrote:
> Hi
> 
> On Tue, Jan 31, 2017 at 12:08 AM Paolo Bonzini  > wrote:
> 
> 
> 
> On 30/01/2017 08:39, Marc-André Lureau wrote:
> > The class kind is necessary to lookup the chardev name in
> > qmp_chardev_add() after calling qemu_chr_new_from_opts() and to set
> > the appropriate ChardevBackend (mainly to free the right
> > fields).
> >
> > qemu_chr_new_from_opts() can be changed to use a non-qmp function
> > using the chardev class typename. Introduce qemu_chardev_add() to be
> > called from qemu_chr_new_from_opts() and remove the class chardev kind
> > field. Set the backend->type in the parse callback (when non-common
> > fields are added).
> >
> > Signed-off-by: Marc-André Lureau  >
> > ---
> >  include/sysemu/char.h |  1 -
> >  backends/baum.c   |  1 -
> >  backends/msmouse.c|  1 -
> >  backends/testdev.c|  1 -
> >  qemu-char.c   | 99
> +--
> >  spice-qemu-char.c |  4 +--
> >  ui/console.c  |  2 +-
> >  ui/gtk.c  |  1 -
> >  8 files changed, 51 insertions(+), 59 deletions(-)
> 
> I am not sure about this patch.  Why not remove backend->type
> altogether, and instead look at ChardevClass with object_dynamic_cast?
> 
> 
> For the reason I gave in the patch summary: qapi_free_ChardevBackend()
> dispatch based on ChardevBackendKind enum, not on Chardev object type.

Oops, I was confusing ChardevBackend and CharBackend!

Paolo