[PATCH v6 0/7] chardev: postpone connect

Vladimir Sementsov-Ogievskiy posted 7 patches 1 week, 3 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20251104101715.76691-1-vsementsov@yandex-team.ru
Maintainers: "Marc-André Lureau" <marcandre.lureau@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Eduardo Habkost <eduardo@habkost.net>
chardev/char-fe.c                   |  22 +++++-
chardev/char-socket.c               |  64 ++++++++-------
chardev/char.c                      | 118 ++++++++++++++++++++--------
hw/core/qdev-properties-system.c    |  26 +++++-
include/chardev/char-fe.h           |   9 ++-
include/chardev/char-socket.h       |   1 +
include/chardev/char.h              |  30 ++++++-
include/hw/qdev-properties-system.h |   3 +
tests/unit/test-char.c              |  14 ++--
ui/dbus-chardev.c                   |  12 ++-
10 files changed, 223 insertions(+), 76 deletions(-)
[PATCH v6 0/7] chardev: postpone connect
Posted by Vladimir Sementsov-Ogievskiy 1 week, 3 days ago
Hi all!

That's a preparation for backend-transfer migration of
vhost-user-blk, and introduced DEFINE_PROP_CHR_NO_CONNECT()
is unused now.

v3 of "vhost-user-blk: live-backend local migration" is coming
soon, and will be based on this series (and will use
DEFINE_PROP_CHR_NO_CONNECT of course).

changes in v6:
05: move connect() call into "if (s)"
07: - drop assertion
    - improve doc comment, to cover @s==NULL relations with @connect
    - add r-b by Marc-André

Vladimir Sementsov-Ogievskiy (7):
  chardev/char-socket: simplify reconnect-ms handling
  chardev/char: split chardev_init_common() 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
  chardev: introduce DEFINE_PROP_CHR_NO_CONNECT

 chardev/char-fe.c                   |  22 +++++-
 chardev/char-socket.c               |  64 ++++++++-------
 chardev/char.c                      | 118 ++++++++++++++++++++--------
 hw/core/qdev-properties-system.c    |  26 +++++-
 include/chardev/char-fe.h           |   9 ++-
 include/chardev/char-socket.h       |   1 +
 include/chardev/char.h              |  30 ++++++-
 include/hw/qdev-properties-system.h |   3 +
 tests/unit/test-char.c              |  14 ++--
 ui/dbus-chardev.c                   |  12 ++-
 10 files changed, 223 insertions(+), 76 deletions(-)

-- 
2.48.1


Re: [PATCH v6 0/7] chardev: postpone connect
Posted by Marc-André Lureau 1 week, 3 days ago
Hi Vladimir

On Tue, Nov 4, 2025 at 2:17 PM Vladimir Sementsov-Ogievskiy <
vsementsov@yandex-team.ru> wrote:

> Hi all!
>
> That's a preparation for backend-transfer migration of
> vhost-user-blk, and introduced DEFINE_PROP_CHR_NO_CONNECT()
> is unused now.
>
> v3 of "vhost-user-blk: live-backend local migration" is coming
> soon, and will be based on this series (and will use
> DEFINE_PROP_CHR_NO_CONNECT of course).
>
> changes in v6:
> 05: move connect() call into "if (s)"
> 07: - drop assertion
>     - improve doc comment, to cover @s==NULL relations with @connect
>     - add r-b by Marc-André
>
> Vladimir Sementsov-Ogievskiy (7):
>   chardev/char-socket: simplify reconnect-ms handling
>   chardev/char: split chardev_init_common() 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
>   chardev: introduce DEFINE_PROP_CHR_NO_CONNECT
>

Do you need this series in 10.2? We are at soft-freeze today, this is
closer to a feature than a simple refactoring, we may just wait for the
next dev phase.



