[PATCH 24/33] chardev-add: support local migration

Vladimir Sementsov-Ogievskiy posted 33 patches 3 months ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Stefano Garzarella <sgarzare@redhat.com>, "Gonglei (Arei)" <arei.gonglei@huawei.com>, Zhenwei Pi <pizhenwei@bytedance.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, Raphael Norwitz <raphael@enfabrica.net>, Jason Wang <jasowang@redhat.com>, Fam Zheng <fam@euphon.net>, "Alex Bennée" <alex.bennee@linaro.org>, "Daniel P. Berrangé" <berrange@redhat.com>, Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>, Thomas Huth <thuth@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Laurent Vivier <lvivier@redhat.com>
There is a newer version of this series
[PATCH 24/33] chardev-add: support local migration
Posted by Vladimir Sementsov-Ogievskiy 3 months ago
This commit introduces a possibility to migrate open chardev
socket fd through migration channel without reconnecting.

For this, user should:
 - enable new migration capability local-char-socket
 - mark the socket by an option support-local-migration=true
 - on target add local-incoming=true option to the socket

Motivation for the API:

1. We don't want to migrate all sockets. For example, QMP-connection is
   bad candidate, as it is separate on source and target. So, we need
   @support-local-migration option to mark sockets, which we want to
   migrate (after this series, we'll want to migrate chardev used to
   connect with vhost-user-server).

2. Still, for remote migration, we can't migrate any sockets, so, we
   need a capability, to enable/disable the whole feature.

3. And finally, we need a sign for the socket to not open a connection
   on initialization, but wait for incoming migration. We can't use
   @support-local-migration option for it, as it may be enabled, but we
   are in incoming-remote migration. Also, we can't rely on the
   migration capability, as user is free to setup capabilities before or
   after chardev creation, and it would be a bad precedent to create
   relations here.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 chardev/char-socket.c         | 101 +++++++++++++++++++++++++++++++++-
 include/chardev/char-socket.h |   3 +
 migration/options.c           |   7 +++
 migration/options.h           |   1 +
 qapi/char.json                |  16 +++++-
 qapi/migration.json           |   8 ++-
 stubs/meson.build             |   1 +
 stubs/qemu_file.c             |  15 +++++
 stubs/vmstate.c               |   6 ++
 tests/qtest/meson.build       |   2 +-
 tests/unit/meson.build        |   4 +-
 11 files changed, 158 insertions(+), 6 deletions(-)
 create mode 100644 stubs/qemu_file.c

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 1e8313915b..db6616e2f2 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -24,6 +24,7 @@
 
 #include "qemu/osdep.h"
 #include "chardev/char.h"
+#include "qapi-types-char.h"
 #include "io/channel-socket.h"
 #include "io/channel-websock.h"
 #include "qemu/error-report.h"
@@ -34,6 +35,10 @@
 #include "qapi/qapi-visit-sockets.h"
 #include "qemu/yank.h"
 #include "trace.h"
+#include "migration/vmstate.h"
+#include "migration/qemu-file.h"
+#include "migration/migration.h"
+#include "hw/vmstate-if.h"
 
 #include "chardev/char-io.h"
 #include "chardev/char-socket.h"
@@ -1118,6 +1123,7 @@ static void char_socket_finalize(Object *obj)
         object_unref(OBJECT(s->tls_creds));
     }
     g_free(s->tls_authz);
