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

Vladimir Sementsov-Ogievskiy posted 7 patches 1 month 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 v2 7/7] char: vhost-user-blk call connect by hand
Posted by Vladimir Sementsov-Ogievskiy 1 month ago
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 7/7] char: vhost-user-blk call connect by hand
Posted by Marc-André Lureau 1 month ago
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 7/7] char: vhost-user-blk call connect by hand
Posted by Vladimir Sementsov-Ogievskiy 1 month ago
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