>
>  chardev/char-fe.c                   |  22 +++++-
>  chardev/char-socket.c               |  64 ++++++++-------
>  chardev/char.c                      | 118 ++++++++++++++++++++--------
>  hw/core/qdev-properties-system.c    |  26 +++++-
>  include/chardev/char-fe.h           |   9 ++-
>  include/chardev/char-socket.h       |   1 +
>  include/chardev/char.h              |  30 ++++++-
>  include/hw/qdev-properties-system.h |   3 +
>  tests/unit/test-char.c              |  14 ++--
>  ui/dbus-chardev.c                   |  12 ++-
>  10 files changed, 223 insertions(+), 76 deletions(-)
>
> --
> 2.48.1
>
>
Re: [PATCH v6 0/7] chardev: postpone connect
Posted by Vladimir Sementsov-Ogievskiy 1 week, 3 days ago
On 04.11.25 14:35, Marc-André Lureau wrote:
> Hi Vladimir
> 
> On Tue, Nov 4, 2025 at 2:17 PM Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru <mailto:vsementsov@yandex-team.ru>> wrote:
> 
>     Hi all!
> 
>     That's a preparation for backend-transfer migration of
>     vhost-user-blk, and introduced DEFINE_PROP_CHR_NO_CONNECT()
>     is unused now.
> 
>     v3 of "vhost-user-blk: live-backend local migration" is coming
>     soon, and will be based on this series (and will use
>     DEFINE_PROP_CHR_NO_CONNECT of course).
> 
>     changes in v6:
>     05: move connect() call into "if (s)"
>     07: - drop assertion
>          - improve doc comment, to cover @s==NULL relations with @connect
>          - add r-b by Marc-André
> 
>     Vladimir Sementsov-Ogievskiy (7):
>        chardev/char-socket: simplify reconnect-ms handling
>        chardev/char: split chardev_init_common() 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
>        chardev: introduce DEFINE_PROP_CHR_NO_CONNECT
> 
> 
> Do you need this series in 10.2? We are at soft-freeze today

Time flies fast :(

, this is closer to a feature than a simple refactoring, we may just wait for the next dev phase.

Of course. Moreover, that's an unused feature without the next part of the series. Let's consider this as a 11.0 material.

> 
> 
>       chardev/char-fe.c                   |  22 +++++-
>       chardev/char-socket.c               |  64 ++++++++-------
>       chardev/char.c                      | 118 ++++++++++++++++++++--------
>       hw/core/qdev-properties-system.c    |  26 +++++-
>       include/chardev/char-fe.h           |   9 ++-
>       include/chardev/char-socket.h       |   1 +
>       include/chardev/char.h              |  30 ++++++-
>       include/hw/qdev-properties-system.h |   3 +
>       tests/unit/test-char.c              |  14 ++--
>       ui/dbus-chardev.c                   |  12 ++-
>       10 files changed, 223 insertions(+), 76 deletions(-)
> 
>     -- 
>     2.48.1
> 


-- 
Best regards,
Vladimir

Re: [PATCH v6 0/7] chardev: postpone connect
Posted by Daniel P. Berrangé 1 week, 3 days ago
On Tue, Nov 04, 2025 at 03:35:17PM +0400, Marc-André Lureau wrote:
> Hi Vladimir
> 
> On Tue, Nov 4, 2025 at 2:17 PM Vladimir Sementsov-Ogievskiy <
> vsementsov@yandex-team.ru> wrote:
> 
> > Hi all!
> >
> > That's a preparation for backend-transfer migration of
> > vhost-user-blk, and introduced DEFINE_PROP_CHR_NO_CONNECT()
> > is unused now.
> >
> > v3 of "vhost-user-blk: live-backend local migration" is coming
> > soon, and will be based on this series (and will use
> > DEFINE_PROP_CHR_NO_CONNECT of course).
> >
> > changes in v6:
> > 05: move connect() call into "if (s)"
> > 07: - drop assertion
> >     - improve doc comment, to cover @s==NULL relations with @connect
> >     - add r-b by Marc-André
> >
> > Vladimir Sementsov-Ogievskiy (7):
> >   chardev/char-socket: simplify reconnect-ms handling
> >   chardev/char: split chardev_init_common() 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
> >   chardev: introduce DEFINE_PROP_CHR_NO_CONNECT
> >
> 
> Do you need this series in 10.2? We are at soft-freeze today, this is
> closer to a feature than a simple refactoring, we may just wait for the
> next dev phase.

Back in v2, I had a request to convert the other chardev backends
to the new model too, as IMHO it is undesirable to introduce the
technical debt by only touching 1 backend:

  https://lists.nongnu.org/archive/html/qemu-devel/2025-10/msg03272.html


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 v6 0/7] chardev: postpone connect
Posted by Vladimir Sementsov-Ogievskiy 1 week, 3 days ago
On 04.11.25 14:41, Daniel P. Berrangé wrote:
> On Tue, Nov 04, 2025 at 03:35:17PM +0400, Marc-André Lureau wrote:
>> Hi Vladimir
>>
>> On Tue, Nov 4, 2025 at 2:17 PM Vladimir Sementsov-Ogievskiy <
>> vsementsov@yandex-team.ru> wrote:
>>
>>> Hi all!
>>>
>>> That's a preparation for backend-transfer migration of
>>> vhost-user-blk, and introduced DEFINE_PROP_CHR_NO_CONNECT()
>>> is unused now.
>>>
>>> v3 of "vhost-user-blk: live-backend local migration" is coming
>>> soon, and will be based on this series (and will use
>>> DEFINE_PROP_CHR_NO_CONNECT of course).
>>>
>>> changes in v6:
>>> 05: move connect() call into "if (s)"
>>> 07: - drop assertion
>>>      - improve doc comment, to cover @s==NULL relations with @connect
>>>      - add r-b by Marc-André
>>>
>>> Vladimir Sementsov-Ogievskiy (7):
>>>    chardev/char-socket: simplify reconnect-ms handling
>>>    chardev/char: split chardev_init_common() 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
>>>    chardev: introduce DEFINE_PROP_CHR_NO_CONNECT
>>>
>>
>> Do you need this series in 10.2? We are at soft-freeze today, this is
>> closer to a feature than a simple refactoring, we may just wait for the
>> next dev phase.
> 
> Back in v2, I had a request to convert the other chardev backends
> to the new model too, as IMHO it is undesirable to introduce the
> technical debt by only touching 1 backend:
> 
>    https://lists.nongnu.org/archive/html/qemu-devel/2025-10/msg03272.html
> 

