* [PATCH v4 0/8] chardev: implement backend chardev multiplexing
@ 2024-10-16 10:25 Roman Penyaev
2024-10-16 10:25 ` [PATCH v4 1/8] chardev/char: rename `MuxChardev` struct to `MuxFeChardev` Roman Penyaev
` (7 more replies)
0 siblings, 8 replies; 22+ messages in thread
From: Roman Penyaev @ 2024-10-16 10:25 UTC (permalink / raw)
Cc: Roman Penyaev, Marc-André Lureau, qemu-devel
Mux is a character backend (host side) device, which multiplexes
multiple frontends with one backend device. The following is a
few lines from the QEMU manpage [1]:
A multiplexer is a "1:N" device, and here the "1" end is your
specified chardev backend, and the "N" end is the various parts
of QEMU that can talk to a chardev.
But sadly multiple backends are not supported.
This work implements multiplexing capability of several backend
devices, which opens up an opportunity to use a single frontend
device on the guest, which can be manipulated from several
backend devices.
The motivation is the EVE project [2], where it would be very
convenient to have a virtio console frontend device on the guest that
can be controlled from multiple backend devices, namely VNC and local
TTY emulator. The following is an example of the QEMU command line:
-chardev mux-be,id=mux0 \
-chardev socket,path=/tmp/sock,server=on,wait=off,id=sock0,mux-be-id=mux0 \
-chardev vc,id=vc0,mux-be-id=mux0 \
-device virtconsole,chardev=mux0 \
-vnc 0.0.0.0:0
which creates 2 backend devices: text virtual console (`vc0`) and a
socket (`sock0`) connected to the single virtio hvc console with the
backend multiplexer (`mux0`) help. `vc0` renders text to an image,
which can be shared over the VNC protocol. `sock0` is a socket
backend which provides biderectional communication to the virtio hvc
console.
Once QEMU starts VNC client and any TTY emulator can be used to
control a single hvc console, for example these two different
consoles should have similar input and output due the buffer
multiplexing:
# VNC client
vncviewer :0
# TTY emulator
socat unix-connect:/tmp/sock pty,link=/tmp/pty
tio /tmp/pty
v3 .. v4:
* Rebase on latest chardev changes
* Add unit tests which test corner cases:
* Inability to remove mux with active frontend
* Inability to add more chardevs to a mux than `MUX_MAX`
* Inability to mix mux-fe and mux-be for the same chardev
v2 .. v3:
* Split frontend and backend multiplexer implementations and
move them to separate files: char-mux-fe.c and char-mux-be.c
v1 .. v2:
* Separate type for the backend multiplexer `mux-be`
* Handle EAGAIN on write to the backend device
* Support of watch of previously failed backend device
* Proper json support of the `mux-be-id` option
* Unit test for the `mux-be` multiplexer
[1] https://www.qemu.org/docs/master/system/qemu-manpage.html#hxtool-6
[2] https://github.com/lf-edge/eve
Roman Penyaev (8):
chardev/char: rename `MuxChardev` struct to `MuxFeChardev`
chardev/char: rename `char-mux.c` to `char-mux-fe.c`
chardev/char: move away mux suspend/resume calls
chardev/char: rename frontend mux calls
chardev/char: introduce `mux-be-id=ID` option
chardev/char-mux: implement backend chardev multiplexing
tests/unit/test-char: add unit test for the `mux-be` multiplexer
qemu-options.hx: describe multiplexing of several backend devices
chardev/char-fe.c | 25 ++-
chardev/char-mux-be.c | 290 ++++++++++++++++++++++++
chardev/{char-mux.c => char-mux-fe.c} | 157 +++++--------
chardev/char.c | 133 +++++++++--
chardev/chardev-internal.h | 55 ++++-
chardev/meson.build | 3 +-
include/chardev/char.h | 8 +-
qapi/char.json | 31 ++-
qemu-options.hx | 78 +++++--
system/vl.c | 4 +-
tests/unit/test-char.c | 306 +++++++++++++++++++++++++-
11 files changed, 922 insertions(+), 168 deletions(-)
create mode 100644 chardev/char-mux-be.c
rename chardev/{char-mux.c => char-mux-fe.c} (71%)
Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>
Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
Cc: qemu-devel@nongnu.org
--
2.34.1
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v4 1/8] chardev/char: rename `MuxChardev` struct to `MuxFeChardev`
2024-10-16 10:25 [PATCH v4 0/8] chardev: implement backend chardev multiplexing Roman Penyaev
@ 2024-10-16 10:25 ` Roman Penyaev
2024-10-16 10:25 ` [PATCH v4 2/8] chardev/char: rename `char-mux.c` to `char-mux-fe.c` Roman Penyaev
` (6 subsequent siblings)
7 siblings, 0 replies; 22+ messages in thread
From: Roman Penyaev @ 2024-10-16 10:25 UTC (permalink / raw)
Cc: Roman Penyaev, Marc-André Lureau, qemu-devel
In the following patches backend multiplexer will be
introduced. This patch renames the structure and a
few mux macros to reflect that this mux is responsible
only for multiplexing of frontend devices.
This patch does the following:
s/MuxChardev/MuxFeChardev/g
s/CHARDEV_IS_MUX/CHARDEV_IS_MUX_FE/g
s/MUX_CHARDEV/MUX_FE_CHARDEV/g
s/TYPE_CHARDEV_MUX/TYPE_CHARDEV_MUX_FE/g
No json or string types are changed for the sake of
compatibility.
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 | 10 +++++-----
chardev/char-mux.c | 36 ++++++++++++++++++------------------
chardev/char.c | 8 ++++----
chardev/chardev-internal.h | 16 ++++++++--------
include/chardev/char.h | 2 +-
5 files changed, 36 insertions(+), 36 deletions(-)
diff --git a/chardev/char-fe.c b/chardev/char-fe.c
index 8ac6bebb6f74..7b1ae16c62a4 100644
--- a/chardev/char-fe.c
+++ b/chardev/char-fe.c
@@ -194,8 +194,8 @@ bool qemu_chr_fe_init(CharBackend *b, Chardev *s, Error **errp)
unsigned int tag = 0;
if (s) {
- if (CHARDEV_IS_MUX(s)) {
- MuxChardev *d = MUX_CHARDEV(s);
+ if (CHARDEV_IS_MUX_FE(s)) {
+ MuxFeChardev *d = MUX_FE_CHARDEV(s);
if (!mux_chr_attach_frontend(d, b, &tag, errp)) {
return false;
@@ -223,8 +223,8 @@ void qemu_chr_fe_deinit(CharBackend *b, bool del)
if (b->chr->be == b) {
b->chr->be = NULL;
}
- if (CHARDEV_IS_MUX(b->chr)) {
- MuxChardev *d = MUX_CHARDEV(b->chr);
+ if (CHARDEV_IS_MUX_FE(b->chr)) {
+ MuxFeChardev *d = MUX_FE_CHARDEV(b->chr);
mux_chr_detach_frontend(d, b->tag);
}
if (del) {
@@ -305,7 +305,7 @@ void qemu_chr_fe_take_focus(CharBackend *b)
return;
}
- if (CHARDEV_IS_MUX(b->chr)) {
+ if (CHARDEV_IS_MUX_FE(b->chr)) {
mux_set_focus(b->chr, b->tag);
}
}
diff --git a/chardev/char-mux.c b/chardev/char-mux.c
index bda5c45e6058..dfaea5aefac3 100644
--- a/chardev/char-mux.c
+++ b/chardev/char-mux.c
@@ -44,7 +44,7 @@ static bool muxes_opened = true;
/* Called with chr_write_lock held. */
static int mux_chr_write(Chardev *chr, const uint8_t *buf, int len)
{
- MuxChardev *d = MUX_CHARDEV(chr);
+ MuxFeChardev *d = MUX_FE_CHARDEV(chr);
int ret;
if (!d->timestamps) {
ret = qemu_chr_fe_write(&d->chr, buf, len);
@@ -125,7 +125,7 @@ static void mux_print_help(Chardev *chr)
}
}
-static void mux_chr_send_event(MuxChardev *d, unsigned int mux_nr,
+static void mux_chr_send_event(MuxFeChardev *d, unsigned int mux_nr,
QEMUChrEvent event)
{
CharBackend *be = d->backends[mux_nr];
@@ -137,14 +137,14 @@ static void mux_chr_send_event(MuxChardev *d, unsigned int mux_nr,
static void mux_chr_be_event(Chardev *chr, QEMUChrEvent event)
{
- MuxChardev *d = MUX_CHARDEV(chr);
+ MuxFeChardev *d = MUX_FE_CHARDEV(chr);
if (d->focus != -1) {
mux_chr_send_event(d, d->focus, event);
}
}
-static int mux_proc_byte(Chardev *chr, MuxChardev *d, int ch)
+static int mux_proc_byte(Chardev *chr, MuxFeChardev *d, int ch)
{
if (d->term_got_escape) {
d->term_got_escape = false;
@@ -198,7 +198,7 @@ static int mux_proc_byte(Chardev *chr, MuxChardev *d, int ch)
static void mux_chr_accept_input(Chardev *chr)
{
- MuxChardev *d = MUX_CHARDEV(chr);
+ MuxFeChardev *d = MUX_FE_CHARDEV(chr);
int m = d->focus;
CharBackend *be = d->backends[m];
@@ -211,7 +211,7 @@ static void mux_chr_accept_input(Chardev *chr)
static int mux_chr_can_read(void *opaque)
{
- MuxChardev *d = MUX_CHARDEV(opaque);
+ MuxFeChardev *d = MUX_FE_CHARDEV(opaque);
int m = d->focus;
CharBackend *be = d->backends[m];
@@ -229,7 +229,7 @@ static int mux_chr_can_read(void *opaque)
static void mux_chr_read(void *opaque, const uint8_t *buf, int size)
{
Chardev *chr = CHARDEV(opaque);
- MuxChardev *d = MUX_CHARDEV(opaque);
+ MuxFeChardev *d = MUX_FE_CHARDEV(opaque);
int m = d->focus;
CharBackend *be = d->backends[m];
int i;
@@ -250,7 +250,7 @@ static void mux_chr_read(void *opaque, const uint8_t *buf, int size)
void mux_chr_send_all_event(Chardev *chr, QEMUChrEvent event)
{
- MuxChardev *d = MUX_CHARDEV(chr);
+ MuxFeChardev *d = MUX_FE_CHARDEV(chr);
int bit;
if (!muxes_opened) {
@@ -271,7 +271,7 @@ static void mux_chr_event(void *opaque, QEMUChrEvent event)
static GSource *mux_chr_add_watch(Chardev *s, GIOCondition cond)
{
- MuxChardev *d = MUX_CHARDEV(s);
+ MuxFeChardev *d = MUX_FE_CHARDEV(s);
Chardev *chr = qemu_chr_fe_get_driver(&d->chr);
ChardevClass *cc = CHARDEV_GET_CLASS(chr);
@@ -284,7 +284,7 @@ static GSource *mux_chr_add_watch(Chardev *s, GIOCondition cond)
static void char_mux_finalize(Object *obj)
{
- MuxChardev *d = MUX_CHARDEV(obj);
+ MuxFeChardev *d = MUX_FE_CHARDEV(obj);
int bit;
bit = -1;
@@ -299,7 +299,7 @@ static void char_mux_finalize(Object *obj)
static void mux_chr_update_read_handlers(Chardev *chr)
{
- MuxChardev *d = MUX_CHARDEV(chr);
+ MuxFeChardev *d = MUX_FE_CHARDEV(chr);
/* Fix up the real driver with mux routines */
qemu_chr_fe_set_handlers_full(&d->chr,
@@ -311,7 +311,7 @@ static void mux_chr_update_read_handlers(Chardev *chr)
chr->gcontext, true, false);
}
-bool mux_chr_attach_frontend(MuxChardev *d, CharBackend *b,
+bool mux_chr_attach_frontend(MuxFeChardev *d, CharBackend *b,
unsigned int *tag, Error **errp)
{
unsigned int bit;
@@ -332,7 +332,7 @@ bool mux_chr_attach_frontend(MuxChardev *d, CharBackend *b,
return true;
}
-bool mux_chr_detach_frontend(MuxChardev *d, unsigned int tag)
+bool mux_chr_detach_frontend(MuxFeChardev *d, unsigned int tag)
{
unsigned int bit;
@@ -349,7 +349,7 @@ bool mux_chr_detach_frontend(MuxChardev *d, unsigned int tag)
void mux_set_focus(Chardev *chr, unsigned int focus)
{
- MuxChardev *d = MUX_CHARDEV(chr);
+ MuxFeChardev *d = MUX_FE_CHARDEV(chr);
assert(find_next_bit(&d->mux_bitset, MAX_MUX, focus) == focus);
@@ -369,7 +369,7 @@ static void qemu_chr_open_mux(Chardev *chr,
{
ChardevMux *mux = backend->u.mux.data;
Chardev *drv;
- MuxChardev *d = MUX_CHARDEV(chr);
+ MuxFeChardev *d = MUX_FE_CHARDEV(chr);
drv = qemu_chr_find(mux->chardev);
if (drv == NULL) {
@@ -434,7 +434,7 @@ static int chardev_options_parsed_cb(Object *child, void *opaque)
{
Chardev *chr = (Chardev *)child;
- if (!chr->be_open && CHARDEV_IS_MUX(chr)) {
+ if (!chr->be_open && CHARDEV_IS_MUX_FE(chr)) {
open_muxes(chr);
}
@@ -462,10 +462,10 @@ static void char_mux_class_init(ObjectClass *oc, void *data)
}
static const TypeInfo char_mux_type_info = {
- .name = TYPE_CHARDEV_MUX,
+ .name = TYPE_CHARDEV_MUX_FE,
.parent = TYPE_CHARDEV,
.class_init = char_mux_class_init,
- .instance_size = sizeof(MuxChardev),
+ .instance_size = sizeof(MuxFeChardev),
.instance_finalize = char_mux_finalize,
};
diff --git a/chardev/char.c b/chardev/char.c
index a1722aa076d9..e077773cdece 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -331,8 +331,8 @@ static const TypeInfo char_type_info = {
static bool qemu_chr_is_busy(Chardev *s)
{
- if (CHARDEV_IS_MUX(s)) {
- MuxChardev *d = MUX_CHARDEV(s);
+ if (CHARDEV_IS_MUX_FE(s)) {
+ MuxFeChardev *d = MUX_FE_CHARDEV(s);
return d->mux_bitset != 0;
} else {
return s->be != NULL;
@@ -687,7 +687,7 @@ static Chardev *__qemu_chr_new_from_opts(QemuOpts *opts, GMainContext *context,
backend->type = CHARDEV_BACKEND_KIND_MUX;
backend->u.mux.data = g_new0(ChardevMux, 1);
backend->u.mux.data->chardev = g_strdup(bid);
- mux = qemu_chardev_new(id, TYPE_CHARDEV_MUX, backend, context, errp);
+ mux = qemu_chardev_new(id, TYPE_CHARDEV_MUX_FE, backend, context, errp);
if (mux == NULL) {
object_unparent(OBJECT(chr));
chr = NULL;
@@ -1104,7 +1104,7 @@ ChardevReturn *qmp_chardev_change(const char *id, ChardevBackend *backend,
return NULL;
}
- if (CHARDEV_IS_MUX(chr)) {
+ if (CHARDEV_IS_MUX_FE(chr)) {
error_setg(errp, "Mux device hotswap not supported yet");
return NULL;
}
diff --git a/chardev/chardev-internal.h b/chardev/chardev-internal.h
index 853807f3cb88..321051bb9cc5 100644
--- a/chardev/chardev-internal.h
+++ b/chardev/chardev-internal.h
@@ -33,7 +33,7 @@
#define MUX_BUFFER_SIZE 32 /* Must be a power of 2. */
#define MUX_BUFFER_MASK (MUX_BUFFER_SIZE - 1)
-struct MuxChardev {
+struct MuxFeChardev {
Chardev parent;
CharBackend *backends[MAX_MUX];
CharBackend chr;
@@ -52,16 +52,16 @@ struct MuxChardev {
bool linestart;
int64_t timestamps_start;
};
-typedef struct MuxChardev MuxChardev;
+typedef struct MuxFeChardev MuxFeChardev;
-DECLARE_INSTANCE_CHECKER(MuxChardev, MUX_CHARDEV,
- TYPE_CHARDEV_MUX)
-#define CHARDEV_IS_MUX(chr) \
- object_dynamic_cast(OBJECT(chr), TYPE_CHARDEV_MUX)
+DECLARE_INSTANCE_CHECKER(MuxFeChardev, MUX_FE_CHARDEV,
+ TYPE_CHARDEV_MUX_FE)
+#define CHARDEV_IS_MUX_FE(chr) \
+ object_dynamic_cast(OBJECT(chr), TYPE_CHARDEV_MUX_FE)
-bool mux_chr_attach_frontend(MuxChardev *d, CharBackend *b,
+bool mux_chr_attach_frontend(MuxFeChardev *d, CharBackend *b,
unsigned int *tag, Error **errp);
-bool mux_chr_detach_frontend(MuxChardev *d, unsigned int tag);
+bool mux_chr_detach_frontend(MuxFeChardev *d, unsigned int tag);
void mux_set_focus(Chardev *chr, unsigned int focus);
void mux_chr_send_all_event(Chardev *chr, QEMUChrEvent event);
diff --git a/include/chardev/char.h b/include/chardev/char.h
index 01df55f9e8c8..d9d23b6232db 100644
--- a/include/chardev/char.h
+++ b/include/chardev/char.h
@@ -231,7 +231,7 @@ int qemu_chr_wait_connected(Chardev *chr, Error **errp);
OBJECT_DECLARE_TYPE(Chardev, ChardevClass, CHARDEV)
#define TYPE_CHARDEV_NULL "chardev-null"
-#define TYPE_CHARDEV_MUX "chardev-mux"
+#define TYPE_CHARDEV_MUX_FE "chardev-mux"
#define TYPE_CHARDEV_RINGBUF "chardev-ringbuf"
#define TYPE_CHARDEV_PTY "chardev-pty"
#define TYPE_CHARDEV_CONSOLE "chardev-console"
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v4 2/8] chardev/char: rename `char-mux.c` to `char-mux-fe.c`
2024-10-16 10:25 [PATCH v4 0/8] chardev: implement backend chardev multiplexing Roman Penyaev
2024-10-16 10:25 ` [PATCH v4 1/8] chardev/char: rename `MuxChardev` struct to `MuxFeChardev` Roman Penyaev
@ 2024-10-16 10:25 ` Roman Penyaev
2024-10-16 10:26 ` [PATCH v4 3/8] chardev/char: move away mux suspend/resume calls Roman Penyaev
` (5 subsequent siblings)
7 siblings, 0 replies; 22+ messages in thread
From: Roman Penyaev @ 2024-10-16 10:25 UTC (permalink / raw)
Cc: Roman Penyaev, Marc-André Lureau, qemu-devel
In the following patches backend multiplexer will be
introduced and the implementation will be named as
follows: `char-mux-be.c`. This patch renames the
frontend multiplexer from `char-mux.c` to
`char-mux-fe.c`.
Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>
Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
Cc: qemu-devel@nongnu.org
---
chardev/{char-mux.c => char-mux-fe.c} | 0
chardev/meson.build | 2 +-
2 files changed, 1 insertion(+), 1 deletion(-)
rename chardev/{char-mux.c => char-mux-fe.c} (100%)
diff --git a/chardev/char-mux.c b/chardev/char-mux-fe.c
similarity index 100%
rename from chardev/char-mux.c
rename to chardev/char-mux-fe.c
diff --git a/chardev/meson.build b/chardev/meson.build
index 70070a8279a9..778444a00ca6 100644
--- a/chardev/meson.build
+++ b/chardev/meson.build
@@ -2,7 +2,7 @@ chardev_ss.add(files(
'char-fe.c',
'char-file.c',
'char-io.c',
- 'char-mux.c',
+ 'char-mux-fe.c',
'char-null.c',
'char-pipe.c',
'char-ringbuf.c',
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v4 3/8] chardev/char: move away mux suspend/resume calls
2024-10-16 10:25 [PATCH v4 0/8] chardev: implement backend chardev multiplexing Roman Penyaev
2024-10-16 10:25 ` [PATCH v4 1/8] chardev/char: rename `MuxChardev` struct to `MuxFeChardev` Roman Penyaev
2024-10-16 10:25 ` [PATCH v4 2/8] chardev/char: rename `char-mux.c` to `char-mux-fe.c` Roman Penyaev
@ 2024-10-16 10:26 ` Roman Penyaev
2024-10-16 10:26 ` [PATCH v4 4/8] chardev/char: rename frontend mux calls Roman Penyaev
` (4 subsequent siblings)
7 siblings, 0 replies; 22+ messages in thread
From: Roman Penyaev @ 2024-10-16 10:26 UTC (permalink / raw)
Cc: Roman Penyaev, Marc-André Lureau, qemu-devel
The suspend/resume open multiplexer calls are generic
and will be used for frontend (current mux) and backend
(will follow) implementations. Move them away from the
`char-mux-fe.c` to more generic `char.c` file. Also
for the sake of clarity these renames were made:
s/suspend_mux_open/mux_suspend_open/g
s/resume_mux_open/mux_resume_open/g
No functional changes are made.
Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>
Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
Cc: qemu-devel@nongnu.org
---
chardev/char-mux-fe.c | 63 ++-------------------------------
chardev/char.c | 72 ++++++++++++++++++++++++++++++++++++++
chardev/chardev-internal.h | 3 ++
include/chardev/char.h | 5 +--
system/vl.c | 4 +--
5 files changed, 82 insertions(+), 65 deletions(-)
diff --git a/chardev/char-mux-fe.c b/chardev/char-mux-fe.c
index dfaea5aefac3..6a195390a3c9 100644
--- a/chardev/char-mux-fe.c
+++ b/chardev/char-mux-fe.c
@@ -34,13 +34,6 @@
/* MUX driver for serial I/O splitting */
-/*
- * Set to false by suspend_mux_open. Open events are delayed until
- * resume_mux_open. Usually suspend_mux_open is called before
- * command line processing and resume_mux_open afterwards.
- */
-static bool muxes_opened = true;
-
/* Called with chr_write_lock held. */
static int mux_chr_write(Chardev *chr, const uint8_t *buf, int len)
{
@@ -248,15 +241,10 @@ static void mux_chr_read(void *opaque, const uint8_t *buf, int size)
}
}
-void mux_chr_send_all_event(Chardev *chr, QEMUChrEvent event)
+void mux_fe_chr_send_all_event(MuxFeChardev *d, QEMUChrEvent event)
{
- MuxFeChardev *d = MUX_FE_CHARDEV(chr);
int bit;
- if (!muxes_opened) {
- return;
- }
-
/* Send the event to all registered listeners */
bit = -1;
while ((bit = find_next_bit(&d->mux_bitset, MAX_MUX, bit + 1)) < MAX_MUX) {
@@ -381,7 +369,7 @@ static void qemu_chr_open_mux(Chardev *chr,
/* only default to opened state if we've realized the initial
* set of muxes
*/
- *be_opened = muxes_opened;
+ *be_opened = mux_is_opened();
qemu_chr_fe_init(&d->chr, drv, errp);
}
@@ -401,53 +389,6 @@ static void qemu_chr_parse_mux(QemuOpts *opts, ChardevBackend *backend,
mux->chardev = g_strdup(chardev);
}
-/**
- * Called after processing of default and command-line-specified
- * chardevs to deliver CHR_EVENT_OPENED events to any FEs attached
- * to a mux chardev. This is done here to ensure that
- * output/prompts/banners are only displayed for the FE that has
- * focus when initial command-line processing/machine init is
- * completed.
- *
- * After this point, any new FE attached to any new or existing
- * mux will receive CHR_EVENT_OPENED notifications for the BE
- * immediately.
- */
-static void open_muxes(Chardev *chr)
-{
- /* send OPENED to all already-attached FEs */
- mux_chr_send_all_event(chr, CHR_EVENT_OPENED);
-
- /*
- * mark mux as OPENED so any new FEs will immediately receive
- * OPENED event
- */
- chr->be_open = 1;
-}
-
-void suspend_mux_open(void)
-{
- muxes_opened = false;
-}
-
-static int chardev_options_parsed_cb(Object *child, void *opaque)
-{
- Chardev *chr = (Chardev *)child;
-
- if (!chr->be_open && CHARDEV_IS_MUX_FE(chr)) {
- open_muxes(chr);
- }
-
- return 0;
-}
-
-void resume_mux_open(void)
-{
- muxes_opened = true;
- object_child_foreach(get_chardevs_root(),
- chardev_options_parsed_cb, NULL);
-}
-
static void char_mux_class_init(ObjectClass *oc, void *data)
{
ChardevClass *cc = CHARDEV_CLASS(oc);
diff --git a/chardev/char.c b/chardev/char.c
index e077773cdece..d8dbdb6f84f1 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -43,6 +43,13 @@
#include "chardev-internal.h"
+/*
+ * Set to false by mux_suspend_open(). Open events are delayed until
+ * mux_resume_open(). Usually mux_suspend_open() is called before
+ * command line processing and mux_resume_open() afterwards.
+ */
+static bool muxes_opened = true;
+
/***********************************************************/
/* character device */
@@ -1259,6 +1266,71 @@ void qemu_chr_cleanup(void)
object_unparent(get_chardevs_root());
}
+/**
+ * Called after processing of default and command-line-specified
+ * chardevs to deliver CHR_EVENT_OPENED events to any FEs attached
+ * to a mux chardev. This is done here to ensure that
+ * output/prompts/banners are only displayed for the FE that has
+ * focus when initial command-line processing/machine init is
+ * completed.
+ *
+ * After this point, any new FE attached to any new or existing
+ * mux will receive CHR_EVENT_OPENED notifications for the BE
+ * immediately.
+ */
+static void open_muxes(Chardev *chr)
+{
+ /* send OPENED to all already-attached FEs */
+ mux_chr_send_all_event(chr, CHR_EVENT_OPENED);
+
+ /*
+ * mark mux as OPENED so any new FEs will immediately receive
+ * OPENED event
+ */
+ chr->be_open = 1;
+}
+
+void mux_suspend_open(void)
+{
+ muxes_opened = false;
+}
+
+static int chardev_options_parsed_cb(Object *child, void *opaque)
+{
+ Chardev *chr = (Chardev *)child;
+
+ if (!chr->be_open && CHARDEV_IS_MUX_FE(chr)) {
+ open_muxes(chr);
+ }
+
+ return 0;
+}
+
+void mux_resume_open(void)
+{
+ muxes_opened = true;
+ object_child_foreach(get_chardevs_root(),
+ chardev_options_parsed_cb, NULL);
+}
+
+bool mux_is_opened(void)
+{
+ return muxes_opened;
+}
+
+void mux_chr_send_all_event(Chardev *chr, QEMUChrEvent event)
+{
+ if (!mux_is_opened()) {
+ return;
+ }
+
+ if (CHARDEV_IS_MUX_FE(chr)) {
+ MuxFeChardev *d = MUX_FE_CHARDEV(chr);
+
+ mux_fe_chr_send_all_event(d, event);
+ }
+}
+
static void register_types(void)
{
type_register_static(&char_type_info);
diff --git a/chardev/chardev-internal.h b/chardev/chardev-internal.h
index 321051bb9cc5..c874850a41ad 100644
--- a/chardev/chardev-internal.h
+++ b/chardev/chardev-internal.h
@@ -65,6 +65,9 @@ bool mux_chr_detach_frontend(MuxFeChardev *d, unsigned int tag);
void mux_set_focus(Chardev *chr, unsigned int focus);
void mux_chr_send_all_event(Chardev *chr, QEMUChrEvent event);
+/* Mux type dependent calls */
+void mux_fe_chr_send_all_event(MuxFeChardev *d, QEMUChrEvent event);
+
Object *get_chardevs_root(void);
#endif /* CHARDEV_INTERNAL_H */
diff --git a/include/chardev/char.h b/include/chardev/char.h
index d9d23b6232db..0bec974f9d73 100644
--- a/include/chardev/char.h
+++ b/include/chardev/char.h
@@ -317,7 +317,8 @@ extern int term_escape_char;
GSource *qemu_chr_timeout_add_ms(Chardev *chr, guint ms,
GSourceFunc func, void *private);
-void suspend_mux_open(void);
-void resume_mux_open(void);
+bool mux_is_opened(void);
+void mux_suspend_open(void);
+void mux_resume_open(void);
#endif
diff --git a/system/vl.c b/system/vl.c
index e83b3b2608bf..b3cbfe2c0f84 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -3679,7 +3679,7 @@ void qemu_init(int argc, char **argv)
qemu_create_machine(machine_opts_dict);
- suspend_mux_open();
+ mux_suspend_open();
qemu_disable_default_devices();
qemu_setup_display();
@@ -3757,5 +3757,5 @@ void qemu_init(int argc, char **argv)
qemu_init_displays();
accel_setup_post(current_machine);
os_setup_post();
- resume_mux_open();
+ mux_resume_open();
}
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v4 4/8] chardev/char: rename frontend mux calls
2024-10-16 10:25 [PATCH v4 0/8] chardev: implement backend chardev multiplexing Roman Penyaev
` (2 preceding siblings ...)
2024-10-16 10:26 ` [PATCH v4 3/8] chardev/char: move away mux suspend/resume calls Roman Penyaev
@ 2024-10-16 10:26 ` Roman Penyaev
2024-10-16 10:26 ` [PATCH v4 5/8] chardev/char: introduce `mux-be-id=ID` option Roman Penyaev
` (3 subsequent siblings)
7 siblings, 0 replies; 22+ messages in thread
From: Roman Penyaev @ 2024-10-16 10:26 UTC (permalink / raw)
Cc: Roman Penyaev, Marc-André Lureau, qemu-devel
This patch renames calls in the frontend mux implementation
to reflect its frontend nature. Patch does the following:
s/mux_chr/mux_fe_chr/g
No functional changes are made.
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 | 6 ++--
chardev/char-mux-fe.c | 68 +++++++++++++++++++-------------------
chardev/chardev-internal.h | 8 ++---
3 files changed, 41 insertions(+), 41 deletions(-)
diff --git a/chardev/char-fe.c b/chardev/char-fe.c
index 7b1ae16c62a4..a2b5bff39fd9 100644
--- a/chardev/char-fe.c
+++ b/chardev/char-fe.c
@@ -197,7 +197,7 @@ bool qemu_chr_fe_init(CharBackend *b, Chardev *s, Error **errp)
if (CHARDEV_IS_MUX_FE(s)) {
MuxFeChardev *d = MUX_FE_CHARDEV(s);
- if (!mux_chr_attach_frontend(d, b, &tag, errp)) {
+ if (!mux_fe_chr_attach_frontend(d, b, &tag, errp)) {
return false;
}
} else if (s->be) {
@@ -225,7 +225,7 @@ void qemu_chr_fe_deinit(CharBackend *b, bool del)
}
if (CHARDEV_IS_MUX_FE(b->chr)) {
MuxFeChardev *d = MUX_FE_CHARDEV(b->chr);
- mux_chr_detach_frontend(d, b->tag);
+ mux_fe_chr_detach_frontend(d, b->tag);
}
if (del) {
Object *obj = OBJECT(b->chr);
@@ -306,7 +306,7 @@ void qemu_chr_fe_take_focus(CharBackend *b)
}
if (CHARDEV_IS_MUX_FE(b->chr)) {
- mux_set_focus(b->chr, b->tag);
+ mux_fe_chr_set_focus(b->chr, b->tag);
}
}
diff --git a/chardev/char-mux-fe.c b/chardev/char-mux-fe.c
index 6a195390a3c9..dcfce099ea9a 100644
--- a/chardev/char-mux-fe.c
+++ b/chardev/char-mux-fe.c
@@ -35,7 +35,7 @@
/* MUX driver for serial I/O splitting */
/* Called with chr_write_lock held. */
-static int mux_chr_write(Chardev *chr, const uint8_t *buf, int len)
+static int mux_fe_chr_write(Chardev *chr, const uint8_t *buf, int len)
{
MuxFeChardev *d = MUX_FE_CHARDEV(chr);
int ret;
@@ -118,8 +118,8 @@ static void mux_print_help(Chardev *chr)
}
}
-static void mux_chr_send_event(MuxFeChardev *d, unsigned int mux_nr,
- QEMUChrEvent event)
+static void mux_fe_chr_send_event(MuxFeChardev *d, unsigned int mux_nr,
+ QEMUChrEvent event)
{
CharBackend *be = d->backends[mux_nr];
@@ -128,12 +128,12 @@ static void mux_chr_send_event(MuxFeChardev *d, unsigned int mux_nr,
}
}
-static void mux_chr_be_event(Chardev *chr, QEMUChrEvent event)
+static void mux_fe_chr_be_event(Chardev *chr, QEMUChrEvent event)
{
MuxFeChardev *d = MUX_FE_CHARDEV(chr);
if (d->focus != -1) {
- mux_chr_send_event(d, d->focus, event);
+ mux_fe_chr_send_event(d, d->focus, event);
}
}
@@ -172,7 +172,7 @@ static int mux_proc_byte(Chardev *chr, MuxFeChardev *d, int ch)
if (bit >= MAX_MUX) {
bit = find_next_bit(&d->mux_bitset, MAX_MUX, 0);
}
- mux_set_focus(chr, bit);
+ mux_fe_chr_set_focus(chr, bit);
break;
} case 't':
d->timestamps = !d->timestamps;
@@ -189,7 +189,7 @@ static int mux_proc_byte(Chardev *chr, MuxFeChardev *d, int ch)
return 0;
}
-static void mux_chr_accept_input(Chardev *chr)
+static void mux_fe_chr_accept_input(Chardev *chr)
{
MuxFeChardev *d = MUX_FE_CHARDEV(chr);
int m = d->focus;
@@ -202,7 +202,7 @@ static void mux_chr_accept_input(Chardev *chr)
}
}
-static int mux_chr_can_read(void *opaque)
+static int mux_fe_chr_can_read(void *opaque)
{
MuxFeChardev *d = MUX_FE_CHARDEV(opaque);
int m = d->focus;
@@ -219,7 +219,7 @@ static int mux_chr_can_read(void *opaque)
return 0;
}
-static void mux_chr_read(void *opaque, const uint8_t *buf, int size)
+static void mux_fe_chr_read(void *opaque, const uint8_t *buf, int size)
{
Chardev *chr = CHARDEV(opaque);
MuxFeChardev *d = MUX_FE_CHARDEV(opaque);
@@ -227,7 +227,7 @@ static void mux_chr_read(void *opaque, const uint8_t *buf, int size)
CharBackend *be = d->backends[m];
int i;
- mux_chr_accept_input(opaque);
+ mux_fe_chr_accept_input(opaque);
for (i = 0; i < size; i++)
if (mux_proc_byte(chr, d, buf[i])) {
@@ -248,16 +248,16 @@ void mux_fe_chr_send_all_event(MuxFeChardev *d, QEMUChrEvent event)
/* Send the event to all registered listeners */
bit = -1;
while ((bit = find_next_bit(&d->mux_bitset, MAX_MUX, bit + 1)) < MAX_MUX) {
- mux_chr_send_event(d, bit, event);
+ mux_fe_chr_send_event(d, bit, event);
}
}
-static void mux_chr_event(void *opaque, QEMUChrEvent event)
+static void mux_fe_chr_event(void *opaque, QEMUChrEvent event)
{
mux_chr_send_all_event(CHARDEV(opaque), event);
}
-static GSource *mux_chr_add_watch(Chardev *s, GIOCondition cond)
+static GSource *mux_fe_chr_add_watch(Chardev *s, GIOCondition cond)
{
MuxFeChardev *d = MUX_FE_CHARDEV(s);
Chardev *chr = qemu_chr_fe_get_driver(&d->chr);
@@ -270,7 +270,7 @@ static GSource *mux_chr_add_watch(Chardev *s, GIOCondition cond)
return cc->chr_add_watch(chr, cond);
}
-static void char_mux_finalize(Object *obj)
+static void char_mux_fe_finalize(Object *obj)
{
MuxFeChardev *d = MUX_FE_CHARDEV(obj);
int bit;
@@ -285,22 +285,22 @@ static void char_mux_finalize(Object *obj)
qemu_chr_fe_deinit(&d->chr, false);
}
-static void mux_chr_update_read_handlers(Chardev *chr)
+static void mux_fe_chr_update_read_handlers(Chardev *chr)
{
MuxFeChardev *d = MUX_FE_CHARDEV(chr);
/* Fix up the real driver with mux routines */
qemu_chr_fe_set_handlers_full(&d->chr,
- mux_chr_can_read,
- mux_chr_read,
- mux_chr_event,
+ mux_fe_chr_can_read,
+ mux_fe_chr_read,
+ mux_fe_chr_event,
NULL,
chr,
chr->gcontext, true, false);
}
-bool mux_chr_attach_frontend(MuxFeChardev *d, CharBackend *b,
- unsigned int *tag, Error **errp)
+bool mux_fe_chr_attach_frontend(MuxFeChardev *d, CharBackend *b,
+ unsigned int *tag, Error **errp)
{
unsigned int bit;
@@ -320,7 +320,7 @@ bool mux_chr_attach_frontend(MuxFeChardev *d, CharBackend *b,
return true;
}
-bool mux_chr_detach_frontend(MuxFeChardev *d, unsigned int tag)
+bool mux_fe_chr_detach_frontend(MuxFeChardev *d, unsigned int tag)
{
unsigned int bit;
@@ -335,19 +335,19 @@ bool mux_chr_detach_frontend(MuxFeChardev *d, unsigned int tag)
return true;
}
-void mux_set_focus(Chardev *chr, unsigned int focus)
+void mux_fe_chr_set_focus(Chardev *chr, unsigned int focus)
{
MuxFeChardev *d = MUX_FE_CHARDEV(chr);
assert(find_next_bit(&d->mux_bitset, MAX_MUX, focus) == focus);
if (d->focus != -1) {
- mux_chr_send_event(d, d->focus, CHR_EVENT_MUX_OUT);
+ mux_fe_chr_send_event(d, d->focus, CHR_EVENT_MUX_OUT);
}
d->focus = focus;
chr->be = d->backends[focus];
- mux_chr_send_event(d, d->focus, CHR_EVENT_MUX_IN);
+ mux_fe_chr_send_event(d, d->focus, CHR_EVENT_MUX_IN);
}
static void qemu_chr_open_mux(Chardev *chr,
@@ -389,30 +389,30 @@ static void qemu_chr_parse_mux(QemuOpts *opts, ChardevBackend *backend,
mux->chardev = g_strdup(chardev);
}
-static void char_mux_class_init(ObjectClass *oc, void *data)
+static void char_mux_fe_class_init(ObjectClass *oc, void *data)
{
ChardevClass *cc = CHARDEV_CLASS(oc);
cc->parse = qemu_chr_parse_mux;
cc->open = qemu_chr_open_mux;
- cc->chr_write = mux_chr_write;
- cc->chr_accept_input = mux_chr_accept_input;
- cc->chr_add_watch = mux_chr_add_watch;
- cc->chr_be_event = mux_chr_be_event;
- cc->chr_update_read_handler = mux_chr_update_read_handlers;
+ cc->chr_write = mux_fe_chr_write;
+ cc->chr_accept_input = mux_fe_chr_accept_input;
+ cc->chr_add_watch = mux_fe_chr_add_watch;
+ cc->chr_be_event = mux_fe_chr_be_event;
+ cc->chr_update_read_handler = mux_fe_chr_update_read_handlers;
}
-static const TypeInfo char_mux_type_info = {
+static const TypeInfo char_mux_fe_type_info = {
.name = TYPE_CHARDEV_MUX_FE,
.parent = TYPE_CHARDEV,
- .class_init = char_mux_class_init,
+ .class_init = char_mux_fe_class_init,
.instance_size = sizeof(MuxFeChardev),
- .instance_finalize = char_mux_finalize,
+ .instance_finalize = char_mux_fe_finalize,
};
static void register_types(void)
{
- type_register_static(&char_mux_type_info);
+ type_register_static(&char_mux_fe_type_info);
}
type_init(register_types);
diff --git a/chardev/chardev-internal.h b/chardev/chardev-internal.h
index c874850a41ad..94c8d07ac235 100644
--- a/chardev/chardev-internal.h
+++ b/chardev/chardev-internal.h
@@ -59,14 +59,14 @@ DECLARE_INSTANCE_CHECKER(MuxFeChardev, MUX_FE_CHARDEV,
#define CHARDEV_IS_MUX_FE(chr) \
object_dynamic_cast(OBJECT(chr), TYPE_CHARDEV_MUX_FE)
-bool mux_chr_attach_frontend(MuxFeChardev *d, CharBackend *b,
- unsigned int *tag, Error **errp);
-bool mux_chr_detach_frontend(MuxFeChardev *d, unsigned int tag);
-void mux_set_focus(Chardev *chr, unsigned int focus);
void mux_chr_send_all_event(Chardev *chr, QEMUChrEvent event);
/* Mux type dependent calls */
+void mux_fe_chr_set_focus(Chardev *d, unsigned int focus);
void mux_fe_chr_send_all_event(MuxFeChardev *d, QEMUChrEvent event);
+bool mux_fe_chr_attach_frontend(MuxFeChardev *d, CharBackend *b,
+ unsigned int *tag, Error **errp);
+bool mux_fe_chr_detach_frontend(MuxFeChardev *d, unsigned int tag);
Object *get_chardevs_root(void);
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v4 5/8] chardev/char: introduce `mux-be-id=ID` option
2024-10-16 10:25 [PATCH v4 0/8] chardev: implement backend chardev multiplexing Roman Penyaev
` (3 preceding siblings ...)
2024-10-16 10:26 ` [PATCH v4 4/8] chardev/char: rename frontend mux calls Roman Penyaev
@ 2024-10-16 10:26 ` Roman Penyaev
2024-10-16 10:26 ` [PATCH v4 6/8] chardev/char-mux: implement backend chardev multiplexing Roman Penyaev
` (2 subsequent siblings)
7 siblings, 0 replies; 22+ messages in thread
From: Roman Penyaev @ 2024-10-16 10:26 UTC (permalink / raw)
Cc: Roman Penyaev, Marc-André Lureau, qemu-devel
Patch introduces `mux-be-id=ID` option for all chardev devices.
This is necessary to attach chardev to `mux-be` for backend
multiplexing. Actual implementation wimplementation will follow.
Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>
Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
Cc: qemu-devel@nongnu.org
---
chardev/char.c | 3 +++
qapi/char.json | 6 +++++-
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/chardev/char.c b/chardev/char.c
index d8dbdb6f84f1..cffe60860589 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -933,6 +933,9 @@ QemuOptsList qemu_chardev_opts = {
},{
.name = "mux",
.type = QEMU_OPT_BOOL,
+ },{
+ .name = "mux-be-id",
+ .type = QEMU_OPT_STRING,
},{
.name = "signal",
.type = QEMU_OPT_BOOL,
diff --git a/qapi/char.json b/qapi/char.json
index e04535435034..fb0dedb24383 100644
--- a/qapi/char.json
+++ b/qapi/char.json
@@ -199,11 +199,15 @@
# @logappend: true to append instead of truncate (default to false to
# truncate)
#
+# @mux-be-id: id of the mux-be device for backend multiplexing
+# (since: 9.2)
+#
# Since: 2.6
##
{ 'struct': 'ChardevCommon',
'data': { '*logfile': 'str',
- '*logappend': 'bool' } }
+ '*logappend': 'bool',
+ '*mux-be-id': 'str' } }
##
# @ChardevFile:
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v4 6/8] chardev/char-mux: implement backend chardev multiplexing
2024-10-16 10:25 [PATCH v4 0/8] chardev: implement backend chardev multiplexing Roman Penyaev
` (4 preceding siblings ...)
2024-10-16 10:26 ` [PATCH v4 5/8] chardev/char: introduce `mux-be-id=ID` option Roman Penyaev
@ 2024-10-16 10:26 ` Roman Penyaev
2024-10-16 11:13 ` Marc-André Lureau
2024-10-16 10:26 ` [PATCH v4 7/8] tests/unit/test-char: add unit test for the `mux-be` multiplexer Roman Penyaev
2024-10-16 10:26 ` [PATCH v4 8/8] qemu-options.hx: describe multiplexing of several backend devices Roman Penyaev
7 siblings, 1 reply; 22+ messages in thread
From: Roman Penyaev @ 2024-10-16 10:26 UTC (permalink / raw)
Cc: Roman Penyaev, Marc-André Lureau, qemu-devel
This patch implements multiplexing capability of several backend
devices, which opens up an opportunity to use a single frontend
device on the guest, which can be manipulated from several
backend devices.
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.
Patch implements another multiplexer type `mux-be`. The following
is QEMU command line example:
-chardev mux-be,id=mux0 \
-chardev socket,path=/tmp/sock,server=on,wait=off,id=sock0,mux-be-id=mux0 \
-chardev vc,id=vc0,mux-be-id=mux0 \
-device virtconsole,chardev=mux0 \
-vnc 0.0.0.0:0
Which creates 2 backend devices: text virtual console (`vc0`) and a
socket (`sock0`) connected to the single virtio hvc console with the
backend multiplexer (`mux0`) help. `vc0` renders text to an image,
which can be shared over the VNC protocol. `sock0` is a socket
backend which provides biderectional communication to the virtio hvc
console.
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-mux-be.c | 290 +++++++++++++++++++++++++++++++++++++
chardev/char.c | 56 +++++--
chardev/chardev-internal.h | 34 ++++-
chardev/meson.build | 1 +
include/chardev/char.h | 1 +
qapi/char.json | 25 ++++
7 files changed, 403 insertions(+), 13 deletions(-)
create mode 100644 chardev/char-mux-be.c
diff --git a/chardev/char-fe.c b/chardev/char-fe.c
index a2b5bff39fd9..2f794674563b 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_fe_chr_attach_frontend(d, b, &tag, errp)) {
return false;
}
+ } else if (CHARDEV_IS_MUX_BE(s)) {
+ MuxBeChardev *d = MUX_BE_CHARDEV(s);
+
+ if (!mux_be_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_FE(b->chr)) {
MuxFeChardev *d = MUX_FE_CHARDEV(b->chr);
mux_fe_chr_detach_frontend(d, b->tag);
+ } else if (CHARDEV_IS_MUX_BE(b->chr)) {
+ MuxBeChardev *d = MUX_BE_CHARDEV(b->chr);
+ mux_be_chr_detach_frontend(d);
}
if (del) {
Object *obj = OBJECT(b->chr);
diff --git a/chardev/char-mux-be.c b/chardev/char-mux-be.c
new file mode 100644
index 000000000000..64a4f2c00034
--- /dev/null
+++ b/chardev/char-mux-be.c
@@ -0,0 +1,290 @@
+/*
+ * QEMU Character Backend Multiplexer
+ *
+ * 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/module.h"
+#include "qemu/option.h"
+#include "qemu/cutils.h"
+#include "chardev/char.h"
+#include "sysemu/block-backend.h"
+#include "qapi/qapi-commands-control.h"
+#include "qapi/clone-visitor.h"
+#include "qapi/qapi-builtin-visit.h"
+#include "chardev-internal.h"
+
+/*
+ * MUX-BE driver for multiplexing 1 frontend device with N 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 mux_be_chr_write_to_all(MuxBeChardev *d, const uint8_t *buf, int len)
+{
+ int r, i, ret = len;
+ unsigned int written;
+
+ for (i = 0; i < d->be_cnt; i++) {
+ 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], buf, len);
+ if (r < 0 && errno == EAGAIN) {
+ /*
+ * Fail immediately if write would block. Expect to be called
+ * soon on watch wake up.
+ */
+ 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.
+ */
+ 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 mux_be_chr_write(Chardev *chr, const uint8_t *buf, int len)
+{
+ MuxBeChardev *d = MUX_BE_CHARDEV(chr);
+ return mux_be_chr_write_to_all(d, buf, len);
+}
+
+static void mux_be_chr_send_event(MuxBeChardev *d, QEMUChrEvent event)
+{
+ CharBackend *fe = d->frontend;
+
+ if (fe && fe->chr_event) {
+ fe->chr_event(fe->opaque, event);
+ }
+}
+
+static void mux_be_chr_be_event(Chardev *chr, QEMUChrEvent event)
+{
+ MuxBeChardev *d = MUX_BE_CHARDEV(chr);
+
+ mux_be_chr_send_event(d, event);
+}
+
+static int mux_be_chr_can_read(void *opaque)
+{
+ MuxBeChardev *d = MUX_BE_CHARDEV(opaque);
+ CharBackend *fe = d->frontend;
+
+ if (fe && fe->chr_can_read) {
+ return fe->chr_can_read(fe->opaque);
+ }
+
+ return 0;
+}
+
+static void mux_be_chr_read(void *opaque, const uint8_t *buf, int size)
+{
+ MuxBeChardev *d = MUX_BE_CHARDEV(opaque);
+ CharBackend *fe = d->frontend;
+
+ if (fe && fe->chr_read) {
+ fe->chr_read(fe->opaque, buf, size);
+ }
+}
+
+void mux_be_chr_send_all_event(MuxBeChardev *d, QEMUChrEvent event)
+{
+ mux_be_chr_send_event(d, event);
+}
+
+static void mux_be_chr_event(void *opaque, QEMUChrEvent event)
+{
+ mux_chr_send_all_event(CHARDEV(opaque), event);
+}
+
+static GSource *mux_be_chr_add_watch(Chardev *s, GIOCondition cond)
+{
+ MuxBeChardev *d = MUX_BE_CHARDEV(s);
+ Chardev *chr;
+ ChardevClass *cc;
+ unsigned int written;
+ int i;
+
+ for (i = 0; i < d->be_cnt; i++) {
+ written = d->be_written[i] - d->be_min_written;
+ if (written) {
+ /* We skip the device with already written buffer */
+ continue;
+ }
+
+ /*
+ * The first device that has no data written to it must be
+ * the device that recently returned EAGAIN and should be
+ * watched.
+ */
+
+ chr = qemu_chr_fe_get_driver(&d->backends[i]);
+ cc = CHARDEV_GET_CLASS(chr);
+
+ if (!cc->chr_add_watch) {
+ return NULL;
+ }
+
+ return cc->chr_add_watch(chr, cond);
+ }
+
+ return NULL;
+}
+
+bool mux_be_chr_attach_chardev(MuxBeChardev *d, Chardev *chr, Error **errp)
+{
+ bool ret;
+
+ if (d->be_cnt >= MAX_MUX) {
+ error_setg(errp, "too many uses of multiplexed chardev '%s'"
+ " (maximum is " stringify(MAX_MUX) ")",
+ d->parent.label);
+ return false;
+ }
+ ret = qemu_chr_fe_init(&d->backends[d->be_cnt], chr, errp);
+ if (ret) {
+ /* Catch up with what was already written */
+ d->be_written[d->be_cnt] = d->be_min_written;
+ d->be_cnt += 1;
+ }
+
+ return ret;
+}
+
+static void char_mux_be_finalize(Object *obj)
+{
+ MuxBeChardev *d = MUX_BE_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], false);
+ }
+}
+
+static void mux_be_chr_update_read_handlers(Chardev *chr)
+{
+ MuxBeChardev *d = MUX_BE_CHARDEV(chr);
+ int i;
+
+ for (i = 0; i < d->be_cnt; i++) {
+ /* Fix up the real driver with mux routines */
+ qemu_chr_fe_set_handlers_full(&d->backends[i],
+ mux_be_chr_can_read,
+ mux_be_chr_read,
+ mux_be_chr_event,
+ NULL,
+ chr,
+ chr->gcontext, true, false);
+ }
+}
+
+bool mux_be_chr_attach_frontend(MuxBeChardev *d, CharBackend *b, Error **errp)
+{
+ if (d->frontend) {
+ error_setg(errp,
+ "multiplexed chardev '%s' is already used "
+ "for multiplexing", d->parent.label);
+ return false;
+ }
+ d->frontend = b;
+
+ return true;
+}
+
+void mux_be_chr_detach_frontend(MuxBeChardev *d)
+{
+ d->frontend = NULL;
+}
+
+static void qemu_chr_open_mux_be(Chardev *chr,
+ ChardevBackend *backend,
+ bool *be_opened,
+ Error **errp)
+{
+ /*
+ * Only default to opened state if we've realized the initial
+ * set of muxes
+ */
+ *be_opened = mux_is_opened();
+}
+
+static void qemu_chr_parse_mux_be(QemuOpts *opts, ChardevBackend *backend,
+ Error **errp)
+{
+ ChardevMuxBe *mux;
+
+ backend->type = CHARDEV_BACKEND_KIND_MUX_BE;
+ mux = backend->u.mux_be.data = g_new0(ChardevMuxBe, 1);
+ qemu_chr_parse_common(opts, qapi_ChardevMuxBe_base(mux));
+}
+
+static void char_mux_be_class_init(ObjectClass *oc, void *data)
+{
+ ChardevClass *cc = CHARDEV_CLASS(oc);
+
+ cc->parse = qemu_chr_parse_mux_be;
+ cc->open = qemu_chr_open_mux_be;
+ cc->chr_write = mux_be_chr_write;
+ cc->chr_add_watch = mux_be_chr_add_watch;
+ cc->chr_be_event = mux_be_chr_be_event;
+ cc->chr_update_read_handler = mux_be_chr_update_read_handlers;
+}
+
+static const TypeInfo char_mux_be_type_info = {
+ .name = TYPE_CHARDEV_MUX_BE,
+ .parent = TYPE_CHARDEV,
+ .class_init = char_mux_be_class_init,
+ .instance_size = sizeof(MuxBeChardev),
+ .instance_finalize = char_mux_be_finalize,
+};
+
+static void register_types(void)
+{
+ type_register_static(&char_mux_be_type_info);
+}
+
+type_init(register_types);
diff --git a/chardev/char.c b/chardev/char.c
index cffe60860589..58fa8ac70a1e 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -341,6 +341,9 @@ static bool qemu_chr_is_busy(Chardev *s)
if (CHARDEV_IS_MUX_FE(s)) {
MuxFeChardev *d = MUX_FE_CHARDEV(s);
return d->mux_bitset != 0;
+ } else if (CHARDEV_IS_MUX_BE(s)) {
+ MuxBeChardev *d = MUX_BE_CHARDEV(s);
+ return d->frontend != NULL;
} else {
return s->be != NULL;
}
@@ -648,7 +651,8 @@ static Chardev *__qemu_chr_new_from_opts(QemuOpts *opts, GMainContext *context,
ChardevBackend *backend = NULL;
const char *name = qemu_opt_get(opts, "backend");
const char *id = qemu_opts_id(opts);
- char *bid = NULL;
+ const char *mux_be_id = NULL;
+ char *mux_fe_id = NULL;
if (name && is_help_option(name)) {
GString *str = g_string_new("");
@@ -676,10 +680,16 @@ static Chardev *__qemu_chr_new_from_opts(QemuOpts *opts, GMainContext *context,
}
if (qemu_opt_get_bool(opts, "mux", 0)) {
- bid = g_strdup_printf("%s-base", id);
+ mux_fe_id = g_strdup_printf("%s-base", id);
+ }
+ mux_be_id = qemu_opt_get(opts, "mux-be-id");
+ if (mux_be_id && mux_fe_id) {
+ error_setg(errp, "chardev: mux and mux-be can't be used for the same "
+ "device");
+ goto out;
}
- chr = qemu_chardev_new(bid ? bid : id,
+ chr = qemu_chardev_new(mux_fe_id ? mux_fe_id : id,
object_class_get_name(OBJECT_CLASS(cc)),
backend, context, errp);
if (chr == NULL) {
@@ -687,25 +697,40 @@ static Chardev *__qemu_chr_new_from_opts(QemuOpts *opts, GMainContext *context,
}
base = chr;
- if (bid) {
+ if (mux_fe_id) {
Chardev *mux;
qapi_free_ChardevBackend(backend);
backend = g_new0(ChardevBackend, 1);
backend->type = CHARDEV_BACKEND_KIND_MUX;
backend->u.mux.data = g_new0(ChardevMux, 1);
- backend->u.mux.data->chardev = g_strdup(bid);
+ backend->u.mux.data->chardev = g_strdup(mux_fe_id);
mux = qemu_chardev_new(id, TYPE_CHARDEV_MUX_FE, backend, context, errp);
if (mux == NULL) {
- object_unparent(OBJECT(chr));
- chr = NULL;
- goto out;
+ goto unparent_and_out;
}
chr = mux;
+ } else if (mux_be_id) {
+ Chardev *s;
+
+ s = qemu_chr_find(mux_be_id);
+ if (!s) {
+ error_setg(errp, "chardev: mux-be device can't be found by id '%s'",
+ mux_be_id);
+ goto unparent_and_out;
+ }
+ if (!CHARDEV_IS_MUX_BE(s)) {
+ error_setg(errp, "chardev: device '%s' is not a multiplexer device"
+ " of 'mux-be' type", mux_be_id);
+ goto unparent_and_out;
+ }
+ if (!mux_be_chr_attach_chardev(MUX_BE_CHARDEV(s), chr, errp)) {
+ goto unparent_and_out;
+ }
}
out:
qapi_free_ChardevBackend(backend);
- g_free(bid);
+ g_free(mux_fe_id);
if (replay && base) {
/* RR should be set on the base device, not the mux */
@@ -713,6 +738,11 @@ out:
}
return chr;
+
+unparent_and_out:
+ object_unparent(OBJECT(chr));
+ chr = NULL;
+ goto out;
}
Chardev *qemu_chr_new_from_opts(QemuOpts *opts, GMainContext *context,
@@ -1114,7 +1144,7 @@ ChardevReturn *qmp_chardev_change(const char *id, ChardevBackend *backend,
return NULL;
}
- if (CHARDEV_IS_MUX_FE(chr)) {
+ if (CHARDEV_IS_MUX_FE(chr) || CHARDEV_IS_MUX_BE(chr)) {
error_setg(errp, "Mux device hotswap not supported yet");
return NULL;
}
@@ -1302,7 +1332,7 @@ static int chardev_options_parsed_cb(Object *child, void *opaque)
{
Chardev *chr = (Chardev *)child;
- if (!chr->be_open && CHARDEV_IS_MUX_FE(chr)) {
+ if (!chr->be_open && (CHARDEV_IS_MUX_FE(chr) || CHARDEV_IS_MUX_BE(chr))) {
open_muxes(chr);
}
@@ -1329,8 +1359,10 @@ void mux_chr_send_all_event(Chardev *chr, QEMUChrEvent event)
if (CHARDEV_IS_MUX_FE(chr)) {
MuxFeChardev *d = MUX_FE_CHARDEV(chr);
-
mux_fe_chr_send_all_event(d, event);
+ } else if (CHARDEV_IS_MUX_BE(chr)) {
+ MuxBeChardev *d = MUX_BE_CHARDEV(chr);
+ mux_be_chr_send_all_event(d, event);
}
}
diff --git a/chardev/chardev-internal.h b/chardev/chardev-internal.h
index 94c8d07ac235..8ea1258f8ff4 100644
--- a/chardev/chardev-internal.h
+++ b/chardev/chardev-internal.h
@@ -35,7 +35,9 @@
struct MuxFeChardev {
Chardev parent;
+ /* Linked frontends */
CharBackend *backends[MAX_MUX];
+ /* Linked backend */
CharBackend chr;
unsigned long mux_bitset;
int focus;
@@ -54,10 +56,36 @@ struct MuxFeChardev {
};
typedef struct MuxFeChardev MuxFeChardev;
+struct MuxBeChardev {
+ Chardev parent;
+ /* Linked frontend */
+ CharBackend *frontend;
+ /* Linked backends */
+ CharBackend backends[MAX_MUX];
+ /*
+ * Number of backends attached to this mux. Once attached, a
+ * backend can't be detached, so the counter is only increasing.
+ * To safely remove a backend, mux has to be removed first.
+ */
+ unsigned int be_cnt;
+ /*
+ * Counters of written bytes from a single frontend device
+ * to multiple backend devices.
+ */
+ unsigned int be_written[MAX_MUX];
+ unsigned int be_min_written;
+};
+typedef struct MuxBeChardev MuxBeChardev;
+
DECLARE_INSTANCE_CHECKER(MuxFeChardev, MUX_FE_CHARDEV,
TYPE_CHARDEV_MUX_FE)
-#define CHARDEV_IS_MUX_FE(chr) \
+DECLARE_INSTANCE_CHECKER(MuxBeChardev, MUX_BE_CHARDEV,
+ TYPE_CHARDEV_MUX_BE)
+
+#define CHARDEV_IS_MUX_FE(chr) \
object_dynamic_cast(OBJECT(chr), TYPE_CHARDEV_MUX_FE)
+#define CHARDEV_IS_MUX_BE(chr) \
+ object_dynamic_cast(OBJECT(chr), TYPE_CHARDEV_MUX_BE)
void mux_chr_send_all_event(Chardev *chr, QEMUChrEvent event);
@@ -67,6 +95,10 @@ void mux_fe_chr_send_all_event(MuxFeChardev *d, QEMUChrEvent event);
bool mux_fe_chr_attach_frontend(MuxFeChardev *d, CharBackend *b,
unsigned int *tag, Error **errp);
bool mux_fe_chr_detach_frontend(MuxFeChardev *d, unsigned int tag);
+void mux_be_chr_send_all_event(MuxBeChardev *d, QEMUChrEvent event);
+bool mux_be_chr_attach_chardev(MuxBeChardev *d, Chardev *chr, Error **errp);
+bool mux_be_chr_attach_frontend(MuxBeChardev *d, CharBackend *b, Error **errp);
+void mux_be_chr_detach_frontend(MuxBeChardev *d);
Object *get_chardevs_root(void);
diff --git a/chardev/meson.build b/chardev/meson.build
index 778444a00ca6..3a9f5565372b 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-fe.c',
+ 'char-mux-be.c',
'char-null.c',
'char-pipe.c',
'char-ringbuf.c',
diff --git a/include/chardev/char.h b/include/chardev/char.h
index 0bec974f9d73..c58c11c4eeaf 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_FE "chardev-mux"
+#define TYPE_CHARDEV_MUX_BE "chardev-mux-be"
#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 fb0dedb24383..cdec8f9cf4e2 100644
--- a/qapi/char.json
+++ b/qapi/char.json
@@ -336,6 +336,17 @@
'data': { 'chardev': 'str' },
'base': 'ChardevCommon' }
+##
+# @ChardevMuxBe:
+#
+# Configuration info for mux-be chardevs.
+#
+# Since: 9.2
+##
+{ 'struct': 'ChardevMuxBe',
+ 'data': { },
+ 'base': 'ChardevCommon' }
+
##
# @ChardevStdio:
#
@@ -483,6 +494,8 @@
#
# @mux: (since 1.5)
#
+# @mux-be: (since 9.2)
+#
# @msmouse: emulated Microsoft serial mouse (since 1.5)
#
# @wctablet: emulated Wacom Penpartner serial tablet (since 2.9)
@@ -525,6 +538,7 @@
'pty',
'null',
'mux',
+ 'mux-be',
'msmouse',
'wctablet',
{ 'name': 'braille', 'if': 'CONFIG_BRLAPI' },
@@ -599,6 +613,16 @@
{ 'struct': 'ChardevMuxWrapper',
'data': { 'data': 'ChardevMux' } }
+##
+# @ChardevMuxBeWrapper:
+#
+# @data: Configuration info for mux-be chardevs
+#
+# Since: 9.2
+##
+{ 'struct': 'ChardevMuxBeWrapper',
+ 'data': { 'data': 'ChardevMuxBe' } }
+
##
# @ChardevStdioWrapper:
#
@@ -707,6 +731,7 @@
'pty': 'ChardevPtyWrapper',
'null': 'ChardevCommonWrapper',
'mux': 'ChardevMuxWrapper',
+ 'mux-be': 'ChardevMuxBeWrapper',
'msmouse': 'ChardevCommonWrapper',
'wctablet': 'ChardevCommonWrapper',
'braille': { 'type': 'ChardevCommonWrapper',
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v4 7/8] tests/unit/test-char: add unit test for the `mux-be` multiplexer
2024-10-16 10:25 [PATCH v4 0/8] chardev: implement backend chardev multiplexing Roman Penyaev
` (5 preceding siblings ...)
2024-10-16 10:26 ` [PATCH v4 6/8] chardev/char-mux: implement backend chardev multiplexing Roman Penyaev
@ 2024-10-16 10:26 ` Roman Penyaev
2024-10-16 11:36 ` Marc-André Lureau
2024-10-16 10:26 ` [PATCH v4 8/8] qemu-options.hx: describe multiplexing of several backend devices Roman Penyaev
7 siblings, 1 reply; 22+ messages in thread
From: Roman Penyaev @ 2024-10-16 10:26 UTC (permalink / raw)
Cc: Roman Penyaev, Marc-André Lureau, qemu-devel
The test is trivial: several backends, 1 `mux-be`, 1 frontend
do the buffer write and read. Pipe is used for EAGAIN verification.
Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>
Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
Cc: qemu-devel@nongnu.org
---
tests/unit/test-char.c | 306 ++++++++++++++++++++++++++++++++++++++++-
1 file changed, 304 insertions(+), 2 deletions(-)
diff --git a/tests/unit/test-char.c b/tests/unit/test-char.c
index a1c6bb874c8e..3eb0692b199f 100644
--- a/tests/unit/test-char.c
+++ b/tests/unit/test-char.c
@@ -178,7 +178,7 @@ static void char_ringbuf_test(void)
qemu_opts_del(opts);
}
-static void char_mux_test(void)
+static void char_mux_fe_test(void)
{
QemuOpts *opts;
Chardev *chr, *base;
@@ -359,6 +359,307 @@ static void char_mux_test(void)
qmp_chardev_remove("mux-label", &error_abort);
}
+static void char_mux_be_test(void)
+{
+ QemuOpts *opts;
+ Chardev *mux_be, *chr1, *chr2, *base;
+ char *data;
+ FeHandler h = { 0, false, 0, false, };
+ Error *error = NULL;
+ CharBackend chr_be;
+ int ret, i;
+
+#define RB_SIZE 128
+
+ /* Create mux-be */
+ opts = qemu_opts_create(qemu_find_opts("chardev"), "mux0",
+ 1, &error_abort);
+ qemu_opt_set(opts, "backend", "mux-be", &error_abort);
+ mux_be = qemu_chr_new_from_opts(opts, NULL, &error_abort);
+ g_assert_nonnull(mux_be);
+ qemu_opts_del(opts);
+
+ /* Check maximum allowed backends */
+ for (i = 0; true; i++) {
+ char name[8];
+
+ snprintf(name, sizeof(name), "chr%d", i);
+ opts = qemu_opts_create(qemu_find_opts("chardev"), name,
+ 1, &error_abort);
+ qemu_opt_set(opts, "backend", "ringbuf", &error_abort);
+ qemu_opt_set(opts, "size", stringify(RB_SIZE), &error_abort);
+ qemu_opt_set(opts, "mux-be-id", "mux0", &error_abort);
+ base = qemu_chr_new_from_opts(opts, NULL, &error);
+ if (error) {
+ const char *err_fmt =
+ "too many uses of multiplexed chardev 'mux0' (maximum is %u)";
+ unsigned n;
+
+ ret = sscanf(error_get_pretty(error), err_fmt, &n);
+ error_free(error);
+ error = NULL;
+ g_assert_cmpint(ret, ==, 1);
+ g_assert_cmpint(i, ==, n);
+ break;
+ }
+ g_assert_nonnull(base);
+ qemu_opts_del(opts);
+ }
+ /* Finalize mux0 */
+ qmp_chardev_remove("mux0", &error_abort);
+
+ /* Finalize all backends */
+ while (i--) {
+ char name[8];
+ snprintf(name, sizeof(name), "chr%d", i);
+ qmp_chardev_remove(name, &error_abort);
+ }
+
+ /* Create mux-be */
+ opts = qemu_opts_create(qemu_find_opts("chardev"), "mux0",
+ 1, &error_abort);
+ qemu_opt_set(opts, "backend", "mux-be", &error_abort);
+ mux_be = qemu_chr_new_from_opts(opts, NULL, &error_abort);
+ g_assert_nonnull(mux_be);
+ qemu_opts_del(opts);
+
+ /* Create chardev which fails */
+ opts = qemu_opts_create(qemu_find_opts("chardev"), "chr1",
+ 1, &error_abort);
+ qemu_opt_set(opts, "backend", "ringbuf", &error_abort);
+ qemu_opt_set(opts, "size", stringify(RB_SIZE), &error_abort);
+ qemu_opt_set(opts, "mux-be-id", "mux0", &error_abort);
+ qemu_opt_set(opts, "mux", "on", &error_abort);
+ chr1 = qemu_chr_new_from_opts(opts, NULL, &error);
+ g_assert_cmpstr(error_get_pretty(error), ==, "chardev: mux and mux-be "
+ "can't be used for the same device");
+ error_free(error);
+ error = NULL;
+ qemu_opts_del(opts);
+
+ /* Create first chardev */
+ opts = qemu_opts_create(qemu_find_opts("chardev"), "chr1",
+ 1, &error_abort);
+ qemu_opt_set(opts, "backend", "ringbuf", &error_abort);
+ qemu_opt_set(opts, "size", stringify(RB_SIZE), &error_abort);
+ qemu_opt_set(opts, "mux-be-id", "mux0", &error_abort);
+ chr1 = qemu_chr_new_from_opts(opts, NULL, &error_abort);
+ g_assert_nonnull(chr1);
+ qemu_opts_del(opts);
+
+ /* Create second chardev */
+ opts = qemu_opts_create(qemu_find_opts("chardev"), "chr2",
+ 1, &error_abort);
+ qemu_opt_set(opts, "backend", "ringbuf", &error_abort);
+ qemu_opt_set(opts, "size", stringify(RB_SIZE), &error_abort);
+ qemu_opt_set(opts, "mux-be-id", "mux0", &error_abort);
+ chr2 = qemu_chr_new_from_opts(opts, NULL, &error_abort);
+ g_assert_nonnull(chr2);
+ qemu_opts_del(opts);
+
+ /* Attach mux-be to a frontend */
+ qemu_chr_fe_init(&chr_be, mux_be, &error_abort);
+ qemu_chr_fe_set_handlers(&chr_be,
+ fe_can_read,
+ fe_read,
+ fe_event,
+ NULL,
+ &h,
+ NULL, true);
+
+ /* Fails second time */
+ qemu_chr_fe_init(&chr_be, mux_be, &error);
+ g_assert_cmpstr(error_get_pretty(error), ==, "multiplexed chardev 'mux0' "
+ "is already used for multiplexing");
+ error_free(error);
+ error = NULL;
+
+ /* Write to backend, chr1 */
+ base = qemu_chr_find("chr1");
+ g_assert_cmpint(qemu_chr_be_can_write(base), !=, 0);
+
+ qemu_chr_be_write(base, (void *)"hello", 6);
+ g_assert_cmpint(h.read_count, ==, 6);
+ g_assert_cmpstr(h.read_buf, ==, "hello");
+ h.read_count = 0;
+
+ /* Write to backend, chr2 */
+ base = qemu_chr_find("chr2");
+ g_assert_cmpint(qemu_chr_be_can_write(base), !=, 0);
+
+ qemu_chr_be_write(base, (void *)"olleh", 6);
+ g_assert_cmpint(h.read_count, ==, 6);
+ g_assert_cmpstr(h.read_buf, ==, "olleh");
+ h.read_count = 0;
+
+ /* Write to frontend, chr_be */
+ ret = qemu_chr_fe_write(&chr_be, (void *)"heyhey", 6);
+ g_assert_cmpint(ret, ==, 6);
+
+ data = qmp_ringbuf_read("chr1", RB_SIZE, false, 0, &error_abort);
+ g_assert_cmpint(strlen(data), ==, 6);
+ g_assert_cmpstr(data, ==, "heyhey");
+ g_free(data);
+
+ data = qmp_ringbuf_read("chr2", RB_SIZE, false, 0, &error_abort);
+ g_assert_cmpint(strlen(data), ==, 6);
+ g_assert_cmpstr(data, ==, "heyhey");
+ g_free(data);
+
+
+#ifndef _WIN32
+ /*
+ * Create third chardev to simulate EAGAIN and watcher.
+ * Mainly copied from char_pipe_test().
+ */
+ {
+ gchar *tmp_path = g_dir_make_tmp("qemu-test-char.XXXXXX", NULL);
+ gchar *in, *out, *pipe = g_build_filename(tmp_path, "pipe", NULL);
+ Chardev *chr3;
+ int fd, len;
+ char buf[128];
+
+ in = g_strdup_printf("%s.in", pipe);
+ if (mkfifo(in, 0600) < 0) {
+ abort();
+ }
+ out = g_strdup_printf("%s.out", pipe);
+ if (mkfifo(out, 0600) < 0) {
+ abort();
+ }
+
+ opts = qemu_opts_create(qemu_find_opts("chardev"), "chr3",
+ 1, &error_abort);
+ qemu_opt_set(opts, "backend", "pipe", &error_abort);
+ qemu_opt_set(opts, "path", pipe, &error_abort);
+ qemu_opt_set(opts, "mux-be-id", "mux0", &error_abort);
+ chr3 = qemu_chr_new_from_opts(opts, NULL, &error_abort);
+ g_assert_nonnull(chr3);
+
+ /* Write to frontend, chr_be */
+ ret = qemu_chr_fe_write(&chr_be, (void *)"thisis", 6);
+ g_assert_cmpint(ret, ==, 6);
+
+ data = qmp_ringbuf_read("chr1", RB_SIZE, false, 0, &error_abort);
+ g_assert_cmpint(strlen(data), ==, 6);
+ g_assert_cmpstr(data, ==, "thisis");
+ g_free(data);
+
+ data = qmp_ringbuf_read("chr2", RB_SIZE, false, 0, &error_abort);
+ g_assert_cmpint(strlen(data), ==, 6);
+ g_assert_cmpstr(data, ==, "thisis");
+ g_free(data);
+
+ fd = open(out, O_RDWR);
+ ret = read(fd, buf, sizeof(buf));
+ g_assert_cmpint(ret, ==, 6);
+ buf[ret] = 0;
+ g_assert_cmpstr(buf, ==, "thisis");
+ close(fd);
+
+ /* Add watch. 0 indicates no watches if nothing to wait for */
+ ret = qemu_chr_fe_add_watch(&chr_be, G_IO_OUT | G_IO_HUP,
+ NULL, NULL);
+ g_assert_cmpint(ret, ==, 0);
+
+ /*
+ * Write to frontend, chr_be, until EAGAIN. Make sure length is
+ * power of two to fit nicely the whole pipe buffer.
+ */
+ len = 0;
+ while ((ret = qemu_chr_fe_write(&chr_be, (void *)"thisisit", 8))
+ != -1) {
+ len += ret;
+ }
+ g_assert_cmpint(errno, ==, EAGAIN);
+
+ /* Further all writes should cause EAGAIN */
+ ret = qemu_chr_fe_write(&chr_be, (void *)"b", 1);
+ g_assert_cmpint(ret, ==, -1);
+ g_assert_cmpint(errno, ==, EAGAIN);
+
+ /*
+ * Add watch. Non 0 indicates we have a blocked chardev, which
+ * can wakes us up when write is possible.
+ */
+ ret = qemu_chr_fe_add_watch(&chr_be, G_IO_OUT | G_IO_HUP,
+ NULL, NULL);
+ g_assert_cmpint(ret, !=, 0);
+ g_source_remove(ret);
+
+ /* Drain pipe and ring buffers */
+ fd = open(out, O_RDWR);
+ while ((ret = read(fd, buf, MIN(sizeof(buf), len))) != -1 && len > 0) {
+ len -= ret;
+ }
+ close(fd);
+
+ data = qmp_ringbuf_read("chr1", RB_SIZE, false, 0, &error_abort);
+ g_assert_cmpint(strlen(data), ==, 128);
+ g_free(data);
+
+ data = qmp_ringbuf_read("chr2", RB_SIZE, false, 0, &error_abort);
+ g_assert_cmpint(strlen(data), ==, 128);
+ g_free(data);
+
+ /*
+ * Now we are good to go, first repeat "lost" sequence, which
+ * was already consumed and drained by the ring buffers, but
+ * pipe have not recieved that yet.
+ */
+ ret = qemu_chr_fe_write(&chr_be, (void *)"thisisit", 8);
+ g_assert_cmpint(ret, ==, 8);
+
+ ret = qemu_chr_fe_write(&chr_be, (void *)"streamisrestored", 16);
+ g_assert_cmpint(ret, ==, 16);
+
+ data = qmp_ringbuf_read("chr1", RB_SIZE, false, 0, &error_abort);
+ g_assert_cmpint(strlen(data), ==, 16);
+ /* Only last 16 bytes, see big comment above */
+ g_assert_cmpstr(data, ==, "streamisrestored");
+ g_free(data);
+
+ data = qmp_ringbuf_read("chr2", RB_SIZE, false, 0, &error_abort);
+ g_assert_cmpint(strlen(data), ==, 16);
+ /* Only last 16 bytes, see big comment above */
+ g_assert_cmpstr(data, ==, "streamisrestored");
+ g_free(data);
+
+ fd = open(out, O_RDWR);
+ ret = read(fd, buf, sizeof(buf));
+ g_assert_cmpint(ret, ==, 24);
+ buf[ret] = 0;
+ /* Both 8 and 16 bytes */
+ g_assert_cmpstr(buf, ==, "thisisitstreamisrestored");
+ close(fd);
+ }
+#endif
+
+ /* Can't be removed, depends on mux0 */
+ qmp_chardev_remove("chr1", &error);
+ g_assert_cmpstr(error_get_pretty(error), ==, "Chardev 'chr1' is busy");
+ error_free(error);
+ error = NULL;
+
+ /* Can't be removed, depends on frontend chr_be */
+ qmp_chardev_remove("mux0", &error);
+ g_assert_cmpstr(error_get_pretty(error), ==, "Chardev 'mux0' is busy");
+ error_free(error);
+ error = NULL;
+
+ /* Finalize frontend */
+ qemu_chr_fe_deinit(&chr_be, false);
+
+ /* Finalize mux0 */
+ qmp_chardev_remove("mux0", &error_abort);
+
+ /* Finalize backend chardevs */
+ qmp_chardev_remove("chr1", &error_abort);
+ qmp_chardev_remove("chr2", &error_abort);
+#ifndef _WIN32
+ qmp_chardev_remove("chr3", &error_abort);
+#endif
+}
static void websock_server_read(void *opaque, const uint8_t *buf, int size)
{
@@ -1506,7 +1807,8 @@ int main(int argc, char **argv)
g_test_add_func("/char/null", char_null_test);
g_test_add_func("/char/invalid", char_invalid_test);
g_test_add_func("/char/ringbuf", char_ringbuf_test);
- g_test_add_func("/char/mux", char_mux_test);
+ g_test_add_func("/char/mux", char_mux_fe_test);
+ g_test_add_func("/char/mux-be", char_mux_be_test);
#ifdef _WIN32
g_test_add_func("/char/console/subprocess", char_console_test_subprocess);
g_test_add_func("/char/console", char_console_test);
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v4 8/8] qemu-options.hx: describe multiplexing of several backend devices
2024-10-16 10:25 [PATCH v4 0/8] chardev: implement backend chardev multiplexing Roman Penyaev
` (6 preceding siblings ...)
2024-10-16 10:26 ` [PATCH v4 7/8] tests/unit/test-char: add unit test for the `mux-be` multiplexer Roman Penyaev
@ 2024-10-16 10:26 ` Roman Penyaev
2024-10-16 11:27 ` Marc-André Lureau
7 siblings, 1 reply; 22+ messages in thread
From: Roman Penyaev @ 2024-10-16 10:26 UTC (permalink / raw)
Cc: Roman Penyaev, Marc-André Lureau, qemu-devel
This adds a few lines describing `mux-be` multiplexer configuration
for multiplexing several backend devices with a single frontend
device.
Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>
Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
Cc: qemu-devel@nongnu.org
---
qemu-options.hx | 78 ++++++++++++++++++++++++++++++++++++-------------
1 file changed, 58 insertions(+), 20 deletions(-)
diff --git a/qemu-options.hx b/qemu-options.hx
index daae49414740..dd5dfe8596f0 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3677,37 +3677,37 @@ DEFHEADING(Character device options:)
DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
"-chardev help\n"
- "-chardev null,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
+ "-chardev null,id=id[,mux=on|off][,mux-be-id=id][,logfile=PATH][,logappend=on|off]\n"
"-chardev socket,id=id[,host=host],port=port[,to=to][,ipv4=on|off][,ipv6=on|off][,nodelay=on|off]\n"
- " [,server=on|off][,wait=on|off][,telnet=on|off][,websocket=on|off][,reconnect=seconds][,mux=on|off]\n"
+ " [,server=on|off][,wait=on|off][,telnet=on|off][,websocket=on|off][,reconnect=seconds][,mux=on|off][,mux-be-id=id]\n"
" [,logfile=PATH][,logappend=on|off][,tls-creds=ID][,tls-authz=ID] (tcp)\n"
"-chardev socket,id=id,path=path[,server=on|off][,wait=on|off][,telnet=on|off][,websocket=on|off][,reconnect=seconds]\n"
- " [,mux=on|off][,logfile=PATH][,logappend=on|off][,abstract=on|off][,tight=on|off] (unix)\n"
+ " [,mux=on|off][,mux-be-id=id][,logfile=PATH][,logappend=on|off][,abstract=on|off][,tight=on|off] (unix)\n"
"-chardev udp,id=id[,host=host],port=port[,localaddr=localaddr]\n"
- " [,localport=localport][,ipv4=on|off][,ipv6=on|off][,mux=on|off]\n"
+ " [,localport=localport][,ipv4=on|off][,ipv6=on|off][,mux=on|off][,mux-be-id=id]\n"
" [,logfile=PATH][,logappend=on|off]\n"
- "-chardev msmouse,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
+ "-chardev msmouse,id=id[,mux=on|off][,mux-be-id=id][,logfile=PATH][,logappend=on|off]\n"
"-chardev vc,id=id[[,width=width][,height=height]][[,cols=cols][,rows=rows]]\n"
- " [,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
+ " [,mux=on|off][,mux-be-id=id][,logfile=PATH][,logappend=on|off]\n"
"-chardev ringbuf,id=id[,size=size][,logfile=PATH][,logappend=on|off]\n"
- "-chardev file,id=id,path=path[,input-path=input-file][,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
- "-chardev pipe,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
+ "-chardev file,id=id,path=path[,input-path=input-file][,mux=on|off][,mux-be-id=id][,logfile=PATH][,logappend=on|off]\n"
+ "-chardev pipe,id=id,path=path[,mux=on|off][,mux-be-id=id][,logfile=PATH][,logappend=on|off]\n"
#ifdef _WIN32
- "-chardev console,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
- "-chardev serial,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
+ "-chardev console,id=id[,mux=on|off][,mux-be-id=id][,logfile=PATH][,logappend=on|off]\n"
+ "-chardev serial,id=id,path=path[,mux=on|off][,mux-be-id=id][,logfile=PATH][,logappend=on|off]\n"
#else
- "-chardev pty,id=id[,path=path][,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
- "-chardev stdio,id=id[,mux=on|off][,signal=on|off][,logfile=PATH][,logappend=on|off]\n"
+ "-chardev pty,id=id[,path=path][,mux=on|off][,mux-be-id=id][,logfile=PATH][,logappend=on|off]\n"
+ "-chardev stdio,id=id[,mux=on|off][,mux-be-id=id][,signal=on|off][,logfile=PATH][,logappend=on|off]\n"
#endif
#ifdef CONFIG_BRLAPI
- "-chardev braille,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
+ "-chardev braille,id=id[,mux=on|off][,mux-be-id=id][,logfile=PATH][,logappend=on|off]\n"
#endif
#if defined(__linux__) || defined(__sun__) || defined(__FreeBSD__) \
|| defined(__NetBSD__) || defined(__OpenBSD__) || defined(__DragonFly__)
- "-chardev serial,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
+ "-chardev serial,id=id,path=path[,mux=on|off][,mux-be-id=id][,logfile=PATH][,logappend=on|off]\n"
#endif
#if defined(__linux__) || defined(__FreeBSD__) || defined(__DragonFly__)
- "-chardev parallel,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
+ "-chardev parallel,id=id,path=path[,mux=on|off][,mux-be-id=id][,logfile=PATH][,logappend=on|off]\n"
#endif
#if defined(CONFIG_SPICE)
"-chardev spicevmc,id=id,name=name[,debug=debug][,logfile=PATH][,logappend=on|off]\n"
@@ -3719,8 +3719,8 @@ DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
SRST
The general form of a character device option is:
-``-chardev backend,id=id[,mux=on|off][,options]``
- Backend is one of: ``null``, ``socket``, ``udp``, ``msmouse``,
+``-chardev backend,id=id[,mux=on|off][,mux-be-id=id][,options]``
+ Backend is one of: ``null``, ``socket``, ``udp``, ``msmouse``, ``mux-be``,
``vc``, ``ringbuf``, ``file``, ``pipe``, ``console``, ``serial``,
``pty``, ``stdio``, ``braille``, ``parallel``,
``spicevmc``, ``spiceport``. The specific backend will determine the
@@ -3777,9 +3777,10 @@ The general form of a character device option is:
the QEMU monitor, and ``-nographic`` also multiplexes the console
and the monitor to stdio.
- There is currently no support for multiplexing in the other
- direction (where a single QEMU front end takes input and output from
- multiple chardevs).
+ If you need to multiplex in the opposite direction (where one QEMU
+ interface receives input and output from multiple chardev devices),
+ each character device needs ``mux-be-id=id`` option. Please refer
+ to the paragraph below regarding chardev ``mux-be`` configuration.
Every backend supports the ``logfile`` option, which supplies the
path to a file to record all data transmitted via the backend. The
@@ -3879,6 +3880,43 @@ The available backends are:
Forward QEMU's emulated msmouse events to the guest. ``msmouse``
does not take any options.
+``-chardev mux-be,id=id``
+ Explicitly create chardev backend multiplexer with possibility to
+ multiplex in the opposite direction, where one QEMU interface
+ (frontend device) receives input and output from multiple chardev
+ backend devices.
+
+ For example the following is a use case of 2 backend devices: text
+ virtual console ``vc0`` and a socket ``sock0`` connected
+ to a single virtio hvc console frontend device with multiplexer
+ ``mux0`` help. Virtual console renders text to an image, which
+ can be shared over the VNC protocol, in turn socket backend provides
+ biderectional communication to the virtio hvc console over socket.
+ The example configuration can be the following:
+
+ ::
+
+ -chardev mux-be,id=mux0 \
+ -chardev socket,path=/tmp/sock,server=on,wait=off,id=sock0,mux-be-id=mux0 \
+ -chardev vc,id=vc0,mux-be-id=mux0 \
+ -device virtconsole,chardev=mux0 \
+ -vnc 0.0.0.0:0
+
+ Once QEMU starts VNC client and any TTY emulator can be used to
+ control a single hvc console:
+
+ ::
+
+ # VNC client
+ vncviewer :0
+
+ # TTY emulator
+ socat unix-connect:/tmp/sock pty,link=/tmp/pty & \
+ tio /tmp/pty
+
+ Multiplexing of several backend devices with serveral frontend devices
+ is not supported.
+
``-chardev vc,id=id[[,width=width][,height=height]][[,cols=cols][,rows=rows]]``
Connect to a QEMU text console. ``vc`` may optionally be given a
specific size.
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v4 6/8] chardev/char-mux: implement backend chardev multiplexing
2024-10-16 10:26 ` [PATCH v4 6/8] chardev/char-mux: implement backend chardev multiplexing Roman Penyaev
@ 2024-10-16 11:13 ` Marc-André Lureau
2024-10-16 11:18 ` Marc-André Lureau
` (3 more replies)
0 siblings, 4 replies; 22+ messages in thread
From: Marc-André Lureau @ 2024-10-16 11:13 UTC (permalink / raw)
To: Roman Penyaev, Markus Armbruster; +Cc: qemu-devel, Kevin Wolf
[-- Attachment #1: Type: text/plain, Size: 23006 bytes --]
Hi
On Wed, Oct 16, 2024 at 2:29 PM Roman Penyaev <r.peniaev@gmail.com> wrote:
> This patch implements multiplexing capability of several backend
> devices, which opens up an opportunity to use a single frontend
> device on the guest, which can be manipulated from several
> backend devices.
>
> 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.
>
> Patch implements another multiplexer type `mux-be`. The following
> is QEMU command line example:
>
> -chardev mux-be,id=mux0 \
> -chardev
> socket,path=/tmp/sock,server=on,wait=off,id=sock0,mux-be-id=mux0 \
> -chardev vc,id=vc0,mux-be-id=mux0 \
>
I am not sure about adding "mux-be-id" to all chardev. It avoids the issue
of expressing a list of ids in mux-be though (while it may have potential
loop!)
Markus, do you have a suggestion to take an array of chardev ids as a CLI
option? It looks like we could require QAPIfy -chardev from Kevin here..
thanks
-device virtconsole,chardev=mux0 \
>
-vnc 0.0.0.0:0
>
> Which creates 2 backend devices: text virtual console (`vc0`) and a
> socket (`sock0`) connected to the single virtio hvc console with the
> backend multiplexer (`mux0`) help. `vc0` renders text to an image,
> which can be shared over the VNC protocol. `sock0` is a socket
> backend which provides biderectional communication to the virtio hvc
> console.
>
> 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-mux-be.c | 290 +++++++++++++++++++++++++++++++++++++
> chardev/char.c | 56 +++++--
> chardev/chardev-internal.h | 34 ++++-
> chardev/meson.build | 1 +
> include/chardev/char.h | 1 +
> qapi/char.json | 25 ++++
> 7 files changed, 403 insertions(+), 13 deletions(-)
> create mode 100644 chardev/char-mux-be.c
>
> diff --git a/chardev/char-fe.c b/chardev/char-fe.c
> index a2b5bff39fd9..2f794674563b 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_fe_chr_attach_frontend(d, b, &tag, errp)) {
> return false;
> }
> + } else if (CHARDEV_IS_MUX_BE(s)) {
> + MuxBeChardev *d = MUX_BE_CHARDEV(s);
> +
> + if (!mux_be_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_FE(b->chr)) {
> MuxFeChardev *d = MUX_FE_CHARDEV(b->chr);
> mux_fe_chr_detach_frontend(d, b->tag);
> + } else if (CHARDEV_IS_MUX_BE(b->chr)) {
> + MuxBeChardev *d = MUX_BE_CHARDEV(b->chr);
> + mux_be_chr_detach_frontend(d);
> }
> if (del) {
> Object *obj = OBJECT(b->chr);
> diff --git a/chardev/char-mux-be.c b/chardev/char-mux-be.c
> new file mode 100644
> index 000000000000..64a4f2c00034
> --- /dev/null
> +++ b/chardev/char-mux-be.c
> @@ -0,0 +1,290 @@
> +/*
> + * QEMU Character Backend Multiplexer
> + *
> + * 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/module.h"
> +#include "qemu/option.h"
> +#include "qemu/cutils.h"
> +#include "chardev/char.h"
> +#include "sysemu/block-backend.h"
> +#include "qapi/qapi-commands-control.h"
> +#include "qapi/clone-visitor.h"
> +#include "qapi/qapi-builtin-visit.h"
> +#include "chardev-internal.h"
> +
> +/*
> + * MUX-BE driver for multiplexing 1 frontend device with N 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 mux_be_chr_write_to_all(MuxBeChardev *d, const uint8_t *buf,
> int len)
> +{
> + int r, i, ret = len;
> + unsigned int written;
> +
> + for (i = 0; i < d->be_cnt; i++) {
> + 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], buf, len);
> + if (r < 0 && errno == EAGAIN) {
> + /*
> + * Fail immediately if write would block. Expect to be called
> + * soon on watch wake up.
> + */
> + 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.
> + */
> + 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 mux_be_chr_write(Chardev *chr, const uint8_t *buf, int len)
> +{
> + MuxBeChardev *d = MUX_BE_CHARDEV(chr);
> + return mux_be_chr_write_to_all(d, buf, len);
> +}
> +
> +static void mux_be_chr_send_event(MuxBeChardev *d, QEMUChrEvent event)
> +{
> + CharBackend *fe = d->frontend;
> +
> + if (fe && fe->chr_event) {
> + fe->chr_event(fe->opaque, event);
> + }
> +}
> +
> +static void mux_be_chr_be_event(Chardev *chr, QEMUChrEvent event)
> +{
> + MuxBeChardev *d = MUX_BE_CHARDEV(chr);
> +
> + mux_be_chr_send_event(d, event);
> +}
> +
> +static int mux_be_chr_can_read(void *opaque)
> +{
> + MuxBeChardev *d = MUX_BE_CHARDEV(opaque);
> + CharBackend *fe = d->frontend;
> +
> + if (fe && fe->chr_can_read) {
> + return fe->chr_can_read(fe->opaque);
> + }
> +
> + return 0;
> +}
> +
> +static void mux_be_chr_read(void *opaque, const uint8_t *buf, int size)
> +{
> + MuxBeChardev *d = MUX_BE_CHARDEV(opaque);
> + CharBackend *fe = d->frontend;
> +
> + if (fe && fe->chr_read) {
> + fe->chr_read(fe->opaque, buf, size);
> + }
> +}
> +
> +void mux_be_chr_send_all_event(MuxBeChardev *d, QEMUChrEvent event)
> +{
> + mux_be_chr_send_event(d, event);
> +}
> +
> +static void mux_be_chr_event(void *opaque, QEMUChrEvent event)
> +{
> + mux_chr_send_all_event(CHARDEV(opaque), event);
> +}
> +
> +static GSource *mux_be_chr_add_watch(Chardev *s, GIOCondition cond)
> +{
> + MuxBeChardev *d = MUX_BE_CHARDEV(s);
> + Chardev *chr;
> + ChardevClass *cc;
> + unsigned int written;
> + int i;
> +
> + for (i = 0; i < d->be_cnt; i++) {
> + written = d->be_written[i] - d->be_min_written;
> + if (written) {
> + /* We skip the device with already written buffer */
> + continue;
> + }
> +
> + /*
> + * The first device that has no data written to it must be
> + * the device that recently returned EAGAIN and should be
> + * watched.
> + */
> +
> + chr = qemu_chr_fe_get_driver(&d->backends[i]);
> + cc = CHARDEV_GET_CLASS(chr);
> +
> + if (!cc->chr_add_watch) {
> + return NULL;
> + }
> +
> + return cc->chr_add_watch(chr, cond);
> + }
> +
> + return NULL;
> +}
> +
> +bool mux_be_chr_attach_chardev(MuxBeChardev *d, Chardev *chr, Error
> **errp)
> +{
> + bool ret;
> +
> + if (d->be_cnt >= MAX_MUX) {
> + error_setg(errp, "too many uses of multiplexed chardev '%s'"
> + " (maximum is " stringify(MAX_MUX) ")",
> + d->parent.label);
> + return false;
> + }
> + ret = qemu_chr_fe_init(&d->backends[d->be_cnt], chr, errp);
> + if (ret) {
> + /* Catch up with what was already written */
> + d->be_written[d->be_cnt] = d->be_min_written;
> + d->be_cnt += 1;
> + }
> +
> + return ret;
> +}
> +
> +static void char_mux_be_finalize(Object *obj)
> +{
> + MuxBeChardev *d = MUX_BE_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], false);
> + }
> +}
> +
> +static void mux_be_chr_update_read_handlers(Chardev *chr)
> +{
> + MuxBeChardev *d = MUX_BE_CHARDEV(chr);
> + int i;
> +
> + for (i = 0; i < d->be_cnt; i++) {
> + /* Fix up the real driver with mux routines */
> + qemu_chr_fe_set_handlers_full(&d->backends[i],
> + mux_be_chr_can_read,
> + mux_be_chr_read,
> + mux_be_chr_event,
> + NULL,
> + chr,
> + chr->gcontext, true, false);
> + }
> +}
> +
> +bool mux_be_chr_attach_frontend(MuxBeChardev *d, CharBackend *b, Error
> **errp)
> +{
> + if (d->frontend) {
> + error_setg(errp,
> + "multiplexed chardev '%s' is already used "
> + "for multiplexing", d->parent.label);
> + return false;
> + }
> + d->frontend = b;
> +
> + return true;
> +}
> +
> +void mux_be_chr_detach_frontend(MuxBeChardev *d)
> +{
> + d->frontend = NULL;
> +}
> +
> +static void qemu_chr_open_mux_be(Chardev *chr,
> + ChardevBackend *backend,
> + bool *be_opened,
> + Error **errp)
> +{
> + /*
> + * Only default to opened state if we've realized the initial
> + * set of muxes
> + */
> + *be_opened = mux_is_opened();
> +}
> +
> +static void qemu_chr_parse_mux_be(QemuOpts *opts, ChardevBackend *backend,
> + Error **errp)
> +{
> + ChardevMuxBe *mux;
> +
> + backend->type = CHARDEV_BACKEND_KIND_MUX_BE;
> + mux = backend->u.mux_be.data = g_new0(ChardevMuxBe, 1);
> + qemu_chr_parse_common(opts, qapi_ChardevMuxBe_base(mux));
> +}
> +
> +static void char_mux_be_class_init(ObjectClass *oc, void *data)
> +{
> + ChardevClass *cc = CHARDEV_CLASS(oc);
> +
> + cc->parse = qemu_chr_parse_mux_be;
> + cc->open = qemu_chr_open_mux_be;
> + cc->chr_write = mux_be_chr_write;
> + cc->chr_add_watch = mux_be_chr_add_watch;
> + cc->chr_be_event = mux_be_chr_be_event;
> + cc->chr_update_read_handler = mux_be_chr_update_read_handlers;
> +}
> +
> +static const TypeInfo char_mux_be_type_info = {
> + .name = TYPE_CHARDEV_MUX_BE,
> + .parent = TYPE_CHARDEV,
> + .class_init = char_mux_be_class_init,
> + .instance_size = sizeof(MuxBeChardev),
> + .instance_finalize = char_mux_be_finalize,
> +};
> +
> +static void register_types(void)
> +{
> + type_register_static(&char_mux_be_type_info);
> +}
> +
> +type_init(register_types);
> diff --git a/chardev/char.c b/chardev/char.c
> index cffe60860589..58fa8ac70a1e 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -341,6 +341,9 @@ static bool qemu_chr_is_busy(Chardev *s)
> if (CHARDEV_IS_MUX_FE(s)) {
> MuxFeChardev *d = MUX_FE_CHARDEV(s);
> return d->mux_bitset != 0;
> + } else if (CHARDEV_IS_MUX_BE(s)) {
> + MuxBeChardev *d = MUX_BE_CHARDEV(s);
> + return d->frontend != NULL;
> } else {
> return s->be != NULL;
> }
> @@ -648,7 +651,8 @@ static Chardev *__qemu_chr_new_from_opts(QemuOpts
> *opts, GMainContext *context,
> ChardevBackend *backend = NULL;
> const char *name = qemu_opt_get(opts, "backend");
> const char *id = qemu_opts_id(opts);
> - char *bid = NULL;
> + const char *mux_be_id = NULL;
> + char *mux_fe_id = NULL;
>
> if (name && is_help_option(name)) {
> GString *str = g_string_new("");
> @@ -676,10 +680,16 @@ static Chardev *__qemu_chr_new_from_opts(QemuOpts
> *opts, GMainContext *context,
> }
>
> if (qemu_opt_get_bool(opts, "mux", 0)) {
> - bid = g_strdup_printf("%s-base", id);
> + mux_fe_id = g_strdup_printf("%s-base", id);
> + }
> + mux_be_id = qemu_opt_get(opts, "mux-be-id");
> + if (mux_be_id && mux_fe_id) {
> + error_setg(errp, "chardev: mux and mux-be can't be used for the
> same "
> + "device");
> + goto out;
> }
>
> - chr = qemu_chardev_new(bid ? bid : id,
> + chr = qemu_chardev_new(mux_fe_id ? mux_fe_id : id,
> object_class_get_name(OBJECT_CLASS(cc)),
> backend, context, errp);
> if (chr == NULL) {
> @@ -687,25 +697,40 @@ static Chardev *__qemu_chr_new_from_opts(QemuOpts
> *opts, GMainContext *context,
> }
>
> base = chr;
> - if (bid) {
> + if (mux_fe_id) {
> Chardev *mux;
> qapi_free_ChardevBackend(backend);
> backend = g_new0(ChardevBackend, 1);
> backend->type = CHARDEV_BACKEND_KIND_MUX;
> backend->u.mux.data = g_new0(ChardevMux, 1);
> - backend->u.mux.data->chardev = g_strdup(bid);
> + backend->u.mux.data->chardev = g_strdup(mux_fe_id);
> mux = qemu_chardev_new(id, TYPE_CHARDEV_MUX_FE, backend, context,
> errp);
> if (mux == NULL) {
> - object_unparent(OBJECT(chr));
> - chr = NULL;
> - goto out;
> + goto unparent_and_out;
> }
> chr = mux;
> + } else if (mux_be_id) {
> + Chardev *s;
> +
> + s = qemu_chr_find(mux_be_id);
> + if (!s) {
> + error_setg(errp, "chardev: mux-be device can't be found by id
> '%s'",
> + mux_be_id);
> + goto unparent_and_out;
> + }
> + if (!CHARDEV_IS_MUX_BE(s)) {
> + error_setg(errp, "chardev: device '%s' is not a multiplexer
> device"
> + " of 'mux-be' type", mux_be_id);
> + goto unparent_and_out;
> + }
> + if (!mux_be_chr_attach_chardev(MUX_BE_CHARDEV(s), chr, errp)) {
> + goto unparent_and_out;
> + }
> }
>
> out:
> qapi_free_ChardevBackend(backend);
> - g_free(bid);
> + g_free(mux_fe_id);
>
> if (replay && base) {
> /* RR should be set on the base device, not the mux */
> @@ -713,6 +738,11 @@ out:
> }
>
> return chr;
> +
> +unparent_and_out:
> + object_unparent(OBJECT(chr));
> + chr = NULL;
> + goto out;
> }
>
> Chardev *qemu_chr_new_from_opts(QemuOpts *opts, GMainContext *context,
> @@ -1114,7 +1144,7 @@ ChardevReturn *qmp_chardev_change(const char *id,
> ChardevBackend *backend,
> return NULL;
> }
>
> - if (CHARDEV_IS_MUX_FE(chr)) {
> + if (CHARDEV_IS_MUX_FE(chr) || CHARDEV_IS_MUX_BE(chr)) {
> error_setg(errp, "Mux device hotswap not supported yet");
> return NULL;
> }
> @@ -1302,7 +1332,7 @@ static int chardev_options_parsed_cb(Object *child,
> void *opaque)
> {
> Chardev *chr = (Chardev *)child;
>
> - if (!chr->be_open && CHARDEV_IS_MUX_FE(chr)) {
> + if (!chr->be_open && (CHARDEV_IS_MUX_FE(chr) ||
> CHARDEV_IS_MUX_BE(chr))) {
> open_muxes(chr);
> }
>
> @@ -1329,8 +1359,10 @@ void mux_chr_send_all_event(Chardev *chr,
> QEMUChrEvent event)
>
> if (CHARDEV_IS_MUX_FE(chr)) {
> MuxFeChardev *d = MUX_FE_CHARDEV(chr);
> -
> mux_fe_chr_send_all_event(d, event);
> + } else if (CHARDEV_IS_MUX_BE(chr)) {
> + MuxBeChardev *d = MUX_BE_CHARDEV(chr);
> + mux_be_chr_send_all_event(d, event);
> }
> }
>
> diff --git a/chardev/chardev-internal.h b/chardev/chardev-internal.h
> index 94c8d07ac235..8ea1258f8ff4 100644
> --- a/chardev/chardev-internal.h
> +++ b/chardev/chardev-internal.h
> @@ -35,7 +35,9 @@
>
> struct MuxFeChardev {
> Chardev parent;
> + /* Linked frontends */
> CharBackend *backends[MAX_MUX];
> + /* Linked backend */
> CharBackend chr;
>
Maybe a patch to rename those fields would help.
>
> unsigned long mux_bitset;
> int focus;
> @@ -54,10 +56,36 @@ struct MuxFeChardev {
> };
> typedef struct MuxFeChardev MuxFeChardev;
>
> +struct MuxBeChardev {
> + Chardev parent;
> + /* Linked frontend */
> + CharBackend *frontend;
> + /* Linked backends */
> + CharBackend backends[MAX_MUX];
> + /*
> + * Number of backends attached to this mux. Once attached, a
> + * backend can't be detached, so the counter is only increasing.
> + * To safely remove a backend, mux has to be removed first.
> + */
> + unsigned int be_cnt;
> + /*
> + * Counters of written bytes from a single frontend device
> + * to multiple backend devices.
> + */
> + unsigned int be_written[MAX_MUX];
> + unsigned int be_min_written;
> +};
> +typedef struct MuxBeChardev MuxBeChardev;
> +
> DECLARE_INSTANCE_CHECKER(MuxFeChardev, MUX_FE_CHARDEV,
> TYPE_CHARDEV_MUX_FE)
> -#define CHARDEV_IS_MUX_FE(chr) \
> +DECLARE_INSTANCE_CHECKER(MuxBeChardev, MUX_BE_CHARDEV,
> + TYPE_CHARDEV_MUX_BE)
> +
> +#define CHARDEV_IS_MUX_FE(chr) \
> object_dynamic_cast(OBJECT(chr), TYPE_CHARDEV_MUX_FE)
> +#define CHARDEV_IS_MUX_BE(chr) \
> + object_dynamic_cast(OBJECT(chr), TYPE_CHARDEV_MUX_BE)
>
> void mux_chr_send_all_event(Chardev *chr, QEMUChrEvent event);
>
> @@ -67,6 +95,10 @@ void mux_fe_chr_send_all_event(MuxFeChardev *d,
> QEMUChrEvent event);
> bool mux_fe_chr_attach_frontend(MuxFeChardev *d, CharBackend *b,
> unsigned int *tag, Error **errp);
> bool mux_fe_chr_detach_frontend(MuxFeChardev *d, unsigned int tag);
> +void mux_be_chr_send_all_event(MuxBeChardev *d, QEMUChrEvent event);
> +bool mux_be_chr_attach_chardev(MuxBeChardev *d, Chardev *chr, Error
> **errp);
> +bool mux_be_chr_attach_frontend(MuxBeChardev *d, CharBackend *b, Error
> **errp);
> +void mux_be_chr_detach_frontend(MuxBeChardev *d);
>
> Object *get_chardevs_root(void);
>
> diff --git a/chardev/meson.build b/chardev/meson.build
> index 778444a00ca6..3a9f5565372b 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-fe.c',
> + 'char-mux-be.c',
> 'char-null.c',
> 'char-pipe.c',
> 'char-ringbuf.c',
> diff --git a/include/chardev/char.h b/include/chardev/char.h
> index 0bec974f9d73..c58c11c4eeaf 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_FE "chardev-mux"
> +#define TYPE_CHARDEV_MUX_BE "chardev-mux-be"
> #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 fb0dedb24383..cdec8f9cf4e2 100644
> --- a/qapi/char.json
> +++ b/qapi/char.json
> @@ -336,6 +336,17 @@
> 'data': { 'chardev': 'str' },
> 'base': 'ChardevCommon' }
>
> +##
> +# @ChardevMuxBe:
> +#
> +# Configuration info for mux-be chardevs.
> +#
> +# Since: 9.2
> +##
> +{ 'struct': 'ChardevMuxBe',
> + 'data': { },
> + 'base': 'ChardevCommon' }
> +
> ##
> # @ChardevStdio:
> #
> @@ -483,6 +494,8 @@
> #
> # @mux: (since 1.5)
> #
> +# @mux-be: (since 9.2)
> +#
> # @msmouse: emulated Microsoft serial mouse (since 1.5)
> #
> # @wctablet: emulated Wacom Penpartner serial tablet (since 2.9)
> @@ -525,6 +538,7 @@
> 'pty',
> 'null',
> 'mux',
> + 'mux-be',
> 'msmouse',
> 'wctablet',
> { 'name': 'braille', 'if': 'CONFIG_BRLAPI' },
> @@ -599,6 +613,16 @@
> { 'struct': 'ChardevMuxWrapper',
> 'data': { 'data': 'ChardevMux' } }
>
> +##
> +# @ChardevMuxBeWrapper:
> +#
> +# @data: Configuration info for mux-be chardevs
> +#
> +# Since: 9.2
> +##
> +{ 'struct': 'ChardevMuxBeWrapper',
> + 'data': { 'data': 'ChardevMuxBe' } }
> +
> ##
> # @ChardevStdioWrapper:
> #
> @@ -707,6 +731,7 @@
> 'pty': 'ChardevPtyWrapper',
> 'null': 'ChardevCommonWrapper',
> 'mux': 'ChardevMuxWrapper',
> + 'mux-be': 'ChardevMuxBeWrapper',
> 'msmouse': 'ChardevCommonWrapper',
> 'wctablet': 'ChardevCommonWrapper',
> 'braille': { 'type': 'ChardevCommonWrapper',
> --
> 2.34.1
>
>
>
--
Marc-André Lureau
[-- Attachment #2: Type: text/html, Size: 28541 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 6/8] chardev/char-mux: implement backend chardev multiplexing
2024-10-16 11:13 ` Marc-André Lureau
@ 2024-10-16 11:18 ` Marc-André Lureau
2024-10-16 14:19 ` Roman Penyaev
` (2 subsequent siblings)
3 siblings, 0 replies; 22+ messages in thread
From: Marc-André Lureau @ 2024-10-16 11:18 UTC (permalink / raw)
To: Roman Penyaev, Markus Armbruster; +Cc: qemu-devel, Kevin Wolf
[-- Attachment #1: Type: text/plain, Size: 1203 bytes --]
Hi
On Wed, Oct 16, 2024 at 3:13 PM Marc-André Lureau <
marcandre.lureau@gmail.com> wrote:
> Hi
>
> On Wed, Oct 16, 2024 at 2:29 PM Roman Penyaev <r.peniaev@gmail.com> wrote:
>
>> This patch implements multiplexing capability of several backend
>> devices, which opens up an opportunity to use a single frontend
>> device on the guest, which can be manipulated from several
>> backend devices.
>>
>> 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.
>>
>> Patch implements another multiplexer type `mux-be`. The following
>> is QEMU command line example:
>>
>> -chardev mux-be,id=mux0 \
>> -chardev
>> socket,path=/tmp/sock,server=on,wait=off,id=sock0,mux-be-id=mux0 \
>> -chardev vc,id=vc0,mux-be-id=mux0 \
>>
>
> I am not sure about adding "mux-be-id" to all chardev. It avoids the issue
> of expressing a list of ids in mux-be though (while it may have potential
> loop!)
>
>
(well, the loop can be expressed with an array list as well, and deepen.. I
don't think we have enough sanity check around that, especially at run
time).
--
Marc-André Lureau
[-- Attachment #2: Type: text/html, Size: 2047 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 8/8] qemu-options.hx: describe multiplexing of several backend devices
2024-10-16 10:26 ` [PATCH v4 8/8] qemu-options.hx: describe multiplexing of several backend devices Roman Penyaev
@ 2024-10-16 11:27 ` Marc-André Lureau
0 siblings, 0 replies; 22+ messages in thread
From: Marc-André Lureau @ 2024-10-16 11:27 UTC (permalink / raw)
To: Roman Penyaev; +Cc: qemu-devel
Hi
On Wed, Oct 16, 2024 at 2:28 PM Roman Penyaev <r.peniaev@gmail.com> wrote:
>
> This adds a few lines describing `mux-be` multiplexer configuration
> for multiplexing several backend devices with a single frontend
> device.
>
> Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>
> Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
> Cc: qemu-devel@nongnu.org
> ---
> qemu-options.hx | 78 ++++++++++++++++++++++++++++++++++++-------------
> 1 file changed, 58 insertions(+), 20 deletions(-)
>
> diff --git a/qemu-options.hx b/qemu-options.hx
> index daae49414740..dd5dfe8596f0 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -3677,37 +3677,37 @@ DEFHEADING(Character device options:)
>
> DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
> "-chardev help\n"
> - "-chardev null,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
> + "-chardev null,id=id[,mux=on|off][,mux-be-id=id][,logfile=PATH][,logappend=on|off]\n"
> "-chardev socket,id=id[,host=host],port=port[,to=to][,ipv4=on|off][,ipv6=on|off][,nodelay=on|off]\n"
> - " [,server=on|off][,wait=on|off][,telnet=on|off][,websocket=on|off][,reconnect=seconds][,mux=on|off]\n"
> + " [,server=on|off][,wait=on|off][,telnet=on|off][,websocket=on|off][,reconnect=seconds][,mux=on|off][,mux-be-id=id]\n"
> " [,logfile=PATH][,logappend=on|off][,tls-creds=ID][,tls-authz=ID] (tcp)\n"
> "-chardev socket,id=id,path=path[,server=on|off][,wait=on|off][,telnet=on|off][,websocket=on|off][,reconnect=seconds]\n"
> - " [,mux=on|off][,logfile=PATH][,logappend=on|off][,abstract=on|off][,tight=on|off] (unix)\n"
> + " [,mux=on|off][,mux-be-id=id][,logfile=PATH][,logappend=on|off][,abstract=on|off][,tight=on|off] (unix)\n"
> "-chardev udp,id=id[,host=host],port=port[,localaddr=localaddr]\n"
> - " [,localport=localport][,ipv4=on|off][,ipv6=on|off][,mux=on|off]\n"
> + " [,localport=localport][,ipv4=on|off][,ipv6=on|off][,mux=on|off][,mux-be-id=id]\n"
> " [,logfile=PATH][,logappend=on|off]\n"
> - "-chardev msmouse,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
> + "-chardev msmouse,id=id[,mux=on|off][,mux-be-id=id][,logfile=PATH][,logappend=on|off]\n"
> "-chardev vc,id=id[[,width=width][,height=height]][[,cols=cols][,rows=rows]]\n"
> - " [,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
> + " [,mux=on|off][,mux-be-id=id][,logfile=PATH][,logappend=on|off]\n"
> "-chardev ringbuf,id=id[,size=size][,logfile=PATH][,logappend=on|off]\n"
> - "-chardev file,id=id,path=path[,input-path=input-file][,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
> - "-chardev pipe,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
> + "-chardev file,id=id,path=path[,input-path=input-file][,mux=on|off][,mux-be-id=id][,logfile=PATH][,logappend=on|off]\n"
> + "-chardev pipe,id=id,path=path[,mux=on|off][,mux-be-id=id][,logfile=PATH][,logappend=on|off]\n"
> #ifdef _WIN32
> - "-chardev console,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
> - "-chardev serial,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
> + "-chardev console,id=id[,mux=on|off][,mux-be-id=id][,logfile=PATH][,logappend=on|off]\n"
> + "-chardev serial,id=id,path=path[,mux=on|off][,mux-be-id=id][,logfile=PATH][,logappend=on|off]\n"
> #else
> - "-chardev pty,id=id[,path=path][,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
> - "-chardev stdio,id=id[,mux=on|off][,signal=on|off][,logfile=PATH][,logappend=on|off]\n"
> + "-chardev pty,id=id[,path=path][,mux=on|off][,mux-be-id=id][,logfile=PATH][,logappend=on|off]\n"
> + "-chardev stdio,id=id[,mux=on|off][,mux-be-id=id][,signal=on|off][,logfile=PATH][,logappend=on|off]\n"
> #endif
> #ifdef CONFIG_BRLAPI
> - "-chardev braille,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
> + "-chardev braille,id=id[,mux=on|off][,mux-be-id=id][,logfile=PATH][,logappend=on|off]\n"
> #endif
> #if defined(__linux__) || defined(__sun__) || defined(__FreeBSD__) \
> || defined(__NetBSD__) || defined(__OpenBSD__) || defined(__DragonFly__)
> - "-chardev serial,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
> + "-chardev serial,id=id,path=path[,mux=on|off][,mux-be-id=id][,logfile=PATH][,logappend=on|off]\n"
> #endif
> #if defined(__linux__) || defined(__FreeBSD__) || defined(__DragonFly__)
> - "-chardev parallel,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
> + "-chardev parallel,id=id,path=path[,mux=on|off][,mux-be-id=id][,logfile=PATH][,logappend=on|off]\n"
> #endif
> #if defined(CONFIG_SPICE)
> "-chardev spicevmc,id=id,name=name[,debug=debug][,logfile=PATH][,logappend=on|off]\n"
> @@ -3719,8 +3719,8 @@ DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
> SRST
> The general form of a character device option is:
>
> -``-chardev backend,id=id[,mux=on|off][,options]``
> - Backend is one of: ``null``, ``socket``, ``udp``, ``msmouse``,
> +``-chardev backend,id=id[,mux=on|off][,mux-be-id=id][,options]``
> + Backend is one of: ``null``, ``socket``, ``udp``, ``msmouse``, ``mux-be``,
> ``vc``, ``ringbuf``, ``file``, ``pipe``, ``console``, ``serial``,
> ``pty``, ``stdio``, ``braille``, ``parallel``,
> ``spicevmc``, ``spiceport``. The specific backend will determine the
> @@ -3777,9 +3777,10 @@ The general form of a character device option is:
> the QEMU monitor, and ``-nographic`` also multiplexes the console
> and the monitor to stdio.
>
> - There is currently no support for multiplexing in the other
> - direction (where a single QEMU front end takes input and output from
> - multiple chardevs).
> + If you need to multiplex in the opposite direction (where one QEMU
> + interface receives input and output from multiple chardev devices),
> + each character device needs ``mux-be-id=id`` option. Please refer
> + to the paragraph below regarding chardev ``mux-be`` configuration.
>
> Every backend supports the ``logfile`` option, which supplies the
> path to a file to record all data transmitted via the backend. The
> @@ -3879,6 +3880,43 @@ The available backends are:
> Forward QEMU's emulated msmouse events to the guest. ``msmouse``
> does not take any options.
>
> +``-chardev mux-be,id=id``
> + Explicitly create chardev backend multiplexer with possibility to
> + multiplex in the opposite direction, where one QEMU interface
> + (frontend device) receives input and output from multiple chardev
> + backend devices.
We are not very good at documenting the version, but you could add (Since 9.2)
> +
> + For example the following is a use case of 2 backend devices: text
> + virtual console ``vc0`` and a socket ``sock0`` connected
> + to a single virtio hvc console frontend device with multiplexer
> + ``mux0`` help. Virtual console renders text to an image, which
> + can be shared over the VNC protocol, in turn socket backend provides
> + biderectional communication to the virtio hvc console over socket.
bidirectional
> + The example configuration can be the following:
> +
> + ::
> +
> + -chardev mux-be,id=mux0 \
> + -chardev socket,path=/tmp/sock,server=on,wait=off,id=sock0,mux-be-id=mux0 \
> + -chardev vc,id=vc0,mux-be-id=mux0 \
> + -device virtconsole,chardev=mux0 \
> + -vnc 0.0.0.0:0
> +
> + Once QEMU starts VNC client and any TTY emulator can be used to
> + control a single hvc console:
> +
> + ::
> +
> + # VNC client
> + vncviewer :0
> +
> + # TTY emulator
> + socat unix-connect:/tmp/sock pty,link=/tmp/pty & \
> + tio /tmp/pty
> +
> + Multiplexing of several backend devices with serveral frontend devices
several
> + is not supported.
> +
> ``-chardev vc,id=id[[,width=width][,height=height]][[,cols=cols][,rows=rows]]``
> Connect to a QEMU text console. ``vc`` may optionally be given a
> specific size.
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 7/8] tests/unit/test-char: add unit test for the `mux-be` multiplexer
2024-10-16 10:26 ` [PATCH v4 7/8] tests/unit/test-char: add unit test for the `mux-be` multiplexer Roman Penyaev
@ 2024-10-16 11:36 ` Marc-André Lureau
2024-10-17 13:48 ` Roman Penyaev
0 siblings, 1 reply; 22+ messages in thread
From: Marc-André Lureau @ 2024-10-16 11:36 UTC (permalink / raw)
To: Roman Penyaev; +Cc: qemu-devel
Hi
On Wed, Oct 16, 2024 at 2:28 PM Roman Penyaev <r.peniaev@gmail.com> wrote:
>
> The test is trivial: several backends, 1 `mux-be`, 1 frontend
> do the buffer write and read. Pipe is used for EAGAIN verification.
>
> Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>
> Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
> Cc: qemu-devel@nongnu.org
> ---
> tests/unit/test-char.c | 306 ++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 304 insertions(+), 2 deletions(-)
please fix the few leaks (found with --enable-asan)
>
> diff --git a/tests/unit/test-char.c b/tests/unit/test-char.c
> index a1c6bb874c8e..3eb0692b199f 100644
> --- a/tests/unit/test-char.c
> +++ b/tests/unit/test-char.c
> @@ -178,7 +178,7 @@ static void char_ringbuf_test(void)
> qemu_opts_del(opts);
> }
>
> -static void char_mux_test(void)
> +static void char_mux_fe_test(void)
> {
> QemuOpts *opts;
> Chardev *chr, *base;
> @@ -359,6 +359,307 @@ static void char_mux_test(void)
> qmp_chardev_remove("mux-label", &error_abort);
> }
>
> +static void char_mux_be_test(void)
> +{
> + QemuOpts *opts;
> + Chardev *mux_be, *chr1, *chr2, *base;
> + char *data;
> + FeHandler h = { 0, false, 0, false, };
> + Error *error = NULL;
> + CharBackend chr_be;
> + int ret, i;
> +
> +#define RB_SIZE 128
> +
> + /* Create mux-be */
> + opts = qemu_opts_create(qemu_find_opts("chardev"), "mux0",
> + 1, &error_abort);
> + qemu_opt_set(opts, "backend", "mux-be", &error_abort);
> + mux_be = qemu_chr_new_from_opts(opts, NULL, &error_abort);
> + g_assert_nonnull(mux_be);
> + qemu_opts_del(opts);
> +
> + /* Check maximum allowed backends */
> + for (i = 0; true; i++) {
> + char name[8];
> +
> + snprintf(name, sizeof(name), "chr%d", i);
> + opts = qemu_opts_create(qemu_find_opts("chardev"), name,
> + 1, &error_abort);
> + qemu_opt_set(opts, "backend", "ringbuf", &error_abort);
> + qemu_opt_set(opts, "size", stringify(RB_SIZE), &error_abort);
> + qemu_opt_set(opts, "mux-be-id", "mux0", &error_abort);
> + base = qemu_chr_new_from_opts(opts, NULL, &error);
> + if (error) {
> + const char *err_fmt =
> + "too many uses of multiplexed chardev 'mux0' (maximum is %u)";
> + unsigned n;
> +
> + ret = sscanf(error_get_pretty(error), err_fmt, &n);
> + error_free(error);
> + error = NULL;
> + g_assert_cmpint(ret, ==, 1);
> + g_assert_cmpint(i, ==, n);
> + break;
> + }
> + g_assert_nonnull(base);
> + qemu_opts_del(opts);
> + }
> + /* Finalize mux0 */
> + qmp_chardev_remove("mux0", &error_abort);
> +
> + /* Finalize all backends */
> + while (i--) {
> + char name[8];
> + snprintf(name, sizeof(name), "chr%d", i);
> + qmp_chardev_remove(name, &error_abort);
> + }
> +
> + /* Create mux-be */
> + opts = qemu_opts_create(qemu_find_opts("chardev"), "mux0",
> + 1, &error_abort);
> + qemu_opt_set(opts, "backend", "mux-be", &error_abort);
> + mux_be = qemu_chr_new_from_opts(opts, NULL, &error_abort);
> + g_assert_nonnull(mux_be);
> + qemu_opts_del(opts);
> +
> + /* Create chardev which fails */
> + opts = qemu_opts_create(qemu_find_opts("chardev"), "chr1",
> + 1, &error_abort);
> + qemu_opt_set(opts, "backend", "ringbuf", &error_abort);
> + qemu_opt_set(opts, "size", stringify(RB_SIZE), &error_abort);
> + qemu_opt_set(opts, "mux-be-id", "mux0", &error_abort);
> + qemu_opt_set(opts, "mux", "on", &error_abort);
> + chr1 = qemu_chr_new_from_opts(opts, NULL, &error);
> + g_assert_cmpstr(error_get_pretty(error), ==, "chardev: mux and mux-be "
> + "can't be used for the same device");
> + error_free(error);
> + error = NULL;
> + qemu_opts_del(opts);
> +
> + /* Create first chardev */
> + opts = qemu_opts_create(qemu_find_opts("chardev"), "chr1",
> + 1, &error_abort);
> + qemu_opt_set(opts, "backend", "ringbuf", &error_abort);
> + qemu_opt_set(opts, "size", stringify(RB_SIZE), &error_abort);
> + qemu_opt_set(opts, "mux-be-id", "mux0", &error_abort);
> + chr1 = qemu_chr_new_from_opts(opts, NULL, &error_abort);
> + g_assert_nonnull(chr1);
> + qemu_opts_del(opts);
> +
> + /* Create second chardev */
> + opts = qemu_opts_create(qemu_find_opts("chardev"), "chr2",
> + 1, &error_abort);
> + qemu_opt_set(opts, "backend", "ringbuf", &error_abort);
> + qemu_opt_set(opts, "size", stringify(RB_SIZE), &error_abort);
> + qemu_opt_set(opts, "mux-be-id", "mux0", &error_abort);
> + chr2 = qemu_chr_new_from_opts(opts, NULL, &error_abort);
> + g_assert_nonnull(chr2);
> + qemu_opts_del(opts);
> +
> + /* Attach mux-be to a frontend */
> + qemu_chr_fe_init(&chr_be, mux_be, &error_abort);
> + qemu_chr_fe_set_handlers(&chr_be,
> + fe_can_read,
> + fe_read,
> + fe_event,
> + NULL,
> + &h,
> + NULL, true);
> +
> + /* Fails second time */
> + qemu_chr_fe_init(&chr_be, mux_be, &error);
> + g_assert_cmpstr(error_get_pretty(error), ==, "multiplexed chardev 'mux0' "
> + "is already used for multiplexing");
> + error_free(error);
> + error = NULL;
> +
> + /* Write to backend, chr1 */
> + base = qemu_chr_find("chr1");
> + g_assert_cmpint(qemu_chr_be_can_write(base), !=, 0);
> +
> + qemu_chr_be_write(base, (void *)"hello", 6);
> + g_assert_cmpint(h.read_count, ==, 6);
> + g_assert_cmpstr(h.read_buf, ==, "hello");
> + h.read_count = 0;
> +
> + /* Write to backend, chr2 */
> + base = qemu_chr_find("chr2");
> + g_assert_cmpint(qemu_chr_be_can_write(base), !=, 0);
> +
> + qemu_chr_be_write(base, (void *)"olleh", 6);
> + g_assert_cmpint(h.read_count, ==, 6);
> + g_assert_cmpstr(h.read_buf, ==, "olleh");
> + h.read_count = 0;
> +
> + /* Write to frontend, chr_be */
> + ret = qemu_chr_fe_write(&chr_be, (void *)"heyhey", 6);
> + g_assert_cmpint(ret, ==, 6);
> +
> + data = qmp_ringbuf_read("chr1", RB_SIZE, false, 0, &error_abort);
> + g_assert_cmpint(strlen(data), ==, 6);
> + g_assert_cmpstr(data, ==, "heyhey");
> + g_free(data);
> +
> + data = qmp_ringbuf_read("chr2", RB_SIZE, false, 0, &error_abort);
> + g_assert_cmpint(strlen(data), ==, 6);
> + g_assert_cmpstr(data, ==, "heyhey");
> + g_free(data);
> +
> +
> +#ifndef _WIN32
> + /*
> + * Create third chardev to simulate EAGAIN and watcher.
> + * Mainly copied from char_pipe_test().
> + */
> + {
> + gchar *tmp_path = g_dir_make_tmp("qemu-test-char.XXXXXX", NULL);
> + gchar *in, *out, *pipe = g_build_filename(tmp_path, "pipe", NULL);
> + Chardev *chr3;
> + int fd, len;
> + char buf[128];
> +
> + in = g_strdup_printf("%s.in", pipe);
> + if (mkfifo(in, 0600) < 0) {
> + abort();
> + }
> + out = g_strdup_printf("%s.out", pipe);
> + if (mkfifo(out, 0600) < 0) {
> + abort();
> + }
> +
> + opts = qemu_opts_create(qemu_find_opts("chardev"), "chr3",
> + 1, &error_abort);
> + qemu_opt_set(opts, "backend", "pipe", &error_abort);
> + qemu_opt_set(opts, "path", pipe, &error_abort);
> + qemu_opt_set(opts, "mux-be-id", "mux0", &error_abort);
> + chr3 = qemu_chr_new_from_opts(opts, NULL, &error_abort);
> + g_assert_nonnull(chr3);
> +
> + /* Write to frontend, chr_be */
> + ret = qemu_chr_fe_write(&chr_be, (void *)"thisis", 6);
> + g_assert_cmpint(ret, ==, 6);
> +
> + data = qmp_ringbuf_read("chr1", RB_SIZE, false, 0, &error_abort);
> + g_assert_cmpint(strlen(data), ==, 6);
> + g_assert_cmpstr(data, ==, "thisis");
> + g_free(data);
> +
> + data = qmp_ringbuf_read("chr2", RB_SIZE, false, 0, &error_abort);
> + g_assert_cmpint(strlen(data), ==, 6);
> + g_assert_cmpstr(data, ==, "thisis");
> + g_free(data);
> +
> + fd = open(out, O_RDWR);
> + ret = read(fd, buf, sizeof(buf));
> + g_assert_cmpint(ret, ==, 6);
> + buf[ret] = 0;
> + g_assert_cmpstr(buf, ==, "thisis");
> + close(fd);
> +
> + /* Add watch. 0 indicates no watches if nothing to wait for */
> + ret = qemu_chr_fe_add_watch(&chr_be, G_IO_OUT | G_IO_HUP,
> + NULL, NULL);
> + g_assert_cmpint(ret, ==, 0);
> +
> + /*
> + * Write to frontend, chr_be, until EAGAIN. Make sure length is
> + * power of two to fit nicely the whole pipe buffer.
> + */
> + len = 0;
> + while ((ret = qemu_chr_fe_write(&chr_be, (void *)"thisisit", 8))
> + != -1) {
> + len += ret;
> + }
> + g_assert_cmpint(errno, ==, EAGAIN);
> +
> + /* Further all writes should cause EAGAIN */
> + ret = qemu_chr_fe_write(&chr_be, (void *)"b", 1);
> + g_assert_cmpint(ret, ==, -1);
> + g_assert_cmpint(errno, ==, EAGAIN);
> +
> + /*
> + * Add watch. Non 0 indicates we have a blocked chardev, which
> + * can wakes us up when write is possible.
> + */
> + ret = qemu_chr_fe_add_watch(&chr_be, G_IO_OUT | G_IO_HUP,
> + NULL, NULL);
> + g_assert_cmpint(ret, !=, 0);
> + g_source_remove(ret);
> +
> + /* Drain pipe and ring buffers */
> + fd = open(out, O_RDWR);
> + while ((ret = read(fd, buf, MIN(sizeof(buf), len))) != -1 && len > 0) {
> + len -= ret;
> + }
> + close(fd);
> +
> + data = qmp_ringbuf_read("chr1", RB_SIZE, false, 0, &error_abort);
> + g_assert_cmpint(strlen(data), ==, 128);
> + g_free(data);
> +
> + data = qmp_ringbuf_read("chr2", RB_SIZE, false, 0, &error_abort);
> + g_assert_cmpint(strlen(data), ==, 128);
> + g_free(data);
> +
> + /*
> + * Now we are good to go, first repeat "lost" sequence, which
> + * was already consumed and drained by the ring buffers, but
> + * pipe have not recieved that yet.
> + */
> + ret = qemu_chr_fe_write(&chr_be, (void *)"thisisit", 8);
> + g_assert_cmpint(ret, ==, 8);
> +
> + ret = qemu_chr_fe_write(&chr_be, (void *)"streamisrestored", 16);
> + g_assert_cmpint(ret, ==, 16);
> +
> + data = qmp_ringbuf_read("chr1", RB_SIZE, false, 0, &error_abort);
> + g_assert_cmpint(strlen(data), ==, 16);
> + /* Only last 16 bytes, see big comment above */
> + g_assert_cmpstr(data, ==, "streamisrestored");
> + g_free(data);
> +
> + data = qmp_ringbuf_read("chr2", RB_SIZE, false, 0, &error_abort);
> + g_assert_cmpint(strlen(data), ==, 16);
> + /* Only last 16 bytes, see big comment above */
> + g_assert_cmpstr(data, ==, "streamisrestored");
> + g_free(data);
> +
> + fd = open(out, O_RDWR);
> + ret = read(fd, buf, sizeof(buf));
> + g_assert_cmpint(ret, ==, 24);
> + buf[ret] = 0;
> + /* Both 8 and 16 bytes */
> + g_assert_cmpstr(buf, ==, "thisisitstreamisrestored");
> + close(fd);
> + }
> +#endif
> +
> + /* Can't be removed, depends on mux0 */
> + qmp_chardev_remove("chr1", &error);
> + g_assert_cmpstr(error_get_pretty(error), ==, "Chardev 'chr1' is busy");
> + error_free(error);
> + error = NULL;
> +
> + /* Can't be removed, depends on frontend chr_be */
> + qmp_chardev_remove("mux0", &error);
> + g_assert_cmpstr(error_get_pretty(error), ==, "Chardev 'mux0' is busy");
> + error_free(error);
> + error = NULL;
> +
> + /* Finalize frontend */
> + qemu_chr_fe_deinit(&chr_be, false);
> +
> + /* Finalize mux0 */
> + qmp_chardev_remove("mux0", &error_abort);
> +
> + /* Finalize backend chardevs */
> + qmp_chardev_remove("chr1", &error_abort);
> + qmp_chardev_remove("chr2", &error_abort);
> +#ifndef _WIN32
> + qmp_chardev_remove("chr3", &error_abort);
> +#endif
> +}
>
> static void websock_server_read(void *opaque, const uint8_t *buf, int size)
> {
> @@ -1506,7 +1807,8 @@ int main(int argc, char **argv)
> g_test_add_func("/char/null", char_null_test);
> g_test_add_func("/char/invalid", char_invalid_test);
> g_test_add_func("/char/ringbuf", char_ringbuf_test);
> - g_test_add_func("/char/mux", char_mux_test);
> + g_test_add_func("/char/mux", char_mux_fe_test);
> + g_test_add_func("/char/mux-be", char_mux_be_test);
> #ifdef _WIN32
> g_test_add_func("/char/console/subprocess", char_console_test_subprocess);
> g_test_add_func("/char/console", char_console_test);
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 6/8] chardev/char-mux: implement backend chardev multiplexing
2024-10-16 11:13 ` Marc-André Lureau
2024-10-16 11:18 ` Marc-André Lureau
@ 2024-10-16 14:19 ` Roman Penyaev
2024-11-20 8:00 ` Roman Penyaev
2024-12-11 9:42 ` Markus Armbruster
3 siblings, 0 replies; 22+ messages in thread
From: Roman Penyaev @ 2024-10-16 14:19 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: Markus Armbruster, qemu-devel, Kevin Wolf
Hi,
On Wed, Oct 16, 2024 at 1:14 PM Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
>
> Hi
>
> On Wed, Oct 16, 2024 at 2:29 PM Roman Penyaev <r.peniaev@gmail.com> wrote:
>>
>> This patch implements multiplexing capability of several backend
>> devices, which opens up an opportunity to use a single frontend
>> device on the guest, which can be manipulated from several
>> backend devices.
>>
>> 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.
>>
>> Patch implements another multiplexer type `mux-be`. The following
>> is QEMU command line example:
>>
>> -chardev mux-be,id=mux0 \
>> -chardev socket,path=/tmp/sock,server=on,wait=off,id=sock0,mux-be-id=mux0 \
>> -chardev vc,id=vc0,mux-be-id=mux0 \
>
>
> I am not sure about adding "mux-be-id" to all chardev. It avoids the issue of expressing a list of ids in mux-be though (while it may have potential loop!)
Loop is a good point, but actually can be easily fixed by forbidding
the use of stacked muxes and a reference on itself. Do you think that
would be enough?
--
Roman
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 7/8] tests/unit/test-char: add unit test for the `mux-be` multiplexer
2024-10-16 11:36 ` Marc-André Lureau
@ 2024-10-17 13:48 ` Roman Penyaev
0 siblings, 0 replies; 22+ messages in thread
From: Roman Penyaev @ 2024-10-17 13:48 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: qemu-devel
Hi Marc-André,
On Wed, Oct 16, 2024 at 1:36 PM Marc-André Lureau
<marcandre.lureau@redhat.com> wrote:
>
> Hi
>
> On Wed, Oct 16, 2024 at 2:28 PM Roman Penyaev <r.peniaev@gmail.com> wrote:
> >
> > The test is trivial: several backends, 1 `mux-be`, 1 frontend
> > do the buffer write and read. Pipe is used for EAGAIN verification.
> >
> > Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>
> > Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
> > Cc: qemu-devel@nongnu.org
> > ---
> > tests/unit/test-char.c | 306 ++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 304 insertions(+), 2 deletions(-)
>
> please fix the few leaks (found with --enable-asan)
Did not know about this option. Should be easy, thanks.
--
Roman
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 6/8] chardev/char-mux: implement backend chardev multiplexing
2024-10-16 11:13 ` Marc-André Lureau
2024-10-16 11:18 ` Marc-André Lureau
2024-10-16 14:19 ` Roman Penyaev
@ 2024-11-20 8:00 ` Roman Penyaev
2024-12-11 9:42 ` Markus Armbruster
3 siblings, 0 replies; 22+ messages in thread
From: Roman Penyaev @ 2024-11-20 8:00 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: Markus Armbruster, qemu-devel, Kevin Wolf
Hi all,
Just a quick reminder about the patchset I sent a while
back for the `mux-be-id` property. I’d really like to move
forward with the mux-be implementation, but I’m still not
sure what the best approach would be. Any feedback and
comments would be appreciated.
Thanks.
--
Roman
On Wed, Oct 16, 2024 at 1:14 PM Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
>
> Hi
>
> On Wed, Oct 16, 2024 at 2:29 PM Roman Penyaev <r.peniaev@gmail.com> wrote:
>>
>> This patch implements multiplexing capability of several backend
>> devices, which opens up an opportunity to use a single frontend
>> device on the guest, which can be manipulated from several
>> backend devices.
>>
>> 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.
>>
>> Patch implements another multiplexer type `mux-be`. The following
>> is QEMU command line example:
>>
>> -chardev mux-be,id=mux0 \
>> -chardev socket,path=/tmp/sock,server=on,wait=off,id=sock0,mux-be-id=mux0 \
>> -chardev vc,id=vc0,mux-be-id=mux0 \
>
>
> I am not sure about adding "mux-be-id" to all chardev. It avoids the issue of expressing a list of ids in mux-be though (while it may have potential loop!)
>
> Markus, do you have a suggestion to take an array of chardev ids as a CLI option? It looks like we could require QAPIfy -chardev from Kevin here..
>
> thanks
>
>> -device virtconsole,chardev=mux0 \
>>
>> -vnc 0.0.0.0:0
>>
>> Which creates 2 backend devices: text virtual console (`vc0`) and a
>> socket (`sock0`) connected to the single virtio hvc console with the
>> backend multiplexer (`mux0`) help. `vc0` renders text to an image,
>> which can be shared over the VNC protocol. `sock0` is a socket
>> backend which provides biderectional communication to the virtio hvc
>> console.
>>
>> 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-mux-be.c | 290 +++++++++++++++++++++++++++++++++++++
>> chardev/char.c | 56 +++++--
>> chardev/chardev-internal.h | 34 ++++-
>> chardev/meson.build | 1 +
>> include/chardev/char.h | 1 +
>> qapi/char.json | 25 ++++
>> 7 files changed, 403 insertions(+), 13 deletions(-)
>> create mode 100644 chardev/char-mux-be.c
>>
>> diff --git a/chardev/char-fe.c b/chardev/char-fe.c
>> index a2b5bff39fd9..2f794674563b 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_fe_chr_attach_frontend(d, b, &tag, errp)) {
>> return false;
>> }
>> + } else if (CHARDEV_IS_MUX_BE(s)) {
>> + MuxBeChardev *d = MUX_BE_CHARDEV(s);
>> +
>> + if (!mux_be_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_FE(b->chr)) {
>> MuxFeChardev *d = MUX_FE_CHARDEV(b->chr);
>> mux_fe_chr_detach_frontend(d, b->tag);
>> + } else if (CHARDEV_IS_MUX_BE(b->chr)) {
>> + MuxBeChardev *d = MUX_BE_CHARDEV(b->chr);
>> + mux_be_chr_detach_frontend(d);
>> }
>> if (del) {
>> Object *obj = OBJECT(b->chr);
>> diff --git a/chardev/char-mux-be.c b/chardev/char-mux-be.c
>> new file mode 100644
>> index 000000000000..64a4f2c00034
>> --- /dev/null
>> +++ b/chardev/char-mux-be.c
>> @@ -0,0 +1,290 @@
>> +/*
>> + * QEMU Character Backend Multiplexer
>> + *
>> + * 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/module.h"
>> +#include "qemu/option.h"
>> +#include "qemu/cutils.h"
>> +#include "chardev/char.h"
>> +#include "sysemu/block-backend.h"
>> +#include "qapi/qapi-commands-control.h"
>> +#include "qapi/clone-visitor.h"
>> +#include "qapi/qapi-builtin-visit.h"
>> +#include "chardev-internal.h"
>> +
>> +/*
>> + * MUX-BE driver for multiplexing 1 frontend device with N 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 mux_be_chr_write_to_all(MuxBeChardev *d, const uint8_t *buf, int len)
>> +{
>> + int r, i, ret = len;
>> + unsigned int written;
>> +
>> + for (i = 0; i < d->be_cnt; i++) {
>> + 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], buf, len);
>> + if (r < 0 && errno == EAGAIN) {
>> + /*
>> + * Fail immediately if write would block. Expect to be called
>> + * soon on watch wake up.
>> + */
>> + 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.
>> + */
>> + 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 mux_be_chr_write(Chardev *chr, const uint8_t *buf, int len)
>> +{
>> + MuxBeChardev *d = MUX_BE_CHARDEV(chr);
>> + return mux_be_chr_write_to_all(d, buf, len);
>> +}
>> +
>> +static void mux_be_chr_send_event(MuxBeChardev *d, QEMUChrEvent event)
>> +{
>> + CharBackend *fe = d->frontend;
>> +
>> + if (fe && fe->chr_event) {
>> + fe->chr_event(fe->opaque, event);
>> + }
>> +}
>> +
>> +static void mux_be_chr_be_event(Chardev *chr, QEMUChrEvent event)
>> +{
>> + MuxBeChardev *d = MUX_BE_CHARDEV(chr);
>> +
>> + mux_be_chr_send_event(d, event);
>> +}
>> +
>> +static int mux_be_chr_can_read(void *opaque)
>> +{
>> + MuxBeChardev *d = MUX_BE_CHARDEV(opaque);
>> + CharBackend *fe = d->frontend;
>> +
>> + if (fe && fe->chr_can_read) {
>> + return fe->chr_can_read(fe->opaque);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void mux_be_chr_read(void *opaque, const uint8_t *buf, int size)
>> +{
>> + MuxBeChardev *d = MUX_BE_CHARDEV(opaque);
>> + CharBackend *fe = d->frontend;
>> +
>> + if (fe && fe->chr_read) {
>> + fe->chr_read(fe->opaque, buf, size);
>> + }
>> +}
>> +
>> +void mux_be_chr_send_all_event(MuxBeChardev *d, QEMUChrEvent event)
>> +{
>> + mux_be_chr_send_event(d, event);
>> +}
>> +
>> +static void mux_be_chr_event(void *opaque, QEMUChrEvent event)
>> +{
>> + mux_chr_send_all_event(CHARDEV(opaque), event);
>> +}
>> +
>> +static GSource *mux_be_chr_add_watch(Chardev *s, GIOCondition cond)
>> +{
>> + MuxBeChardev *d = MUX_BE_CHARDEV(s);
>> + Chardev *chr;
>> + ChardevClass *cc;
>> + unsigned int written;
>> + int i;
>> +
>> + for (i = 0; i < d->be_cnt; i++) {
>> + written = d->be_written[i] - d->be_min_written;
>> + if (written) {
>> + /* We skip the device with already written buffer */
>> + continue;
>> + }
>> +
>> + /*
>> + * The first device that has no data written to it must be
>> + * the device that recently returned EAGAIN and should be
>> + * watched.
>> + */
>> +
>> + chr = qemu_chr_fe_get_driver(&d->backends[i]);
>> + cc = CHARDEV_GET_CLASS(chr);
>> +
>> + if (!cc->chr_add_watch) {
>> + return NULL;
>> + }
>> +
>> + return cc->chr_add_watch(chr, cond);
>> + }
>> +
>> + return NULL;
>> +}
>> +
>> +bool mux_be_chr_attach_chardev(MuxBeChardev *d, Chardev *chr, Error **errp)
>> +{
>> + bool ret;
>> +
>> + if (d->be_cnt >= MAX_MUX) {
>> + error_setg(errp, "too many uses of multiplexed chardev '%s'"
>> + " (maximum is " stringify(MAX_MUX) ")",
>> + d->parent.label);
>> + return false;
>> + }
>> + ret = qemu_chr_fe_init(&d->backends[d->be_cnt], chr, errp);
>> + if (ret) {
>> + /* Catch up with what was already written */
>> + d->be_written[d->be_cnt] = d->be_min_written;
>> + d->be_cnt += 1;
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +static void char_mux_be_finalize(Object *obj)
>> +{
>> + MuxBeChardev *d = MUX_BE_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], false);
>> + }
>> +}
>> +
>> +static void mux_be_chr_update_read_handlers(Chardev *chr)
>> +{
>> + MuxBeChardev *d = MUX_BE_CHARDEV(chr);
>> + int i;
>> +
>> + for (i = 0; i < d->be_cnt; i++) {
>> + /* Fix up the real driver with mux routines */
>> + qemu_chr_fe_set_handlers_full(&d->backends[i],
>> + mux_be_chr_can_read,
>> + mux_be_chr_read,
>> + mux_be_chr_event,
>> + NULL,
>> + chr,
>> + chr->gcontext, true, false);
>> + }
>> +}
>> +
>> +bool mux_be_chr_attach_frontend(MuxBeChardev *d, CharBackend *b, Error **errp)
>> +{
>> + if (d->frontend) {
>> + error_setg(errp,
>> + "multiplexed chardev '%s' is already used "
>> + "for multiplexing", d->parent.label);
>> + return false;
>> + }
>> + d->frontend = b;
>> +
>> + return true;
>> +}
>> +
>> +void mux_be_chr_detach_frontend(MuxBeChardev *d)
>> +{
>> + d->frontend = NULL;
>> +}
>> +
>> +static void qemu_chr_open_mux_be(Chardev *chr,
>> + ChardevBackend *backend,
>> + bool *be_opened,
>> + Error **errp)
>> +{
>> + /*
>> + * Only default to opened state if we've realized the initial
>> + * set of muxes
>> + */
>> + *be_opened = mux_is_opened();
>> +}
>> +
>> +static void qemu_chr_parse_mux_be(QemuOpts *opts, ChardevBackend *backend,
>> + Error **errp)
>> +{
>> + ChardevMuxBe *mux;
>> +
>> + backend->type = CHARDEV_BACKEND_KIND_MUX_BE;
>> + mux = backend->u.mux_be.data = g_new0(ChardevMuxBe, 1);
>> + qemu_chr_parse_common(opts, qapi_ChardevMuxBe_base(mux));
>> +}
>> +
>> +static void char_mux_be_class_init(ObjectClass *oc, void *data)
>> +{
>> + ChardevClass *cc = CHARDEV_CLASS(oc);
>> +
>> + cc->parse = qemu_chr_parse_mux_be;
>> + cc->open = qemu_chr_open_mux_be;
>> + cc->chr_write = mux_be_chr_write;
>> + cc->chr_add_watch = mux_be_chr_add_watch;
>> + cc->chr_be_event = mux_be_chr_be_event;
>> + cc->chr_update_read_handler = mux_be_chr_update_read_handlers;
>> +}
>> +
>> +static const TypeInfo char_mux_be_type_info = {
>> + .name = TYPE_CHARDEV_MUX_BE,
>> + .parent = TYPE_CHARDEV,
>> + .class_init = char_mux_be_class_init,
>> + .instance_size = sizeof(MuxBeChardev),
>> + .instance_finalize = char_mux_be_finalize,
>> +};
>> +
>> +static void register_types(void)
>> +{
>> + type_register_static(&char_mux_be_type_info);
>> +}
>> +
>> +type_init(register_types);
>> diff --git a/chardev/char.c b/chardev/char.c
>> index cffe60860589..58fa8ac70a1e 100644
>> --- a/chardev/char.c
>> +++ b/chardev/char.c
>> @@ -341,6 +341,9 @@ static bool qemu_chr_is_busy(Chardev *s)
>> if (CHARDEV_IS_MUX_FE(s)) {
>> MuxFeChardev *d = MUX_FE_CHARDEV(s);
>> return d->mux_bitset != 0;
>> + } else if (CHARDEV_IS_MUX_BE(s)) {
>> + MuxBeChardev *d = MUX_BE_CHARDEV(s);
>> + return d->frontend != NULL;
>> } else {
>> return s->be != NULL;
>> }
>> @@ -648,7 +651,8 @@ static Chardev *__qemu_chr_new_from_opts(QemuOpts *opts, GMainContext *context,
>> ChardevBackend *backend = NULL;
>> const char *name = qemu_opt_get(opts, "backend");
>> const char *id = qemu_opts_id(opts);
>> - char *bid = NULL;
>> + const char *mux_be_id = NULL;
>> + char *mux_fe_id = NULL;
>>
>> if (name && is_help_option(name)) {
>> GString *str = g_string_new("");
>> @@ -676,10 +680,16 @@ static Chardev *__qemu_chr_new_from_opts(QemuOpts *opts, GMainContext *context,
>> }
>>
>> if (qemu_opt_get_bool(opts, "mux", 0)) {
>> - bid = g_strdup_printf("%s-base", id);
>> + mux_fe_id = g_strdup_printf("%s-base", id);
>> + }
>> + mux_be_id = qemu_opt_get(opts, "mux-be-id");
>> + if (mux_be_id && mux_fe_id) {
>> + error_setg(errp, "chardev: mux and mux-be can't be used for the same "
>> + "device");
>> + goto out;
>> }
>>
>> - chr = qemu_chardev_new(bid ? bid : id,
>> + chr = qemu_chardev_new(mux_fe_id ? mux_fe_id : id,
>> object_class_get_name(OBJECT_CLASS(cc)),
>> backend, context, errp);
>> if (chr == NULL) {
>> @@ -687,25 +697,40 @@ static Chardev *__qemu_chr_new_from_opts(QemuOpts *opts, GMainContext *context,
>> }
>>
>> base = chr;
>> - if (bid) {
>> + if (mux_fe_id) {
>> Chardev *mux;
>> qapi_free_ChardevBackend(backend);
>> backend = g_new0(ChardevBackend, 1);
>> backend->type = CHARDEV_BACKEND_KIND_MUX;
>> backend->u.mux.data = g_new0(ChardevMux, 1);
>> - backend->u.mux.data->chardev = g_strdup(bid);
>> + backend->u.mux.data->chardev = g_strdup(mux_fe_id);
>> mux = qemu_chardev_new(id, TYPE_CHARDEV_MUX_FE, backend, context, errp);
>> if (mux == NULL) {
>> - object_unparent(OBJECT(chr));
>> - chr = NULL;
>> - goto out;
>> + goto unparent_and_out;
>> }
>> chr = mux;
>> + } else if (mux_be_id) {
>> + Chardev *s;
>> +
>> + s = qemu_chr_find(mux_be_id);
>> + if (!s) {
>> + error_setg(errp, "chardev: mux-be device can't be found by id '%s'",
>> + mux_be_id);
>> + goto unparent_and_out;
>> + }
>> + if (!CHARDEV_IS_MUX_BE(s)) {
>> + error_setg(errp, "chardev: device '%s' is not a multiplexer device"
>> + " of 'mux-be' type", mux_be_id);
>> + goto unparent_and_out;
>> + }
>> + if (!mux_be_chr_attach_chardev(MUX_BE_CHARDEV(s), chr, errp)) {
>> + goto unparent_and_out;
>> + }
>> }
>>
>> out:
>> qapi_free_ChardevBackend(backend);
>> - g_free(bid);
>> + g_free(mux_fe_id);
>>
>> if (replay && base) {
>> /* RR should be set on the base device, not the mux */
>> @@ -713,6 +738,11 @@ out:
>> }
>>
>> return chr;
>> +
>> +unparent_and_out:
>> + object_unparent(OBJECT(chr));
>> + chr = NULL;
>> + goto out;
>> }
>>
>> Chardev *qemu_chr_new_from_opts(QemuOpts *opts, GMainContext *context,
>> @@ -1114,7 +1144,7 @@ ChardevReturn *qmp_chardev_change(const char *id, ChardevBackend *backend,
>> return NULL;
>> }
>>
>> - if (CHARDEV_IS_MUX_FE(chr)) {
>> + if (CHARDEV_IS_MUX_FE(chr) || CHARDEV_IS_MUX_BE(chr)) {
>> error_setg(errp, "Mux device hotswap not supported yet");
>> return NULL;
>> }
>> @@ -1302,7 +1332,7 @@ static int chardev_options_parsed_cb(Object *child, void *opaque)
>> {
>> Chardev *chr = (Chardev *)child;
>>
>> - if (!chr->be_open && CHARDEV_IS_MUX_FE(chr)) {
>> + if (!chr->be_open && (CHARDEV_IS_MUX_FE(chr) || CHARDEV_IS_MUX_BE(chr))) {
>> open_muxes(chr);
>> }
>>
>> @@ -1329,8 +1359,10 @@ void mux_chr_send_all_event(Chardev *chr, QEMUChrEvent event)
>>
>> if (CHARDEV_IS_MUX_FE(chr)) {
>> MuxFeChardev *d = MUX_FE_CHARDEV(chr);
>> -
>> mux_fe_chr_send_all_event(d, event);
>> + } else if (CHARDEV_IS_MUX_BE(chr)) {
>> + MuxBeChardev *d = MUX_BE_CHARDEV(chr);
>> + mux_be_chr_send_all_event(d, event);
>> }
>> }
>>
>> diff --git a/chardev/chardev-internal.h b/chardev/chardev-internal.h
>> index 94c8d07ac235..8ea1258f8ff4 100644
>> --- a/chardev/chardev-internal.h
>> +++ b/chardev/chardev-internal.h
>> @@ -35,7 +35,9 @@
>>
>> struct MuxFeChardev {
>> Chardev parent;
>> + /* Linked frontends */
>> CharBackend *backends[MAX_MUX];
>> + /* Linked backend */
>> CharBackend chr;
>
>
> Maybe a patch to rename those fields would help.
>
>
>>
>> unsigned long mux_bitset;
>> int focus;
>> @@ -54,10 +56,36 @@ struct MuxFeChardev {
>> };
>> typedef struct MuxFeChardev MuxFeChardev;
>>
>> +struct MuxBeChardev {
>> + Chardev parent;
>> + /* Linked frontend */
>> + CharBackend *frontend;
>> + /* Linked backends */
>> + CharBackend backends[MAX_MUX];
>> + /*
>> + * Number of backends attached to this mux. Once attached, a
>> + * backend can't be detached, so the counter is only increasing.
>> + * To safely remove a backend, mux has to be removed first.
>> + */
>> + unsigned int be_cnt;
>> + /*
>> + * Counters of written bytes from a single frontend device
>> + * to multiple backend devices.
>> + */
>> + unsigned int be_written[MAX_MUX];
>> + unsigned int be_min_written;
>> +};
>> +typedef struct MuxBeChardev MuxBeChardev;
>> +
>> DECLARE_INSTANCE_CHECKER(MuxFeChardev, MUX_FE_CHARDEV,
>> TYPE_CHARDEV_MUX_FE)
>> -#define CHARDEV_IS_MUX_FE(chr) \
>> +DECLARE_INSTANCE_CHECKER(MuxBeChardev, MUX_BE_CHARDEV,
>> + TYPE_CHARDEV_MUX_BE)
>> +
>> +#define CHARDEV_IS_MUX_FE(chr) \
>> object_dynamic_cast(OBJECT(chr), TYPE_CHARDEV_MUX_FE)
>> +#define CHARDEV_IS_MUX_BE(chr) \
>> + object_dynamic_cast(OBJECT(chr), TYPE_CHARDEV_MUX_BE)
>>
>> void mux_chr_send_all_event(Chardev *chr, QEMUChrEvent event);
>>
>> @@ -67,6 +95,10 @@ void mux_fe_chr_send_all_event(MuxFeChardev *d, QEMUChrEvent event);
>> bool mux_fe_chr_attach_frontend(MuxFeChardev *d, CharBackend *b,
>> unsigned int *tag, Error **errp);
>> bool mux_fe_chr_detach_frontend(MuxFeChardev *d, unsigned int tag);
>> +void mux_be_chr_send_all_event(MuxBeChardev *d, QEMUChrEvent event);
>> +bool mux_be_chr_attach_chardev(MuxBeChardev *d, Chardev *chr, Error **errp);
>> +bool mux_be_chr_attach_frontend(MuxBeChardev *d, CharBackend *b, Error **errp);
>> +void mux_be_chr_detach_frontend(MuxBeChardev *d);
>>
>> Object *get_chardevs_root(void);
>>
>> diff --git a/chardev/meson.build b/chardev/meson.build
>> index 778444a00ca6..3a9f5565372b 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-fe.c',
>> + 'char-mux-be.c',
>> 'char-null.c',
>> 'char-pipe.c',
>> 'char-ringbuf.c',
>> diff --git a/include/chardev/char.h b/include/chardev/char.h
>> index 0bec974f9d73..c58c11c4eeaf 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_FE "chardev-mux"
>> +#define TYPE_CHARDEV_MUX_BE "chardev-mux-be"
>> #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 fb0dedb24383..cdec8f9cf4e2 100644
>> --- a/qapi/char.json
>> +++ b/qapi/char.json
>> @@ -336,6 +336,17 @@
>> 'data': { 'chardev': 'str' },
>> 'base': 'ChardevCommon' }
>>
>> +##
>> +# @ChardevMuxBe:
>> +#
>> +# Configuration info for mux-be chardevs.
>> +#
>> +# Since: 9.2
>> +##
>> +{ 'struct': 'ChardevMuxBe',
>> + 'data': { },
>> + 'base': 'ChardevCommon' }
>> +
>> ##
>> # @ChardevStdio:
>> #
>> @@ -483,6 +494,8 @@
>> #
>> # @mux: (since 1.5)
>> #
>> +# @mux-be: (since 9.2)
>> +#
>> # @msmouse: emulated Microsoft serial mouse (since 1.5)
>> #
>> # @wctablet: emulated Wacom Penpartner serial tablet (since 2.9)
>> @@ -525,6 +538,7 @@
>> 'pty',
>> 'null',
>> 'mux',
>> + 'mux-be',
>> 'msmouse',
>> 'wctablet',
>> { 'name': 'braille', 'if': 'CONFIG_BRLAPI' },
>> @@ -599,6 +613,16 @@
>> { 'struct': 'ChardevMuxWrapper',
>> 'data': { 'data': 'ChardevMux' } }
>>
>> +##
>> +# @ChardevMuxBeWrapper:
>> +#
>> +# @data: Configuration info for mux-be chardevs
>> +#
>> +# Since: 9.2
>> +##
>> +{ 'struct': 'ChardevMuxBeWrapper',
>> + 'data': { 'data': 'ChardevMuxBe' } }
>> +
>> ##
>> # @ChardevStdioWrapper:
>> #
>> @@ -707,6 +731,7 @@
>> 'pty': 'ChardevPtyWrapper',
>> 'null': 'ChardevCommonWrapper',
>> 'mux': 'ChardevMuxWrapper',
>> + 'mux-be': 'ChardevMuxBeWrapper',
>> 'msmouse': 'ChardevCommonWrapper',
>> 'wctablet': 'ChardevCommonWrapper',
>> 'braille': { 'type': 'ChardevCommonWrapper',
>> --
>> 2.34.1
>>
>>
>
>
> --
> Marc-André Lureau
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 6/8] chardev/char-mux: implement backend chardev multiplexing
2024-10-16 11:13 ` Marc-André Lureau
` (2 preceding siblings ...)
2024-11-20 8:00 ` Roman Penyaev
@ 2024-12-11 9:42 ` Markus Armbruster
2024-12-17 10:32 ` Roman Penyaev
3 siblings, 1 reply; 22+ messages in thread
From: Markus Armbruster @ 2024-12-11 9:42 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: Roman Penyaev, qemu-devel, Kevin Wolf
I'm awfully, awfully late. My apologies!
Marc-André Lureau <marcandre.lureau@gmail.com> writes:
> Hi
>
> On Wed, Oct 16, 2024 at 2:29 PM Roman Penyaev <r.peniaev@gmail.com> wrote:
>
>> This patch implements multiplexing capability of several backend
>> devices, which opens up an opportunity to use a single frontend
>> device on the guest, which can be manipulated from several
>> backend devices.
>>
>> 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.
>>
>> Patch implements another multiplexer type `mux-be`. The following
>> is QEMU command line example:
>>
>> -chardev mux-be,id=mux0 \
>> -chardev socket,path=/tmp/sock,server=on,wait=off,id=sock0,mux-be-id=mux0 \
>> -chardev vc,id=vc0,mux-be-id=mux0 \
>>
>
> I am not sure about adding "mux-be-id" to all chardev. It avoids the issue
> of expressing a list of ids in mux-be though (while it may have potential
> loop!)
>
> Markus, do you have a suggestion to take an array of chardev ids as a CLI
> option? It looks like we could require QAPIfy -chardev from Kevin here..
We've developed a number of ways of array-shaped configuration bits.
The most recent one relies on QAPI. To not get bogged down in
compatibility considerations, let me show a new option first.
Create a QAPI type FooOptions for the option's argument, say
{ 'struct': 'FooOptions',
'data': { 'ids': ['str'] } }
Create the new option -foo, and use qobject_input_visitor_new_str() and
visit_type_T() to parse its argument into a T.
The new option now supports both JSON and dotted keys syntax for its
argument.
JSON example:
-foo '{"ids": ["eins", "zwei", "drei"]}'
Same in dotted keys:
-foo ids.0=eins,ids.1=zwei,ids.2=drei
Note: dotted keys are slightly less expressive than JSON. For instance,
they can't do empty arrays. Users need to fall back to JSON then.
Peruse the big comment in util/keyval.c if you're curious.
Things can get messy when QAPIfying an existing option argument. Dotted
keys are designed to be close to QemuOpts, but they're not identical.
If existing usage of the option argument relies on funky QemuOpts
features dotted keys don't replicate, we have a compatibility problem.
For complicated arguments, we may not know whether we have a
compatibility problem.
We can sacrifice dotted key syntax to avoid compatibility problems: if
the argument isn't JSON, fall back to the old option parsing code
instead. Ugly, because we then have effectively two interfaces instead
of a single interface with a choice of syntax.
Hope this still helps at least some.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 6/8] chardev/char-mux: implement backend chardev multiplexing
2024-12-11 9:42 ` Markus Armbruster
@ 2024-12-17 10:32 ` Roman Penyaev
2024-12-19 13:45 ` Markus Armbruster
2025-01-16 11:27 ` Kevin Wolf
0 siblings, 2 replies; 22+ messages in thread
From: Roman Penyaev @ 2024-12-17 10:32 UTC (permalink / raw)
To: Markus Armbruster; +Cc: Marc-André Lureau, qemu-devel, Kevin Wolf
Hi Markus,
Thanks for the explicit info. But I have a lot to ask :)
Do I understand correctly that there are two ways to parse
arguments: classic, via qemu_opts_parse_noisily() and modern, via
qobject_input_visitor_new_str()? (for example, I look in
net/net.c, netdev_parse_modern()). My goal is not to create a
completely new option, but to add (extend) parameters for
chardev, namely to add a new type of backend device. This
complicates everything, since chardev uses
qemu_opts_parse_noisily() for parsing and bypasses the modern
way (I hope I'm not mistaken, maybe Marc can comment). And I'm
not sure that it's easy to replace the classic way of parsing
arguments with the modern way without breaking anything. I can,
of course, be wrong, but if I understand correctly, util/keyval.c
does not work with QemuOpts, and the entire char/* is very much
tied to this classic way of getting arguments. Is there a
transitional way to parse the arguments? Use the modern way, but
still represent the arguments as QemuOpts?
--
Roman
On Wed, Dec 11, 2024 at 10:42 AM Markus Armbruster <armbru@redhat.com> wrote:
>
> I'm awfully, awfully late. My apologies!
> Marc-André Lureau <marcandre.lureau@gmail.com> writes:
>
> > Hi
> >
> > On Wed, Oct 16, 2024 at 2:29 PM Roman Penyaev <r.peniaev@gmail.com> wrote:
> >
> >> This patch implements multiplexing capability of several backend
> >> devices, which opens up an opportunity to use a single frontend
> >> device on the guest, which can be manipulated from several
> >> backend devices.
> >>
> >> 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.
> >>
> >> Patch implements another multiplexer type `mux-be`. The following
> >> is QEMU command line example:
> >>
> >> -chardev mux-be,id=mux0 \
> >> -chardev socket,path=/tmp/sock,server=on,wait=off,id=sock0,mux-be-id=mux0 \
> >> -chardev vc,id=vc0,mux-be-id=mux0 \
> >>
> >
> > I am not sure about adding "mux-be-id" to all chardev. It avoids the issue
> > of expressing a list of ids in mux-be though (while it may have potential
> > loop!)
> >
> > Markus, do you have a suggestion to take an array of chardev ids as a CLI
> > option? It looks like we could require QAPIfy -chardev from Kevin here..
>
> We've developed a number of ways of array-shaped configuration bits.
> The most recent one relies on QAPI. To not get bogged down in
> compatibility considerations, let me show a new option first.
>
> Create a QAPI type FooOptions for the option's argument, say
>
> { 'struct': 'FooOptions',
> 'data': { 'ids': ['str'] } }
>
> Create the new option -foo, and use qobject_input_visitor_new_str() and
> visit_type_T() to parse its argument into a T.
>
> The new option now supports both JSON and dotted keys syntax for its
> argument.
>
> JSON example:
>
> -foo '{"ids": ["eins", "zwei", "drei"]}'
>
> Same in dotted keys:
>
> -foo ids.0=eins,ids.1=zwei,ids.2=drei
>
> Note: dotted keys are slightly less expressive than JSON. For instance,
> they can't do empty arrays. Users need to fall back to JSON then.
> Peruse the big comment in util/keyval.c if you're curious.
>
> Things can get messy when QAPIfying an existing option argument. Dotted
> keys are designed to be close to QemuOpts, but they're not identical.
> If existing usage of the option argument relies on funky QemuOpts
> features dotted keys don't replicate, we have a compatibility problem.
> For complicated arguments, we may not know whether we have a
> compatibility problem.
>
> We can sacrifice dotted key syntax to avoid compatibility problems: if
> the argument isn't JSON, fall back to the old option parsing code
> instead. Ugly, because we then have effectively two interfaces instead
> of a single interface with a choice of syntax.
>
> Hope this still helps at least some.
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 6/8] chardev/char-mux: implement backend chardev multiplexing
2024-12-17 10:32 ` Roman Penyaev
@ 2024-12-19 13:45 ` Markus Armbruster
2025-01-16 11:27 ` Kevin Wolf
1 sibling, 0 replies; 22+ messages in thread
From: Markus Armbruster @ 2024-12-19 13:45 UTC (permalink / raw)
To: Roman Penyaev; +Cc: Marc-André Lureau, qemu-devel, Kevin Wolf
Roman Penyaev <r.peniaev@gmail.com> writes:
> Hi Markus,
>
> Thanks for the explicit info. But I have a lot to ask :)
> Do I understand correctly that there are two ways to parse
> arguments: classic, via qemu_opts_parse_noisily() and modern, via
> qobject_input_visitor_new_str()?
Three, to be honest:
* QemuOpts, commonly with qemu_opts_parse_noisily()
* QAPI, commonly with qobject_input_visitor_new_str()
* Ad hoc parsers (you don't wan't to know more)
> (for example, I look in
> net/net.c, netdev_parse_modern()). My goal is not to create a
> completely new option, but to add (extend) parameters for
> chardev, namely to add a new type of backend device. This
> complicates everything, since chardev uses
> qemu_opts_parse_noisily() for parsing and bypasses the modern
> way (I hope I'm not mistaken, maybe Marc can comment). And I'm
> not sure that it's easy to replace the classic way of parsing
> arguments with the modern way without breaking anything.
It's not easy.
The main difficulty is assessing compatibility breaks, and whether they
matter.
> I can,
> of course, be wrong, but if I understand correctly, util/keyval.c
> does not work with QemuOpts,
Correct.
> and the entire char/* is very much
> tied to this classic way of getting arguments.
In the beginning, there was the command line (CLI), and then the human
monitor (HMP).
As CLI options and HMP commands were implemented with QemuOpts, the
underlying internal interfaces tended to be adjusted to take QemuOpts
arguments.
Then there was QMP, and then there was QAPI. As QMP commands were
implemented with QAPI, the underlying internal interfaces tended to be
adjusted to take generated QAPI type arguments.
This got us two internal interfaces doing the same thing. To not have
two implementations, one interface needs to wrap around the other.
Wrapping the QemuOpts one around the QAPI one is more sane.
Have a look at qmp_chardev_add() and hmp_chardev_add().
qmp_chardev_add() wraps around chardev_new(), which takes QAPI type
ChardevBackend.
hmp_chardev_add() wraps around qemu_chr_new_from_opts(), which wraps
around do_qemu_chr_new_from_opts(), which wraps around
qemu_chardev_new(), which wraps around chardev_new(). CLI -chardev
works the same way.
So... Yes, at some point the entire chardev/ was very much tied to
QemuOpts. By now, its core *should* be untied from it. There *may* be
remnants of the old way that still need to be untied.
> Is there a
> transitional way to parse the arguments? Use the modern way, but
> still represent the arguments as QemuOpts?
You could convert manually from QAPI to QemuOpts, but that would be a
mistake. We know, because we made the mistake with device_add and
netdev_add. Fixing the mistake for netdev_add was painful (see commit
db2a380c84574d8c76d7193b8af8535234fe5156 (net: Complete qapi-fication of
netdev_add)). device_add remains unfixed, which has been a source of
trouble.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 6/8] chardev/char-mux: implement backend chardev multiplexing
2024-12-17 10:32 ` Roman Penyaev
2024-12-19 13:45 ` Markus Armbruster
@ 2025-01-16 11:27 ` Kevin Wolf
2025-01-17 8:03 ` Roman Penyaev
1 sibling, 1 reply; 22+ messages in thread
From: Kevin Wolf @ 2025-01-16 11:27 UTC (permalink / raw)
To: Roman Penyaev; +Cc: Markus Armbruster, Marc-André Lureau, qemu-devel
Am 17.12.2024 um 11:32 hat Roman Penyaev geschrieben:
> Hi Markus,
>
> Thanks for the explicit info. But I have a lot to ask :)
> Do I understand correctly that there are two ways to parse
> arguments: classic, via qemu_opts_parse_noisily() and modern, via
> qobject_input_visitor_new_str()? (for example, I look in
> net/net.c, netdev_parse_modern()). My goal is not to create a
> completely new option, but to add (extend) parameters for
> chardev, namely to add a new type of backend device. This
> complicates everything, since chardev uses
> qemu_opts_parse_noisily() for parsing and bypasses the modern
> way (I hope I'm not mistaken, maybe Marc can comment). And I'm
> not sure that it's easy to replace the classic way of parsing
> arguments with the modern way without breaking anything.
A few years ago, I tried to unify the QMP and CLI code paths for
creating chardevs and this involved using QAPI for everything. As far as
I can remember, chardevs don't use any of the QemuOpts features that
don't exist in they keyval parser, so it's easy from that angle.
What makes it more complicated is that CLI and QMP options have diverged
quite a bit, and while generally the same functionality is available, it
sometimes uses different names or is negated in one compared to the
other etc.
So I ended up writing compatibility code that translated legacy CLI
options into QAPI-compatible ones, and then I could use the exising QAPI
types. Part of it made use of aliases, which would have been a new
feature in QAPI, but Markus didn't like them in the end. They could have
been replaced by manual conversion code, but I didn't have time (nor, to
be honest, motivation) to work it any more when Markus had finally made
that decision. It shouldn't actually be very hard.
Anyway, if it's of any use for you, feel free to resurrect parts of it:
https://repo.or.cz/qemu/kevin.git/shortlog/refs/tags/qapi-alias-chardev-v4
(Or maybe I will eventually...)
Whatever you choose to do, my one request for you would be that you
really make sure that CLI and QMP are structured and behave exactly the
same with your new option, to avoid making the problem worse than it
already is.
Kevin
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 6/8] chardev/char-mux: implement backend chardev multiplexing
2025-01-16 11:27 ` Kevin Wolf
@ 2025-01-17 8:03 ` Roman Penyaev
2025-01-17 13:20 ` Kevin Wolf
0 siblings, 1 reply; 22+ messages in thread
From: Roman Penyaev @ 2025-01-17 8:03 UTC (permalink / raw)
To: Kevin Wolf; +Cc: Markus Armbruster, Marc-André Lureau, qemu-devel
Hi Kevin,
On Thu, Jan 16, 2025 at 12:27 PM Kevin Wolf <kwolf@redhat.com> wrote:
>
> Am 17.12.2024 um 11:32 hat Roman Penyaev geschrieben:
> > Hi Markus,
> >
> > Thanks for the explicit info. But I have a lot to ask :)
> > Do I understand correctly that there are two ways to parse
> > arguments: classic, via qemu_opts_parse_noisily() and modern, via
> > qobject_input_visitor_new_str()? (for example, I look in
> > net/net.c, netdev_parse_modern()). My goal is not to create a
> > completely new option, but to add (extend) parameters for
> > chardev, namely to add a new type of backend device. This
> > complicates everything, since chardev uses
> > qemu_opts_parse_noisily() for parsing and bypasses the modern
> > way (I hope I'm not mistaken, maybe Marc can comment). And I'm
> > not sure that it's easy to replace the classic way of parsing
> > arguments with the modern way without breaking anything.
>
> A few years ago, I tried to unify the QMP and CLI code paths for
> creating chardevs and this involved using QAPI for everything. As far as
> I can remember, chardevs don't use any of the QemuOpts features that
> don't exist in they keyval parser, so it's easy from that angle.
>
> What makes it more complicated is that CLI and QMP options have diverged
> quite a bit, and while generally the same functionality is available, it
> sometimes uses different names or is negated in one compared to the
> other etc.
Right, I noticed that as well.
>
> So I ended up writing compatibility code that translated legacy CLI
> options into QAPI-compatible ones, and then I could use the exising QAPI
> types. Part of it made use of aliases, which would have been a new
> feature in QAPI, but Markus didn't like them in the end. They could have
> been replaced by manual conversion code, but I didn't have time (nor, to
> be honest, motivation) to work it any more when Markus had finally made
> that decision. It shouldn't actually be very hard.
>
> Anyway, if it's of any use for you, feel free to resurrect parts of it:
>
> https://repo.or.cz/qemu/kevin.git/shortlog/refs/tags/qapi-alias-chardev-v4
Nice, thanks for sharing, will take a look.
>
> (Or maybe I will eventually...)
>
> Whatever you choose to do, my one request for you would be that you
> really make sure that CLI and QMP are structured and behave exactly the
> same with your new option, to avoid making the problem worse than it
> already is.
I understand this as keeping compatibility between CLI and QAPI at the
command line interface level. In simple words: a new command line option
"list.0=id0,list.1=id1" should be parsed in CLI exactly as QAPI will parse
it in the possible bright future (I mean once the whole chardev is switched
to QAPI). If my understanding is correct, then, with Markus and Mark-Andre
help, I'm on the right track.
--
Roman
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 6/8] chardev/char-mux: implement backend chardev multiplexing
2025-01-17 8:03 ` Roman Penyaev
@ 2025-01-17 13:20 ` Kevin Wolf
0 siblings, 0 replies; 22+ messages in thread
From: Kevin Wolf @ 2025-01-17 13:20 UTC (permalink / raw)
To: Roman Penyaev; +Cc: Markus Armbruster, Marc-André Lureau, qemu-devel
Am 17.01.2025 um 09:03 hat Roman Penyaev geschrieben:
> On Thu, Jan 16, 2025 at 12:27 PM Kevin Wolf <kwolf@redhat.com> wrote:
> > Whatever you choose to do, my one request for you would be that you
> > really make sure that CLI and QMP are structured and behave exactly the
> > same with your new option, to avoid making the problem worse than it
> > already is.
>
> I understand this as keeping compatibility between CLI and QAPI at the
> command line interface level. In simple words: a new command line option
> "list.0=id0,list.1=id1" should be parsed in CLI exactly as QAPI will parse
> it in the possible bright future (I mean once the whole chardev is switched
> to QAPI). If my understanding is correct, then, with Markus and Mark-Andre
> help, I'm on the right track.
Yes, that's exactly what I mean. Good to hear that you're already
planning to keep this in mind.
Kevin
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2025-01-17 13:21 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-16 10:25 [PATCH v4 0/8] chardev: implement backend chardev multiplexing Roman Penyaev
2024-10-16 10:25 ` [PATCH v4 1/8] chardev/char: rename `MuxChardev` struct to `MuxFeChardev` Roman Penyaev
2024-10-16 10:25 ` [PATCH v4 2/8] chardev/char: rename `char-mux.c` to `char-mux-fe.c` Roman Penyaev
2024-10-16 10:26 ` [PATCH v4 3/8] chardev/char: move away mux suspend/resume calls Roman Penyaev
2024-10-16 10:26 ` [PATCH v4 4/8] chardev/char: rename frontend mux calls Roman Penyaev
2024-10-16 10:26 ` [PATCH v4 5/8] chardev/char: introduce `mux-be-id=ID` option Roman Penyaev
2024-10-16 10:26 ` [PATCH v4 6/8] chardev/char-mux: implement backend chardev multiplexing Roman Penyaev
2024-10-16 11:13 ` Marc-André Lureau
2024-10-16 11:18 ` Marc-André Lureau
2024-10-16 14:19 ` Roman Penyaev
2024-11-20 8:00 ` Roman Penyaev
2024-12-11 9:42 ` Markus Armbruster
2024-12-17 10:32 ` Roman Penyaev
2024-12-19 13:45 ` Markus Armbruster
2025-01-16 11:27 ` Kevin Wolf
2025-01-17 8:03 ` Roman Penyaev
2025-01-17 13:20 ` Kevin Wolf
2024-10-16 10:26 ` [PATCH v4 7/8] tests/unit/test-char: add unit test for the `mux-be` multiplexer Roman Penyaev
2024-10-16 11:36 ` Marc-André Lureau
2024-10-17 13:48 ` Roman Penyaev
2024-10-16 10:26 ` [PATCH v4 8/8] qemu-options.hx: describe multiplexing of several backend devices Roman Penyaev
2024-10-16 11:27 ` Marc-André Lureau
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).