Re: [PATCH v6 6/8] chardev/char-mux: implement backend chardev multiplexing

[Roman Penyaev]

Hi,

On Tue, Jan 7, 2025 at 3:57 PM Marc-André Lureau
<marcandre.lureau@redhat.com> wrote:
>
> Hi

[cut]

> > > But next attempt to write will loop over the same backend again, which
> > > will see the "same" write multiple times.
> >
> > This case is handled by checking the difference between counters
> > `d->be_written[i]` and `d->be_min_written`. The idea is that device, which
> > already "swallowed" some portion of data, will be skipped from writing to 
> > it,
> > until it catches up with the stream.
>
> ok, I see. This looks fragile though, I/one will need to do a more
> thorough review.

Yes, please, a thorough review would be great. I'm really keen to improve
what needs to be improved and hopefully finish this.

Do you want me to resend this as v7 to make patchew to grab this series
for further review?

>
> >
> > Please take a look into the `char_mux_be_test()` test case, where the
> > EAGAIN scenario is tested. The line test-char.c:716 explicitly shows the
> > repeat of the write procedure after EAGAIN was received.
> >
> > >
> > > > +        } 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.
> > > > +             */
> > > > +            r = len;
> > > > +        }
> > > > +        d->be_written[i] += r;
> > > > +        ret = MIN(r, ret);
> > > > +    }
> > > > +    d->be_min_written += ret;
> > > > +
> > > > +    return ret;
> > > > +}
> > >
> > > I am not sure what is the correct way to handle write here. This
> > > mux-be behaviour is different from mux-fe, since it handles all
> > > backend I/Os, and does not select one... it's more of a "mixer",
> > > right, Is this wanted?
> >
> > Right. The intention is to have both consoles simultaneously
> > working, for example having char-based tio (over a socket chardev)
> > and image-based vnc (over a vc chardev):
> >
> >     -chardev socket,path=/tmp/sock,server=on,wait=off,id=sock0 \
> >     -chardev vc,id=vc0 \
> >
> > and both are connected to the same frontend device.
> >
> > I agree with you on the "mixer" naming concern, this did not come to
> > my mind. As far as I understand the logic of `mux-fe`, it just doesn't seem
> > possible to have both frontends running at the same time, because they
> > will both generate output, at least that's the case for virtual consoles:
> > imagine you have two virtual console frontends working at the same time
> > and one backend. Any command you enter from a backend causes the two
> > separate frontends to output completely different data.
> >
> > On the other hand, several backend devices can easily be simultaneously
> > attached to one frontend, the analogy is simple: several monitors, several
> > keyboards, etc work perfectly fine with a single PC. At least this is how
> > I see this, please correct me if I'm wrong.
>
> Whether we talk about multiplexing front-end or back-end, the issues
> are similar. In general, mixing input will create issues. Teeing
> output is less problematic, except to handle the buffering...

I understand your concerns. What exact issues do you have in mind?
Are these issues related to the input buffer handling, so technical issues?
Or issues with usability?

>
> >
> > Do you think we need to artificially introduce multiplexing logic to be 
> > fully
> > compliant with multiplexer naming? It's not hard to do, repeating
> > `mux_proc_byte()` from `mux-fe`. In my use-case, I'll still need to disable
> > multiplexing in favor of 'mixing', for example with the 'mixer=on' option,
> > i.e. '-chardev mux-be,mixer=on,...`. Or do you think it should be some
> > completely different beast, something like mixer chardev?
>
> I think it would be saner to have the muxer be selectors: only work
> with one selected be or fe. Otherwise, we can run into various issues.

In multiplexing (not mixing) for the use-case that I am describing, there is one
serious drawback: as soon as you switch the "focus" to another input device
(for example from vnc to socket chardev), you will not be able to switch back
from the same input console - the input now works on another device. This looks
strange and does not add convenience to the final user. Perhaps, for a case
other than console, this would be reasonable, but for console input -
I would like
to keep the mixer option: the front-end receives input from both back-ends.

> I hope some qemu maintainers can comment too.

Do I need to add someone else in CC to gain more feedback?

--
Roman