+    g_free(s->vmstate_name);
     if (s->registered_yank) {
         /*
          * In the chardev-change special-case, we shouldn't unregister the yank
@@ -1276,8 +1282,15 @@ static int qmp_chardev_open_socket_client(Chardev *chr,
 {
     SocketChardev *s = SOCKET_CHARDEV(chr);
 
+    s->reconnect_time_ms = reconnect_ms;
+
+    if (s->local_incoming) {
+        /* We'll get fd at migreation load. This field works once */
+        s->local_incoming = false;
+        return 0;
+    }
+
     if (reconnect_ms > 0) {
-        s->reconnect_time_ms = reconnect_ms;
         tcp_chr_connect_client_async(chr);
         return 0;
     } else {
@@ -1367,6 +1380,52 @@ static bool qmp_chardev_validate_socket(ChardevSocket *sock,
     return true;
 }
 
+static int char_socket_save(QEMUFile *f, void *opaque, size_t size,
+                           const VMStateField *field, JSONWriter *vmdesc)
+{
+    SocketChardev *s = opaque;
+
+    warn_report("%s", __func__);
+    return qemu_file_put_fd(f, s->sioc->fd);
+}
+
+static int char_socket_load(QEMUFile *f, void *opaque, size_t size,
+                           const VMStateField *field)
+{
+    Chardev *chr = opaque;
+
+    int fd = qemu_file_get_fd(f);
+    warn_report("%s %d", __func__, fd);
+    if (fd < 0) {
+        return fd;
+    }
+    return tcp_chr_add_client(chr, fd);
+}
+
+static bool char_socket_needed(void *opaque)
+{
+    SocketChardev *s = opaque;
+
+    return !!s->vmstate_name;
+}
+
+const VMStateDescription vmstate_char_socket = {
+    .name = "char_socket",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = char_socket_needed,
+    .fields = (VMStateField[]) {
+        {
+            .name = "fd",
+            .info = &(const VMStateInfo) {
+                .name = "fd",
+                .get = char_socket_load,
+                .put = char_socket_save,
+            },
+        },
+        VMSTATE_END_OF_LIST()
+    },
+};
 
 static void qmp_chardev_open_socket(Chardev *chr,
                                     ChardevBackend *backend,
@@ -1381,14 +1440,36 @@ static void qmp_chardev_open_socket(Chardev *chr,
     bool is_tn3270      = sock->has_tn3270  ? sock->tn3270  : false;
     bool is_waitconnect = sock->has_wait    ? sock->wait    : false;
     bool is_websock     = sock->has_websocket ? sock->websocket : false;
+    bool support_local_mig = sock->has_support_local_migration
+                                 ? sock->support_local_migration
+                                 : false;
+    bool local_incoming = sock->local_incoming;
     int64_t reconnect_ms = 0;
     SocketAddress *addr;
 
+    if (support_local_mig && is_listen) {
+        error_setg(errp,
+                   "local migration is not supported for listening sockets");
+        return;
+    }
+
+    if (support_local_mig && !(chr->label && chr->label[0])) {
+        error_setg(errp,
+                   "local migration is not supported for unnamed chardevs");
+        return;
+    }
+
     s->is_listen = is_listen;
     s->is_telnet = is_telnet;
     s->is_tn3270 = is_tn3270;
     s->is_websock = is_websock;
     s->do_nodelay = do_nodelay;
+    s->local_incoming = local_incoming;
+
+    if (support_local_mig) {
+        s->vmstate_name = g_strdup_printf("__yc-chardev-%s", chr->label);
+    }
+
     if (sock->tls_creds) {
         Object *creds;
         creds = object_resolve_path_component(
@@ -1448,6 +1529,7 @@ static void qmp_chardev_open_socket(Chardev *chr,
     update_disconnected_filename(s);
 
     if (s->is_listen) {
+        assert(!s->vmstate_name);
         if (qmp_chardev_open_socket_server(chr, is_telnet || is_tn3270,
                                            is_waitconnect, errp) < 0) {
             return;
@@ -1463,6 +1545,8 @@ static void qmp_chardev_open_socket(Chardev *chr,
             return;
         }
     }
+
+    vmstate_register(VMSTATE_IF(s), -1, &vmstate_char_socket, s);
 }
 
 static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,
@@ -1581,9 +1665,20 @@ char_socket_get_connected(Object *obj, Error **errp)
     return s->state == TCP_CHARDEV_STATE_CONNECTED;
 }
 
+static char *
+char_socket_if_get_id(VMStateIf *obj)
+{
+    SocketChardev *s = SOCKET_CHARDEV(obj);
+
+    return s->vmstate_name;
+}
+
 static void char_socket_class_init(ObjectClass *oc, const void *data)
 {
     ChardevClass *cc = CHARDEV_CLASS(oc);
+    VMStateIfClass *vc = VMSTATE_IF_CLASS(oc);
+
+    vc->get_id = char_socket_if_get_id;
 
     cc->supports_yank = true;
 
@@ -1613,6 +1708,10 @@ static const TypeInfo char_socket_type_info = {
     .instance_size = sizeof(SocketChardev),
     .instance_finalize = char_socket_finalize,
     .class_init = char_socket_class_init,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_VMSTATE_IF },
+        { }
+    }
 };
 
 static void register_types(void)
diff --git a/include/chardev/char-socket.h b/include/chardev/char-socket.h
index d6d13ad37f..69b9609215 100644
--- a/include/chardev/char-socket.h
+++ b/include/chardev/char-socket.h
@@ -78,6 +78,9 @@ struct SocketChardev {
     bool connect_err_reported;
 
     QIOTask *connect_task;
+
+    char *vmstate_name;
+    bool local_incoming;
 };
 typedef struct SocketChardev SocketChardev;
 
diff --git a/migration/options.c b/migration/options.c
index 4e923a2e07..dffb6910f4 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -262,6 +262,13 @@ bool migrate_mapped_ram(void)
     return s->capabilities[MIGRATION_CAPABILITY_MAPPED_RAM];
 }
 
+bool migrate_local_char_socket(void)
+{
+    MigrationState *s = migrate_get_current();
+
+    return s->capabilities[MIGRATION_CAPABILITY_LOCAL_CHAR_SOCKET];
+}
+
 bool migrate_ignore_shared(void)
 {
     MigrationState *s = migrate_get_current();
diff --git a/migration/options.h b/migration/options.h
index 82d839709e..40971f0aa0 100644
--- a/migration/options.h
+++ b/migration/options.h
@@ -30,6 +30,7 @@ bool migrate_colo(void);
 bool migrate_dirty_bitmaps(void);
 bool migrate_events(void);
 bool migrate_mapped_ram(void);
+bool migrate_local_char_socket(void);
 bool migrate_ignore_shared(void);
 bool migrate_late_block_activate(void);
 bool migrate_multifd(void);
diff --git a/qapi/char.json b/qapi/char.json
index f0a53f742c..5b535c196a 100644
--- a/qapi/char.json
+++ b/qapi/char.json
@@ -280,11 +280,23 @@
 #     mutually exclusive with @reconnect.
 #     (default: 0) (Since: 9.2)
 #
+# @support-local-migration: The socket open file descriptor will
+#     migrate if this field is true and local-char-socket migration
+#     capability enabled (default: false) (Since: 10.2)
+#
+# @local-incoming: Do load open file descriptor for the socket
+#     on incoming migration. May be used only if QEMU is started
+#     for incoming migration and only together with local-char-socket
+#     migration capability (default: false) (Since: 10.2)
+#
 # Features:
 #
 # @deprecated: Member @reconnect is deprecated.  Use @reconnect-ms
 #     instead.
 #
+# @unstable: Members @support-local-migration and @local-incoming
+#            are experimental
+#
 # Since: 1.4
 ##
 { 'struct': 'ChardevSocket',
@@ -298,7 +310,9 @@
             '*tn3270': 'bool',
             '*websocket': 'bool',
             '*reconnect': { 'type': 'int', 'features': [ 'deprecated' ] },
-            '*reconnect-ms': 'int' },
+            '*reconnect-ms': 'int',
+            '*support-local-migration': { 'type': 'bool', 'features': [ 'unstable' ] },
+            '*local-incoming': { 'type': 'bool', 'features': [ 'unstable' ] } },
   'base': 'ChardevCommon' }
 
 ##
diff --git a/qapi/migration.json b/qapi/migration.json
index 2387c21e9c..4f282d168e 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -517,6 +517,11 @@
 #     each RAM page.  Requires a migration URI that supports seeking,
 #     such as a file.  (since 9.0)
 #
+# @local-char-socket: Migrate socket chardevs open file descriptors.
+#     Only may be used when migration channel is unix socket. Only
+#     involves socket chardevs with "support-local-migration" option
+#     enabled.  (since 10.2)
+#
 # Features:
 #
 # @unstable: Members @x-colo and @x-ignore-shared are experimental.
@@ -536,7 +541,8 @@
            { 'name': 'x-ignore-shared', 'features': [ 'unstable' ] },
            'validate-uuid', 'background-snapshot',
            'zero-copy-send', 'postcopy-preempt', 'switchover-ack',
-           'dirty-limit', 'mapped-ram'] }
+           'dirty-limit', 'mapped-ram',
+           { 'name': 'local-char-socket', 'features': [ 'unstable' ] } ] }
 
 ##
 # @MigrationCapabilityStatus:
diff --git a/stubs/meson.build b/stubs/meson.build
index cef046e685..7855483639 100644
--- a/stubs/meson.build
+++ b/stubs/meson.build
@@ -50,6 +50,7 @@ if have_block or have_user
   stub_ss.add(files('qtest.c'))
   stub_ss.add(files('vm-stop.c'))
   stub_ss.add(files('vmstate.c'))
+  stub_ss.add(files('qemu_file.c'))
 endif
 
 if have_user
diff --git a/stubs/qemu_file.c b/stubs/qemu_file.c
new file mode 100644
index 0000000000..854d4c13cd
--- /dev/null
+++ b/stubs/qemu_file.c
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#include "qemu/osdep.h"
+#include "migration/qemu-file.h"
+
+
+int qemu_file_get_fd(QEMUFile *f)
+{
+    return -1;
+}
+
+int qemu_file_put_fd(QEMUFile *f, int fd)
+{
+    return -1;
+}
diff --git a/stubs/vmstate.c b/stubs/vmstate.c
index c190762d7c..f345edf3c9 100644
--- a/stubs/vmstate.c
+++ b/stubs/vmstate.c
@@ -2,6 +2,7 @@
 #include "migration/vmstate.h"
 #include "qapi/qapi-types-migration.h"
 #include "migration/client-options.h"