Right. I remember it, and have a draft converting series. It turned out to be more than expected,
about 24 commits, and personally I'm not sure, that it worth doing.. I'll send an RFC too look at.

-- 
Best regards,
Vladimir

Re: [PATCH v6 0/7] chardev: postpone connect
Posted by Daniel P. Berrangé 1 week, 3 days ago
On Tue, Nov 04, 2025 at 02:44:49PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> On 04.11.25 14:41, Daniel P. Berrangé wrote:
> > On Tue, Nov 04, 2025 at 03:35:17PM +0400, Marc-André Lureau wrote:
> > > Hi Vladimir
> > > 
> > > On Tue, Nov 4, 2025 at 2:17 PM Vladimir Sementsov-Ogievskiy <
> > > vsementsov@yandex-team.ru> wrote:
> > > 
> > > > Hi all!
> > > > 
> > > > That's a preparation for backend-transfer migration of
> > > > vhost-user-blk, and introduced DEFINE_PROP_CHR_NO_CONNECT()
> > > > is unused now.
> > > > 
> > > > v3 of "vhost-user-blk: live-backend local migration" is coming
> > > > soon, and will be based on this series (and will use
> > > > DEFINE_PROP_CHR_NO_CONNECT of course).
> > > > 
> > > > changes in v6:
> > > > 05: move connect() call into "if (s)"
> > > > 07: - drop assertion
> > > >      - improve doc comment, to cover @s==NULL relations with @connect
> > > >      - add r-b by Marc-André
> > > > 
> > > > Vladimir Sementsov-Ogievskiy (7):
> > > >    chardev/char-socket: simplify reconnect-ms handling
> > > >    chardev/char: split chardev_init_common() 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
> > > >    chardev: introduce DEFINE_PROP_CHR_NO_CONNECT
> > > > 
> > > 
> > > Do you need this series in 10.2? We are at soft-freeze today, this is
> > > closer to a feature than a simple refactoring, we may just wait for the
> > > next dev phase.
> > 
> > Back in v2, I had a request to convert the other chardev backends
> > to the new model too, as IMHO it is undesirable to introduce the
> > technical debt by only touching 1 backend:
> > 
> >    https://lists.nongnu.org/archive/html/qemu-devel/2025-10/msg03272.html
> > 
> 
> Right. I remember it, and have a draft converting series. It turned out to be more than expected,
> about 24 commits, and personally I'm not sure, that it worth doing.. I'll send an RFC too look at.

