On 9/12/2025 10:56 AM, Markus Armbruster wrote:
Vladimir Sementsov-Ogievskiy <[email protected]> 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 <[email protected]>

[...]

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/[email protected]
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/[email protected]/

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 <[email protected]>
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 <[email protected]>
---
 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

Reply via email to