Re: [PATCH v13 00/10] migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration

[Fabiano Rosas <farosas@suse.de>]

Het Gala <het.gala@nutanix.com> writes:

> Fabiano, would your below commits impact this patchset 'make check' 
> tests ? Because you have added tests for file based migration, which is 
> still not included in this patchset.

AFAICS, the tests shouldn't break.

> tests/qtest: migration-test: Add tests for file-based migration
> tests/qtest/migration: Add a test for the analyze-migration script
>
> I have tried to address all the g_autoptr() issues observed eariler, and 
> make check tests failing as a result. All the tests were passing (even 
> -qcow2 181) when the patch was posted for review. What can be the next 
> steps here for us? Do we need to add support for file based migration in 
> these patches or as you said eariler, you will introduce those patches 
> on top of my patches. Please let me know.

Right, your series is next on my queue for reviewing. I'll get to it
soon.

Here's what I was intending to send on top of it:

-->8--
From 0fc533989366a1a1e19737916d65938f64426c9b Mon Sep 17 00:00:00 2001
From: Fabiano Rosas <farosas@suse.de>
Date: Tue, 10 Oct 2023 11:01:32 -0300
Subject: [PATCH] migration: Convert the file transport to new migration api

Convert the file: URI to support the new QAPI syntax.

Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 migration/file.c      | 22 +++++++---------------
 migration/file.h      |  8 ++++++--
 migration/migration.c | 17 +++++++++++------
 qapi/migration.json   | 20 ++++++++++++++++++--
 4 files changed, 42 insertions(+), 25 deletions(-)

diff --git a/migration/file.c b/migration/file.c
index cf5b1bf365..e67c81dd2c 100644
--- a/migration/file.c
+++ b/migration/file.c
@@ -19,7 +19,7 @@
 
 /* Remove the offset option from @filespec and return it in @offsetp. */
 
-static int file_parse_offset(char *filespec, uint64_t *offsetp, Error **errp)
+int file_parse_offset(char *filespec, uint64_t *offsetp, Error **errp)
 {
     char *option = strstr(filespec, OFFSET_OPTION);
     int ret;
@@ -36,20 +36,16 @@ static int file_parse_offset(char *filespec, uint64_t *offsetp, Error **errp)
     return 0;
 }
 
