[PATCH v4 0/2] migration: Support socket fd for CPR and fix

[PATCH v4 0/2] migration: Support socket fd for CPR and fix

[Jaehoon Kim]

Hello,

This is v4 of the patch series.

Changes since v3:
- [Patch 1/2]: Applid review feedback from Steve Sistare - now using
  '%d' in g_strdup_printf

- [Patch 2/2]: No changes; Kept as-is with Reviewed-by from Steve
  Sistare.

Jaehoon Kim (2):
  tests/migration: Setup pre-listened cpr.sock to remove race-condition.
  migration: Support fd-based socket address in cpr_transfer_input

 migration/cpr-transfer.c          |  7 +++++--
 tests/qtest/migration/cpr-tests.c | 14 ++++++++++++--
 2 files changed, 17 insertions(+), 4 deletions(-)

-- 
2.49.0

[PATCH v4 1/2] tests/migration: Setup pre-listened cpr.sock to remove race-condition.

[Jaehoon Kim]

When the source VM attempts to connect to the destination VM's Unix
domain socket (cpr.sock) during a cpr-transfer test, race conditions can
occur if the socket file isn't ready. This can lead to connection
failures when running tests.

This patch creates and listens on the socket in advance, and passes the
pre-listened FD directly. This avoids timing issues and improves the
reliability of CPR tests.

Reviewed-by: Jason J. Herne <jjherne@linux.ibm.com>
Signed-off-by: Jaehoon Kim <jhkim@linux.ibm.com>
---
 tests/qtest/migration/cpr-tests.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/migration/cpr-tests.c b/tests/qtest/migration/cpr-tests.c
index 5536e14610..f7bd5c4666 100644
--- a/tests/qtest/migration/cpr-tests.c
+++ b/tests/qtest/migration/cpr-tests.c
@@ -60,13 +60,12 @@ static void test_mode_transfer_common(bool incoming_defer)
     g_autofree char *cpr_path = g_strdup_printf("%s/cpr.sock", tmpfs);
     g_autofree char *mig_path = g_strdup_printf("%s/migsocket", tmpfs);
     g_autofree char *uri = g_strdup_printf("unix:%s", mig_path);
+    g_autofree char *opts_target;
 
     const char *opts = "-machine aux-ram-share=on -nodefaults";
     g_autofree const char *cpr_channel = g_strdup_printf(
         "cpr,addr.transport=socket,addr.type=unix,addr.path=%s",
         cpr_path);
-    g_autofree char *opts_target = g_strdup_printf("-incoming %s %s",
-                                                   cpr_channel, opts);
 
     g_autofree char *connect_channels = g_strdup_printf(
         "[ { 'channel-type': 'main',"
@@ -75,6 +74,17 @@ static void test_mode_transfer_common(bool incoming_defer)
         "              'path': '%s' } } ]",
         mig_path);
 
