[PATCH v2 0/7] chardev: postpone connect

[PATCH v2 0/7] chardev: postpone connect

[Vladimir Sementsov-Ogievskiy]

Hi all!

That's a preparation for backend-transfer migration of
vhost-user-blk. For such migration we are going to transfer
vhost-user-blk fds, including backend chardev fd to the target
in migration stream (backed by UNIX domain socket).

So, on the target, we want to know, should we call connect(),
or is it a backend-transfer migration, and we should wait for
incoming fd.

But at initialization time we can't know it: user may setup
migration parameters (enabling backend-transfer) later.

So, let's postpone chardev open/connect phase up to attaching
to frontend. At this point we can check:

- if it's vhost-user-blk, do nothing, let vhost-user-blk decide
  when to do connect()
- otherwise, do connect() at this point

These are new patches, but called v2, as that's a rework of
previous try to handle vhost-user-blk chardev in

  [PATCH 00/33] vhost-user-blk: live-backend local migration

That previous try required additional option for chardevs,
so users should mark chardevs on target by a new option to
enable backend-transfer (and addition option on source was
needed, and addition migration capability).
The new upcoming design will require only one migration
parameter to enable the whole feature. But it requires more
complex logic, to postpone opens/connects, realized for
chardevs in these series.

The series is based on
  [PATCH 0/2] remove deprecated 'reconnect' options
Based-on: <20250924133309.334631-1-vsementsov@yandex-team.ru>

Vladimir Sementsov-Ogievskiy (7):
  chardev/char-socket: simplify reconnect-ms handling
  chardev/char: split chardev_init_logfd() out of qemu_char_open()
  chardev/char: qemu_char_open(): add return value
  chardev/char: move filename and be_opened handling to qemu_char_open()
  chardev/char: introduce .init() + .connect() initialization interface
  chardev/char-socket: move to .init + .connect api
  char: vhost-user-blk call connect by hand

 chardev/char-fe.c                |  19 ++++-
 chardev/char-socket.c            |  64 +++++++++--------
 chardev/char.c                   | 117 +++++++++++++++++++++++--------
 hw/core/qdev-properties-system.c |   2 +-
 include/chardev/char-fe.h        |   2 +
 include/chardev/char-socket.h    |   1 +
 include/chardev/char.h           |  22 +++++-
 tests/unit/test-char.c           |  14 ++--
 ui/dbus-chardev.c                |  12 +++-
 9 files changed, 181 insertions(+), 72 deletions(-)

-- 
2.48.1

[PATCH v2 1/7] chardev/char-socket: simplify reconnect-ms handling

[Vladimir Sementsov-Ogievskiy]