+#include "migration/options.h"
 
 int vmstate_register_with_alias_id(VMStateIf *obj,
                                    uint32_t instance_id,
@@ -28,3 +29,8 @@ MigMode migrate_mode(void)
 {
     return MIG_MODE_NORMAL;
 }
+
+bool migrate_local_char_socket(void)
+{
+    return false;
+}
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 669d07c06b..8be0c1dc7c 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -381,7 +381,7 @@ qtests = {
   'pxe-test': files('boot-sector.c'),
   'pnv-xive2-test': files('pnv-xive2-common.c', 'pnv-xive2-flush-sync.c',
                           'pnv-xive2-nvpg_bar.c'),
-  'qos-test': [chardev, io, qos_test_ss.apply({}).sources()],
+  'qos-test': [chardev, io, qos_test_ss.apply({}).sources(), '../../hw/core/vmstate-if.c'],
   'tpm-crb-swtpm-test': [io, tpmemu_files],
   'tpm-crb-test': [io, tpmemu_files],
   'tpm-tis-swtpm-test': [io, tpmemu_files, 'tpm-tis-util.c'],
diff --git a/tests/unit/meson.build b/tests/unit/meson.build
index d5248ae51d..b96f4dcabe 100644
--- a/tests/unit/meson.build
+++ b/tests/unit/meson.build
@@ -139,7 +139,7 @@ if have_system
     'test-bufferiszero': [],
     'test-smp-parse': [qom, meson.project_source_root() / 'hw/core/machine-smp.c'],
     'test-vmstate': [migration, io],
-    'test-yank': ['socket-helpers.c', qom, io, chardev]
+    'test-yank': ['socket-helpers.c', qom, io, chardev, '../../hw/core/vmstate-if.c']
   }
   if config_host_data.get('CONFIG_INOTIFY1')
     tests += {'test-util-filemonitor': []}
@@ -151,7 +151,7 @@ if have_system
   if not get_option('tsan')
     if host_os != 'windows'
         tests += {
-          'test-char': ['socket-helpers.c', qom, io, chardev]
+          'test-char': ['socket-helpers.c', qom, io, chardev, '../../hw/core/vmstate-if.c']
         }
     endif
 
-- 
2.48.1
Re: [PATCH 24/33] chardev-add: support local migration
Posted by Markus Armbruster 2 months ago
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:

> This commit introduces a possibility to migrate open chardev
> socket fd through migration channel without reconnecting.
>
> For this, user should:
>  - enable new migration capability local-char-socket
>  - mark the socket by an option support-local-migration=true
>  - on target add local-incoming=true option to the socket
>
> Motivation for the API:
>
> 1. We don't want to migrate all sockets. For example, QMP-connection is
>    bad candidate, as it is separate on source and target. So, we need
>    @support-local-migration option to mark sockets, which we want to
>    migrate (after this series, we'll want to migrate chardev used to
>    connect with vhost-user-server).
>
> 2. Still, for remote migration, we can't migrate any sockets, so, we
>    need a capability, to enable/disable the whole feature.
>
> 3. And finally, we need a sign for the socket to not open a connection
>    on initialization, but wait for incoming migration. We can't use
>    @support-local-migration option for it, as it may be enabled, but we
>    are in incoming-remote migration. Also, we can't rely on the
>    migration capability, as user is free to setup capabilities before or
>    after chardev creation, and it would be a bad precedent to create
>    relations here.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

[...]

> diff --git a/qapi/char.json b/qapi/char.json
> index f0a53f742c..5b535c196a 100644
> --- a/qapi/char.json
> +++ b/qapi/char.json
> @@ -280,11 +280,23 @@
>  #     mutually exclusive with @reconnect.
>  #     (default: 0) (Since: 9.2)
>  #
> +# @support-local-migration: The socket open file descriptor will
> +#     migrate if this field is true and local-char-socket migration
> +#     capability enabled (default: false) (Since: 10.2)
> +#
> +# @local-incoming: Do load open file descriptor for the socket
> +#     on incoming migration. May be used only if QEMU is started
> +#     for incoming migration and only together with local-char-socket
> +#     migration capability (default: false) (Since: 10.2)
> +#
>  # Features:
>  #
>  # @deprecated: Member @reconnect is deprecated.  Use @reconnect-ms
>  #     instead.
>  #
> +# @unstable: Members @support-local-migration and @local-incoming
> +#            are experimental
> +#
>  # Since: 1.4
>  ##
>  { 'struct': 'ChardevSocket',
> @@ -298,7 +310,9 @@
>              '*tn3270': 'bool',
>              '*websocket': 'bool',
>              '*reconnect': { 'type': 'int', 'features': [ 'deprecated' ] },
> -            '*reconnect-ms': 'int' },
> +            '*reconnect-ms': 'int',
> +            '*support-local-migration': { 'type': 'bool', 'features': [ 'unstable' ] },
> +            '*local-incoming': { 'type': 'bool', 'features': [ 'unstable' ] } },
>    'base': 'ChardevCommon' }
>  
>  ##
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 2387c21e9c..4f282d168e 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -517,6 +517,11 @@
>  #     each RAM page.  Requires a migration URI that supports seeking,
>  #     such as a file.  (since 9.0)
>  #
> +# @local-char-socket: Migrate socket chardevs open file descriptors.
> +#     Only may be used when migration channel is unix socket. Only
> +#     involves socket chardevs with "support-local-migration" option
> +#     enabled.  (since 10.2)
> +#
>  # Features:
>  #
>  # @unstable: Members @x-colo and @x-ignore-shared are experimental.
> @@ -536,7 +541,8 @@
>             { 'name': 'x-ignore-shared', 'features': [ 'unstable' ] },
>             'validate-uuid', 'background-snapshot',
>             'zero-copy-send', 'postcopy-preempt', 'switchover-ack',
> -           'dirty-limit', 'mapped-ram'] }
> +           'dirty-limit', 'mapped-ram',
> +           { 'name': 'local-char-socket', 'features': [ 'unstable' ] } ] }
>  
>  ##
>  # @MigrationCapabilityStatus:

I understand why we need a knob to enable the feature.  A
MigrationCapability looks fine to me.  We could perhaps come up with a
better name, but let's leave that for later.

I'm unsure about making users mark the sockets (really: the sockets
wrapped in a character device backend) to be migrated that way.

Which sockets are users supposed to mark, and how would they know?

What happens when a user marks the QMP socket?  You called that a "bad
candidate".

Doesn't feel like good user interface design.

Could QEMU decide (in principle) which sockets are suitable for
sending down the migration channel?

If yes, could we make it do the right thing automatically?  Or at least
a check that stops the user from doing the wrong thing?

