Re: [Qemu-devel] [PATCH 15/54] chardev: qom-ify

[Eric Blake <eblake@redhat.com>]

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

On 12/12/2016 04:42 PM, Marc-André Lureau wrote:
> Turn Chardev into Object.
> 
> qemu_chr_alloc() is replaced by the qemu_chardev_new() constructor. It
> will call qemu_char_open() to open/intialize the chardev with the
> ChardevCommon *backend settings.

Review part 2:

>  spice-qemu-char.c     |  144 +++---
>  ui/console.c          |   73 +--
>  ui/gtk.c              |   51 +-
>  vl.c                  |    2 +

> +++ b/spice-qemu-char.c
> @@ -18,6 +18,12 @@ typedef struct SpiceChardev {
>      QLIST_ENTRY(SpiceChardev) next;
>  } SpiceChardev;
>  
> +#define TYPE_CHARDEV_SPICE "chardev-spice"
> +#define TYPE_CHARDEV_SPICEVMC "chardev-spicevmc"
> +#define TYPE_CHARDEV_SPICEPORT "chardev-spiceport"

Why do we have a lot of TYPE_CHARDEV_* in sysemu/char.h, but not these?
Should they all live in the same place?

> +++ b/ui/console.c
> @@ -1040,9 +1040,12 @@ typedef struct VCChardev {
>      QemuConsole *console;
>  } VCChardev;
>  
> -static int console_puts(Chardev *chr, const uint8_t *buf, int len)
> +#define TYPE_CHARDEV_VC "chardev-vc"
> +#define VC_CHARDEV(obj) OBJECT_CHECK(VCChardev, (obj), TYPE_CHARDEV_VC)
> +
> +static int vc_chr_write(Chardev *chr, const uint8_t *buf, int len)
...

> @@ -1951,9 +1954,9 @@ int qemu_console_get_height(QemuConsole *con, int fallback)
>      return con ? surface_height(con->surface) : fallback;
>  }
>  
> -static void text_console_set_echo(Chardev *chr, bool echo)
> +static void vc_chr_set_echo(Chardev *chr, bool echo)

Should these function renames be split to a separate patch?

> +++ b/ui/gtk.c
> @@ -184,6 +184,9 @@ typedef struct VCChardev {
>      bool echo;
>  } VCChardev;
>  
> +#define TYPE_CHARDEV_VC "chardev-vc"
> +#define VC_CHARDEV(obj) OBJECT_CHECK(VCChardev, (obj), TYPE_CHARDEV_VC)

Why do we have TYPE_CHARDEV_VC and VC_CHARDEV() defined in two different
.c files?

Overall the conversion looks good; as said elsewhere in the thread, I
think you can post a v2 of 1-15 and get that merged first, while still
hammering out the details of the rest of the series.

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