Yep, I'd be interested to see what it looks like, even if it is not
finished / not functional.


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 v6 0/7] chardev: postpone connect
Posted by Vladimir Sementsov-Ogievskiy 1 week ago
On 04.11.25 14:48, Daniel P. Berrangé wrote:
> On Tue, Nov 04, 2025 at 02:44:49PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> On 04.11.25 14:41, Daniel P. Berrangé wrote:
>>> On Tue, Nov 04, 2025 at 03:35:17PM +0400, Marc-André Lureau wrote:
>>>> Hi Vladimir
>>>>
>>>> On Tue, Nov 4, 2025 at 2:17 PM Vladimir Sementsov-Ogievskiy <
>>>> vsementsov@yandex-team.ru> wrote:
>>>>
>>>>> Hi all!
>>>>>
>>>>> That's a preparation for backend-transfer migration of
>>>>> vhost-user-blk, and introduced DEFINE_PROP_CHR_NO_CONNECT()
>>>>> is unused now.
>>>>>
>>>>> v3 of "vhost-user-blk: live-backend local migration" is coming
>>>>> soon, and will be based on this series (and will use
>>>>> DEFINE_PROP_CHR_NO_CONNECT of course).
>>>>>
>>>>> changes in v6:
>>>>> 05: move connect() call into "if (s)"
>>>>> 07: - drop assertion
>>>>>       - improve doc comment, to cover @s==NULL relations with @connect
>>>>>       - add r-b by Marc-André
>>>>>
>>>>> Vladimir Sementsov-Ogievskiy (7):
>>>>>     chardev/char-socket: simplify reconnect-ms handling
>>>>>     chardev/char: split chardev_init_common() 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
>>>>>     chardev: introduce DEFINE_PROP_CHR_NO_CONNECT
>>>>>
>>>>
>>>> Do you need this series in 10.2? We are at soft-freeze today, this is
>>>> closer to a feature than a simple refactoring, we may just wait for the
>>>> next dev phase.
>>>
>>> Back in v2, I had a request to convert the other chardev backends
>>> to the new model too, as IMHO it is undesirable to introduce the
>>> technical debt by only touching 1 backend:
>>>
>>>     https://lists.nongnu.org/archive/html/qemu-devel/2025-10/msg03272.html
>>>
>>
>> Right. I remember it, and have a draft converting series. It turned out to be more than expected,
>> about 24 commits, and personally I'm not sure, that it worth doing.. I'll send an RFC too look at.
> 
> Yep, I'd be interested to see what it looks like, even if it is not
> finished / not functional.
> 

Hmm. Looking through my draft, and in parallel, moving char-socket vmsd
into correct layer (into char-socket), I now see the following:

1. splitting open() logic into init() and connect() make sense only
if it done for and together with implementing vmsd for that chardev
(as we do the split exactly in a manner which makes implementing of
migration possible)

2. implementing backend-transfer for all chardevs is too much for me.

So, seems, correct way is to allow chardevs have only .init, without
.connect when no support for migration.

And, converting all chardevs would be simply rename .open() to .init().
And I'll keep .open() semantics for .init() (i.e. additional handling
.filename and CHR_EVENT_OPENED for chardevs that don't care of it)

So, I can do the rename as first patch of v7 of this series, and we'll
keep common interaface for all chadevs (except for that most of them
will not support migration, and therefore don't implement .connect()
and .vmsd)

-- 
Best regards,
Vladimir