-void file_start_outgoing_migration(MigrationState *s, const char *filespec,
+void file_start_outgoing_migration(MigrationState *s, FileMigrationArgs *file_args,
                                    Error **errp)
 {
-    g_autofree char *filename = g_strdup(filespec);
     g_autoptr(QIOChannelFile) fioc = NULL;
-    uint64_t offset = 0;
+    g_autofree char *filename = g_strdup(file_args->path);
+    uint64_t offset = file_args->offset;
     QIOChannel *ioc;
 
     trace_migration_file_outgoing(filename);
 
-    if (file_parse_offset(filename, &offset, errp)) {
-        return;
-    }
-
     fioc = qio_channel_file_new_path(filename, O_CREAT | O_WRONLY | O_TRUNC,
                                      0600, errp);
     if (!fioc) {
@@ -73,19 +69,15 @@ static gboolean file_accept_incoming_migration(QIOChannel *ioc,
     return G_SOURCE_REMOVE;
 }
 
-void file_start_incoming_migration(const char *filespec, Error **errp)
+void file_start_incoming_migration(FileMigrationArgs *file_args, Error **errp)
 {
-    g_autofree char *filename = g_strdup(filespec);
+    g_autofree char *filename = g_strdup(file_args->path);
     QIOChannelFile *fioc = NULL;
-    uint64_t offset = 0;
+    uint64_t offset = file_args->offset;
     QIOChannel *ioc;
 
     trace_migration_file_incoming(filename);
 
-    if (file_parse_offset(filename, &offset, errp)) {
-        return;
-    }
-
     fioc = qio_channel_file_new_path(filename, O_RDONLY, 0, errp);
     if (!fioc) {
         return;
diff --git a/migration/file.h b/migration/file.h
index 90fa4849e0..155f6aab45 100644
--- a/migration/file.h
+++ b/migration/file.h
@@ -7,8 +7,12 @@
 
 #ifndef QEMU_MIGRATION_FILE_H
 #define QEMU_MIGRATION_FILE_H
-void file_start_incoming_migration(const char *filename, Error **errp);
 
-void file_start_outgoing_migration(MigrationState *s, const char *filename,
+#include "qapi/qapi-types-migration.h"
+
+void file_start_incoming_migration(FileMigrationArgs *file_args, Error **errp);
+
+void file_start_outgoing_migration(MigrationState *s, FileMigrationArgs *file_args,
                                    Error **errp);
+int file_parse_offset(char *filespec, uint64_t *offsetp, Error **errp);
 #endif
diff --git a/migration/migration.c b/migration/migration.c
index a651106bff..0b07b7343b 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -468,6 +468,13 @@ bool migrate_uri_parse(const char *uri, MigrationChannel **channel,
         }
         addr->u.socket.type = saddr->type;
         addr->u.socket.u = saddr->u;
+    } else if (strstart(uri, "file:", NULL)) {
+        addr->transport = MIGRATION_ADDRESS_TYPE_FILE;
+        addr->u.file.path = g_strdup(uri + strlen("file:"));
+        if (file_parse_offset(addr->u.file.path, &addr->u.file.offset, errp)) {
+            g_free(addr->u.file.path);
+            return false;
+        }
     } else {
         error_setg(errp, "unknown migration protocol: %s", uri);
         return false;
@@ -483,7 +490,6 @@ static void qemu_start_incoming_migration(const char *uri, bool has_channels,
                                           MigrationChannelList *channels,
                                           Error **errp)
 {
-    const char *p = NULL;
     MigrationChannel *channel = NULL;
     MigrationAddress *addr = NULL;
 
@@ -535,8 +541,8 @@ static void qemu_start_incoming_migration(const char *uri, bool has_channels,
  #endif
     } else if (addr->transport == MIGRATION_ADDRESS_TYPE_EXEC) {
         exec_start_incoming_migration(addr->u.exec.args, errp);
-    } else if (strstart(uri, "file:", &p)) {
-        file_start_incoming_migration(p, errp);
+    } else if (addr->transport == MIGRATION_ADDRESS_TYPE_FILE) {
+        file_start_incoming_migration(&addr->u.file, errp);
     } else {
         error_setg(errp, "unknown migration protocol: %s", uri);
     }
@@ -1760,7 +1766,6 @@ void qmp_migrate(const char *uri, bool has_channels,
     bool resume_requested;
     Error *local_err = NULL;
     MigrationState *s = migrate_get_current();
-    const char *p = NULL;
     MigrationChannel *channel = NULL;
     MigrationAddress *addr = NULL;
 
@@ -1824,8 +1829,8 @@ void qmp_migrate(const char *uri, bool has_channels,
 #endif
     } else if (addr->transport == MIGRATION_ADDRESS_TYPE_EXEC) {
         exec_start_outgoing_migration(s, addr->u.exec.args, &local_err);
-    } else if (strstart(uri, "file:", &p)) {
-        file_start_outgoing_migration(s, p, &local_err);
+    } else if (addr->transport == MIGRATION_ADDRESS_TYPE_FILE) {
+        file_start_outgoing_migration(s, &addr->u.file, &local_err);
     } else {
         error_setg(&local_err, QERR_INVALID_PARAMETER_VALUE, "uri",
                    "a valid migration protocol");
diff --git a/qapi/migration.json b/qapi/migration.json
index 7b84c04617..bb0639b9d6 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -1489,10 +1489,25 @@
 #
 # @rdma: Migrate via RDMA.
 #
+# @file: Direct the migration stream to a file.
+#
 # Since 8.2
 ##
 { 'enum': 'MigrationAddressType',
-  'data': ['socket', 'exec', 'rdma'] }
+  'data': ['socket', 'exec', 'rdma', 'file'] }
+
+##
+# @FileMigrationArgs:
+#
+# @path: file path
+#
+# @offset: initial offset for the file
+#
+# Since 8.2
+##
+{ 'struct': 'FileMigrationArgs',
+  'data': {'path': 'str',
+           'offset': 'uint64' } }
 
 ##
 # @MigrationExecCommand:
@@ -1517,7 +1532,8 @@
   'data': {
     'socket': 'SocketAddress',
     'exec': 'MigrationExecCommand',
-    'rdma': 'InetSocketAddress' } }
+    'rdma': 'InetSocketAddress',
+    'file': 'FileMigrationArgs' } }
 
 ##
 # @MigrationChannelType:
-- 
2.35.3