For further vhost-user-blk backend-transfer migration realization we
want to give it (vhost-user-blk) a possibility (and responsibility) to
decide when do connect.
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 only provide new macro, to define chardev property,
later it will be used in vhost-user-blk instead of DEFINE_PROP_CHR.
Then, vhost-user-blk will call qemu_chr_connect() by hand when needed
(for example through qemu_chr_fe_wait_connected(), which is already
called in vhost_user_blk_realize_connect()).
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
chardev/char-fe.c | 10 ++++++++--
hw/core/qdev-properties-system.c | 26 +++++++++++++++++++++++---
include/chardev/char-fe.h | 6 +++++-
include/hw/qdev-properties-system.h | 3 +++
4 files changed, 39 insertions(+), 6 deletions(-)
diff --git a/chardev/char-fe.c b/chardev/char-fe.c
index 973fed5bea..a0218393e4 100644
--- a/chardev/char-fe.c
+++ b/chardev/char-fe.c
@@ -189,11 +189,12 @@ 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, bool connect,
+ Error **errp)
{
unsigned int tag = 0;
- if (!qemu_chr_connect(s, errp)) {
+ if (connect && !qemu_chr_connect(s, errp)) {
return false;
}
@@ -218,6 +219,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, true, 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..6a0572ca03 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -266,8 +266,8 @@ static void get_chr(Object *obj, Visitor *v, const char *name, void *opaque,
g_free(p);
}
-static void set_chr(Object *obj, Visitor *v, const char *name, void *opaque,
- Error **errp)
+static void do_set_chr(Object *obj, Visitor *v, const char *name, void *opaque,
+ bool connect, Error **errp)
{
ERRP_GUARD();
const Property *prop = opaque;
@@ -297,13 +297,25 @@ 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, connect, errp)) {
error_prepend(errp, "Property '%s.%s' can't take value '%s': ",
object_get_typename(obj), name, str);
}
g_free(str);
}
+static void set_chr(Object *obj, Visitor *v, const char *name, void *opaque,
+ Error **errp)
+{
+ do_set_chr(obj, v, name, opaque, true, errp);
+}
+
+static void set_chr_no_connect(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+ do_set_chr(obj, v, name, opaque, false, errp);
+}
+
static void release_chr(Object *obj, const char *name, void *opaque)
{
const Property *prop = opaque;
@@ -320,6 +332,14 @@ const PropertyInfo qdev_prop_chr = {
.release = release_chr,
};
+const PropertyInfo qdev_prop_chr_no_connect = {
+ .type = "str",
+ .description = "ID of a chardev to use as a backend",
+ .get = get_chr,
+ .set = set_chr_no_connect,
+ .release = release_chr,
+};
+
/* --- mac address --- */
/*
diff --git a/include/chardev/char-fe.h b/include/chardev/char-fe.h
index 8ef05b3dd0..32013623b3 100644
--- a/include/chardev/char-fe.h
+++ b/include/chardev/char-fe.h
@@ -25,15 +25,19 @@ struct CharBackend {
};
/**
- * qemu_chr_fe_init:
+ * qemu_chr_fe_init(_ex):
*
* Initializes a front end for the given CharBackend and
* Chardev. Call qemu_chr_fe_deinit() to remove the association and
* release the driver.
+ * Call qemu_chr_connect(), except for the case when connect=false
+ * parameter set for _ex() version.
*
* 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, bool connect,
+ Error **errp);
/**
* qemu_chr_fe_deinit:
diff --git a/include/hw/qdev-properties-system.h b/include/hw/qdev-properties-system.h
index 9601a11a09..41f68f60b9 100644
--- a/include/hw/qdev-properties-system.h
+++ b/include/hw/qdev-properties-system.h
@@ -7,6 +7,7 @@ bool qdev_prop_sanitize_s390x_loadparm(uint8_t *loadparm, const char *str,
Error **errp);
extern const PropertyInfo qdev_prop_chr;
+extern const PropertyInfo qdev_prop_chr_no_connect;
extern const PropertyInfo qdev_prop_macaddr;
extern const PropertyInfo qdev_prop_reserved_region;
extern const PropertyInfo qdev_prop_multifd_compression;
@@ -39,6 +40,8 @@ extern const PropertyInfo qdev_prop_virtio_gpu_output_list;
#define DEFINE_PROP_CHR(_n, _s, _f) \
DEFINE_PROP(_n, _s, _f, qdev_prop_chr, CharBackend)
+#define DEFINE_PROP_CHR_NO_CONNECT(_n, _s, _f) \
+ DEFINE_PROP(_n, _s, _f, qdev_prop_chr_no_connect, CharBackend)
#define DEFINE_PROP_NETDEV(_n, _s, _f) \
DEFINE_PROP(_n, _s, _f, qdev_prop_netdev, NICPeers)
#define DEFINE_PROP_DRIVE(_n, _s, _f) \
--
2.48.1Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes: > For further vhost-user-blk backend-transfer migration realization we > want to give it (vhost-user-blk) a possibility (and responsibility) to > decide when do connect. > > 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 only provide new macro, to define chardev property, > later it will be used in vhost-user-blk instead of DEFINE_PROP_CHR. There is no "later" in this series. The new macro is called DEFINE_PROP_CHR_NO_CONNECT(). > Then, vhost-user-blk will call qemu_chr_connect() by hand when needed > (for example through qemu_chr_fe_wait_connected(), which is already > called in vhost_user_blk_realize_connect()). > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Excuse my quick & ignorant questions... I understand ChardevClass provides either methods init() and connect(), or method open(). Is a ChardevClass providing open() usable with DEFINE_PROP_CHR_NO_CONNECT()? Is a ChardevClass providing init() and connect() usable with DEFINE_PROP_CHR()? Could the code do the right thing based on presence of open() vs. init() and connect() instead of DEFINE_PROP_CHR() vs. DEFINE_PROP_CHR_NO_CONNECT()?
On 15.10.25 09:56, Markus Armbruster wrote: > Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes: > >> For further vhost-user-blk backend-transfer migration realization we >> want to give it (vhost-user-blk) a possibility (and responsibility) to >> decide when do connect. >> >> 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 only provide new macro, to define chardev property, >> later it will be used in vhost-user-blk instead of DEFINE_PROP_CHR. > > There is no "later" in this series. > > The new macro is called DEFINE_PROP_CHR_NO_CONNECT(). > >> Then, vhost-user-blk will call qemu_chr_connect() by hand when needed >> (for example through qemu_chr_fe_wait_connected(), which is already >> called in vhost_user_blk_realize_connect()). >> >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> > > Excuse my quick & ignorant questions... > > I understand ChardevClass provides either methods init() and connect(), > or method open(). > > Is a ChardevClass providing open() usable with > DEFINE_PROP_CHR_NO_CONNECT()? Good question. It's usable, but it will work like simple DEFINE_PROP_CHR. I should improve it somehow. Better is to fail than go unexpected way. > > Is a ChardevClass providing init() and connect() usable with > DEFINE_PROP_CHR()? Yes, and works correctly. > > Could the code do the right thing based on presence of open() vs. init() > and connect() instead of DEFINE_PROP_CHR() > vs. DEFINE_PROP_CHR_NO_CONNECT()? > No, because, the frontend should be prepared to work with new _NO_CONNECT (e.g., call _connect() by hand when needed). There are a lot of frontends, which expect already connected backend, updating them all would be big (and unnecessary) work. -- Best regards, Vladimir
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
> On 15.10.25 09:56, Markus Armbruster wrote:
>> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
>>
>>> For further vhost-user-blk backend-transfer migration realization we
>>> want to give it (vhost-user-blk) a possibility (and responsibility) to
>>> decide when do connect.
>>>
>>> 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 only provide new macro, to define chardev property,
>>> later it will be used in vhost-user-blk instead of DEFINE_PROP_CHR.
>>
>> There is no "later" in this series.
>> The new macro is called DEFINE_PROP_CHR_NO_CONNECT().
>>
>>> Then, vhost-user-blk will call qemu_chr_connect() by hand when needed
>>> (for example through qemu_chr_fe_wait_connected(), which is already
>>> called in vhost_user_blk_realize_connect()).
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>>
>> Excuse my quick & ignorant questions...
>>
>> I understand ChardevClass provides either methods init() and connect(),
>> or method open().
>>
>> Is a ChardevClass providing open() usable with
>> DEFINE_PROP_CHR_NO_CONNECT()?
>
> Good question. It's usable, but it will work like simple DEFINE_PROP_CHR.
> I should improve it somehow. Better is to fail than go unexpected way.
>
>> Is a ChardevClass providing init() and connect() usable with
>> DEFINE_PROP_CHR()?
>
> Yes, and works correctly.
>
>> Could the code do the right thing based on presence of open() vs. init()
>> and connect() instead of DEFINE_PROP_CHR()
>> vs. DEFINE_PROP_CHR_NO_CONNECT()?
>>
>
> No, because, the frontend should be prepared to work with new _NO_CONNECT (e.g.,
> call _connect() by hand when needed). There are a lot of frontends, which
> expect already connected backend, updating them all would be big (and
> unnecessary) work.
QMP command
{"execute": "qom-list-types", "arguments": {"implements": "chardev"}}
shows me 23 subtypes of "chardev". Could miss a few not in this build.
Converting them all would be work. It's not a prohibitive amount of
work, though. Whether it's worth our while is not for me to judge.
On 15.10.25 10:55, Markus Armbruster wrote:
> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
>
>> On 15.10.25 09:56, Markus Armbruster wrote:
>>> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
>>>
>>>> For further vhost-user-blk backend-transfer migration realization we
>>>> want to give it (vhost-user-blk) a possibility (and responsibility) to
>>>> decide when do connect.
>>>>
>>>> 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 only provide new macro, to define chardev property,
>>>> later it will be used in vhost-user-blk instead of DEFINE_PROP_CHR.
>>>
>>> There is no "later" in this series.
>>> The new macro is called DEFINE_PROP_CHR_NO_CONNECT().
>>>
>>>> Then, vhost-user-blk will call qemu_chr_connect() by hand when needed
>>>> (for example through qemu_chr_fe_wait_connected(), which is already
>>>> called in vhost_user_blk_realize_connect()).
>>>>
>>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>>>
>>> Excuse my quick & ignorant questions...
>>>
>>> I understand ChardevClass provides either methods init() and connect(),
>>> or method open().
>>>
>>> Is a ChardevClass providing open() usable with
>>> DEFINE_PROP_CHR_NO_CONNECT()?
>>
>> Good question. It's usable, but it will work like simple DEFINE_PROP_CHR.
>> I should improve it somehow. Better is to fail than go unexpected way.
>>
>>> Is a ChardevClass providing init() and connect() usable with
>>> DEFINE_PROP_CHR()?
>>
>> Yes, and works correctly.
>>
>>> Could the code do the right thing based on presence of open() vs. init()
>>> and connect() instead of DEFINE_PROP_CHR()
>>> vs. DEFINE_PROP_CHR_NO_CONNECT()?
>>>
>>
>> No, because, the frontend should be prepared to work with new _NO_CONNECT (e.g.,
>> call _connect() by hand when needed). There are a lot of frontends, which
>> expect already connected backend, updating them all would be big (and
>> unnecessary) work.
>
> QMP command
>
> {"execute": "qom-list-types", "arguments": {"implements": "chardev"}}
>
> shows me 23 subtypes of "chardev". Could miss a few not in this build.
>
> Converting them all would be work. It's not a prohibitive amount of
> work, though. Whether it's worth our while is not for me to judge.
>
I'm not sure we talk about the same thing.
Converting all chardevs to new API .init + .connect is feasible, and I say
in cover letter:
> If the design gets general approval, I'll try to update other
> chardev backends, to avoid supporting two different initialization
> APIs in future.
But converting all chardev users to DEFINE_PROP_CHR_NO_CONNECT is another
thing:
git grep DEFINE_PROP_CHR | wc -l
71
- it would be a huge work, and no benefits. Having a default of "already
connected chardev" seems reasonable. No reason to teach these 70 frontends
to call _connect() by hand.
-
frontend/backend is always misleading terminology for me, especially when
we have more than two components in the system :/
--
Best regards,
Vladimir
© 2016 - 2025 Red Hat, Inc.