1. Patchew
  2. QEMU
  3. [PATCH v4 0/8] chardev: implement backend chardev multiplexing
  4. View patch

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

Roman Penyaev posted 8 patches 1 year, 3 months ago
There is a newer version of this series
[PATCH v4 6/8] chardev/char-mux: implement backend chardev multiplexing
Posted by Roman Penyaev 1 year, 3 months ago 
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
Re: [PATCH v4 6/8] chardev/char-mux: implement backend chardev multiplexing
Posted by Marc-André Lureau 1 year, 3 months ago 
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
Re: [PATCH v4 6/8] chardev/char-mux: implement backend chardev multiplexing
Posted by Markus Armbruster 1 year, 2 months ago 
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.
Re: [PATCH v4 6/8] chardev/char-mux: implement backend chardev multiplexing
Posted by Roman Penyaev 1 year, 1 month ago 
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.
>
Re: [PATCH v4 6/8] chardev/char-mux: implement backend chardev multiplexing
Posted by Kevin Wolf 1 year ago 
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
Re: [PATCH v4 6/8] chardev/char-mux: implement backend chardev multiplexing
Posted by Roman Penyaev 1 year ago 
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
Re: [PATCH v4 6/8] chardev/char-mux: implement backend chardev multiplexing
Posted by Kevin Wolf 1 year ago 
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
Re: [PATCH v4 6/8] chardev/char-mux: implement backend chardev multiplexing
Posted by Markus Armbruster 1 year, 1 month ago 
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.
Re: [PATCH v4 6/8] chardev/char-mux: implement backend chardev multiplexing
Posted by Roman Penyaev 1 year, 2 months ago 
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
Re: [PATCH v4 6/8] chardev/char-mux: implement backend chardev multiplexing
Posted by Roman Penyaev 1 year, 3 months ago 
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
Re: [PATCH v4 6/8] chardev/char-mux: implement backend chardev multiplexing
Posted by Marc-André Lureau 1 year, 3 months ago 
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

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.