[PATCH v2 0/7] chardev: postpone connect

Vladimir Sementsov-Ogievskiy posted 7 patches 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20251013133836.852018-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>
There is a newer version of this series
chardev/char-fe.c                |  19 ++++-
chardev/char-socket.c            |  64 +++++++++--------
chardev/char.c                   | 117 +++++++++++++++++++++++--------
hw/core/qdev-properties-system.c |   2 +-
include/chardev/char-fe.h        |   2 +
include/chardev/char-socket.h    |   1 +
include/chardev/char.h           |  22 +++++-
tests/unit/test-char.c           |  14 ++--
ui/dbus-chardev.c                |  12 +++-
9 files changed, 181 insertions(+), 72 deletions(-)
[PATCH v2 0/7] chardev: postpone connect
Posted by Vladimir Sementsov-Ogievskiy 1 month ago
Hi all!

That's a preparation for backend-transfer migration of
vhost-user-blk. For such migration we are going to transfer
vhost-user-blk fds, including backend chardev fd to the target
in migration stream (backed by UNIX domain socket).

So, on the target, we want to know, should we call connect(),
or is it a backend-transfer migration, and we should wait for
incoming fd.

But at initialization time we can't know it: user may setup
migration parameters (enabling backend-transfer) later.

So, let's postpone chardev open/connect phase up to attaching
to frontend. At this point we can check:

- if it's vhost-user-blk, do nothing, let vhost-user-blk decide
  when to do connect()
- otherwise, do connect() at this point

These are new patches, but called v2, as that's a rework of
previous try to handle vhost-user-blk chardev in

  [PATCH 00/33] vhost-user-blk: live-backend local migration

That previous try required additional option for chardevs,
so users should mark chardevs on target by a new option to
enable backend-transfer (and addition option on source was
needed, and addition migration capability).
The new upcoming design will require only one migration
parameter to enable the whole feature. But it requires more
complex logic, to postpone opens/connects, realized for
chardevs in these series.

The series is based on
  [PATCH 0/2] remove deprecated 'reconnect' options
Based-on: <20250924133309.334631-1-vsementsov@yandex-team.ru>

Vladimir Sementsov-Ogievskiy (7):
  chardev/char-socket: simplify reconnect-ms handling
  chardev/char: split chardev_init_logfd() 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
  char: vhost-user-blk call connect by hand

 chardev/char-fe.c                |  19 ++++-
 chardev/char-socket.c            |  64 +++++++++--------
 chardev/char.c                   | 117 +++++++++++++++++++++++--------
 hw/core/qdev-properties-system.c |   2 +-
 include/chardev/char-fe.h        |   2 +
 include/chardev/char-socket.h    |   1 +
 include/chardev/char.h           |  22 +++++-
 tests/unit/test-char.c           |  14 ++--
 ui/dbus-chardev.c                |  12 +++-
 9 files changed, 181 insertions(+), 72 deletions(-)

-- 
2.48.1
Re: [PATCH v2 0/7] chardev: postpone connect
Posted by Daniel P. Berrangé 1 month ago
On Mon, Oct 13, 2025 at 04:38:29PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
> 
> That's a preparation for backend-transfer migration of
> vhost-user-blk. For such migration we are going to transfer
> vhost-user-blk fds, including backend chardev fd to the target
> in migration stream (backed by UNIX domain socket).
> 
> So, on the target, we want to know, should we call connect(),
> or is it a backend-transfer migration, and we should wait for
> incoming fd.
> 
> But at initialization time we can't know it: user may setup
> migration parameters (enabling backend-transfer) later.
> 
> So, let's postpone chardev open/connect phase up to attaching
> to frontend. At this point we can check:
> 
> - if it's vhost-user-blk, do nothing, let vhost-user-blk decide
>   when to do connect()
> - otherwise, do connect() at this point

I'm finding it quite unpleasant that we've created a new set of
callbacks just for the socket backend, and not the other chardev
backends.

Conceptually it feels like the problem of transferring in pre-
opened FDs from a previous QEMU should be conceptually relevant
to all the backend types. If it is, then I very much want us to
convert all the backends instead of leaving a pile of technical
debt for someone else in the future.

This series also doesn't illustrate usage of the new model with
pre-opened FDs, so I'm finding it hard to validate whether
this design is effective or not.


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 v2 0/7] chardev: postpone connect
Posted by Vladimir Sementsov-Ogievskiy 1 month ago
On 14.10.25 16:40, Daniel P. Berrangé wrote:
> On Mon, Oct 13, 2025 at 04:38:29PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> Hi all!
>>
>> That's a preparation for backend-transfer migration of
>> vhost-user-blk. For such migration we are going to transfer
>> vhost-user-blk fds, including backend chardev fd to the target
>> in migration stream (backed by UNIX domain socket).
>>
>> So, on the target, we want to know, should we call connect(),
>> or is it a backend-transfer migration, and we should wait for
>> incoming fd.
>>
>> But at initialization time we can't know it: user may setup
>> migration parameters (enabling backend-transfer) later.
>>
>> So, let's postpone chardev open/connect phase up to attaching
>> to frontend. At this point we can check:
>>
>> - if it's vhost-user-blk, do nothing, let vhost-user-blk decide
>>    when to do connect()
>> - otherwise, do connect() at this point
> 
> I'm finding it quite unpleasant that we've created a new set of
> callbacks just for the socket backend, and not the other chardev
> backends.
> 
> Conceptually it feels like the problem of transferring in pre-
> opened FDs from a previous QEMU should be conceptually relevant
> to all the backend types. If it is, then I very much want us to
> convert all the backends instead of leaving a pile of technical
> debt for someone else in the future.

Agree. If the design is approved, I'll try to update all other
chardevs as well.

> 
> This series also doesn't illustrate usage of the new model with
> pre-opened FDs, so I'm finding it hard to validate whether
> this design is effective or not.
> 

Understand, of course.

[PATCH v2 00/?] vhost-user-blk: live-backend local migration

is coming soon, so the whole picture will be seen.

Still, I'll first resend this one (chardev: postpone connect) and

[PATCH v2 00/23] vhost refactoring and fixes

to base future v2 vhost-user-blk: live-backend local migration
on clear base.

-- 
Best regards,
Vladimir