+    /*
+     * Set up a UNIX domain socket for the CPR channel before
+     * launching the destination VM, to avoid timing issues
+     * during connection setup.
+     */
+    int cpr_sockfd = qtest_socket_server(cpr_path);
+    g_assert(cpr_sockfd >= 0);
+
+    opts_target = g_strdup_printf("-incoming cpr,addr.transport=socket,"
+                                  "addr.type=fd,addr.str=%d %s",
+                                  cpr_sockfd, opts);
     MigrateCommon args = {
         .start.opts_source = opts,
         .start.opts_target = opts_target,
-- 
2.49.0

Re: [PATCH v4 1/2] tests/migration: Setup pre-listened cpr.sock to remove race-condition.

[Steven Sistare]

On 6/11/2025 4:56 PM, Jaehoon Kim wrote:
> When the source VM attempts to connect to the destination VM's Unix
> domain socket (cpr.sock) during a cpr-transfer test, race conditions can
> occur if the socket file isn't ready. This can lead to connection
> failures when running tests.
> 
> This patch creates and listens on the socket in advance, and passes the
> pre-listened FD directly. This avoids timing issues and improves the
> reliability of CPR tests.
> 
> Reviewed-by: Jason J. Herne <jjherne@linux.ibm.com>
> Signed-off-by: Jaehoon Kim <jhkim@linux.ibm.com>
> ---
>   tests/qtest/migration/cpr-tests.c | 14 ++++++++++++--
>   1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/qtest/migration/cpr-tests.c b/tests/qtest/migration/cpr-tests.c
> index 5536e14610..f7bd5c4666 100644
> --- a/tests/qtest/migration/cpr-tests.c
> +++ b/tests/qtest/migration/cpr-tests.c
> @@ -60,13 +60,12 @@ static void test_mode_transfer_common(bool incoming_defer)
>       g_autofree char *cpr_path = g_strdup_printf("%s/cpr.sock", tmpfs);
>       g_autofree char *mig_path = g_strdup_printf("%s/migsocket", tmpfs);
>       g_autofree char *uri = g_strdup_printf("unix:%s", mig_path);
> +    g_autofree char *opts_target;

This must be initialized. Otherwise, if the function returns before opts_target
is assigned, then a garbage pointer can be freed.

     g_autofree char *opts_target = NULL;

Sorry I did not catch that before.

>       const char *opts = "-machine aux-ram-share=on -nodefaults";
>       g_autofree const char *cpr_channel = g_strdup_printf(
>           "cpr,addr.transport=socket,addr.type=unix,addr.path=%s",
>           cpr_path);
> -    g_autofree char *opts_target = g_strdup_printf("-incoming %s %s",
> -                                                   cpr_channel, opts);
>   
>       g_autofree char *connect_channels = g_strdup_printf(
>           "[ { 'channel-type': 'main',"
> @@ -75,6 +74,17 @@ static void test_mode_transfer_common(bool incoming_defer)
>           "              'path': '%s' } } ]",
>           mig_path);
>   
> +    /*
> +     * Set up a UNIX domain socket for the CPR channel before
> +     * launching the destination VM, to avoid timing issues
> +     * during connection setup.
> +     */
> +    int cpr_sockfd = qtest_socket_server(cpr_path);
> +    g_assert(cpr_sockfd >= 0);
> +
> +    opts_target = g_strdup_printf("-incoming cpr,addr.transport=socket,"
> +                                  "addr.type=fd,addr.str=%d %s",
> +                                  cpr_sockfd, opts);

Or, declare and assign the final value together:

     g_autofree char *opts_target = g_strdup_printf(
         "-incoming cpr,addr.transport=socket,addr.type=fd,addr.str=%d %s",
         cpr_sockfd, opts);

With either change:
Reviewed-by: Steve Sistare <steven.sistare@oracle.com>

Thanks very much for fixing the cpr tests.

- Steve

>       MigrateCommon args = {
>           .start.opts_source = opts,
>           .start.opts_target = opts_target,

[PATCH v4 2/2] migration: Support fd-based socket address in cpr_transfer_input

[Jaehoon Kim]

Extend cpr_transfer_input to handle SOCKET_ADDRESS_TYPE_FD alongside
SOCKET_ADDRESS_TYPE_UNIX. This change supports the use of pre-listened
socket file descriptors for cpr migration channels.

This change is particularly useful in qtest environments, where the
socket may be created externally and passed via fd.

Reviewed-by: Jason J. Herne <jjherne@linux.ibm.com>
Reviewed-by: Steve Sistare <steven.sistare@oracle.com>
Signed-off-by: Jaehoon Kim <jhkim@linux.ibm.com>
---
 migration/cpr-transfer.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/migration/cpr-transfer.c b/migration/cpr-transfer.c
index e1f140359c..00371d17c3 100644
--- a/migration/cpr-transfer.c
+++ b/migration/cpr-transfer.c
@@ -46,7 +46,8 @@ QEMUFile *cpr_transfer_input(MigrationChannel *channel, Error **errp)
     MigrationAddress *addr = channel->addr;
 
     if (addr->transport == MIGRATION_ADDRESS_TYPE_SOCKET &&
-        addr->u.socket.type == SOCKET_ADDRESS_TYPE_UNIX) {
+        (addr->u.socket.type == SOCKET_ADDRESS_TYPE_UNIX ||
+            addr->u.socket.type == SOCKET_ADDRESS_TYPE_FD)) {
 
         g_autoptr(QIOChannelSocket) sioc = NULL;
         SocketAddress *saddr = &addr->u.socket;
@@ -60,7 +61,9 @@ QEMUFile *cpr_transfer_input(MigrationChannel *channel, Error **errp)
 
         sioc = qio_net_listener_wait_client(listener);
         ioc = QIO_CHANNEL(sioc);
-        trace_cpr_transfer_input(addr->u.socket.u.q_unix.path);
+        trace_cpr_transfer_input(
+            addr->u.socket.type == SOCKET_ADDRESS_TYPE_UNIX ?
+            addr->u.socket.u.q_unix.path : addr->u.socket.u.fd.str);
         qio_channel_set_name(ioc, "cpr-in");
         return qemu_file_new_input(ioc);
 
-- 
2.49.0

Re: [PATCH v4 2/2] migration: Support fd-based socket address in cpr_transfer_input

[Daniel P. Berrangé]

On Wed, Jun 11, 2025 at 03:56:10PM -0500, Jaehoon Kim wrote:
> Extend cpr_transfer_input to handle SOCKET_ADDRESS_TYPE_FD alongside
> SOCKET_ADDRESS_TYPE_UNIX. This change supports the use of pre-listened
> socket file descriptors for cpr migration channels.
> 
> This change is particularly useful in qtest environments, where the
> socket may be created externally and passed via fd.
> 
> Reviewed-by: Jason J. Herne <jjherne@linux.ibm.com>
> Reviewed-by: Steve Sistare <steven.sistare@oracle.com>
> Signed-off-by: Jaehoon Kim <jhkim@linux.ibm.com>
> ---
>  migration/cpr-transfer.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)

This patch *MUST* be first in the series, otherwise 'git bisect'
will hit test failures on the former patch.

> 
> diff --git a/migration/cpr-transfer.c b/migration/cpr-transfer.c
> index e1f140359c..00371d17c3 100644
> --- a/migration/cpr-transfer.c
> +++ b/migration/cpr-transfer.c
> @@ -46,7 +46,8 @@ QEMUFile *cpr_transfer_input(MigrationChannel *channel, Error **errp)
>      MigrationAddress *addr = channel->addr;
>  
>      if (addr->transport == MIGRATION_ADDRESS_TYPE_SOCKET &&
> -        addr->u.socket.type == SOCKET_ADDRESS_TYPE_UNIX) {
> +        (addr->u.socket.type == SOCKET_ADDRESS_TYPE_UNIX ||
> +            addr->u.socket.type == SOCKET_ADDRESS_TYPE_FD)) {
>  
>          g_autoptr(QIOChannelSocket) sioc = NULL;
>          SocketAddress *saddr = &addr->u.socket;
> @@ -60,7 +61,9 @@ QEMUFile *cpr_transfer_input(MigrationChannel *channel, Error **errp)
>  
>          sioc = qio_net_listener_wait_client(listener);
>          ioc = QIO_CHANNEL(sioc);
> -        trace_cpr_transfer_input(addr->u.socket.u.q_unix.path);
> +        trace_cpr_transfer_input(
> +            addr->u.socket.type == SOCKET_ADDRESS_TYPE_UNIX ?
> +            addr->u.socket.u.q_unix.path : addr->u.socket.u.fd.str);
>          qio_channel_set_name(ioc, "cpr-in");
>          return qemu_file_new_input(ioc);
>  
> -- 
> 2.49.0
> 
> 

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 v4 2/2] migration: Support fd-based socket address in cpr_transfer_input

[Peter Xu]

On Thu, Jun 12, 2025 at 02:41:15PM +0100, Daniel P. Berrangé wrote:
> On Wed, Jun 11, 2025 at 03:56:10PM -0500, Jaehoon Kim wrote:
> > Extend cpr_transfer_input to handle SOCKET_ADDRESS_TYPE_FD alongside
> > SOCKET_ADDRESS_TYPE_UNIX. This change supports the use of pre-listened
> > socket file descriptors for cpr migration channels.
> > 
> > This change is particularly useful in qtest environments, where the
> > socket may be created externally and passed via fd.
> > 
> > Reviewed-by: Jason J. Herne <jjherne@linux.ibm.com>
> > Reviewed-by: Steve Sistare <steven.sistare@oracle.com>
> > Signed-off-by: Jaehoon Kim <jhkim@linux.ibm.com>
> > ---
> >  migration/cpr-transfer.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> This patch *MUST* be first in the series, otherwise 'git bisect'
> will hit test failures on the former patch.

Yes. I fixed both issues and queued the series, thanks all.  For the other
patch I did null-initialize for the var.

https://gitlab.com/peterx/qemu/-/commits/migration-staging

-- 
Peter Xu