[...]
Re: [PATCH 24/33] chardev-add: support local migration
Posted by Steven Sistare 2 months ago
On 9/12/2025 10:56 AM, Markus Armbruster wrote:
> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
> 
>> This commit introduces a possibility to migrate open chardev
>> socket fd through migration channel without reconnecting.
>>
>> For this, user should:
>>   - enable new migration capability local-char-socket
>>   - mark the socket by an option support-local-migration=true
>>   - on target add local-incoming=true option to the socket
>>
>> Motivation for the API:
>>
>> 1. We don't want to migrate all sockets. For example, QMP-connection is
>>     bad candidate, as it is separate on source and target. So, we need
>>     @support-local-migration option to mark sockets, which we want to
>>     migrate (after this series, we'll want to migrate chardev used to
>>     connect with vhost-user-server).
>>
>> 2. Still, for remote migration, we can't migrate any sockets, so, we
>>     need a capability, to enable/disable the whole feature.
>>
>> 3. And finally, we need a sign for the socket to not open a connection
>>     on initialization, but wait for incoming migration. We can't use
>>     @support-local-migration option for it, as it may be enabled, but we
>>     are in incoming-remote migration. Also, we can't rely on the
>>     migration capability, as user is free to setup capabilities before or
>>     after chardev creation, and it would be a bad precedent to create
>>     relations here.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> 
> [...]
> 
>> diff --git a/qapi/char.json b/qapi/char.json
>> index f0a53f742c..5b535c196a 100644
>> --- a/qapi/char.json
>> +++ b/qapi/char.json
>> @@ -280,11 +280,23 @@
>>   #     mutually exclusive with @reconnect.
>>   #     (default: 0) (Since: 9.2)
>>   #
>> +# @support-local-migration: The socket open file descriptor will
>> +#     migrate if this field is true and local-char-socket migration
>> +#     capability enabled (default: false) (Since: 10.2)
>> +#
>> +# @local-incoming: Do load open file descriptor for the socket
>> +#     on incoming migration. May be used only if QEMU is started
>> +#     for incoming migration and only together with local-char-socket
>> +#     migration capability (default: false) (Since: 10.2)
>> +#
>>   # Features:
>>   #
>>   # @deprecated: Member @reconnect is deprecated.  Use @reconnect-ms
>>   #     instead.
>>   #
>> +# @unstable: Members @support-local-migration and @local-incoming
>> +#            are experimental
>> +#
>>   # Since: 1.4
>>   ##
>>   { 'struct': 'ChardevSocket',
>> @@ -298,7 +310,9 @@
>>               '*tn3270': 'bool',
>>               '*websocket': 'bool',
>>               '*reconnect': { 'type': 'int', 'features': [ 'deprecated' ] },
>> -            '*reconnect-ms': 'int' },
>> +            '*reconnect-ms': 'int',
>> +            '*support-local-migration': { 'type': 'bool', 'features': [ 'unstable' ] },
>> +            '*local-incoming': { 'type': 'bool', 'features': [ 'unstable' ] } },
>>     'base': 'ChardevCommon' }
>>   
>>   ##
>> diff --git a/qapi/migration.json b/qapi/migration.json
>> index 2387c21e9c..4f282d168e 100644
>> --- a/qapi/migration.json
>> +++ b/qapi/migration.json
>> @@ -517,6 +517,11 @@
>>   #     each RAM page.  Requires a migration URI that supports seeking,
>>   #     such as a file.  (since 9.0)
>>   #
>> +# @local-char-socket: Migrate socket chardevs open file descriptors.
>> +#     Only may be used when migration channel is unix socket. Only
>> +#     involves socket chardevs with "support-local-migration" option
>> +#     enabled.  (since 10.2)
>> +#
>>   # Features:
>>   #
>>   # @unstable: Members @x-colo and @x-ignore-shared are experimental.
>> @@ -536,7 +541,8 @@
>>              { 'name': 'x-ignore-shared', 'features': [ 'unstable' ] },
>>              'validate-uuid', 'background-snapshot',
>>              'zero-copy-send', 'postcopy-preempt', 'switchover-ack',
>> -           'dirty-limit', 'mapped-ram'] }
>> +           'dirty-limit', 'mapped-ram',
>> +           { 'name': 'local-char-socket', 'features': [ 'unstable' ] } ] }
>>   
>>   ##
>>   # @MigrationCapabilityStatus:
> 
> I understand why we need a knob to enable the feature.  A
> MigrationCapability looks fine to me.  We could perhaps come up with a
> better name, but let's leave that for later.
> 
> I'm unsure about making users mark the sockets (really: the sockets
> wrapped in a character device backend) to be migrated that way.
> 
> Which sockets are users supposed to mark, and how would they know?
> 
> What happens when a user marks the QMP socket?  You called that a "bad
> candidate".
> 
> Doesn't feel like good user interface design.
> 
> Could QEMU decide (in principle) which sockets are suitable for
> sending down the migration channel?
> 
> If yes, could we make it do the right thing automatically?  Or at least
> a check that stops the user from doing the wrong thing?
> 
> [...]

Hi Vladimir, I did not notice this patch before.
I also submitted patches for preserving chardevs including sockets, here:
   https://lore.kernel.org/qemu-devel/1658851843-236870-40-git-send-email-steven.sistare@oracle.com
and have fixed more bugs since then. I have attached my latest unsubmitted version
from my workspace.

My interface for enabling it is here:
   https://lore.kernel.org/qemu-devel/1658851843-236870-37-git-send-email-steven.sistare@oracle.com/

I am not wedded to either the interface or my socket patch, but the capability
must be supported for CPR.  And an acknowledgement of the prior work would
be nice.

- Steve

From 262eff756f425862c4afbd522128e7bf38d50118 Mon Sep 17 00:00:00 2001
From: Steve Sistare <steven.sistare@oracle.com>
Date: Tue, 20 Feb 2024 06:25:57 -0800
Subject: [PATCH] chardev: cpr for sockets

Save an accepted socket and restore it after cpr.  Re-create the
corresponding listen socket, but do not listen again until the accepted
socket is closed.

Save a connected socket and restore it after cpr, and do not re-connect
via qmp_chardev_open_socket_client.  Hoist the setting of
s->reconnect_time_ms so we still reconnect when the socket is closed.

Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
---
 chardev/char-socket.c         | 83 +++++++++++++++++++++++++++++++++++++++----
 include/chardev/char-socket.h |  1 +
 2 files changed, 78 insertions(+), 6 deletions(-)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 1e83139..f83f078 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -26,6 +26,7 @@
 #include "chardev/char.h"
 #include "io/channel-socket.h"
 #include "io/channel-websock.h"
+#include "migration/cpr.h"
 #include "qemu/error-report.h"
 #include "qemu/module.h"
 #include "qemu/option.h"
@@ -360,11 +361,52 @@ static void char_socket_yank_iochannel(void *opaque)
     qio_channel_shutdown(ioc, QIO_CHANNEL_SHUTDOWN_BOTH, NULL);
 }
 
