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

Vladimir Sementsov-Ogievskiy posted 7 patches 2 weeks ago
Maintainers: "Marc-André Lureau" <marcandre.lureau@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Eduardo Habkost <eduardo@habkost.net>
There is a newer version of this series
[PATCH v5 5/7] chardev/char: introduce .init() + .connect() initialization interface
Posted by Vladimir Sementsov-Ogievskiy 2 weeks ago
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, or 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         | 39 +++++++++++++++++++++++++++++++++++++--
 include/chardev/char.h | 30 +++++++++++++++++++++++++++++-
 3 files changed, 70 insertions(+), 3 deletions(-)

diff --git a/chardev/char-fe.c b/chardev/char-fe.c
index 34b83fc1c4..c67b4d640f 100644
--- a/chardev/char-fe.c
+++ b/chardev/char-fe.c
@@ -193,6 +193,10 @@ bool qemu_chr_fe_init(CharFrontend *c, 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 7a029b0eb9..33eba0cdc0 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);
     }
@@ -1030,6 +1050,7 @@ static Chardev *chardev_new(const char *id, const char *typename,
 {
     Object *obj;
     Chardev *chr = NULL;
+    ChardevClass *cc;
 
     assert(g_str_has_prefix(typename, "chardev-"));
     assert(id);
@@ -1044,8 +1065,22 @@ static Chardev *chardev_new(const char *id, const char *typename,
         goto fail;
     }
 
-    if (!qemu_char_open(chr, backend, typename + 8, errp)) {
-        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 b65e9981c1..24c449b07b 100644
--- a/include/chardev/char.h
+++ b/include/chardev/char.h
@@ -63,6 +63,7 @@ struct Chardev {
     CharFrontend *fe;
     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,7 +261,33 @@ 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 the backend, mutually exclusive
+     * with @open, and should be followed by @connect().
+     * Must set the Chardev's chr->filename on success.
+     */
+    bool (*init)(Chardev *chr, ChardevBackend *backend,
+                 Error **errp);
+
+    /*
+     * Called after @init(), starts the backend, mutually exclusive
+     * with @open. Should care to send CHR_EVENT_OPENED when connected
+     * (this may happen asynchronously after .connect() call alredy
+     * returned).
+     */
+    bool (*connect)(Chardev *chr, Error **errp);
+
+    /*
+     * Called after construction, an alternative to @init + @connect
+     * and should do the work for both: create and start the backend.
+     * Mutually exclusive with @init and @connect.
+     *
+     * May not set the Chardev's chr->filename (generic code will care),
+     * and may not send CHR_EVENT_OPENED when connected (@be_opened
+     * should not be touched in this case, to signal the generic code
+     * to care about CHR_EVENT_OPENED). If the backend cares about
+     * CHR_EVENT_OPENED, it should set @be_opened to false.
+     */
     void (*open)(Chardev *chr, ChardevBackend *backend,
                  bool *be_opened, Error **errp);
 
-- 
2.48.1


Re: [PATCH v5 5/7] chardev/char: introduce .init() + .connect() initialization interface
Posted by Marc-André Lureau 1 week, 5 days ago
Hi

On Fri, Oct 31, 2025 at 7:59 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, or 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         | 39 +++++++++++++++++++++++++++++++++++++--
>  include/chardev/char.h | 30 +++++++++++++++++++++++++++++-
>  3 files changed, 70 insertions(+), 3 deletions(-)
>
> diff --git a/chardev/char-fe.c b/chardev/char-fe.c
> index 34b83fc1c4..c67b4d640f 100644
> --- a/chardev/char-fe.c
> +++ b/chardev/char-fe.c
> @@ -193,6 +193,10 @@ bool qemu_chr_fe_init(CharFrontend *c, Chardev *s,
> Error **errp)
>  {
>      unsigned int tag = 0;
>
> +    if (!qemu_chr_connect(s, errp)) {
> +        return false;
> +    }
>

this should be after "if (s)" condition.

It is moved in a later patch, but better do it now.


> +
>      if (s) {
>          if (CHARDEV_IS_MUX(s)) {
>              MuxChardev *d = MUX_CHARDEV(s);
> diff --git a/chardev/char.c b/chardev/char.c
> index 7a029b0eb9..33eba0cdc0 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);
>      }
> @@ -1030,6 +1050,7 @@ static Chardev *chardev_new(const char *id, const
> char *typename,
>  {
>      Object *obj;
>      Chardev *chr = NULL;
> +    ChardevClass *cc;
>
>      assert(g_str_has_prefix(typename, "chardev-"));
>      assert(id);
> @@ -1044,8 +1065,22 @@ static Chardev *chardev_new(const char *id, const
> char *typename,
>          goto fail;
>      }
>
> -    if (!qemu_char_open(chr, backend, typename + 8, errp)) {
> -        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 b65e9981c1..24c449b07b 100644
> --- a/include/chardev/char.h
> +++ b/include/chardev/char.h
> @@ -63,6 +63,7 @@ struct Chardev {
>      CharFrontend *fe;
>      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,7 +261,33 @@ 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 the backend, mutually exclusive
> +     * with @open, and should be followed by @connect().
> +     * Must set the Chardev's chr->filename on success.
> +     */
> +    bool (*init)(Chardev *chr, ChardevBackend *backend,
> +                 Error **errp);
> +
> +    /*
> +     * Called after @init(), starts the backend, mutually exclusive
> +     * with @open. Should care to send CHR_EVENT_OPENED when connected
> +     * (this may happen asynchronously after .connect() call alredy
> +     * returned).
> +     */
> +    bool (*connect)(Chardev *chr, Error **errp);
> +
> +    /*
> +     * Called after construction, an alternative to @init + @connect
> +     * and should do the work for both: create and start the backend.
> +     * Mutually exclusive with @init and @connect.
> +     *
> +     * May not set the Chardev's chr->filename (generic code will care),
> +     * and may not send CHR_EVENT_OPENED when connected (@be_opened
> +     * should not be touched in this case, to signal the generic code
> +     * to care about CHR_EVENT_OPENED). If the backend cares about
> +     * CHR_EVENT_OPENED, it should set @be_opened to false.
> +     */
>      void (*open)(Chardev *chr, ChardevBackend *backend,
>                   bool *be_opened, Error **errp);
>
> --
> 2.48.1
>
>
Re: [PATCH v5 5/7] chardev/char: introduce .init() + .connect() initialization interface
Posted by Vladimir Sementsov-Ogievskiy 1 week, 3 days ago
On 02.11.25 14:37, Marc-André Lureau wrote:
> Hi
> 
> On Fri, Oct 31, 2025 at 7:59 PM Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru <mailto: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, or from qemu_chr_wait_connected().
> 
>     Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru <mailto:vsementsov@yandex-team.ru>>
>     Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com <mailto:marcandre.lureau@redhat.com>>
>     ---
>       chardev/char-fe.c      |  4 ++++
>       chardev/char.c         | 39 +++++++++++++++++++++++++++++++++++++--
>       include/chardev/char.h | 30 +++++++++++++++++++++++++++++-
>       3 files changed, 70 insertions(+), 3 deletions(-)
> 
>     diff --git a/chardev/char-fe.c b/chardev/char-fe.c
>     index 34b83fc1c4..c67b4d640f 100644
>     --- a/chardev/char-fe.c
>     +++ b/chardev/char-fe.c
>     @@ -193,6 +193,10 @@ bool qemu_chr_fe_init(CharFrontend *c, Chardev *s, Error **errp)
>       {
>           unsigned int tag = 0;
> 
>     +    if (!qemu_chr_connect(s, errp)) {
>     +        return false;
>     +    }
> 
> 
> this should be after "if (s)" condition.
> 
> It is moved in a later patch, but better do it now.

Oh, right, will do.

-- 
Best regards,
Vladimir