[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v7 2/4] chardev/char-hub: implement backend chardev aggregato
|
From: |
Roman Penyaev |
|
Subject: |
Re: [PATCH v7 2/4] chardev/char-hub: implement backend chardev aggregator |
|
Date: |
Tue, 21 Jan 2025 11:56:04 +0100 |
Hi Marc-André,
On Mon, Jan 20, 2025 at 12:21 PM Marc-André Lureau
<marcandre.lureau@redhat.com> wrote:
>
> Hi
>
> On Sat, Jan 18, 2025 at 8:41 PM Roman Penyaev <r.peniaev@gmail.com> wrote:
> >
> > This patch implements a new chardev backend `hub` device, which
> > aggregates input from multiple backend devices and forwards it to a
> > single frontend device. Additionally, `hub` device takes the output
> > from the frontend device and sends it back to all the connected
> > backend devices. This allows for seamless interaction between
> > different backend devices and a single frontend interface.
> >
> > The idea of the change is trivial: keep list of backend devices
> > (up to 4), init them on demand and forward data buffer back and
> > forth.
> >
> > The following is QEMU command line example:
> >
> > -chardev pty,path=/tmp/pty,id=pty0 \
> > -chardev vc,id=vc0 \
> > -chardev hub,id=hub0,chardevs.0=pty0,chardevs.1=vc0 \
> > -device virtconsole,chardev=hub0 \
> > -vnc 0.0.0.0:0
> >
> > Which creates 2 backend devices: text virtual console (`vc0`) and a
> > pseudo TTY (`pty0`) connected to the single virtio hvc console with
> > the backend aggregator (`hub0`) help. `vc0` renders text to an image,
> > which can be shared over the VNC protocol. `pty0` is a pseudo TTY
> > backend which provides biderectional communication to the virtio hvc
> > console.
> >
> > 'chardevs.N' list syntax is used for the sake of compatibility with
> > the representation of JSON lists in 'key=val' pairs format of the
> > util/keyval.c, despite the fact that modern QAPI way of parsing,
> > namely qobject_input_visitor_new_str(), is not used. Choice of keeping
> > QAPI list sytax may help to smoothly switch to modern parsing in the
>
> syntax
Will fix. Thanks.
>
> > future.
> >
> > Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>
> > Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
> > Cc: qemu-devel@nongnu.org
> > ---
> > chardev/char-fe.c | 9 +
> > chardev/char-hub.c | 334 +++++++++++++++++++++++++++++++++++++
> > chardev/char.c | 26 ++-
> > chardev/chardev-internal.h | 56 ++++++-
> > chardev/meson.build | 1 +
> > include/chardev/char.h | 1 +
> > qapi/char.json | 27 +++
> > 7 files changed, 451 insertions(+), 3 deletions(-)
> > create mode 100644 chardev/char-hub.c
> >
> > diff --git a/chardev/char-fe.c b/chardev/char-fe.c
> > index 158a5f4f551e..cfd0577c3f46 100644
> > --- a/chardev/char-fe.c
> > +++ b/chardev/char-fe.c
> > @@ -200,6 +200,12 @@ bool qemu_chr_fe_init(CharBackend *b, Chardev *s,
> > Error **errp)
> > if (!mux_chr_attach_frontend(d, b, &tag, errp)) {
> > return false;
> > }
> > + } else if (CHARDEV_IS_HUB(s)) {
> > + HubChardev *d = HUB_CHARDEV(s);
> > +
> > + if (!hub_chr_attach_frontend(d, b, errp)) {
> > + return false;
> > + }
> > } else if (s->be) {
> > error_setg(errp, "chardev '%s' is already in use", s->label);
> > return false;
> > @@ -226,6 +232,9 @@ void qemu_chr_fe_deinit(CharBackend *b, bool del)
> > if (CHARDEV_IS_MUX(b->chr)) {
> > MuxChardev *d = MUX_CHARDEV(b->chr);
> > mux_chr_detach_frontend(d, b->tag);
> > + } else if (CHARDEV_IS_HUB(b->chr)) {
> > + HubChardev *d = HUB_CHARDEV(b->chr);
> > + hub_chr_detach_frontend(d);
> > }
>
> you don't need this extra attach/detach logic, you can rely on parent
> implementation. See attached patch
Ah, correct. What a service, thank you :) I was focused on the attach
logic that I completely forgot the hub is a char device itself.
Will squash your changes.
>
>
> > if (del) {
> > Object *obj = OBJECT(b->chr);
> > diff --git a/chardev/char-hub.c b/chardev/char-hub.c
> > new file mode 100644
> > index 000000000000..9b53df51de44
> > --- /dev/null
> > +++ b/chardev/char-hub.c
> > @@ -0,0 +1,336 @@
> > +/*
> > + * QEMU Character Hub Device
> > + *
> > + * Author: Roman Penyaev <r.peniaev@gmail.com>
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a
> > copy
> > + * of this software and associated documentation files (the "Software"),
> > to deal
> > + * in the Software without restriction, including without limitation the
> > rights
> > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or
> > sell
> > + * copies of the Software, and to permit persons to whom the Software is
> > + * furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice shall be included
> > in
> > + * all copies or substantial portions of the Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
> > OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> > OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> > FROM,
> > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> > IN
> > + * THE SOFTWARE.
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "qapi/error.h"
> > +#include "qemu/option.h"
> > +#include "chardev/char.h"
> > +#include "chardev-internal.h"
> > +
> > +/*
> > + * Character hub device aggregates input from multiple backend devices
> > + * and forwards it to a single frontend device. Additionally, hub
> > + * device takes the output from the frontend device and sends it back
> > + * to all the connected backend devices.
> > + */
> > +
> > +/*
> > + * Write to all backends. Different backend devices accept data with
> > + * various rate, so it is quite possible that one device returns less,
> > + * then others. In this case we return minimum to the caller,
> > + * expecting caller will repeat operation soon. When repeat happens
> > + * send to the devices which consume data faster must be avoided
> > + * for obvious reasons not to send data, which was already sent.
> > + */
> > +static int hub_chr_write_to_all(HubChardev *d, const uint8_t *buf, int len)
> > +{
> > + int r, i, ret = len;
> > + unsigned int written;
> > +
> > + /* Invalidate index on every write */
> > + d->be_eagain_ind = -1;
> > +
> > + for (i = 0; i < d->be_cnt; i++) {
> > + if (!d->backends[i].be.chr->be_open) {
> > + /* Skip closed backend */
> > + continue;
> > + }
> > + written = d->be_written[i] - d->be_min_written;
> > + if (written) {
> > + /* Written in the previous call so take into account */
> > + ret = MIN(written, ret);
> > + continue;
> > + }
> > + r = qemu_chr_fe_write(&d->backends[i].be, buf, len);
> > + if (r < 0 && errno == EAGAIN) {
> > + /*
> > + * Fail immediately if write would block. Expect to be called
> > + * soon on watch wake up.
> > + */
> > + d->be_eagain_ind = i;
> > + return r;
> > + } else if (r < 0) {
> > + /*
> > + * Ignore all other errors and pretend the entire buffer is
> > + * written to avoid this chardev being watched. This device
> > + * becomes disabled until the following write succeeds, but
> > + * writing continues to others.
> > + */
>
> I wonder if this behaviour is desirable. Why silence the error? What
> if all backends fail?
The intention was to cover the case, when one of the backend devices is
closed, but I need to continue serving writes for the other devices. Later I
discovered the OPENED/CLOSED events and `->be_open` flag. So you
are absolutely correct, this "else if" branch is not needed any more and
can be removed. Thanks.
>
> > + r = len;
> > + }
> > + d->be_written[i] += r;
> > + ret = MIN(r, ret);
> > + }
> > + d->be_min_written += ret;
> > +
> > + return ret;
> > +}
> > +
> > +/* Called with chr_write_lock held. */
> > +static int hub_chr_write(Chardev *chr, const uint8_t *buf, int len)
> > +{
> > + HubChardev *d = HUB_CHARDEV(chr);
> > + return hub_chr_write_to_all(d, buf, len);
>
> no need for an extra function, you can just inline here
Will do, not a big deal.
>
> > +}
> > +
> > +static int hub_chr_can_read(void *opaque)
> > +{
> > + HubCharBackend *backend = opaque;
> > + CharBackend *fe = backend->hub->frontend;
> > +
> > + if (fe && fe->chr_can_read) {
> > + return fe->chr_can_read(fe->opaque);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void hub_chr_read(void *opaque, const uint8_t *buf, int size)
> > +{
> > + HubCharBackend *backend = opaque;
> > + CharBackend *fe = backend->hub->frontend;
> > +
> > +
> > + if (fe && fe->chr_read) {
> > + fe->chr_read(fe->opaque, buf, size);
> > + }
> > +}
> > +
> > +static void hub_chr_event(void *opaque, QEMUChrEvent event)
> > +{
> > + HubCharBackend *backend = opaque;
> > + HubChardev *d = backend->hub;
> > + CharBackend *fe = d->frontend;
> > +
> > + if (event == CHR_EVENT_OPENED) {
> > + /*
> > + * Catch up with what was already written while this backend
> > + * was closed
> > + */
> > + d->be_written[backend->be_ind] = d->be_min_written;
> > +
> > + if (d->be_event_opened_cnt++) {
> > + /* Ignore subsequent open events from other backends */
> > + return;
> > + }
> > + } else if (event == CHR_EVENT_CLOSED) {
> > + if (!d->be_event_opened_cnt) {
> > + /* Don't go below zero. Probably assert is better */
> > + return;
> > + }
> > + if (--d->be_event_opened_cnt) {
> > + /* Serve only the last one close event */
> > + return;
> > + }
>
> I wonder if it really makes sense to open when the first chardev opens
> and close with the last one.
>
> I would rather have a default "safe" behaviour that opens when all are
> open, and close when the first one is closed.
>
> Or a loosy behaviour that would just stay open, regardless of backend status.
>
> Or a "selected" behaviour where one backend dictates the hub state.
The concept is similar to an fd handle: a file is considered opened if there
is a valid reference (fd handle) on it. Backend devices can appear and
disappear at any time, there are no main or selected backend devices, all
are equal.
Closing the frontend when the first backend is closed definitely breaks the
user scenario, since there is no way to predict when the user will open or close
the backend device (attach/detach to a pty device or a connection to a socket
can happen at any time).
Keeping the frontend device open seems to do no harm, but it seemed wrong
to me resource wise on the guest and also makes it impossible to implement
logic on the guest that would determine when the opposite side is open/closed
(this can be relevant for virtualserialport scenario). So I wanted to
do it "correctly".
Do you foresee any issues? I noticed that OPEN/CLOSED events can often
be repeated or not sent at all (the first patch solves this problem
for char-pty).
Although, tests/unit/test-char.c tries to verify the correctness by
counting these
events.
I tested the reference logic using pty/socket/vc backends paired with
virtconsole/virtserialport frontends.
>
>
> > + }
> > +
> > + if (fe && fe->chr_event) {
> > + fe->chr_event(fe->opaque, event);
> > + }
> > +}
> > +
> > +static GSource *hub_chr_add_watch(Chardev *s, GIOCondition cond)
> > +{
> > + HubChardev *d = HUB_CHARDEV(s);
> > + Chardev *chr;
> > + ChardevClass *cc;
> > +
> > + if (d->be_eagain_ind == -1) {
> > + return NULL;
> > + }
> > +
> > + assert(d->be_eagain_ind < d->be_cnt);
> > + chr = qemu_chr_fe_get_driver(&d->backends[d->be_eagain_ind].be);
> > + cc = CHARDEV_GET_CLASS(chr);
> > + if (!cc->chr_add_watch) {
> > + return NULL;
> > + }
> > +
> > + return cc->chr_add_watch(chr, cond);
> > +}
> > +
> > +static bool hub_chr_attach_chardev(HubChardev *d, Chardev *chr,
> > + Error **errp)
> > +{
> > + bool ret;
> > +
> > + if (d->be_cnt >= MAX_HUB) {
> > + error_setg(errp, "hub: too many uses of chardevs '%s'"
> > + " (maximum is " stringify(MAX_HUB) ")",
> > + d->parent.label);
> > + return false;
> > + }
> > + ret = qemu_chr_fe_init(&d->backends[d->be_cnt].be, chr, errp);
> > + if (ret) {
> > + d->backends[d->be_cnt].hub = d;
> > + d->backends[d->be_cnt].be_ind = d->be_cnt;
> > + d->be_cnt += 1;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static void char_hub_finalize(Object *obj)
> > +{
> > + HubChardev *d = HUB_CHARDEV(obj);
> > + CharBackend *fe = d->frontend;
> > + int i;
> > +
> > + if (fe) {
> > + fe->chr = NULL;
> > + }
> > + for (i = 0; i < d->be_cnt; i++) {
> > + qemu_chr_fe_deinit(&d->backends[i].be, false);
> > + }
> > +}
> > +
> > +static void hub_chr_update_read_handlers(Chardev *chr)
> > +{
> > + HubChardev *d = HUB_CHARDEV(chr);
> > + int i;
> > +
> > + for (i = 0; i < d->be_cnt; i++) {
> > + qemu_chr_fe_set_handlers_full(&d->backends[i].be,
> > + hub_chr_can_read,
> > + hub_chr_read,
> > + hub_chr_event,
> > + NULL,
> > + &d->backends[i],
> > + chr->gcontext, true, false);
> > + }
> > +}
> > +
> > +bool hub_chr_attach_frontend(HubChardev *d, CharBackend *b, Error **errp)
> > +{
> > + if (d->frontend) {
> > + error_setg(errp, "hub: multiplexed chardev '%s' is already used "
> > + "for multiplexing", d->parent.label);
> > + return false;
> > + }
> > + d->frontend = b;
> > +
> > + return true;
> > +}
> > +
> > +void hub_chr_detach_frontend(HubChardev *d)
> > +{
> > + d->frontend = NULL;
> > +}
> > +
> > +static void qemu_chr_open_hub(Chardev *chr,
> > + ChardevBackend *backend,
> > + bool *be_opened,
> > + Error **errp)
> > +{
> > + ChardevHub *hub = backend->u.hub.data;
> > + HubChardev *d = HUB_CHARDEV(chr);
> > + strList *list = hub->chardevs;
> > +
> > + d->be_eagain_ind = -1;
> > +
> > + if (list == NULL) {
> > + error_setg(errp, "hub: 'chardevs' list is not defined");
> > + return;
> > + }
> > +
> > + while (list) {
> > + Chardev *s;
> > +
> > + s = qemu_chr_find(list->value);
> > + if (s == NULL) {
> > + error_setg(errp, "hub: chardev can't be found by id '%s'",
> > + list->value);
> > + return;
> > + }
> > + if (CHARDEV_IS_HUB(s) || CHARDEV_IS_MUX(s)) {
> > + error_setg(errp, "hub: multiplexers and hub devices can't be "
> > + "stacked, check chardev '%s', chardev should not "
> > + "be a hub device or have 'mux=on' enabled",
> > + list->value);
> > + return;
> > + }
> > + if (!hub_chr_attach_chardev(d, s, errp)) {
> > + return;
> > + }
> > + list = list->next;
> > + }
> > +
> > + /* Closed until an explicit event from backend */
> > + *be_opened = false;
> > +}
> > +
> > +static void qemu_chr_parse_hub(QemuOpts *opts, ChardevBackend *backend,
> > + Error **errp)
> > +{
> > + ChardevHub *hub;
> > + strList **tail;
> > + int i;
> > +
> > + backend->type = CHARDEV_BACKEND_KIND_HUB;
> > + hub = backend->u.hub.data = g_new0(ChardevHub, 1);
> > + qemu_chr_parse_common(opts, qapi_ChardevHub_base(hub));
> > +
> > + tail = &hub->chardevs;
> > +
> > + for (i = 0; i < MAX_HUB; i++) {
> > + char optbuf[16];
> > + const char *dev;
> > +
> > + snprintf(optbuf, sizeof(optbuf), "chardevs.%u", i);
> > + dev = qemu_opt_get(opts, optbuf);
> > + if (!dev) {
> > + break;
> > + }
> > +
> > + QAPI_LIST_APPEND(tail, g_strdup(dev));
> > + }
> > +}
> > +
> > +static void char_hub_class_init(ObjectClass *oc, void *data)
> > +{
> > + ChardevClass *cc = CHARDEV_CLASS(oc);
> > +
> > + cc->parse = qemu_chr_parse_hub;
> > + cc->open = qemu_chr_open_hub;
> > + cc->chr_write = hub_chr_write;
> > + cc->chr_add_watch = hub_chr_add_watch;
> > + /* We handle events from backends only */
> > + cc->chr_be_event = NULL;
> > + cc->chr_update_read_handler = hub_chr_update_read_handlers;
> > +}
> > +
> > +static const TypeInfo char_hub_type_info = {
> > + .name = TYPE_CHARDEV_HUB,
> > + .parent = TYPE_CHARDEV,
> > + .class_init = char_hub_class_init,
> > + .instance_size = sizeof(HubChardev),
> > + .instance_finalize = char_hub_finalize,
> > +};
> > +
> > +static void register_types(void)
> > +{
> > + type_register_static(&char_hub_type_info);
> > +}
> > +
> > +type_init(register_types);
> > diff --git a/chardev/char.c b/chardev/char.c
> > index 7705da5ad02b..31536f4b7356 100644
> > --- a/chardev/char.c
> > +++ b/chardev/char.c
> > @@ -334,6 +334,9 @@ static bool qemu_chr_is_busy(Chardev *s)
> > if (CHARDEV_IS_MUX(s)) {
> > MuxChardev *d = MUX_CHARDEV(s);
> > return d->mux_bitset != 0;
> > + } else if (CHARDEV_IS_HUB(s)) {
> > + HubChardev *d = HUB_CHARDEV(s);
> > + return d->frontend != NULL;
> > } else {
> > return s->be != NULL;
> > }
> > @@ -943,7 +946,26 @@ QemuOptsList qemu_chardev_opts = {
> > },{
> > .name = "chardev",
> > .type = QEMU_OPT_STRING,
> > + },
> > + /*
> > + * Multiplexer options. Follows QAPI array syntax.
> > + * See MAX_HUB macro to obtain array capacity.
> > + */
> > + {
> > + .name = "chardevs.0",
> > + .type = QEMU_OPT_STRING,
> > + },{
> > + .name = "chardevs.1",
> > + .type = QEMU_OPT_STRING,
> > },{
> > + .name = "chardevs.2",
> > + .type = QEMU_OPT_STRING,
> > + },{
> > + .name = "chardevs.3",
> > + .type = QEMU_OPT_STRING,
> > + },
> > +
> > + {
> > .name = "append",
> > .type = QEMU_OPT_BOOL,
> > },{
> > @@ -1106,8 +1128,8 @@ ChardevReturn *qmp_chardev_change(const char *id,
> > ChardevBackend *backend,
> > return NULL;
> > }
> >
> > - if (CHARDEV_IS_MUX(chr)) {
> > - error_setg(errp, "Mux device hotswap not supported yet");
> > + if (CHARDEV_IS_MUX(chr) || CHARDEV_IS_HUB(chr)) {
> > + error_setg(errp, "For mux or hub device hotswap is not supported
> > yet");
> > return NULL;
> > }
> >
> > diff --git a/chardev/chardev-internal.h b/chardev/chardev-internal.h
> > index 853807f3cb88..ff5432008aad 100644
> > --- a/chardev/chardev-internal.h
> > +++ b/chardev/chardev-internal.h
> > @@ -29,13 +29,16 @@
> > #include "chardev/char-fe.h"
> > #include "qom/object.h"
> >
> > +#define MAX_HUB 4
> > #define MAX_MUX 4
> > #define MUX_BUFFER_SIZE 32 /* Must be a power of 2. */
> > #define MUX_BUFFER_MASK (MUX_BUFFER_SIZE - 1)
> >
> > struct MuxChardev {
> > Chardev parent;
> > + /* Linked frontends */
> > CharBackend *backends[MAX_MUX];
> > + /* Linked backend */
> > CharBackend chr;
> > unsigned long mux_bitset;
> > int focus;
> > @@ -53,11 +56,59 @@ struct MuxChardev {
> > int64_t timestamps_start;
> > };
> > typedef struct MuxChardev MuxChardev;
> > +typedef struct HubChardev HubChardev;
> > +typedef struct HubCharBackend HubCharBackend;
> > +
> > +/*
> > + * Back-pointer on a hub, actual backend and its index in
> > + * `hub->backends` array
> > + */
> > +struct HubCharBackend {
> > + HubChardev *hub;
> > + CharBackend be;
> > + unsigned int be_ind;
> > +};
> > +
> > +struct HubChardev {
> > + Chardev parent;
> > + /* Linked frontend */
> > + CharBackend *frontend;
> > + /* Linked backends */
> > + HubCharBackend backends[MAX_HUB];
> > + /*
> > + * Number of backends attached to this hub. Once attached, a
> > + * backend can't be detached, so the counter is only increasing.
> > + * To safely remove a backend, hub has to be removed first.
> > + */
> > + unsigned int be_cnt;
> > + /*
> > + * Number of CHR_EVEN_OPENED events from all backends. Needed to
> > + * send CHR_EVEN_CLOSED only when counter goes to zero.
> > + */
> > + unsigned int be_event_opened_cnt;
> > + /*
> > + * Counters of written bytes from a single frontend device
> > + * to multiple backend devices.
> > + */
> > + unsigned int be_written[MAX_HUB];
> > + unsigned int be_min_written;
> > + /*
> > + * Index of a backend device which got EAGAIN on last write,
> > + * -1 is invalid index.
> > + */
> > + int be_eagain_ind;
> > +};
> > +typedef struct HubChardev HubChardev;
> >
> > DECLARE_INSTANCE_CHECKER(MuxChardev, MUX_CHARDEV,
> > TYPE_CHARDEV_MUX)
> > -#define CHARDEV_IS_MUX(chr) \
> > +DECLARE_INSTANCE_CHECKER(HubChardev, HUB_CHARDEV,
> > + TYPE_CHARDEV_HUB)
> > +
> > +#define CHARDEV_IS_MUX(chr) \
> > object_dynamic_cast(OBJECT(chr), TYPE_CHARDEV_MUX)
> > +#define CHARDEV_IS_HUB(chr) \
> > + object_dynamic_cast(OBJECT(chr), TYPE_CHARDEV_HUB)
> >
> > bool mux_chr_attach_frontend(MuxChardev *d, CharBackend *b,
> > unsigned int *tag, Error **errp);
> > @@ -65,6 +116,9 @@ bool mux_chr_detach_frontend(MuxChardev *d, unsigned int
> > tag);
> > void mux_set_focus(Chardev *chr, unsigned int focus);
> > void mux_chr_send_all_event(Chardev *chr, QEMUChrEvent event);
> >
> > +bool hub_chr_attach_frontend(HubChardev *d, CharBackend *b, Error **errp);
> > +void hub_chr_detach_frontend(HubChardev *d);
> > +
> > Object *get_chardevs_root(void);
> >
> > #endif /* CHARDEV_INTERNAL_H */
> > diff --git a/chardev/meson.build b/chardev/meson.build
> > index 70070a8279a9..56ee39ac0b01 100644
> > --- a/chardev/meson.build
> > +++ b/chardev/meson.build
> > @@ -3,6 +3,7 @@ chardev_ss.add(files(
> > 'char-file.c',
> > 'char-io.c',
> > 'char-mux.c',
> > + 'char-hub.c',
> > 'char-null.c',
> > 'char-pipe.c',
> > 'char-ringbuf.c',
> > diff --git a/include/chardev/char.h b/include/chardev/char.h
> > index 01df55f9e8c8..429852f8d9d3 100644
> > --- a/include/chardev/char.h
> > +++ b/include/chardev/char.h
> > @@ -232,6 +232,7 @@ OBJECT_DECLARE_TYPE(Chardev, ChardevClass, CHARDEV)
> >
> > #define TYPE_CHARDEV_NULL "chardev-null"
> > #define TYPE_CHARDEV_MUX "chardev-mux"
> > +#define TYPE_CHARDEV_HUB "chardev-hub"
> > #define TYPE_CHARDEV_RINGBUF "chardev-ringbuf"
> > #define TYPE_CHARDEV_PTY "chardev-pty"
> > #define TYPE_CHARDEV_CONSOLE "chardev-console"
> > diff --git a/qapi/char.json b/qapi/char.json
> > index e04535435034..f02b66c06b3e 100644
> > --- a/qapi/char.json
> > +++ b/qapi/char.json
> > @@ -332,6 +332,19 @@
> > 'data': { 'chardev': 'str' },
> > 'base': 'ChardevCommon' }
> >
> > +##
> > +# @ChardevHub:
> > +#
> > +# Configuration info for hub chardevs.
> > +#
> > +# @chardevs: List of chardev IDs, which should be added to this hub
> > +#
> > +# Since: 10.0
> > +##
> > +{ 'struct': 'ChardevHub',
> > + 'data': { 'chardevs': ['str'] },
> > + 'base': 'ChardevCommon' }
> > +
> > ##
> > # @ChardevStdio:
> > #
> > @@ -479,6 +492,8 @@
> > #
> > # @mux: (since 1.5)
> > #
> > +# @hub: (since 10.0)
> > +#
> > # @msmouse: emulated Microsoft serial mouse (since 1.5)
> > #
> > # @wctablet: emulated Wacom Penpartner serial tablet (since 2.9)
> > @@ -521,6 +536,7 @@
> > 'pty',
> > 'null',
> > 'mux',
> > + 'hub',
> > 'msmouse',
> > 'wctablet',
> > { 'name': 'braille', 'if': 'CONFIG_BRLAPI' },
> > @@ -595,6 +611,16 @@
> > { 'struct': 'ChardevMuxWrapper',
> > 'data': { 'data': 'ChardevMux' } }
> >
> > +##
> > +# @ChardevHubWrapper:
> > +#
> > +# @data: Configuration info for hub chardevs
> > +#
> > +# Since: 10.0
> > +##
> > +{ 'struct': 'ChardevHubWrapper',
> > + 'data': { 'data': 'ChardevHub' } }
> > +
> > ##
> > # @ChardevStdioWrapper:
> > #
> > @@ -703,6 +729,7 @@
> > 'pty': 'ChardevPtyWrapper',
> > 'null': 'ChardevCommonWrapper',
> > 'mux': 'ChardevMuxWrapper',
> > + 'hub': 'ChardevHubWrapper',
> > 'msmouse': 'ChardevCommonWrapper',
> > 'wctablet': 'ChardevCommonWrapper',
> > 'braille': { 'type': 'ChardevCommonWrapper',
> > --
> > 2.43.0
> >
>
> looks ok to me otherwise
I'll resend the series, thanks.
--
Roman