+static void cpr_save_socket(Chardev *chr, QIOChannelSocket *sioc)
+{
+    SocketChardev *sockchar = SOCKET_CHARDEV(chr);
+
+    if (sioc && chr->cpr_enabled && !sockchar->cpr_reused) {
+        cpr_save_fd(chr->label, 0, sioc->fd);
+    }
+}
+
+/*
+ * Return 0 if fd is found and restored, -1 if found but an error occurred,
+ * and 1 if no fd found.
+ */
+static int cpr_load_socket(Chardev *chr, Error **errp)
+{
+    ERRP_GUARD();
+    SocketChardev *sockchar = SOCKET_CHARDEV(chr);
+    QIOChannelSocket *sioc;
+    const char *label = chr->label;
+    int fd = cpr_find_fd(label, 0);
+
+    sockchar->cpr_reused = (fd >= 0);
+    if (fd != -1) {
+        sockchar = SOCKET_CHARDEV(chr);
+        sioc = qio_channel_socket_new_fd(fd, errp);
+        if (sioc) {
+            tcp_chr_accept(sockchar->listener, sioc, chr);
+            object_unref(OBJECT(sioc));
+        } else {
+            error_prepend(errp, "could not restore socket for %s", label);
+            return -1;
+        }
+        return 0;
+    }
+    return 1;
+}
+
 static void tcp_chr_free_connection(Chardev *chr)
 {
     SocketChardev *s = SOCKET_CHARDEV(chr);
     int i;
 
+    if (chr->cpr_enabled) {
+        cpr_delete_fd(chr->label, 0);
+    }
+
     if (s->read_msgfds_num) {
         for (i = 0; i < s->read_msgfds_num; i++) {
             close(s->read_msgfds[i]);
@@ -923,6 +965,8 @@ static int tcp_chr_new_client(Chardev *chr, QIOChannelSocket *sioc)
         tcp_chr_connect(chr);
     }
 
+    cpr_save_socket(chr, sioc);
+
     return 0;
 }
 
@@ -1228,6 +1272,7 @@ static gboolean socket_reconnect_timeout(gpointer opaque)
 static int qmp_chardev_open_socket_server(Chardev *chr,
                                           bool is_telnet,
                                           bool is_waitconnect,
+                                          bool do_listen,
                                           Error **errp)
 {
     SocketChardev *s = SOCKET_CHARDEV(chr);
@@ -1259,11 +1304,15 @@ skip_listen:
 
     if (is_waitconnect) {
         tcp_chr_accept_server_sync(chr);
-    } else {
+    } else if (do_listen) {
         qio_net_listener_set_client_func_full(s->listener,
                                               tcp_chr_accept,
                                               chr, NULL,
                                               chr->gcontext);
+    } else {
+        qio_net_listener_set_client_func_full(s->listener,
+                                              NULL, NULL, NULL,
+                                              chr->gcontext);
     }
 
     return 0;
@@ -1271,13 +1320,11 @@ skip_listen:
 
 
 static int qmp_chardev_open_socket_client(Chardev *chr,
-                                          int64_t reconnect_ms,
                                           Error **errp)
 {
     SocketChardev *s = SOCKET_CHARDEV(chr);
 
-    if (reconnect_ms > 0) {
-        s->reconnect_time_ms = reconnect_ms;
+    if (s->reconnect_time_ms > 0) {
         tcp_chr_connect_client_async(chr);
         return 0;
     } else {
@@ -1381,8 +1428,10 @@ static void qmp_chardev_open_socket(Chardev *chr,
     bool is_tn3270      = sock->has_tn3270  ? sock->tn3270  : false;
     bool is_waitconnect = sock->has_wait    ? sock->wait    : false;
     bool is_websock     = sock->has_websocket ? sock->websocket : false;
+    bool do_listen      = is_listen;
     int64_t reconnect_ms = 0;
     SocketAddress *addr;
+    int ret;
 
     s->is_listen = is_listen;
     s->is_telnet = is_telnet;
@@ -1442,14 +1491,31 @@ static void qmp_chardev_open_socket(Chardev *chr,
     }
     s->registered_yank = true;
 
+    qemu_chr_set_feature(chr, QEMU_CHAR_FEATURE_CPR);
+
     /* be isn't opened until we get a connection */
     *be_opened = false;
 
     update_disconnected_filename(s);
 
     if (s->is_listen) {
+        /*
+         * If an accepted socket has been preserved, use it.  It may be closed
+         * later, so still set up the listen socket so it can accept again,
+         * but do not wait, and do not listen yet.
+         */
+
+        ret = cpr_load_socket(chr, errp);
+        if (ret < 0) {
+            return;
+        } else if (ret == 0) {
+            is_waitconnect = false;
+            do_listen = false;
+        }
+
         if (qmp_chardev_open_socket_server(chr, is_telnet || is_tn3270,
-                                           is_waitconnect, errp) < 0) {
+                                           is_waitconnect, do_listen,
+                                           errp) < 0) {
             return;
         }
     } else {
@@ -1458,8 +1524,13 @@ static void qmp_chardev_open_socket(Chardev *chr,
         } else if (sock->has_reconnect_ms) {
             reconnect_ms = sock->reconnect_ms;
         }
+        s->reconnect_time_ms = reconnect_ms;
 
-        if (qmp_chardev_open_socket_client(chr, reconnect_ms, errp) < 0) {
+        /* If a connected socket has been preserved, use it, else connect. */
+        if (cpr_load_socket(chr, errp) <= 0) {
+            return;
+        }
+        if (qmp_chardev_open_socket_client(chr, errp) < 0) {
             return;
         }
     }
diff --git a/include/chardev/char-socket.h b/include/chardev/char-socket.h
index d6d13ad..640d033 100644
--- a/include/chardev/char-socket.h
+++ b/include/chardev/char-socket.h
@@ -63,6 +63,7 @@ struct SocketChardev {
     int *write_msgfds;
     size_t write_msgfds_num;
     bool registered_yank;
+    bool cpr_reused;
 
     SocketAddress *addr;
     bool is_listen;
-- 
1.8.3.1

Re: [PATCH 24/33] chardev-add: support local migration
Posted by Vladimir Sementsov-Ogievskiy 2 months ago
On 12.09.25 18:24, Steven Sistare wrote:
> On 9/12/2025 10:56 AM, Markus Armbruster wrote:
>> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
>>
>>> This commit introduces a possibility to migrate open chardev
>>> socket fd through migration channel without reconnecting.
>>>
>>> For this, user should:
>>>   - enable new migration capability local-char-socket
>>>   - mark the socket by an option support-local-migration=true
>>>   - on target add local-incoming=true option to the socket
>>>
>>> Motivation for the API:
>>>
>>> 1. We don't want to migrate all sockets. For example, QMP-connection is
>>>     bad candidate, as it is separate on source and target. So, we need
>>>     @support-local-migration option to mark sockets, which we want to
>>>     migrate (after this series, we'll want to migrate chardev used to
>>>     connect with vhost-user-server).
>>>
>>> 2. Still, for remote migration, we can't migrate any sockets, so, we
>>>     need a capability, to enable/disable the whole feature.
>>>
>>> 3. And finally, we need a sign for the socket to not open a connection
>>>     on initialization, but wait for incoming migration. We can't use
>>>     @support-local-migration option for it, as it may be enabled, but we
>>>     are in incoming-remote migration. Also, we can't rely on the
>>>     migration capability, as user is free to setup capabilities before or
>>>     after chardev creation, and it would be a bad precedent to create
>>>     relations here.
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>>
>> [...]
>>
>>> diff --git a/qapi/char.json b/qapi/char.json
>>> index f0a53f742c..5b535c196a 100644
>>> --- a/qapi/char.json
>>> +++ b/qapi/char.json
>>> @@ -280,11 +280,23 @@
>>>   #     mutually exclusive with @reconnect.
>>>   #     (default: 0) (Since: 9.2)
>>>   #
>>> +# @support-local-migration: The socket open file descriptor will
>>> +#     migrate if this field is true and local-char-socket migration
>>> +#     capability enabled (default: false) (Since: 10.2)
>>> +#
>>> +# @local-incoming: Do load open file descriptor for the socket
>>> +#     on incoming migration. May be used only if QEMU is started
>>> +#     for incoming migration and only together with local-char-socket
>>> +#     migration capability (default: false) (Since: 10.2)
>>> +#
>>>   # Features:
>>>   #
>>>   # @deprecated: Member @reconnect is deprecated.  Use @reconnect-ms
>>>   #     instead.
>>>   #
>>> +# @unstable: Members @support-local-migration and @local-incoming
>>> +#            are experimental
>>> +#
>>>   # Since: 1.4
>>>   ##
>>>   { 'struct': 'ChardevSocket',
>>> @@ -298,7 +310,9 @@
>>>               '*tn3270': 'bool',
>>>               '*websocket': 'bool',
>>>               '*reconnect': { 'type': 'int', 'features': [ 'deprecated' ] },
>>> -            '*reconnect-ms': 'int' },
>>> +            '*reconnect-ms': 'int',
>>> +            '*support-local-migration': { 'type': 'bool', 'features': [ 'unstable' ] },
>>> +            '*local-incoming': { 'type': 'bool', 'features': [ 'unstable' ] } },
>>>     'base': 'ChardevCommon' }
>>>   ##
>>> diff --git a/qapi/migration.json b/qapi/migration.json
>>> index 2387c21e9c..4f282d168e 100644
>>> --- a/qapi/migration.json
>>> +++ b/qapi/migration.json
>>> @@ -517,6 +517,11 @@
>>>   #     each RAM page.  Requires a migration URI that supports seeking,
>>>   #     such as a file.  (since 9.0)
>>>   #
>>> +# @local-char-socket: Migrate socket chardevs open file descriptors.
>>> +#     Only may be used when migration channel is unix socket. Only
>>> +#     involves socket chardevs with "support-local-migration" option
>>> +#     enabled.  (since 10.2)
>>> +#
>>>   # Features:
>>>   #
>>>   # @unstable: Members @x-colo and @x-ignore-shared are experimental.
>>> @@ -536,7 +541,8 @@
>>>              { 'name': 'x-ignore-shared', 'features': [ 'unstable' ] },
>>>              'validate-uuid', 'background-snapshot',
>>>              'zero-copy-send', 'postcopy-preempt', 'switchover-ack',
>>> -           'dirty-limit', 'mapped-ram'] }
>>> +           'dirty-limit', 'mapped-ram',
>>> +           { 'name': 'local-char-socket', 'features': [ 'unstable' ] } ] }
>>>   ##
>>>   # @MigrationCapabilityStatus:
>>
>> I understand why we need a knob to enable the feature.  A
>> MigrationCapability looks fine to me.  We could perhaps come up with a
>> better name, but let's leave that for later.
>>
>> I'm unsure about making users mark the sockets (really: the sockets
>> wrapped in a character device backend) to be migrated that way.
>>
>> Which sockets are users supposed to mark, and how would they know?
>>
>> What happens when a user marks the QMP socket?  You called that a "bad
>> candidate".
>>
>> Doesn't feel like good user interface design.
>>
>> Could QEMU decide (in principle) which sockets are suitable for
>> sending down the migration channel?
>>
>> If yes, could we make it do the right thing automatically?  Or at least
>> a check that stops the user from doing the wrong thing?
>>
>> [...]
> 
> Hi Vladimir, I did not notice this patch before.
> I also submitted patches for preserving chardevs including sockets, here:
>    https://lore.kernel.org/qemu-devel/1658851843-236870-40-git-send-email-steven.sistare@oracle.com
> and have fixed more bugs since then. I have attached my latest unsubmitted version
> from my workspace.
> 
> My interface for enabling it is here:
>    https://lore.kernel.org/qemu-devel/1658851843-236870-37-git-send-email-steven.sistare@oracle.com/
> 
> I am not wedded to either the interface or my socket patch, but the capability
> must be supported for CPR.  And an acknowledgement of the prior work would
> be nice.
> 

