[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 20/20] chardev: qom-ify
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH 20/20] chardev: qom-ify |
Date: |
Mon, 9 Jan 2017 12:02:49 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0 |
On 01/05/2017 10:53 AM, 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.
>
> The CharDriver::create() callback is turned into a ChardevClass::open()
> which is called from the newly introduced qemu_chardev_open().
>
> "chardev-gdb" and "chardev-hci" are internal chardev and aren't
> creatable directly with -chardev. Use a new internal flag to disable
> them. We may want to use TYPE_USER_CREATABLE interface instead, or
> perhaps allow -chardev usage.
>
> Although in general we keep typename and macros private, unless the type
> is being used by some other file, in this patch, all types and common
> helper macros for qemu-char.c are in char.h. This is to help transition
> now (some types must be declared early, while some aren't shared) and
> when splitting in several units. This is to be improved later.
>
> Signed-off-by: Marc-André Lureau <address@hidden>
> ---
> +++ b/qemu-char.c
> @@ -466,7 +433,8 @@ int qemu_chr_fe_get_msgfds(CharBackend *be, int *fds, int
> len)
> return -1;
> }
>
> - return s->driver->get_msgfds ? s->driver->get_msgfds(s, fds, len) : -1;
> + return CHARDEV_GET_CLASS(s)->get_msgfds ?
> + CHARDEV_GET_CLASS(s)->get_msgfds(s, fds, len) : -1;
I'm not familiar enough with the Object hierarchy to know if there is
any significant runtime cost to CHARDEV_GET_CLASS() that would warrant
capturing the output in a temporary variable, or if it is simple enough
that a compiler can already figure out that it is a reusable value
rather than computing it twice. Probably not worrying about, though,
unless it actually shows up in a hot spot on a trace.
> @@ -4957,47 +4994,96 @@ void qemu_chr_set_feature(Chardev *chr,
> return set_bit(feature, chr->features);
> }
>
> -ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
> - Error **errp)
> +static const ChardevClass *char_get_class(const char *driver, Error **errp)
> +{
> + ObjectClass *oc;
> + const ChardevClass *cc;
> + char *typename = g_strdup_printf("chardev-%s", driver);
> +
> + oc = object_class_by_name(typename);
> + g_free(typename);
char_get_class("tty", NULL) will fail, because that's merely an alias
name rather than the canonical "serial" name. Will that matter to any of
the code? I didn't spot anything, so I'm not letting it stop my review.
Reviewed-by: Eric Blake <address@hidden>
--
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 15/20] spice-char: improve error reporting, (continued)
- [Qemu-devel] [PATCH 16/20] char: use error_report(), Marc-André Lureau, 2017/01/05
- [Qemu-devel] [PATCH 18/20] baum: use a common prefix for chr callbacks, Marc-André Lureau, 2017/01/05
- [Qemu-devel] [PATCH 17/20] gtk: overwrite the console.c char driver, Marc-André Lureau, 2017/01/05
- [Qemu-devel] [PATCH 19/20] vc: use a common prefix for chr callbacks, Marc-André Lureau, 2017/01/05
- [Qemu-devel] [PATCH 13/20] char: rename CharDriverState Chardev, Marc-André Lureau, 2017/01/05
- [Qemu-devel] [PATCH 20/20] chardev: qom-ify, Marc-André Lureau, 2017/01/05
- Re: [Qemu-devel] [PATCH 20/20] chardev: qom-ify,
Eric Blake <=