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         | 39 +++++++++++++++++++++++++++++++++++++--
 include/chardev/char.h | 22 +++++++++++++++++++++-
 3 files changed, 62 insertions(+), 3 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 27290e26fb..409f3aac1c 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 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

Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:

> 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>

[...]

> 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,

What to do mean by "create and open/starts"?

Maybe "create and start"?

> +     * mutual exclusive with .init. .connect must not be defined when

mutually

> +     * .open is defined.
> +     */

Suggest to use @name to refer to a member name.  We do that elsewhere,
and it's easier on the eyes than dots.

>      void (*open)(Chardev *chr, ChardevBackend *backend,
>                   bool *be_opened, Error **errp);
>  
> +    /*
> +     * Called after construction, create the backend, mutual exclusive

mutually

> +     * with .open, and must be accompanied by .connect.

Is it okay to destroy after init() without connect()?

If yes, "must" is misleading.

> +     * Must set chr-filename.

What's chr-filename?

> +     */
> +    bool (*init)(Chardev *chr, ChardevBackend *backend,
> +                 Error **errp);
> +
> +    /*
> +     * Called after .init(), open/starts the backend, mutual exclusive

mutually

> +     * with .open. Must send CHR_EVENT_OPENED.

Must send CHR_EVENT_OPENED when it succeeds, I guess.

> +     */
> +    bool (*connect)(Chardev *chr, Error **errp);
> +
>      /* write buf to the backend */
>      int (*chr_write)(Chardev *s, const uint8_t *buf, int len);

So, a ChardevClass either provides methods init() and connect(), or
their fusion open().  Correct?

Perhaps documentation becomes simpler if you put init() and connect()
before open().  You could then say open() needs to do the work of both.

On 15.10.25 09:50, Markus Armbruster wrote:
> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
> 
>> 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>
> 
> [...]
> 
>> 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,
> 
> What to do mean by "create and open/starts"?
> 
> Maybe "create and start"?

Yes, sounds good.

> 
>> +     * mutual exclusive with .init. .connect must not be defined when
> 
> mutually
> 
>> +     * .open is defined.
>> +     */
> 
> Suggest to use @name to refer to a member name.  We do that elsewhere,
> and it's easier on the eyes than dots.
> 
>>       void (*open)(Chardev *chr, ChardevBackend *backend,
>>                    bool *be_opened, Error **errp);
>>   
>> +    /*
>> +     * Called after construction, create the backend, mutual exclusive
> 
> mutually
> 
>> +     * with .open, and must be accompanied by .connect.
> 
> Is it okay to destroy after init() without connect()?
> 
> If yes, "must" is misleading.

Hmm. "should" is OK then?

> 
>> +     * Must set chr-filename.
> 
> What's chr-filename?

Type. That should be chr->filename. Or better @chr->filename ?

> 
>> +     */
>> +    bool (*init)(Chardev *chr, ChardevBackend *backend,
>> +                 Error **errp);
>> +
>> +    /*
>> +     * Called after .init(), open/starts the backend, mutual exclusive
> 
> mutually
> 
>> +     * with .open. Must send CHR_EVENT_OPENED.
> 
> Must send CHR_EVENT_OPENED when it succeeds, I guess.

Yes, will add.

> 
>> +     */
>> +    bool (*connect)(Chardev *chr, Error **errp);
>> +
>>       /* write buf to the backend */
>>       int (*chr_write)(Chardev *s, const uint8_t *buf, int len);
> 
> So, a ChardevClass either provides methods init() and connect(), or
> their fusion open().  Correct?

Yes

> 
> Perhaps documentation becomes simpler if you put init() and connect()
> before open().  You could then say open() needs to do the work of both.
> 

Agree, will do.

Thanks for reviewing!

-- 
Best regards,
Vladimir

Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:

> On 15.10.25 09:50, Markus Armbruster wrote:
>> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
>> 
>>> 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>
>> [...]
>> 
>>> 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,
>>
>> What to do mean by "create and open/starts"?
>> Maybe "create and start"?
>
> Yes, sounds good.
>
>>> +     * mutual exclusive with .init. .connect must not be defined when
>>
>> mutually
>> 
>>> +     * .open is defined.
>>> +     */
>>
>> Suggest to use @name to refer to a member name.  We do that elsewhere,
>> and it's easier on the eyes than dots.
>> 
>>>      void (*open)(Chardev *chr, ChardevBackend *backend,
>>>                   bool *be_opened, Error **errp);
>>> +    /*
>>> +     * Called after construction, create the backend, mutual exclusive
>>
>> mutually
>> 
>>> +     * with .open, and must be accompanied by .connect.
>>
>> Is it okay to destroy after init() without connect()?
>> If yes, "must" is misleading.
>
> Hmm. "should" is OK then?

"Should be followed by connect()" would work.

Or something like "The backend still needs to be started with init()."

>>> +     * Must set chr-filename.
>>
>> What's chr-filename?
>
> Type. That should be chr->filename. Or better @chr->filename ?

There is no member @chr.  I figure it's CharBackend member @chr.
Perhaps "the CharBackend's chr->filename". 

[...]

> Thanks for reviewing!

You're welcome!