Re: [PATCH v7 2/4] chardev/char-hub: implement backend chardev aggregator

[Roman Penyaev]

On Wed, Jan 22, 2025 at 3:44 PM Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Roman Penyaev <r.peniaev@gmail.com> writes:
>
> > 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.
> >
> <snip>
> > +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;
>
> So I was looking at this to see if I could implement what I wanted which
> was a tee-like copy of a serial port output while maintaining the C-a
> support of the mux.
>
> Normally I just use the shortcut -serial mon:stdio
>
> However that form is a special case so I tried the following and ran
> into the above:
>
>           -chardev stdio,mux=on,id=char0 \
>           -chardev file,path=console.log,id=clog  \
>           -mon chardev=char0,mode=readline \
>           -chardev hub,id=hub0,chardevs.0=char0,chardevs.1=clog
>
> Giving:
>   qemu-system-aarch64: -chardev 
> -hub,id=hub0,chardevs.0=char0,chardevs.1=clog: hub: -multiplexers and hub 
> devices can't be stacked, check chardev
>             -'char0', chardev should not be a hub device or have 'mux=on' 
> -enabled
>
> So what stops this sort of chain?

Hi Alex,

You define 'char0' as a mux device (the "mux=on" option). That is not supported
simply to avoid circle dependencies. To be frank I never considered
this use-case,
although chains might be useful. If you need this use-case we can
think of a more
proper (or advance) check.

--
Roman