Thanks! I'll consider this when preparing a new version for vhost-user-blk.



-- 
Best regards,
Vladimir

Re: [PATCH 24/33] chardev-add: support local migration
Posted by Vladimir Sementsov-Ogievskiy 2 months ago
On 12.09.25 17:56, Markus Armbruster wrote:
> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
> 
>> This commit introduces a possibility to migrate open chardev
>> socket fd through migration channel without reconnecting.
>>
>> For this, user should:
>>   - enable new migration capability local-char-socket
>>   - mark the socket by an option support-local-migration=true
>>   - on target add local-incoming=true option to the socket
>>
>> Motivation for the API:
>>
>> 1. We don't want to migrate all sockets. For example, QMP-connection is
>>     bad candidate, as it is separate on source and target. So, we need
>>     @support-local-migration option to mark sockets, which we want to
>>     migrate (after this series, we'll want to migrate chardev used to
>>     connect with vhost-user-server).
>>
>> 2. Still, for remote migration, we can't migrate any sockets, so, we
>>     need a capability, to enable/disable the whole feature.
>>
>> 3. And finally, we need a sign for the socket to not open a connection
>>     on initialization, but wait for incoming migration. We can't use
>>     @support-local-migration option for it, as it may be enabled, but we
>>     are in incoming-remote migration. Also, we can't rely on the
>>     migration capability, as user is free to setup capabilities before or
>>     after chardev creation, and it would be a bad precedent to create
>>     relations here.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> 
> [...]
> 
>> diff --git a/qapi/char.json b/qapi/char.json
>> index f0a53f742c..5b535c196a 100644
>> --- a/qapi/char.json
>> +++ b/qapi/char.json
>> @@ -280,11 +280,23 @@
>>   #     mutually exclusive with @reconnect.
>>   #     (default: 0) (Since: 9.2)
>>   #
>> +# @support-local-migration: The socket open file descriptor will
>> +#     migrate if this field is true and local-char-socket migration
>> +#     capability enabled (default: false) (Since: 10.2)
>> +#
>> +# @local-incoming: Do load open file descriptor for the socket
>> +#     on incoming migration. May be used only if QEMU is started
>> +#     for incoming migration and only together with local-char-socket
>> +#     migration capability (default: false) (Since: 10.2)
>> +#
>>   # Features:
>>   #
>>   # @deprecated: Member @reconnect is deprecated.  Use @reconnect-ms
>>   #     instead.
>>   #
>> +# @unstable: Members @support-local-migration and @local-incoming
>> +#            are experimental
>> +#
>>   # Since: 1.4
>>   ##
>>   { 'struct': 'ChardevSocket',
>> @@ -298,7 +310,9 @@
>>               '*tn3270': 'bool',
>>               '*websocket': 'bool',
>>               '*reconnect': { 'type': 'int', 'features': [ 'deprecated' ] },
>> -            '*reconnect-ms': 'int' },
>> +            '*reconnect-ms': 'int',
>> +            '*support-local-migration': { 'type': 'bool', 'features': [ 'unstable' ] },
>> +            '*local-incoming': { 'type': 'bool', 'features': [ 'unstable' ] } },
>>     'base': 'ChardevCommon' }
>>   
>>   ##
>> diff --git a/qapi/migration.json b/qapi/migration.json
>> index 2387c21e9c..4f282d168e 100644
>> --- a/qapi/migration.json
>> +++ b/qapi/migration.json
>> @@ -517,6 +517,11 @@
>>   #     each RAM page.  Requires a migration URI that supports seeking,
>>   #     such as a file.  (since 9.0)
>>   #
>> +# @local-char-socket: Migrate socket chardevs open file descriptors.
>> +#     Only may be used when migration channel is unix socket. Only
>> +#     involves socket chardevs with "support-local-migration" option
>> +#     enabled.  (since 10.2)
>> +#
>>   # Features:
>>   #
>>   # @unstable: Members @x-colo and @x-ignore-shared are experimental.
>> @@ -536,7 +541,8 @@
>>              { 'name': 'x-ignore-shared', 'features': [ 'unstable' ] },
>>              'validate-uuid', 'background-snapshot',
>>              'zero-copy-send', 'postcopy-preempt', 'switchover-ack',
>> -           'dirty-limit', 'mapped-ram'] }
>> +           'dirty-limit', 'mapped-ram',
>> +           { 'name': 'local-char-socket', 'features': [ 'unstable' ] } ] }
>>   
>>   ##
>>   # @MigrationCapabilityStatus:
> 
> I understand why we need a knob to enable the feature.  A
> MigrationCapability looks fine to me.  We could perhaps come up with a
> better name, but let's leave that for later.
> 
> I'm unsure about making users mark the sockets (really: the sockets
> wrapped in a character device backend) to be migrated that way.
> 
> Which sockets are users supposed to mark, and how would they know?
> 
> What happens when a user marks the QMP socket?  You called that a "bad
> candidate".
> 
> Doesn't feel like good user interface design.
> 
> Could QEMU decide (in principle) which sockets are suitable for
> sending down the migration channel?
> 
> If yes, could we make it do the right thing automatically?  Or at least
> a check that stops the user from doing the wrong thing?
> 

Yes, I'm thinking about this too. In my live-TAP series, I don't migrate TAP
netdev in separate, but it's migrated as part of virtio-net device. I hope,
it should be possible to do something similar here.

So the good target interface for me now is one "migarate-fds" (or better named)
capability, which turns on the whole feature both for virtio-net and vhost-user-blk
devices (and probably future ones). With additional optional device parameters
to be able to disable fds-migration per-device.

-- 
Best regards,
Vladimir