We pass it to qmp_chardev_open_socket_client() only to write
to s->reconnect_time_ms. Let's simply set this field earlier,
together with other options.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 chardev/char-socket.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 62852e3caf..f3bc6290d2 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -1274,18 +1274,16 @@ skip_listen:
 
 
 static int qmp_chardev_open_socket_client(Chardev *chr,
-                                          int64_t reconnect_ms,
                                           Error **errp)
 {
     SocketChardev *s = SOCKET_CHARDEV(chr);
 
-    if (reconnect_ms > 0) {
-        s->reconnect_time_ms = reconnect_ms;
+    if (s->reconnect_time_ms > 0) {
         tcp_chr_connect_client_async(chr);
         return 0;
-    } else {
-        return tcp_chr_connect_client_sync(chr, errp);
     }
+
+    return tcp_chr_connect_client_sync(chr, errp);
 }
 
 
@@ -1378,7 +1376,6 @@ static void qmp_chardev_open_socket(Chardev *chr,
     bool is_tn3270      = sock->has_tn3270  ? sock->tn3270  : false;
     bool is_waitconnect = sock->has_wait    ? sock->wait    : false;
     bool is_websock     = sock->has_websocket ? sock->websocket : false;
-    int64_t reconnect_ms = sock->has_reconnect_ms ? sock->reconnect_ms : 0;
     SocketAddress *addr;
 
     s->is_listen = is_listen;
@@ -1386,6 +1383,8 @@ static void qmp_chardev_open_socket(Chardev *chr,
     s->is_tn3270 = is_tn3270;
     s->is_websock = is_websock;
     s->do_nodelay = do_nodelay;
+    s->reconnect_time_ms = sock->has_reconnect_ms ? sock->reconnect_ms : 0;
+
     if (sock->tls_creds) {
         Object *creds;
         creds = object_resolve_path_component(
@@ -1450,7 +1449,7 @@ static void qmp_chardev_open_socket(Chardev *chr,
             return;
         }
     } else {
-        if (qmp_chardev_open_socket_client(chr, reconnect_ms, errp) < 0) {
+        if (qmp_chardev_open_socket_client(chr, errp) < 0) {
             return;
         }
     }
-- 
2.48.1

[PATCH v2 2/7] chardev/char: split chardev_init_logfd() out of qemu_char_open()

[Vladimir Sementsov-Ogievskiy]

We are going to share new chardev_init_logfd() with further
alternative initialization interface. Let qemu_char_open() be
a wrapper for .open(), and its artifacts (handle be_opened if
was not set to false by backend, and filename).

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 chardev/char.c | 49 +++++++++++++++++++++++++++++++------------------
 1 file changed, 31 insertions(+), 18 deletions(-)

diff --git a/chardev/char.c b/chardev/char.c
index a43b7e5481..d5a2533e8e 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -250,22 +250,6 @@ static void qemu_char_open(Chardev *chr, ChardevBackend *backend,
                            bool *be_opened, Error **errp)
 {
     ChardevClass *cc = CHARDEV_GET_CLASS(chr);
-    /* Any ChardevCommon member would work */
-    ChardevCommon *common = backend ? backend->u.null.data : NULL;
-
-    if (common && common->logfile) {
-        int flags = O_WRONLY;
-        if (common->has_logappend &&
-            common->logappend) {
-            flags |= O_APPEND;
-        } else {
-            flags |= O_TRUNC;
-        }
-        chr->logfd = qemu_create(common->logfile, flags, 0666, errp);
-        if (chr->logfd < 0) {
-            return;
-        }
-    }
 
     if (cc->open) {
         cc->open(chr, backend, be_opened, errp);
@@ -1000,6 +984,28 @@ void qemu_chr_set_feature(Chardev *chr,
     return set_bit(feature, chr->features);
 }
 
+static bool chardev_init_logfd(Chardev *chr, ChardevBackend *backend,
+                                Error **errp)
+{
+    ChardevCommon *common = backend ? backend->u.null.data : NULL;
+
+    if (common && common->logfile) {
+        int flags = O_WRONLY;
+        if (common->has_logappend &&
+            common->logappend) {
+            flags |= O_APPEND;
+        } else {
+            flags |= O_TRUNC;
+        }
+        chr->logfd = qemu_create(common->logfile, flags, 0666, errp);
+        if (chr->logfd < 0) {
+            return false;
+        }
+    }
+
+    return true;
+}
+
 static Chardev *chardev_new(const char *id, const char *typename,
                             ChardevBackend *backend,
                             GMainContext *gcontext,
@@ -1020,11 +1026,14 @@ static Chardev *chardev_new(const char *id, const char *typename,
     chr->label = g_strdup(id);
     chr->gcontext = gcontext;
 
+    if (!chardev_init_logfd(chr, backend, errp)) {
+        goto fail;
+    }
+
     qemu_char_open(chr, backend, &be_opened, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
-        object_unref(obj);
-        return NULL;
+        goto fail;
     }
 
     if (!chr->filename) {
@@ -1035,6 +1044,10 @@ static Chardev *chardev_new(const char *id, const char *typename,
     }
 
     return chr;
+
+fail:
+    object_unref(obj);
+    return NULL;
 }
 
 Chardev *qemu_chardev_new(const char *id, const char *typename,
-- 
2.48.1

[PATCH v2 3/7] chardev/char: qemu_char_open(): add return value

[Vladimir Sementsov-Ogievskiy]

Accordingly with recommendations in include/qapi/error.h accompany
errp by boolean return value and get rid of error propagation.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 chardev/char.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/chardev/char.c b/chardev/char.c
index d5a2533e8e..64ec60c0f2 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -246,14 +246,20 @@ int qemu_chr_add_client(Chardev *s, int fd)
         CHARDEV_GET_CLASS(s)->chr_add_client(s, fd) : -1;
 }
 
-static void qemu_char_open(Chardev *chr, ChardevBackend *backend,
+static bool qemu_char_open(Chardev *chr, ChardevBackend *backend,
                            bool *be_opened, Error **errp)
 {
+    ERRP_GUARD();
     ChardevClass *cc = CHARDEV_GET_CLASS(chr);
 
     if (cc->open) {
         cc->open(chr, backend, be_opened, errp);
+        if (*errp) {
+            return false;
+        }
     }
+
+    return true;
 }
 
 static void char_init(Object *obj)
@@ -1030,9 +1036,7 @@ static Chardev *chardev_new(const char *id, const char *typename,
         goto fail;
     }
 
-    qemu_char_open(chr, backend, &be_opened, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
+    if (!qemu_char_open(chr, backend, &be_opened, &local_err)) {
         goto fail;
     }
 
-- 
2.48.1

[PATCH v2 4/7] chardev/char: move filename and be_opened handling to qemu_char_open()

[Vladimir Sementsov-Ogievskiy]

Absent filename and necessity to send CHR_EVENT_OPENED are artifacts
of .open(). Handle them in qemu_char_open().

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 chardev/char.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/chardev/char.c b/chardev/char.c
index 64ec60c0f2..6498d53daa 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -247,18 +247,27 @@ int qemu_chr_add_client(Chardev *s, int fd)
 }
 
 static bool qemu_char_open(Chardev *chr, ChardevBackend *backend,
-                           bool *be_opened, Error **errp)
+                           const char *default_filename, Error **errp)
 {
     ERRP_GUARD();
     ChardevClass *cc = CHARDEV_GET_CLASS(chr);
+    bool be_opened = true;
 
     if (cc->open) {
-        cc->open(chr, backend, be_opened, errp);
+        cc->open(chr, backend, &be_opened, errp);
         if (*errp) {
             return false;
         }
     }
 
+    if (!chr->filename) {
+        chr->filename = g_strdup(default_filename);
+    }
+
+    if (be_opened) {
+        qemu_chr_be_event(chr, CHR_EVENT_OPENED);
+    }
+
     return true;
 }
 
@@ -1021,7 +1030,6 @@ static Chardev *chardev_new(const char *id, const char *typename,
     Object *obj;
     Chardev *chr = NULL;
     Error *local_err = NULL;
-    bool be_opened = true;
 
     assert(g_str_has_prefix(typename, "chardev-"));
     assert(id);
@@ -1036,17 +1044,10 @@ static Chardev *chardev_new(const char *id, const char *typename,
         goto fail;
     }
 
-    if (!qemu_char_open(chr, backend, &be_opened, &local_err)) {
+    if (!qemu_char_open(chr, backend, typename + 8, &local_err)) {
         goto fail;
     }
 
-    if (!chr->filename) {
-        chr->filename = g_strdup(typename + 8);
-    }
-    if (be_opened) {
-        qemu_chr_be_event(chr, CHR_EVENT_OPENED);
-    }
-
     return chr;
 
 fail:
-- 
2.48.1

[PATCH v2 5/7] chardev/char: introduce .init() + .connect() initialization interface

[Vladimir Sementsov-Ogievskiy]

We'll need a possibility to postpone connect step to later point in
time to implement backend-transfer migration feature for vhost-user-blk
in further commits. Let's start with new char interface for backends.

.init() takes QAPI parameters and should parse them, called early

.connect() should actually establish a connection, and postponed to
the point of attaching to frontend. Called at later point, either
at time of attaching frontend, either from qemu_chr_wait_connected().

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 chardev/char-fe.c      |  4 ++++
 chardev/char.c         | 40 +++++++++++++++++++++++++++++++++++++---
 include/chardev/char.h | 22 +++++++++++++++++++++-
 3 files changed, 62 insertions(+), 4 deletions(-)

diff --git a/chardev/char-fe.c b/chardev/char-fe.c
index 158a5f4f55..973fed5bea 100644
--- a/chardev/char-fe.c
+++ b/chardev/char-fe.c
@@ -193,6 +193,10 @@ bool qemu_chr_fe_init(CharBackend *b, Chardev *s, Error **errp)
 {
     unsigned int tag = 0;
 
+    if (!qemu_chr_connect(s, errp)) {
+        return false;
+    }
+
     if (s) {
         if (CHARDEV_IS_MUX(s)) {
             MuxChardev *d = MUX_CHARDEV(s);
diff --git a/chardev/char.c b/chardev/char.c
index 6498d53daa..01ffe37acf 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -33,6 +33,7 @@
 #include "qapi/error.h"
 #include "qapi/qapi-commands-char.h"
 #include "qapi/qmp/qerror.h"
+#include "qom/object.h"
 #include "system/replay.h"
 #include "qemu/help_option.h"
 #include "qemu/module.h"
@@ -338,10 +339,29 @@ static bool qemu_chr_is_busy(Chardev *s)
     }
 }
 
+bool qemu_chr_connect(Chardev *chr, Error **errp)
+{
+    ChardevClass *cc = CHARDEV_GET_CLASS(chr);
+
+    if (chr->connect_postponed) {
+        assert(cc->connect);
+        chr->connect_postponed = false;
+        if (!cc->connect(chr, errp)) {
+            return false;
+        }
+    }
+
+    return true;
+}
+
 int qemu_chr_wait_connected(Chardev *chr, Error **errp)
 {
     ChardevClass *cc = CHARDEV_GET_CLASS(chr);
 
+    if (!qemu_chr_connect(chr, errp)) {
+        return -1;
+    }
+
     if (cc->chr_wait_connected) {
         return cc->chr_wait_connected(chr, errp);
     }
@@ -1029,7 +1049,7 @@ static Chardev *chardev_new(const char *id, const char *typename,
 {
     Object *obj;
     Chardev *chr = NULL;
-    Error *local_err = NULL;
+    ChardevClass *cc;
 
     assert(g_str_has_prefix(typename, "chardev-"));
     assert(id);
@@ -1044,8 +1064,22 @@ static Chardev *chardev_new(const char *id, const char *typename,
         goto fail;
     }
 
-    if (!qemu_char_open(chr, backend, typename + 8, &local_err)) {
-        goto fail;
+    cc = CHARDEV_GET_CLASS(chr);
+
+    if (cc->init) {
+        assert(!cc->open);
+        assert(cc->connect);
+
+        if (!cc->init(chr, backend, errp)) {
+            goto fail;
+        }
+        assert(chr->filename);
+
+        chr->connect_postponed = true;
+    } else {
+        if (!qemu_char_open(chr, backend, typename + 8, errp)) {
+            goto fail;
+        }
     }
 
     return chr;
diff --git a/include/chardev/char.h b/include/chardev/char.h
index 429852f8d9..ebadaf3482 100644
--- a/include/chardev/char.h
+++ b/include/chardev/char.h
@@ -63,6 +63,7 @@ struct Chardev {
     CharBackend *be;
     char *label;
     char *filename;
+    bool connect_postponed;
     int logfd;
     int be_open;
     /* used to coordinate the chardev-change special-case: */
@@ -225,6 +226,7 @@ QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename,
                                 bool permit_mux_mon);
 int qemu_chr_write(Chardev *s, const uint8_t *buf, int len, bool write_all);
 #define qemu_chr_write_all(s, buf, len) qemu_chr_write(s, buf, len, true)
+bool qemu_chr_connect(Chardev *chr, Error **errp);
 int qemu_chr_wait_connected(Chardev *chr, Error **errp);
 
 #define TYPE_CHARDEV "chardev"
@@ -259,10 +261,28 @@ struct ChardevClass {
     /* parse command line options and populate QAPI @backend */
     void (*parse)(QemuOpts *opts, ChardevBackend *backend, Error **errp);
 
-    /* called after construction, open/starts the backend */
+    /*
+     * Called after construction, create and open/starts the backend,
+     * mutual exclusive with .init. .connect must not be defined when
+     * .open is defined.
+     */
     void (*open)(Chardev *chr, ChardevBackend *backend,
                  bool *be_opened, Error **errp);
 
+    /*
+     * Called after construction, create the backend, mutual exclusive
+     * with .open, and must be accompanied by .connect.
+     * Must set chr-filename.
+     */
+    bool (*init)(Chardev *chr, ChardevBackend *backend,
+                 Error **errp);
+
+    /*
+     * Called after .init(), open/starts the backend, mutual exclusive
+     * with .open. Must send CHR_EVENT_OPENED.
+     */
+    bool (*connect)(Chardev *chr, Error **errp);
+
     /* write buf to the backend */
     int (*chr_write)(Chardev *s, const uint8_t *buf, int len);
 
-- 
2.48.1

[PATCH v2 6/7] chardev/char-socket: move to .init + .connect api

[Vladimir Sementsov-Ogievskiy]

Move char-socket to new API. This will help to realize backend-transfer
feature for vhost-user-blk.

With this commit qemu_chr_fe_init() starts to do connecting, so we
should handle its errors instead of passing &error_abort.

Also, move qemu_chr_fe_init() in test-char.c, to trigger connect
before trying to get address.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 chardev/char-socket.c         | 55 ++++++++++++++++++++---------------
 chardev/char.c                |  7 +++--
 include/chardev/char-socket.h |  1 +
 tests/unit/test-char.c        | 14 ++++-----
 ui/dbus-chardev.c             | 12 ++++++--
 5 files changed, 54 insertions(+), 35 deletions(-)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index f3bc6290d2..0a5738c158 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -1287,6 +1287,25 @@ static int qmp_chardev_open_socket_client(Chardev *chr,
 }
 
 
+static bool char_socket_connect(Chardev *chr, Error **errp)
+{
+    SocketChardev *s = SOCKET_CHARDEV(chr);
+
+    if (s->is_listen) {
+        if (qmp_chardev_open_socket_server(chr, s->is_telnet || s->is_tn3270,
+                                           s->is_waitconnect, errp) < 0) {
+            return false;
+        }
+    } else {
+        if (qmp_chardev_open_socket_client(chr, errp) < 0) {
+            return false;
+        }
+    }
+
+    return true;
+}
+
+
 static bool qmp_chardev_validate_socket(ChardevSocket *sock,
                                         SocketAddress *addr,
                                         Error **errp)
@@ -1363,10 +1382,9 @@ static bool qmp_chardev_validate_socket(ChardevSocket *sock,
 }
 
 
-static void qmp_chardev_open_socket(Chardev *chr,
-                                    ChardevBackend *backend,
-                                    bool *be_opened,
-                                    Error **errp)
+static bool char_socket_init(Chardev *chr,
+                             ChardevBackend *backend,
+                             Error **errp)
 {
     SocketChardev *s = SOCKET_CHARDEV(chr);
     ChardevSocket *sock = backend->u.socket.data;
@@ -1374,7 +1392,6 @@ static void qmp_chardev_open_socket(Chardev *chr,
     bool is_listen      = sock->has_server  ? sock->server  : true;
     bool is_telnet      = sock->has_telnet  ? sock->telnet  : false;
     bool is_tn3270      = sock->has_tn3270  ? sock->tn3270  : false;
-    bool is_waitconnect = sock->has_wait    ? sock->wait    : false;
     bool is_websock     = sock->has_websocket ? sock->websocket : false;
     SocketAddress *addr;
 
@@ -1383,6 +1400,7 @@ static void qmp_chardev_open_socket(Chardev *chr,
     s->is_tn3270 = is_tn3270;
     s->is_websock = is_websock;
     s->do_nodelay = do_nodelay;
+    s->is_waitconnect = sock->has_wait ? sock->wait : false;
     s->reconnect_time_ms = sock->has_reconnect_ms ? sock->reconnect_ms : 0;
 
     if (sock->tls_creds) {
@@ -1392,7 +1410,7 @@ static void qmp_chardev_open_socket(Chardev *chr,
         if (!creds) {
             error_setg(errp, "No TLS credentials with id '%s'",
                        sock->tls_creds);
-            return;
+            return false;
         }
         s->tls_creds = (QCryptoTLSCreds *)
             object_dynamic_cast(creds,
@@ -1400,7 +1418,7 @@ static void qmp_chardev_open_socket(Chardev *chr,
         if (!s->tls_creds) {
             error_setg(errp, "Object with id '%s' is not TLS credentials",
                        sock->tls_creds);
-            return;
+            return false;
         }
         object_ref(OBJECT(s->tls_creds));
         if (!qcrypto_tls_creds_check_endpoint(s->tls_creds,
@@ -1408,7 +1426,7 @@ static void qmp_chardev_open_socket(Chardev *chr,
                                           ? QCRYPTO_TLS_CREDS_ENDPOINT_SERVER
                                           : QCRYPTO_TLS_CREDS_ENDPOINT_CLIENT,
                                           errp)) {
-            return;
+            return false;
         }
     }
     s->tls_authz = g_strdup(sock->tls_authz);
@@ -1416,7 +1434,7 @@ static void qmp_chardev_open_socket(Chardev *chr,
     s->addr = addr = socket_address_flatten(sock->addr);
 
     if (!qmp_chardev_validate_socket(sock, addr, errp)) {
-        return;
+        return false;
     }
 
     qemu_chr_set_feature(chr, QEMU_CHAR_FEATURE_RECONNECTABLE);
@@ -1433,26 +1451,14 @@ static void qmp_chardev_open_socket(Chardev *chr,
      */
     if (!chr->handover_yank_instance) {
         if (!yank_register_instance(CHARDEV_YANK_INSTANCE(chr->label), errp)) {
-            return;
+            return false;
         }
     }
     s->registered_yank = true;
 
-    /* be isn't opened until we get a connection */
-    *be_opened = false;
-
     update_disconnected_filename(s);
 
-    if (s->is_listen) {
-        if (qmp_chardev_open_socket_server(chr, is_telnet || is_tn3270,
-                                           is_waitconnect, errp) < 0) {
-            return;
-        }
-    } else {
-        if (qmp_chardev_open_socket_client(chr, errp) < 0) {
-            return;
-        }
-    }
+    return true;
 }
 
 static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,
@@ -1576,7 +1582,8 @@ static void char_socket_class_init(ObjectClass *oc, const void *data)
     cc->supports_yank = true;
 
     cc->parse = qemu_chr_parse_socket;
-    cc->open = qmp_chardev_open_socket;
+    cc->init = char_socket_init;
+    cc->connect = char_socket_connect;
     cc->chr_wait_connected = tcp_chr_wait_connected;
     cc->chr_write = tcp_chr_write;
     cc->chr_sync_read = tcp_chr_sync_read;
diff --git a/chardev/char.c b/chardev/char.c
index 01ffe37acf..4d151f537c 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -1221,12 +1221,15 @@ ChardevReturn *qmp_chardev_change(const char *id, ChardevBackend *backend,
     }
 
     chr->be = NULL;
-    qemu_chr_fe_init(be, chr_new, &error_abort);
+    if (!qemu_chr_fe_init(be, chr_new, errp)) {
+        object_unref(OBJECT(chr_new));
+        return NULL;
+    }
 
     if (be->chr_be_change(be->opaque) < 0) {
         error_setg(errp, "Chardev '%s' change failed", chr_new->label);
         chr_new->be = NULL;
-        qemu_chr_fe_init(be, chr, &error_abort);
+        qemu_chr_fe_init(be, chr, NULL);
         if (closed_sent) {
             qemu_chr_be_event(chr, CHR_EVENT_OPENED);
         }
diff --git a/include/chardev/char-socket.h b/include/chardev/char-socket.h
index d6d13ad37f..0109727eaa 100644
--- a/include/chardev/char-socket.h
+++ b/include/chardev/char-socket.h
@@ -68,6 +68,7 @@ struct SocketChardev {
     bool is_listen;
     bool is_telnet;
     bool is_tn3270;
+    bool is_waitconnect;
     GSource *telnet_source;
     TCPChardevTelnetInit *telnet_init;
 
diff --git a/tests/unit/test-char.c b/tests/unit/test-char.c
index f30a39f61f..5c9482a478 100644
--- a/tests/unit/test-char.c
+++ b/tests/unit/test-char.c
@@ -845,6 +845,7 @@ static void char_websock_test(void)
                               0xef, 0xaa, 0xc5, 0x97, /* Masking key */
                               0xec, 0x42              /* Status code */ };
 
+    qemu_chr_fe_init(&be, chr, &error_abort);
     addr = object_property_get_qobject(OBJECT(chr), "addr", &error_abort);
     qdict = qobject_to(QDict, addr);
     port = qdict_get_str(qdict, "port");
@@ -852,7 +853,6 @@ static void char_websock_test(void)
     handshake_port = g_strdup_printf(handshake, port, port);
     qobject_unref(qdict);
 
-    qemu_chr_fe_init(&be, chr, &error_abort);
     qemu_chr_fe_set_handlers(&be, websock_server_can_read, websock_server_read,
                              NULL, NULL, chr, NULL, true);
 
@@ -1216,6 +1216,8 @@ static void char_socket_server_test(gconstpointer opaque)
     g_assert_nonnull(chr);
     g_assert(!object_property_get_bool(OBJECT(chr), "connected", &error_abort));
 
+    qemu_chr_fe_init(&be, chr, &error_abort);
+
     qaddr = object_property_get_qobject(OBJECT(chr), "addr", &error_abort);
     g_assert_nonnull(qaddr);
 
@@ -1224,8 +1226,6 @@ static void char_socket_server_test(gconstpointer opaque)
     visit_free(v);
     qobject_unref(qaddr);
 
-    qemu_chr_fe_init(&be, chr, &error_abort);
-
  reconnect:
     data.event = -1;
     data.be = &be;
@@ -1417,6 +1417,8 @@ static void char_socket_client_test(gconstpointer opaque)
     qemu_opts_del(opts);
     g_assert_nonnull(chr);
 
+    qemu_chr_fe_init(&be, chr, &error_abort);
+
     if (config->reconnect) {
         /*
          * If reconnect is set, the connection will be
@@ -1431,8 +1433,6 @@ static void char_socket_client_test(gconstpointer opaque)
                                           &error_abort));
     }
 
-    qemu_chr_fe_init(&be, chr, &error_abort);
-
  reconnect:
     data.event = -1;
     data.be = &be;
@@ -1550,6 +1550,8 @@ static void char_socket_server_two_clients_test(gconstpointer opaque)
     g_assert_nonnull(chr);
     g_assert(!object_property_get_bool(OBJECT(chr), "connected", &error_abort));
 
+    qemu_chr_fe_init(&be, chr, &error_abort);
+
     qaddr = object_property_get_qobject(OBJECT(chr), "addr", &error_abort);
     g_assert_nonnull(qaddr);
 
@@ -1558,8 +1560,6 @@ static void char_socket_server_two_clients_test(gconstpointer opaque)
     visit_free(v);
     qobject_unref(qaddr);
 
-    qemu_chr_fe_init(&be, chr, &error_abort);
-
     qemu_chr_fe_set_handlers(&be, char_socket_can_read, char_socket_discard_read,
                              count_closed_event, NULL,
                              &closed, NULL, true);
diff --git a/ui/dbus-chardev.c b/ui/dbus-chardev.c
index d05dddaf81..23cf9d6ee9 100644
--- a/ui/dbus-chardev.c
+++ b/ui/dbus-chardev.c
@@ -210,8 +210,14 @@ dbus_chr_open(Chardev *chr, ChardevBackend *backend,
     if (*errp) {
         return;
     }
-    CHARDEV_CLASS(object_class_by_name(TYPE_CHARDEV_SOCKET))->open(
-        chr, be, be_opened, errp);
+    if (!CHARDEV_CLASS(object_class_by_name(TYPE_CHARDEV_SOCKET))->init(
+        chr, be, errp)) {
+        return;
+    }
+    if (!CHARDEV_CLASS(object_class_by_name(TYPE_CHARDEV_SOCKET))->connect(
+        chr, errp)) {
+        return;
+    }
 }
 
 static void
@@ -276,6 +282,8 @@ char_dbus_class_init(ObjectClass *oc, const void *data)
 
     cc->parse = dbus_chr_parse;
     cc->open = dbus_chr_open;
+    cc->init = NULL;
+    cc->connect = NULL;
     cc->chr_set_fe_open = dbus_chr_set_fe_open;
     cc->chr_set_echo = dbus_chr_set_echo;
     klass->parent_chr_be_event = cc->chr_be_event;
-- 
2.48.1

[PATCH v2 7/7] char: vhost-user-blk call connect by hand

[Vladimir Sementsov-Ogievskiy]

Give vhost-user-blk a possibility (and responsibility) to decide when
do connect. It will help us to realize vhost-user-blk backend-transfer
migration feature:

For incoming migration we'll need to postpone connect at least until
early stage of migrate-incoming command, when we already know all
migration parameters and can decide, are we going to do incoming
backend-transfer (and get chardev fd from incoming stream), or we
finally need to connect.

With this patch, we don't modify vhost-user-blk.c: it already
do call qemu_chr_fe_wait_connected(), which in turn calls
qemu_chr_connect().

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 chardev/char-fe.c                | 21 +++++++++++++++++----
 hw/core/qdev-properties-system.c |  2 +-
 include/chardev/char-fe.h        |  2 ++
 3 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/chardev/char-fe.c b/chardev/char-fe.c
index 973fed5bea..9e2c658cb9 100644
--- a/chardev/char-fe.c
+++ b/chardev/char-fe.c
@@ -189,12 +189,20 @@ bool qemu_chr_fe_backend_open(CharBackend *be)
     return be->chr && be->chr->be_open;
 }
 
-bool qemu_chr_fe_init(CharBackend *b, Chardev *s, Error **errp)
+bool qemu_chr_fe_init_ex(CharBackend *b, Chardev *s, ObjectClass *oc,
+                         Error **errp)
 {
     unsigned int tag = 0;
-
-    if (!qemu_chr_connect(s, errp)) {
-        return false;
+    const char *driver = oc ? object_class_get_name(oc) : NULL;
+
+    /*
+     * vhost-user-blk wants call qemu_chr_connect by hand,
+     * to support backend-transfer migration feature.
+     */
+    if (!driver || !g_str_has_prefix(driver, "vhost-user-blk")) {
+        if (!qemu_chr_connect(s, errp)) {
+            return false;
+        }
     }
 
     if (s) {
@@ -218,6 +226,11 @@ bool qemu_chr_fe_init(CharBackend *b, Chardev *s, Error **errp)
     return true;
 }
 
+bool qemu_chr_fe_init(CharBackend *b, Chardev *s, Error **errp)
+{
+    return qemu_chr_fe_init_ex(b, s, NULL, errp);
+}
+
 void qemu_chr_fe_deinit(CharBackend *b, bool del)
 {
     assert(b);
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 1f810b7ddf..35eed17d4d 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -297,7 +297,7 @@ static void set_chr(Object *obj, Visitor *v, const char *name, void *opaque,
     if (s == NULL) {
         error_setg(errp, "Property '%s.%s' can't find value '%s'",
                    object_get_typename(obj), name, str);
-    } else if (!qemu_chr_fe_init(be, s, errp)) {
+    } else if (!qemu_chr_fe_init_ex(be, s, obj->class, errp)) {
         error_prepend(errp, "Property '%s.%s' can't take value '%s': ",
                       object_get_typename(obj), name, str);
     }
diff --git a/include/chardev/char-fe.h b/include/chardev/char-fe.h
index 8ef05b3dd0..f1c05cc5ed 100644
--- a/include/chardev/char-fe.h
+++ b/include/chardev/char-fe.h
@@ -34,6 +34,8 @@ struct CharBackend {
  * Returns: false on error.
  */
 bool qemu_chr_fe_init(CharBackend *b, Chardev *s, Error **errp);
+bool qemu_chr_fe_init_ex(CharBackend *b, Chardev *s, ObjectClass *oc,
+                         Error **errp);
 
 /**
  * qemu_chr_fe_deinit:
-- 
2.48.1

Re: [PATCH v2 3/7] chardev/char: qemu_char_open(): add return value

[Marc-André Lureau]

Hi

On Mon, Oct 13, 2025 at 5:40 PM Vladimir Sementsov-Ogievskiy
<vsementsov@yandex-team.ru> wrote:
>
> Accordingly with recommendations in include/qapi/error.h accompany
> errp by boolean return value and get rid of error propagation.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>  chardev/char.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/chardev/char.c b/chardev/char.c
> index d5a2533e8e..64ec60c0f2 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -246,14 +246,20 @@ int qemu_chr_add_client(Chardev *s, int fd)
>          CHARDEV_GET_CLASS(s)->chr_add_client(s, fd) : -1;
>  }
>
> -static void qemu_char_open(Chardev *chr, ChardevBackend *backend,
> +static bool qemu_char_open(Chardev *chr, ChardevBackend *backend,
>                             bool *be_opened, Error **errp)
>  {
> +    ERRP_GUARD();
>      ChardevClass *cc = CHARDEV_GET_CLASS(chr);
>
>      if (cc->open) {
>          cc->open(chr, backend, be_opened, errp);
> +        if (*errp) {
> +            return false;
> +        }
>      }
> +
> +    return true;
>  }
>
>  static void char_init(Object *obj)
> @@ -1030,9 +1036,7 @@ static Chardev *chardev_new(const char *id, const char *typename,
>          goto fail;
>      }
>
> -    qemu_char_open(chr, backend, &be_opened, &local_err);
> -    if (local_err) {
> -        error_propagate(errp, local_err);
> +    if (!qemu_char_open(chr, backend, &be_opened, &local_err)) {

You meant to pass errp instead, since you dropped error_propagate ?

>          goto fail;
>      }
>
> --
> 2.48.1
>
>


-- 
Marc-André Lureau

Re: [PATCH v2 7/7] char: vhost-user-blk call connect by hand

[Marc-André Lureau]

Hi

On Mon, Oct 13, 2025 at 5:39 PM Vladimir Sementsov-Ogievskiy
<vsementsov@yandex-team.ru> wrote:
>
> Give vhost-user-blk a possibility (and responsibility) to decide when
> do connect. It will help us to realize vhost-user-blk backend-transfer
> migration feature:
>
> For incoming migration we'll need to postpone connect at least until
> early stage of migrate-incoming command, when we already know all
> migration parameters and can decide, are we going to do incoming
> backend-transfer (and get chardev fd from incoming stream), or we
> finally need to connect.
>
> With this patch, we don't modify vhost-user-blk.c: it already
> do call qemu_chr_fe_wait_connected(), which in turn calls
> qemu_chr_connect().
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>  chardev/char-fe.c                | 21 +++++++++++++++++----
>  hw/core/qdev-properties-system.c |  2 +-
>  include/chardev/char-fe.h        |  2 ++
>  3 files changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/chardev/char-fe.c b/chardev/char-fe.c
> index 973fed5bea..9e2c658cb9 100644
> --- a/chardev/char-fe.c
> +++ b/chardev/char-fe.c
> @@ -189,12 +189,20 @@ bool qemu_chr_fe_backend_open(CharBackend *be)
>      return be->chr && be->chr->be_open;
>  }
>
> -bool qemu_chr_fe_init(CharBackend *b, Chardev *s, Error **errp)
> +bool qemu_chr_fe_init_ex(CharBackend *b, Chardev *s, ObjectClass *oc,
> +                         Error **errp)
>  {
>      unsigned int tag = 0;
> -
> -    if (!qemu_chr_connect(s, errp)) {
> -        return false;
> +    const char *driver = oc ? object_class_get_name(oc) : NULL;
> +
> +    /*
> +     * vhost-user-blk wants call qemu_chr_connect by hand,
> +     * to support backend-transfer migration feature.
> +     */
> +    if (!driver || !g_str_has_prefix(driver, "vhost-user-blk")) {
> +        if (!qemu_chr_connect(s, errp)) {
> +            return false;
> +        }

Why not pass a bool "connect" instead?

DEFINE_PROP_CHR() would use "true".

+ Introduce DEFINE_PROP_CHR_NO_CONNECT() that would use "false".

and wire it with a new PropertyInfo and modified callbacks.

>      }
>
>      if (s) {
> @@ -218,6 +226,11 @@ bool qemu_chr_fe_init(CharBackend *b, Chardev *s, Error **errp)
>      return true;
>  }
>
> +bool qemu_chr_fe_init(CharBackend *b, Chardev *s, Error **errp)
> +{
> +    return qemu_chr_fe_init_ex(b, s, NULL, errp);
> +}
> +
>  void qemu_chr_fe_deinit(CharBackend *b, bool del)
>  {
>      assert(b);
> diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
> index 1f810b7ddf..35eed17d4d 100644
> --- a/hw/core/qdev-properties-system.c
> +++ b/hw/core/qdev-properties-system.c
> @@ -297,7 +297,7 @@ static void set_chr(Object *obj, Visitor *v, const char *name, void *opaque,
>      if (s == NULL) {
>          error_setg(errp, "Property '%s.%s' can't find value '%s'",
>                     object_get_typename(obj), name, str);
> -    } else if (!qemu_chr_fe_init(be, s, errp)) {
> +    } else if (!qemu_chr_fe_init_ex(be, s, obj->class, errp)) {
>          error_prepend(errp, "Property '%s.%s' can't take value '%s': ",
>                        object_get_typename(obj), name, str);
>      }
> diff --git a/include/chardev/char-fe.h b/include/chardev/char-fe.h
> index 8ef05b3dd0..f1c05cc5ed 100644
> --- a/include/chardev/char-fe.h
> +++ b/include/chardev/char-fe.h
> @@ -34,6 +34,8 @@ struct CharBackend {
>   * Returns: false on error.
>   */
>  bool qemu_chr_fe_init(CharBackend *b, Chardev *s, Error **errp);
> +bool qemu_chr_fe_init_ex(CharBackend *b, Chardev *s, ObjectClass *oc,
> +                         Error **errp);
>
>  /**
>   * qemu_chr_fe_deinit:
> --
> 2.48.1
>
>


-- 
Marc-André Lureau

Re: [PATCH v2 2/7] chardev/char: split chardev_init_logfd() out of qemu_char_open()

[Marc-André Lureau]

On Mon, Oct 13, 2025 at 5:41 PM Vladimir Sementsov-Ogievskiy
<vsementsov@yandex-team.ru> wrote:
>
> We are going to share new chardev_init_logfd() with further
> alternative initialization interface. Let qemu_char_open() be
> a wrapper for .open(), and its artifacts (handle be_opened if
> was not set to false by backend, and filename).
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  chardev/char.c | 49 +++++++++++++++++++++++++++++++------------------
>  1 file changed, 31 insertions(+), 18 deletions(-)
>
> diff --git a/chardev/char.c b/chardev/char.c
> index a43b7e5481..d5a2533e8e 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -250,22 +250,6 @@ static void qemu_char_open(Chardev *chr, ChardevBackend *backend,
>                             bool *be_opened, Error **errp)
>  {
>      ChardevClass *cc = CHARDEV_GET_CLASS(chr);
> -    /* Any ChardevCommon member would work */

maybe keep that comment?


> -    ChardevCommon *common = backend ? backend->u.null.data : NULL;
> -
> -    if (common && common->logfile) {
> -        int flags = O_WRONLY;
> -        if (common->has_logappend &&
> -            common->logappend) {
> -            flags |= O_APPEND;
> -        } else {
> -            flags |= O_TRUNC;
> -        }
> -        chr->logfd = qemu_create(common->logfile, flags, 0666, errp);
> -        if (chr->logfd < 0) {
> -            return;
> -        }
> -    }
>
>      if (cc->open) {
>          cc->open(chr, backend, be_opened, errp);
> @@ -1000,6 +984,28 @@ void qemu_chr_set_feature(Chardev *chr,
>      return set_bit(feature, chr->features);
>  }
>
> +static bool chardev_init_logfd(Chardev *chr, ChardevBackend *backend,
> +                                Error **errp)
> +{
> +    ChardevCommon *common = backend ? backend->u.null.data : NULL;
> +
> +    if (common && common->logfile) {
> +        int flags = O_WRONLY;
> +        if (common->has_logappend &&
> +            common->logappend) {
> +            flags |= O_APPEND;
> +        } else {
> +            flags |= O_TRUNC;
> +        }
> +        chr->logfd = qemu_create(common->logfile, flags, 0666, errp);
> +        if (chr->logfd < 0) {
> +            return false;
> +        }
> +    }
> +
> +    return true;
> +}
> +
>  static Chardev *chardev_new(const char *id, const char *typename,
>                              ChardevBackend *backend,
>                              GMainContext *gcontext,
> @@ -1020,11 +1026,14 @@ static Chardev *chardev_new(const char *id, const char *typename,
>      chr->label = g_strdup(id);
>      chr->gcontext = gcontext;
>
> +    if (!chardev_init_logfd(chr, backend, errp)) {
> +        goto fail;
> +    }
> +
>      qemu_char_open(chr, backend, &be_opened, &local_err);
>      if (local_err) {
>          error_propagate(errp, local_err);
> -        object_unref(obj);
> -        return NULL;
> +        goto fail;
>      }
>
>      if (!chr->filename) {
> @@ -1035,6 +1044,10 @@ static Chardev *chardev_new(const char *id, const char *typename,
>      }
>
>      return chr;
> +
> +fail:
> +    object_unref(obj);
> +    return NULL;
>  }
>
>  Chardev *qemu_chardev_new(const char *id, const char *typename,
> --
> 2.48.1
>
>


--
Marc-André Lureau

Re: [PATCH v2 5/7] chardev/char: introduce .init() + .connect() initialization interface

[Marc-André Lureau]

On Mon, Oct 13, 2025 at 5:40 PM Vladimir Sementsov-Ogievskiy
<vsementsov@yandex-team.ru> wrote:
>
> We'll need a possibility to postpone connect step to later point in
> time to implement backend-transfer migration feature for vhost-user-blk
> in further commits. Let's start with new char interface for backends.
>
> .init() takes QAPI parameters and should parse them, called early
>
> .connect() should actually establish a connection, and postponed to
> the point of attaching to frontend. Called at later point, either
> at time of attaching frontend, either from qemu_chr_wait_connected().
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  chardev/char-fe.c      |  4 ++++
>  chardev/char.c         | 40 +++++++++++++++++++++++++++++++++++++---
>  include/chardev/char.h | 22 +++++++++++++++++++++-
>  3 files changed, 62 insertions(+), 4 deletions(-)
>
> diff --git a/chardev/char-fe.c b/chardev/char-fe.c
> index 158a5f4f55..973fed5bea 100644
> --- a/chardev/char-fe.c
> +++ b/chardev/char-fe.c
> @@ -193,6 +193,10 @@ bool qemu_chr_fe_init(CharBackend *b, Chardev *s, Error **errp)
>  {
>      unsigned int tag = 0;
>
> +    if (!qemu_chr_connect(s, errp)) {
> +        return false;
> +    }
> +
>      if (s) {
>          if (CHARDEV_IS_MUX(s)) {
>              MuxChardev *d = MUX_CHARDEV(s);
> diff --git a/chardev/char.c b/chardev/char.c
> index 6498d53daa..01ffe37acf 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -33,6 +33,7 @@
>  #include "qapi/error.h"
>  #include "qapi/qapi-commands-char.h"
>  #include "qapi/qmp/qerror.h"
> +#include "qom/object.h"
>  #include "system/replay.h"
>  #include "qemu/help_option.h"
>  #include "qemu/module.h"
> @@ -338,10 +339,29 @@ static bool qemu_chr_is_busy(Chardev *s)
>      }
>  }
>
> +bool qemu_chr_connect(Chardev *chr, Error **errp)
> +{
> +    ChardevClass *cc = CHARDEV_GET_CLASS(chr);
> +
> +    if (chr->connect_postponed) {
> +        assert(cc->connect);
> +        chr->connect_postponed = false;
> +        if (!cc->connect(chr, errp)) {
> +            return false;
> +        }
> +    }
> +
> +    return true;
> +}
> +
>  int qemu_chr_wait_connected(Chardev *chr, Error **errp)
>  {
>      ChardevClass *cc = CHARDEV_GET_CLASS(chr);
>
> +    if (!qemu_chr_connect(chr, errp)) {
> +        return -1;
> +    }
> +
>      if (cc->chr_wait_connected) {
>          return cc->chr_wait_connected(chr, errp);
>      }
> @@ -1029,7 +1049,7 @@ static Chardev *chardev_new(const char *id, const char *typename,
>  {
>      Object *obj;
>      Chardev *chr = NULL;
> -    Error *local_err = NULL;
> +    ChardevClass *cc;
>
>      assert(g_str_has_prefix(typename, "chardev-"));
>      assert(id);
> @@ -1044,8 +1064,22 @@ static Chardev *chardev_new(const char *id, const char *typename,
>          goto fail;
>      }
>
> -    if (!qemu_char_open(chr, backend, typename + 8, &local_err)) {
> -        goto fail;
> +    cc = CHARDEV_GET_CLASS(chr);
> +
> +    if (cc->init) {
> +        assert(!cc->open);
> +        assert(cc->connect);
> +
> +        if (!cc->init(chr, backend, errp)) {
> +            goto fail;
> +        }
> +        assert(chr->filename);
> +
> +        chr->connect_postponed = true;
> +    } else {
> +        if (!qemu_char_open(chr, backend, typename + 8, errp)) {
> +            goto fail;
> +        }
>      }
>
>      return chr;
> diff --git a/include/chardev/char.h b/include/chardev/char.h
> index 429852f8d9..ebadaf3482 100644
> --- a/include/chardev/char.h
> +++ b/include/chardev/char.h
> @@ -63,6 +63,7 @@ struct Chardev {
>      CharBackend *be;
>      char *label;
>      char *filename;
> +    bool connect_postponed;
>      int logfd;
>      int be_open;
>      /* used to coordinate the chardev-change special-case: */
> @@ -225,6 +226,7 @@ QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename,
>                                  bool permit_mux_mon);
>  int qemu_chr_write(Chardev *s, const uint8_t *buf, int len, bool write_all);
>  #define qemu_chr_write_all(s, buf, len) qemu_chr_write(s, buf, len, true)
> +bool qemu_chr_connect(Chardev *chr, Error **errp);
>  int qemu_chr_wait_connected(Chardev *chr, Error **errp);
>
>  #define TYPE_CHARDEV "chardev"
> @@ -259,10 +261,28 @@ struct ChardevClass {
>      /* parse command line options and populate QAPI @backend */
>      void (*parse)(QemuOpts *opts, ChardevBackend *backend, Error **errp);
>
> -    /* called after construction, open/starts the backend */
> +    /*
> +     * Called after construction, create and open/starts the backend,
> +     * mutual exclusive with .init. .connect must not be defined when
> +     * .open is defined.
> +     */
>      void (*open)(Chardev *chr, ChardevBackend *backend,
>                   bool *be_opened, Error **errp);
>
> +    /*
> +     * Called after construction, create the backend, mutual exclusive
> +     * with .open, and must be accompanied by .connect.
> +     * Must set chr-filename.
> +     */
> +    bool (*init)(Chardev *chr, ChardevBackend *backend,
> +                 Error **errp);
> +
> +    /*
> +     * Called after .init(), open/starts the backend, mutual exclusive
> +     * with .open. Must send CHR_EVENT_OPENED.
> +     */
> +    bool (*connect)(Chardev *chr, Error **errp);
> +
>      /* write buf to the backend */
>      int (*chr_write)(Chardev *s, const uint8_t *buf, int len);
>
> --
> 2.48.1
>
>


--
Marc-André Lureau

Re: [PATCH v2 4/7] chardev/char: move filename and be_opened handling to qemu_char_open()

[Marc-André Lureau]

On Mon, Oct 13, 2025 at 5:40 PM Vladimir Sementsov-Ogievskiy
<vsementsov@yandex-team.ru> wrote:
>
> Absent filename and necessity to send CHR_EVENT_OPENED are artifacts
> of .open(). Handle them in qemu_char_open().
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  chardev/char.c | 23 ++++++++++++-----------
>  1 file changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/chardev/char.c b/chardev/char.c
> index 64ec60c0f2..6498d53daa 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -247,18 +247,27 @@ int qemu_chr_add_client(Chardev *s, int fd)
>  }
>
>  static bool qemu_char_open(Chardev *chr, ChardevBackend *backend,
> -                           bool *be_opened, Error **errp)
> +                           const char *default_filename, Error **errp)
>  {
>      ERRP_GUARD();
>      ChardevClass *cc = CHARDEV_GET_CLASS(chr);
> +    bool be_opened = true;
>
>      if (cc->open) {
> -        cc->open(chr, backend, be_opened, errp);
> +        cc->open(chr, backend, &be_opened, errp);
>          if (*errp) {
>              return false;
>          }
>      }
>
> +    if (!chr->filename) {
> +        chr->filename = g_strdup(default_filename);
> +    }
> +
> +    if (be_opened) {
> +        qemu_chr_be_event(chr, CHR_EVENT_OPENED);
> +    }
> +
>      return true;
>  }
>
> @@ -1021,7 +1030,6 @@ static Chardev *chardev_new(const char *id, const char *typename,
>      Object *obj;
>      Chardev *chr = NULL;
>      Error *local_err = NULL;
> -    bool be_opened = true;
>
>      assert(g_str_has_prefix(typename, "chardev-"));
>      assert(id);
> @@ -1036,17 +1044,10 @@ static Chardev *chardev_new(const char *id, const char *typename,
>          goto fail;
>      }
>
> -    if (!qemu_char_open(chr, backend, &be_opened, &local_err)) {
> +    if (!qemu_char_open(chr, backend, typename + 8, &local_err)) {
>          goto fail;
>      }
>
> -    if (!chr->filename) {
> -        chr->filename = g_strdup(typename + 8);
> -    }
> -    if (be_opened) {
> -        qemu_chr_be_event(chr, CHR_EVENT_OPENED);
> -    }
> -
>      return chr;
>
>  fail:
> --
> 2.48.1
>
>


--
Marc-André Lureau

Re: [PATCH v2 6/7] chardev/char-socket: move to .init + .connect api

[Marc-André Lureau]

On Mon, Oct 13, 2025 at 5:40 PM Vladimir Sementsov-Ogievskiy
<vsementsov@yandex-team.ru> wrote:
>
> Move char-socket to new API. This will help to realize backend-transfer
> feature for vhost-user-blk.
>
> With this commit qemu_chr_fe_init() starts to do connecting, so we
> should handle its errors instead of passing &error_abort.
>
> Also, move qemu_chr_fe_init() in test-char.c, to trigger connect
> before trying to get address.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  chardev/char-socket.c         | 55 ++++++++++++++++++++---------------
>  chardev/char.c                |  7 +++--
>  include/chardev/char-socket.h |  1 +
>  tests/unit/test-char.c        | 14 ++++-----
>  ui/dbus-chardev.c             | 12 ++++++--
>  5 files changed, 54 insertions(+), 35 deletions(-)
>
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index f3bc6290d2..0a5738c158 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -1287,6 +1287,25 @@ static int qmp_chardev_open_socket_client(Chardev *chr,
>  }
>
>
> +static bool char_socket_connect(Chardev *chr, Error **errp)
> +{
> +    SocketChardev *s = SOCKET_CHARDEV(chr);
> +
> +    if (s->is_listen) {
> +        if (qmp_chardev_open_socket_server(chr, s->is_telnet || s->is_tn3270,
> +                                           s->is_waitconnect, errp) < 0) {
> +            return false;
> +        }
> +    } else {
> +        if (qmp_chardev_open_socket_client(chr, errp) < 0) {
> +            return false;
> +        }
> +    }
> +
> +    return true;
> +}
> +
> +
>  static bool qmp_chardev_validate_socket(ChardevSocket *sock,
>                                          SocketAddress *addr,
>                                          Error **errp)
> @@ -1363,10 +1382,9 @@ static bool qmp_chardev_validate_socket(ChardevSocket *sock,
>  }
>
>
> -static void qmp_chardev_open_socket(Chardev *chr,
> -                                    ChardevBackend *backend,
> -                                    bool *be_opened,
> -                                    Error **errp)
> +static bool char_socket_init(Chardev *chr,
> +                             ChardevBackend *backend,
> +                             Error **errp)
>  {
>      SocketChardev *s = SOCKET_CHARDEV(chr);
>      ChardevSocket *sock = backend->u.socket.data;
> @@ -1374,7 +1392,6 @@ static void qmp_chardev_open_socket(Chardev *chr,
>      bool is_listen      = sock->has_server  ? sock->server  : true;
>      bool is_telnet      = sock->has_telnet  ? sock->telnet  : false;
>      bool is_tn3270      = sock->has_tn3270  ? sock->tn3270  : false;
> -    bool is_waitconnect = sock->has_wait    ? sock->wait    : false;
>      bool is_websock     = sock->has_websocket ? sock->websocket : false;
>      SocketAddress *addr;
>
> @@ -1383,6 +1400,7 @@ static void qmp_chardev_open_socket(Chardev *chr,
>      s->is_tn3270 = is_tn3270;
>      s->is_websock = is_websock;
>      s->do_nodelay = do_nodelay;
> +    s->is_waitconnect = sock->has_wait ? sock->wait : false;
>      s->reconnect_time_ms = sock->has_reconnect_ms ? sock->reconnect_ms : 0;
>
>      if (sock->tls_creds) {
> @@ -1392,7 +1410,7 @@ static void qmp_chardev_open_socket(Chardev *chr,
>          if (!creds) {
>              error_setg(errp, "No TLS credentials with id '%s'",
>                         sock->tls_creds);
> -            return;
> +            return false;
>          }
>          s->tls_creds = (QCryptoTLSCreds *)
>              object_dynamic_cast(creds,
> @@ -1400,7 +1418,7 @@ static void qmp_chardev_open_socket(Chardev *chr,
>          if (!s->tls_creds) {
>              error_setg(errp, "Object with id '%s' is not TLS credentials",
>                         sock->tls_creds);
> -            return;
> +            return false;
>          }
>          object_ref(OBJECT(s->tls_creds));
>          if (!qcrypto_tls_creds_check_endpoint(s->tls_creds,
> @@ -1408,7 +1426,7 @@ static void qmp_chardev_open_socket(Chardev *chr,
>                                            ? QCRYPTO_TLS_CREDS_ENDPOINT_SERVER
>                                            : QCRYPTO_TLS_CREDS_ENDPOINT_CLIENT,
>                                            errp)) {
> -            return;
> +            return false;
>          }
>      }
>      s->tls_authz = g_strdup(sock->tls_authz);
> @@ -1416,7 +1434,7 @@ static void qmp_chardev_open_socket(Chardev *chr,
>      s->addr = addr = socket_address_flatten(sock->addr);
>
>      if (!qmp_chardev_validate_socket(sock, addr, errp)) {
> -        return;
> +        return false;
>      }
>
>      qemu_chr_set_feature(chr, QEMU_CHAR_FEATURE_RECONNECTABLE);
> @@ -1433,26 +1451,14 @@ static void qmp_chardev_open_socket(Chardev *chr,
>       */
>      if (!chr->handover_yank_instance) {
>          if (!yank_register_instance(CHARDEV_YANK_INSTANCE(chr->label), errp)) {
> -            return;
> +            return false;
>          }
>      }
>      s->registered_yank = true;
>
> -    /* be isn't opened until we get a connection */
> -    *be_opened = false;
> -
>      update_disconnected_filename(s);
>
> -    if (s->is_listen) {
> -        if (qmp_chardev_open_socket_server(chr, is_telnet || is_tn3270,
> -                                           is_waitconnect, errp) < 0) {
> -            return;
> -        }
> -    } else {
> -        if (qmp_chardev_open_socket_client(chr, errp) < 0) {
> -            return;
> -        }
> -    }
> +    return true;
>  }
>
>  static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,
> @@ -1576,7 +1582,8 @@ static void char_socket_class_init(ObjectClass *oc, const void *data)
>      cc->supports_yank = true;
>
>      cc->parse = qemu_chr_parse_socket;
> -    cc->open = qmp_chardev_open_socket;
> +    cc->init = char_socket_init;
> +    cc->connect = char_socket_connect;
>      cc->chr_wait_connected = tcp_chr_wait_connected;
>      cc->chr_write = tcp_chr_write;
>      cc->chr_sync_read = tcp_chr_sync_read;
> diff --git a/chardev/char.c b/chardev/char.c
> index 01ffe37acf..4d151f537c 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -1221,12 +1221,15 @@ ChardevReturn *qmp_chardev_change(const char *id, ChardevBackend *backend,
>      }
>
>      chr->be = NULL;
> -    qemu_chr_fe_init(be, chr_new, &error_abort);
> +    if (!qemu_chr_fe_init(be, chr_new, errp)) {
> +        object_unref(OBJECT(chr_new));
> +        return NULL;
> +    }
>
>      if (be->chr_be_change(be->opaque) < 0) {
>          error_setg(errp, "Chardev '%s' change failed", chr_new->label);
>          chr_new->be = NULL;
> -        qemu_chr_fe_init(be, chr, &error_abort);
> +        qemu_chr_fe_init(be, chr, NULL);
>          if (closed_sent) {
>              qemu_chr_be_event(chr, CHR_EVENT_OPENED);
>          }
> diff --git a/include/chardev/char-socket.h b/include/chardev/char-socket.h
> index d6d13ad37f..0109727eaa 100644
> --- a/include/chardev/char-socket.h
> +++ b/include/chardev/char-socket.h
> @@ -68,6 +68,7 @@ struct SocketChardev {
>      bool is_listen;
>      bool is_telnet;
>      bool is_tn3270;
> +    bool is_waitconnect;
>      GSource *telnet_source;
>      TCPChardevTelnetInit *telnet_init;
>
> diff --git a/tests/unit/test-char.c b/tests/unit/test-char.c
> index f30a39f61f..5c9482a478 100644
> --- a/tests/unit/test-char.c
> +++ b/tests/unit/test-char.c
> @@ -845,6 +845,7 @@ static void char_websock_test(void)
>                                0xef, 0xaa, 0xc5, 0x97, /* Masking key */
>                                0xec, 0x42              /* Status code */ };
>
> +    qemu_chr_fe_init(&be, chr, &error_abort);
>      addr = object_property_get_qobject(OBJECT(chr), "addr", &error_abort);
>      qdict = qobject_to(QDict, addr);
>      port = qdict_get_str(qdict, "port");
> @@ -852,7 +853,6 @@ static void char_websock_test(void)
>      handshake_port = g_strdup_printf(handshake, port, port);
>      qobject_unref(qdict);
>
> -    qemu_chr_fe_init(&be, chr, &error_abort);
>      qemu_chr_fe_set_handlers(&be, websock_server_can_read, websock_server_read,
>                               NULL, NULL, chr, NULL, true);
>
> @@ -1216,6 +1216,8 @@ static void char_socket_server_test(gconstpointer opaque)
>      g_assert_nonnull(chr);
>      g_assert(!object_property_get_bool(OBJECT(chr), "connected", &error_abort));
>
> +    qemu_chr_fe_init(&be, chr, &error_abort);
> +
>      qaddr = object_property_get_qobject(OBJECT(chr), "addr", &error_abort);
>      g_assert_nonnull(qaddr);
>
> @@ -1224,8 +1226,6 @@ static void char_socket_server_test(gconstpointer opaque)
>      visit_free(v);
>      qobject_unref(qaddr);
>
> -    qemu_chr_fe_init(&be, chr, &error_abort);
> -
>   reconnect:
>      data.event = -1;
>      data.be = &be;
> @@ -1417,6 +1417,8 @@ static void char_socket_client_test(gconstpointer opaque)
>      qemu_opts_del(opts);
>      g_assert_nonnull(chr);
>
> +    qemu_chr_fe_init(&be, chr, &error_abort);
> +
>      if (config->reconnect) {
>          /*
>           * If reconnect is set, the connection will be
> @@ -1431,8 +1433,6 @@ static void char_socket_client_test(gconstpointer opaque)
>                                            &error_abort));
>      }
>
> -    qemu_chr_fe_init(&be, chr, &error_abort);
> -
>   reconnect:
>      data.event = -1;
>      data.be = &be;
> @@ -1550,6 +1550,8 @@ static void char_socket_server_two_clients_test(gconstpointer opaque)
>      g_assert_nonnull(chr);
>      g_assert(!object_property_get_bool(OBJECT(chr), "connected", &error_abort));
>
> +    qemu_chr_fe_init(&be, chr, &error_abort);
> +
>      qaddr = object_property_get_qobject(OBJECT(chr), "addr", &error_abort);
>      g_assert_nonnull(qaddr);
>
> @@ -1558,8 +1560,6 @@ static void char_socket_server_two_clients_test(gconstpointer opaque)
>      visit_free(v);
>      qobject_unref(qaddr);
>
> -    qemu_chr_fe_init(&be, chr, &error_abort);
> -
>      qemu_chr_fe_set_handlers(&be, char_socket_can_read, char_socket_discard_read,
>                               count_closed_event, NULL,
>                               &closed, NULL, true);
> diff --git a/ui/dbus-chardev.c b/ui/dbus-chardev.c
> index d05dddaf81..23cf9d6ee9 100644
> --- a/ui/dbus-chardev.c
> +++ b/ui/dbus-chardev.c
> @@ -210,8 +210,14 @@ dbus_chr_open(Chardev *chr, ChardevBackend *backend,
>      if (*errp) {
>          return;
>      }
> -    CHARDEV_CLASS(object_class_by_name(TYPE_CHARDEV_SOCKET))->open(
> -        chr, be, be_opened, errp);
> +    if (!CHARDEV_CLASS(object_class_by_name(TYPE_CHARDEV_SOCKET))->init(
> +        chr, be, errp)) {
> +        return;
> +    }
> +    if (!CHARDEV_CLASS(object_class_by_name(TYPE_CHARDEV_SOCKET))->connect(
> +        chr, errp)) {
> +        return;
> +    }
>  }
>
>  static void
> @@ -276,6 +282,8 @@ char_dbus_class_init(ObjectClass *oc, const void *data)
>
>      cc->parse = dbus_chr_parse;
>      cc->open = dbus_chr_open;
> +    cc->init = NULL;
> +    cc->connect = NULL;
>      cc->chr_set_fe_open = dbus_chr_set_fe_open;
>      cc->chr_set_echo = dbus_chr_set_echo;
>      klass->parent_chr_be_event = cc->chr_be_event;
> --
> 2.48.1
>
>


--
Marc-André Lureau

Re: [PATCH v2 1/7] chardev/char-socket: simplify reconnect-ms handling

[Marc-André Lureau]

On Mon, Oct 13, 2025 at 5:39 PM Vladimir Sementsov-Ogievskiy
<vsementsov@yandex-team.ru> wrote:
>
> We pass it to qmp_chardev_open_socket_client() only to write
> to s->reconnect_time_ms. Let's simply set this field earlier,
> together with other options.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  chardev/char-socket.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index 62852e3caf..f3bc6290d2 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -1274,18 +1274,16 @@ skip_listen:
>
>
>  static int qmp_chardev_open_socket_client(Chardev *chr,
> -                                          int64_t reconnect_ms,
>                                            Error **errp)
>  {
>      SocketChardev *s = SOCKET_CHARDEV(chr);
>
> -    if (reconnect_ms > 0) {
> -        s->reconnect_time_ms = reconnect_ms;
> +    if (s->reconnect_time_ms > 0) {
>          tcp_chr_connect_client_async(chr);
>          return 0;
> -    } else {
> -        return tcp_chr_connect_client_sync(chr, errp);
>      }
> +
> +    return tcp_chr_connect_client_sync(chr, errp);
>  }
>
>
> @@ -1378,7 +1376,6 @@ static void qmp_chardev_open_socket(Chardev *chr,
>      bool is_tn3270      = sock->has_tn3270  ? sock->tn3270  : false;
>      bool is_waitconnect = sock->has_wait    ? sock->wait    : false;
>      bool is_websock     = sock->has_websocket ? sock->websocket : false;
> -    int64_t reconnect_ms = sock->has_reconnect_ms ? sock->reconnect_ms : 0;
>      SocketAddress *addr;
>
>      s->is_listen = is_listen;
> @@ -1386,6 +1383,8 @@ static void qmp_chardev_open_socket(Chardev *chr,
>      s->is_tn3270 = is_tn3270;
>      s->is_websock = is_websock;
>      s->do_nodelay = do_nodelay;
> +    s->reconnect_time_ms = sock->has_reconnect_ms ? sock->reconnect_ms : 0;
> +
>      if (sock->tls_creds) {
>          Object *creds;
>          creds = object_resolve_path_component(
> @@ -1450,7 +1449,7 @@ static void qmp_chardev_open_socket(Chardev *chr,
>              return;
>          }
>      } else {
> -        if (qmp_chardev_open_socket_client(chr, reconnect_ms, errp) < 0) {
> +        if (qmp_chardev_open_socket_client(chr, errp) < 0) {
>              return;
>          }
>      }
> --
> 2.48.1
>
>


--
Marc-André Lureau

Re: [PATCH v2 3/7] chardev/char: qemu_char_open(): add return value

[Vladimir Sementsov-Ogievskiy]

On 14.10.25 10:10, Marc-André Lureau wrote:
> Hi
> 
> On Mon, Oct 13, 2025 at 5:40 PM Vladimir Sementsov-Ogievskiy
> <vsementsov@yandex-team.ru> wrote:
>>
>> Accordingly with recommendations in include/qapi/error.h accompany
>> errp by boolean return value and get rid of error propagation.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>> ---
>>   chardev/char.c | 12 ++++++++----
>>   1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/chardev/char.c b/chardev/char.c
>> index d5a2533e8e..64ec60c0f2 100644
>> --- a/chardev/char.c
>> +++ b/chardev/char.c
>> @@ -246,14 +246,20 @@ int qemu_chr_add_client(Chardev *s, int fd)
>>           CHARDEV_GET_CLASS(s)->chr_add_client(s, fd) : -1;
>>   }
>>
>> -static void qemu_char_open(Chardev *chr, ChardevBackend *backend,
>> +static bool qemu_char_open(Chardev *chr, ChardevBackend *backend,
>>                              bool *be_opened, Error **errp)
>>   {
>> +    ERRP_GUARD();
>>       ChardevClass *cc = CHARDEV_GET_CLASS(chr);
>>
>>       if (cc->open) {
>>           cc->open(chr, backend, be_opened, errp);
>> +        if (*errp) {
>> +            return false;
>> +        }
>>       }
>> +
>> +    return true;
>>   }
>>
>>   static void char_init(Object *obj)
>> @@ -1030,9 +1036,7 @@ static Chardev *chardev_new(const char *id, const char *typename,
>>           goto fail;
>>       }
>>
>> -    qemu_char_open(chr, backend, &be_opened, &local_err);
>> -    if (local_err) {
>> -        error_propagate(errp, local_err);
>> +    if (!qemu_char_open(chr, backend, &be_opened, &local_err)) {
> 
> You meant to pass errp instead, since you dropped error_propagate ?
> 

Oops, right

>>           goto fail;
>>       }
>>
>> --
>> 2.48.1
>>
>>
> 
> 


-- 
Best regards,
Vladimir

Re: [PATCH v2 7/7] char: vhost-user-blk call connect by hand

[Vladimir Sementsov-Ogievskiy]

On 14.10.25 10:30, Marc-André Lureau wrote:
> Hi
> 
> On Mon, Oct 13, 2025 at 5:39 PM Vladimir Sementsov-Ogievskiy
> <vsementsov@yandex-team.ru> wrote:
>>
>> Give vhost-user-blk a possibility (and responsibility) to decide when
>> do connect. It will help us to realize vhost-user-blk backend-transfer
>> migration feature:
>>
>> For incoming migration we'll need to postpone connect at least until
>> early stage of migrate-incoming command, when we already know all
>> migration parameters and can decide, are we going to do incoming
>> backend-transfer (and get chardev fd from incoming stream), or we
>> finally need to connect.
>>
>> With this patch, we don't modify vhost-user-blk.c: it already
>> do call qemu_chr_fe_wait_connected(), which in turn calls
>> qemu_chr_connect().
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>> ---
>>   chardev/char-fe.c                | 21 +++++++++++++++++----
>>   hw/core/qdev-properties-system.c |  2 +-
>>   include/chardev/char-fe.h        |  2 ++
>>   3 files changed, 20 insertions(+), 5 deletions(-)
>>
>> diff --git a/chardev/char-fe.c b/chardev/char-fe.c
>> index 973fed5bea..9e2c658cb9 100644
>> --- a/chardev/char-fe.c
>> +++ b/chardev/char-fe.c
>> @@ -189,12 +189,20 @@ bool qemu_chr_fe_backend_open(CharBackend *be)
>>       return be->chr && be->chr->be_open;
>>   }
>>
>> -bool qemu_chr_fe_init(CharBackend *b, Chardev *s, Error **errp)
>> +bool qemu_chr_fe_init_ex(CharBackend *b, Chardev *s, ObjectClass *oc,
>> +                         Error **errp)
>>   {
>>       unsigned int tag = 0;
>> -
>> -    if (!qemu_chr_connect(s, errp)) {
>> -        return false;
>> +    const char *driver = oc ? object_class_get_name(oc) : NULL;
>> +
>> +    /*
>> +     * vhost-user-blk wants call qemu_chr_connect by hand,
>> +     * to support backend-transfer migration feature.
>> +     */
>> +    if (!driver || !g_str_has_prefix(driver, "vhost-user-blk")) {
>> +        if (!qemu_chr_connect(s, errp)) {
>> +            return false;
>> +        }
> 
> Why not pass a bool "connect" instead?
> 
> DEFINE_PROP_CHR() would use "true".
> 
> + Introduce DEFINE_PROP_CHR_NO_CONNECT() that would use "false".
> 
> and wire it with a new PropertyInfo and modified callbacks.
> 

Will try, thanks for idea. I don't like this checking by name too, just design it after
.check_peer_type in net/ code.

>>       }
>>
>>       if (s) {
>> @@ -218,6 +226,11 @@ bool qemu_chr_fe_init(CharBackend *b, Chardev *s, Error **errp)
>>       return true;
>>   }
>>
>> +bool qemu_chr_fe_init(CharBackend *b, Chardev *s, Error **errp)
>> +{
>> +    return qemu_chr_fe_init_ex(b, s, NULL, errp);
>> +}
>> +
>>   void qemu_chr_fe_deinit(CharBackend *b, bool del)
>>   {
>>       assert(b);
>> diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
>> index 1f810b7ddf..35eed17d4d 100644
>> --- a/hw/core/qdev-properties-system.c
>> +++ b/hw/core/qdev-properties-system.c
>> @@ -297,7 +297,7 @@ static void set_chr(Object *obj, Visitor *v, const char *name, void *opaque,
>>       if (s == NULL) {
>>           error_setg(errp, "Property '%s.%s' can't find value '%s'",
>>                      object_get_typename(obj), name, str);
>> -    } else if (!qemu_chr_fe_init(be, s, errp)) {
>> +    } else if (!qemu_chr_fe_init_ex(be, s, obj->class, errp)) {
>>           error_prepend(errp, "Property '%s.%s' can't take value '%s': ",
>>                         object_get_typename(obj), name, str);
>>       }
>> diff --git a/include/chardev/char-fe.h b/include/chardev/char-fe.h
>> index 8ef05b3dd0..f1c05cc5ed 100644
>> --- a/include/chardev/char-fe.h
>> +++ b/include/chardev/char-fe.h
>> @@ -34,6 +34,8 @@ struct CharBackend {
>>    * Returns: false on error.
>>    */
>>   bool qemu_chr_fe_init(CharBackend *b, Chardev *s, Error **errp);
>> +bool qemu_chr_fe_init_ex(CharBackend *b, Chardev *s, ObjectClass *oc,
>> +                         Error **errp);
>>
>>   /**
>>    * qemu_chr_fe_deinit:
>> --
>> 2.48.1
>>
>>
> 
> 


-- 
Best regards,
Vladimir

Re: [PATCH v2 2/7] chardev/char: split chardev_init_logfd() out of qemu_char_open()

[Vladimir Sementsov-Ogievskiy]

On 14.10.25 10:30, Marc-André Lureau wrote:
> On Mon, Oct 13, 2025 at 5:41 PM Vladimir Sementsov-Ogievskiy
> <vsementsov@yandex-team.ru> wrote:
>>
>> We are going to share new chardev_init_logfd() with further
>> alternative initialization interface. Let qemu_char_open() be
>> a wrapper for .open(), and its artifacts (handle be_opened if
>> was not set to false by backend, and filename).
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> 
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
>> ---
>>   chardev/char.c | 49 +++++++++++++++++++++++++++++++------------------
>>   1 file changed, 31 insertions(+), 18 deletions(-)
>>
>> diff --git a/chardev/char.c b/chardev/char.c
>> index a43b7e5481..d5a2533e8e 100644
>> --- a/chardev/char.c
>> +++ b/chardev/char.c
>> @@ -250,22 +250,6 @@ static void qemu_char_open(Chardev *chr, ChardevBackend *backend,
>>                              bool *be_opened, Error **errp)
>>   {
>>       ChardevClass *cc = CHARDEV_GET_CLASS(chr);
>> -    /* Any ChardevCommon member would work */
> 
> maybe keep that comment?

I a bit don't follow it, honestly.. What it mean? That we should
handle members of common structure here?

Not a problem to put it back to chardev_init_logfd().. But maybe, it
then should be renamed to chardev_init_common()

> 
> 
>> -    ChardevCommon *common = backend ? backend->u.null.data : NULL;
>> -
>> -    if (common && common->logfile) {
>> -        int flags = O_WRONLY;
>> -        if (common->has_logappend &&
>> -            common->logappend) {
>> -            flags |= O_APPEND;
>> -        } else {
>> -            flags |= O_TRUNC;
>> -        }
>> -        chr->logfd = qemu_create(common->logfile, flags, 0666, errp);
>> -        if (chr->logfd < 0) {
>> -            return;
>> -        }
>> -    }
>>
>>       if (cc->open) {
>>           cc->open(chr, backend, be_opened, errp);
>> @@ -1000,6 +984,28 @@ void qemu_chr_set_feature(Chardev *chr,
>>       return set_bit(feature, chr->features);
>>   }
>>
>> +static bool chardev_init_logfd(Chardev *chr, ChardevBackend *backend,
>> +                                Error **errp)
>> +{
>> +    ChardevCommon *common = backend ? backend->u.null.data : NULL;
>> +
>> +    if (common && common->logfile) {
>> +        int flags = O_WRONLY;
>> +        if (common->has_logappend &&
>> +            common->logappend) {
>> +            flags |= O_APPEND;
>> +        } else {
>> +            flags |= O_TRUNC;
>> +        }
>> +        chr->logfd = qemu_create(common->logfile, flags, 0666, errp);
>> +        if (chr->logfd < 0) {
>> +            return false;
>> +        }
>> +    }
>> +
>> +    return true;
>> +}
>> +
>>   static Chardev *chardev_new(const char *id, const char *typename,
>>                               ChardevBackend *backend,
>>                               GMainContext *gcontext,
>> @@ -1020,11 +1026,14 @@ static Chardev *chardev_new(const char *id, const char *typename,
>>       chr->label = g_strdup(id);
>>       chr->gcontext = gcontext;
>>
>> +    if (!chardev_init_logfd(chr, backend, errp)) {
>> +        goto fail;
>> +    }
>> +
>>       qemu_char_open(chr, backend, &be_opened, &local_err);
>>       if (local_err) {
>>           error_propagate(errp, local_err);
>> -        object_unref(obj);
>> -        return NULL;
>> +        goto fail;
>>       }
>>
>>       if (!chr->filename) {
>> @@ -1035,6 +1044,10 @@ static Chardev *chardev_new(const char *id, const char *typename,
>>       }
>>
>>       return chr;
>> +
>> +fail:
>> +    object_unref(obj);
>> +    return NULL;
>>   }
>>
>>   Chardev *qemu_chardev_new(const char *id, const char *typename,
>> --
>> 2.48.1
>>
>>
> 
> 
> --
> Marc-André Lureau


-- 
Best regards,
Vladimir

Re: [PATCH v2 0/7] chardev: postpone connect

[Daniel P. Berrangé]

On Mon, Oct 13, 2025 at 04:38:29PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
> 
> That's a preparation for backend-transfer migration of
> vhost-user-blk. For such migration we are going to transfer
> vhost-user-blk fds, including backend chardev fd to the target
> in migration stream (backed by UNIX domain socket).
> 
> So, on the target, we want to know, should we call connect(),
> or is it a backend-transfer migration, and we should wait for
> incoming fd.
> 
> But at initialization time we can't know it: user may setup
> migration parameters (enabling backend-transfer) later.
> 
> So, let's postpone chardev open/connect phase up to attaching
> to frontend. At this point we can check:
> 
> - if it's vhost-user-blk, do nothing, let vhost-user-blk decide
>   when to do connect()
> - otherwise, do connect() at this point

I'm finding it quite unpleasant that we've created a new set of
callbacks just for the socket backend, and not the other chardev
backends.

Conceptually it feels like the problem of transferring in pre-
opened FDs from a previous QEMU should be conceptually relevant
to all the backend types. If it is, then I very much want us to
convert all the backends instead of leaving a pile of technical
debt for someone else in the future.

This series also doesn't illustrate usage of the new model with
pre-opened FDs, so I'm finding it hard to validate whether
this design is effective or not.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v2 2/7] chardev/char: split chardev_init_logfd() out of qemu_char_open()

[Marc-André Lureau]

Hi

On Tue, Oct 14, 2025 at 1:21 PM Vladimir Sementsov-Ogievskiy
<vsementsov@yandex-team.ru> wrote:
>
> On 14.10.25 10:30, Marc-André Lureau wrote:
> > On Mon, Oct 13, 2025 at 5:41 PM Vladimir Sementsov-Ogievskiy
> > <vsementsov@yandex-team.ru> wrote:
> >>
> >> We are going to share new chardev_init_logfd() with further
> >> alternative initialization interface. Let qemu_char_open() be
> >> a wrapper for .open(), and its artifacts (handle be_opened if
> >> was not set to false by backend, and filename).
> >>
> >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> >
> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> >> ---
> >>   chardev/char.c | 49 +++++++++++++++++++++++++++++++------------------
> >>   1 file changed, 31 insertions(+), 18 deletions(-)
> >>
> >> diff --git a/chardev/char.c b/chardev/char.c
> >> index a43b7e5481..d5a2533e8e 100644
> >> --- a/chardev/char.c
> >> +++ b/chardev/char.c
> >> @@ -250,22 +250,6 @@ static void qemu_char_open(Chardev *chr, ChardevBackend *backend,
> >>                              bool *be_opened, Error **errp)
> >>   {
> >>       ChardevClass *cc = CHARDEV_GET_CLASS(chr);
> >> -    /* Any ChardevCommon member would work */
> >
> > maybe keep that comment?
>
> I a bit don't follow it, honestly.. What it mean? That we should
> handle members of common structure here?

The ChardevBackend type is a union, and all the members inherit from
ChardevCommon. Ideally, we wouldn't cast this way, but that would
require some changes in code generator...

>
> Not a problem to put it back to chardev_init_logfd().. But maybe, it
> then should be renamed to chardev_init_common()
>
> >
> >
> >> -    ChardevCommon *common = backend ? backend->u.null.data : NULL;
> >> -
> >> -    if (common && common->logfile) {
> >> -        int flags = O_WRONLY;
> >> -        if (common->has_logappend &&
> >> -            common->logappend) {
> >> -            flags |= O_APPEND;
> >> -        } else {
> >> -            flags |= O_TRUNC;
> >> -        }
> >> -        chr->logfd = qemu_create(common->logfile, flags, 0666, errp);
> >> -        if (chr->logfd < 0) {
> >> -            return;
> >> -        }
> >> -    }
> >>
> >>       if (cc->open) {
> >>           cc->open(chr, backend, be_opened, errp);
> >> @@ -1000,6 +984,28 @@ void qemu_chr_set_feature(Chardev *chr,
> >>       return set_bit(feature, chr->features);
> >>   }
> >>
> >> +static bool chardev_init_logfd(Chardev *chr, ChardevBackend *backend,
> >> +                                Error **errp)
> >> +{
> >> +    ChardevCommon *common = backend ? backend->u.null.data : NULL;
> >> +
> >> +    if (common && common->logfile) {
> >> +        int flags = O_WRONLY;
> >> +        if (common->has_logappend &&
> >> +            common->logappend) {
> >> +            flags |= O_APPEND;
> >> +        } else {
> >> +            flags |= O_TRUNC;
> >> +        }
> >> +        chr->logfd = qemu_create(common->logfile, flags, 0666, errp);
> >> +        if (chr->logfd < 0) {
> >> +            return false;
> >> +        }
> >> +    }
> >> +
> >> +    return true;
> >> +}
> >> +
> >>   static Chardev *chardev_new(const char *id, const char *typename,
> >>                               ChardevBackend *backend,
> >>                               GMainContext *gcontext,
> >> @@ -1020,11 +1026,14 @@ static Chardev *chardev_new(const char *id, const char *typename,
> >>       chr->label = g_strdup(id);
> >>       chr->gcontext = gcontext;
> >>
> >> +    if (!chardev_init_logfd(chr, backend, errp)) {
> >> +        goto fail;
> >> +    }
> >> +
> >>       qemu_char_open(chr, backend, &be_opened, &local_err);
> >>       if (local_err) {
> >>           error_propagate(errp, local_err);
> >> -        object_unref(obj);
> >> -        return NULL;
> >> +        goto fail;
> >>       }
> >>
> >>       if (!chr->filename) {
> >> @@ -1035,6 +1044,10 @@ static Chardev *chardev_new(const char *id, const char *typename,
> >>       }
> >>
> >>       return chr;
> >> +
> >> +fail:
> >> +    object_unref(obj);
> >> +    return NULL;
> >>   }
> >>
> >>   Chardev *qemu_chardev_new(const char *id, const char *typename,
> >> --
> >> 2.48.1
> >>
> >>
> >
> >
> > --
> > Marc-André Lureau
>
>
> --
> Best regards,
> Vladimir



-- 
Marc-André Lureau

Re: [PATCH v2 0/7] chardev: postpone connect

[Vladimir Sementsov-Ogievskiy]

On 14.10.25 16:40, Daniel P. Berrangé wrote:
> On Mon, Oct 13, 2025 at 04:38:29PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> Hi all!
>>
>> That's a preparation for backend-transfer migration of
>> vhost-user-blk. For such migration we are going to transfer
>> vhost-user-blk fds, including backend chardev fd to the target
>> in migration stream (backed by UNIX domain socket).
>>
>> So, on the target, we want to know, should we call connect(),
>> or is it a backend-transfer migration, and we should wait for
>> incoming fd.
>>
>> But at initialization time we can't know it: user may setup
>> migration parameters (enabling backend-transfer) later.
>>
>> So, let's postpone chardev open/connect phase up to attaching
>> to frontend. At this point we can check:
>>
>> - if it's vhost-user-blk, do nothing, let vhost-user-blk decide
>>    when to do connect()
>> - otherwise, do connect() at this point
> 
> I'm finding it quite unpleasant that we've created a new set of
> callbacks just for the socket backend, and not the other chardev
> backends.
> 
> Conceptually it feels like the problem of transferring in pre-
> opened FDs from a previous QEMU should be conceptually relevant
> to all the backend types. If it is, then I very much want us to
> convert all the backends instead of leaving a pile of technical
> debt for someone else in the future.

Agree. If the design is approved, I'll try to update all other
chardevs as well.

> 
> This series also doesn't illustrate usage of the new model with
> pre-opened FDs, so I'm finding it hard to validate whether
> this design is effective or not.
> 

Understand, of course.

[PATCH v2 00/?] vhost-user-blk: live-backend local migration

is coming soon, so the whole picture will be seen.

Still, I'll first resend this one (chardev: postpone connect) and

[PATCH v2 00/23] vhost refactoring and fixes

to base future v2 vhost-user-blk: live-backend local migration
on clear base.

-- 
Best regards,
Vladimir