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
signature.asc
Description: OpenPGP digital signature
