[RFC,V1,4/6] migration: cpr-uri parameter

Commit Message

Steven Sistare June 30, 2024, 7:44 p.m. UTC

Define the cpr-uri migration parameter to specify the URI to which
CPR vmstate is saved for cpr-transfer mode.

Signed-off-by: Steve Sistare <[e-mail]>
---
 migration/migration-hmp-cmds.c | 10 ++++++++++
 migration/options.c            | 29 +++++++++++++++++++++++++++++
 migration/options.h            |  1 +
 qapi/migration.json            | 12 ++++++++++++
 4 files changed, 52 insertions(+)

Comments

Peter Xu Aug. 15, 2024, 8:46 p.m. UTC | #1

On Sun, Jun 30, 2024 at 12:44:06PM -0700, Steve Sistare wrote:
> Define the cpr-uri migration parameter to specify the URI to which
> CPR vmstate is saved for cpr-transfer mode.
> 
> Signed-off-by: Steve Sistare <[e-mail]>

So I left the idea in my reply to the cover letter to reuse "-cpr-uri" in
qemu cmdline on dst node, and I wonder if it'll also save this one on src
with the same idea.

In general, with cpr-transfer we always stick with unix socket, and only
one pair of it.

When migration starts, cpr_state_save() dumps things and cpr_state_load()
loads things.  Then the socket gets reused on both sides to be the
migration channel.

IIUC we can already reuse QMP command URI on src so we don't need anything
new.  On dst, we may need "-incoming-cpr" to tell QEMU we need to kick off
CPR early loads; it's a pity we don't have way to specify migration mode
there, otherwise IIUC we can even save "-incoming-cpr" but reuse "-incoming".

Thanks,

Steven Sistare Aug. 16, 2024, 3:13 p.m. UTC | #2

On 8/15/2024 4:46 PM, Peter Xu wrote:
> On Sun, Jun 30, 2024 at 12:44:06PM -0700, Steve Sistare wrote:
>> Define the cpr-uri migration parameter to specify the URI to which
>> CPR vmstate is saved for cpr-transfer mode.
>>
>> Signed-off-by: Steve Sistare <[e-mail]>
> 
> So I left the idea in my reply to the cover letter to reuse "-cpr-uri" in
> qemu cmdline on dst node, and I wonder if it'll also save this one on src
> with the same idea.
> 
> In general, with cpr-transfer we always stick with unix socket, and only
> one pair of it.
> 
> When migration starts, cpr_state_save() dumps things and cpr_state_load()
> loads things.  Then the socket gets reused on both sides to be the
> migration channel.
> 
> IIUC we can already reuse QMP command URI on src so we don't need anything
> new.  On dst, we may need "-incoming-cpr" to tell QEMU we need to kick off
> CPR early loads; it's a pity we don't have way to specify migration mode
> there, otherwise IIUC we can even save "-incoming-cpr" but reuse "-incoming".

I also considered using the existing migration URI on both the source and target,
and accepting it twice, by defining an -incoming-mode parameter.  But, I still
think it would be a dis-service to our users to eliminate all flexibility for the
URI type.

- Steve

Patch

diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index 16a4b00..4ede831 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -371,6 +371,11 @@  void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict)
                            params->direct_io ? "on" : "off");
         }
 
+        assert(params->cpr_uri);
+        monitor_printf(mon, "%s: '%s'\n",
+            MigrationParameter_str(MIGRATION_PARAMETER_CPR_URI),
+            params->cpr_uri);
+
         assert(params->has_cpr_exec_command);
         monitor_print_cpr_exec_command(mon, params->cpr_exec_command);
     }
@@ -650,6 +655,11 @@  void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
         p->has_direct_io = true;
         visit_type_bool(v, param, &p->direct_io, &err);
         break;
+    case MIGRATION_PARAMETER_CPR_URI:
+        p->cpr_uri = g_new0(StrOrNull, 1);
+        p->cpr_uri->type = QTYPE_QSTRING;
+        visit_type_str(v, param, &p->cpr_uri->u.s, &err);
+        break;
     case MIGRATION_PARAMETER_CPR_EXEC_COMMAND: {
         g_autofree char **strv = g_strsplit(valuestr ?: "", " ", -1);
         strList **tail = &p->cpr_exec_command;
diff --git a/migration/options.c b/migration/options.c
index b8d5f72..7526f9f 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -163,6 +163,8 @@  Property migration_properties[] = {
     DEFINE_PROP_ZERO_PAGE_DETECTION("zero-page-detection", MigrationState,
                        parameters.zero_page_detection,
                        ZERO_PAGE_DETECTION_MULTIFD),
+    DEFINE_PROP_STRING("cpr-uri", MigrationState,
+                       parameters.cpr_uri),
 
     /* Migration capabilities */
     DEFINE_PROP_MIG_CAP("x-xbzrle", MIGRATION_CAPABILITY_XBZRLE),
@@ -848,6 +850,13 @@  ZeroPageDetection migrate_zero_page_detection(void)
     return s->parameters.zero_page_detection;
 }
 
+const char *migrate_cpr_uri(void)
+{
+    MigrationState *s = migrate_get_current();
+
+    return s->parameters.cpr_uri;
+}
+
 /* parameters helpers */
 
 AnnounceParameters *migrate_announce_params(void)
@@ -931,6 +940,7 @@  MigrationParameters *qmp_query_migrate_parameters(Error **errp)
     params->zero_page_detection = s->parameters.zero_page_detection;
     params->has_direct_io = true;
     params->direct_io = s->parameters.direct_io;
+    params->cpr_uri = g_strdup(s->parameters.cpr_uri);
     params->has_cpr_exec_command = true;
     params->cpr_exec_command = QAPI_CLONE(strList,
                                           s->parameters.cpr_exec_command);
@@ -967,6 +977,7 @@  void migrate_params_init(MigrationParameters *params)
     params->has_mode = true;
     params->has_zero_page_detection = true;
     params->has_direct_io = true;
+    params->cpr_uri = g_strdup("");
     params->has_cpr_exec_command = true;
 }
 
@@ -1257,9 +1268,15 @@  static void migrate_params_test_apply(MigrateSetParameters *params,
         dest->direct_io = params->direct_io;
     }
 
+    if (params->cpr_uri) {
+        assert(params->cpr_uri->type == QTYPE_QSTRING);
+        dest->cpr_uri = params->cpr_uri->u.s;
+    }
+
     if (params->has_cpr_exec_command) {
         dest->cpr_exec_command = params->cpr_exec_command;
     }
+
 }
 
 static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
@@ -1390,6 +1407,12 @@  static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
         s->parameters.direct_io = params->direct_io;
     }
 
+    if (params->cpr_uri) {
+        g_free(s->parameters.cpr_uri);
+        assert(params->cpr_uri->type == QTYPE_QSTRING);
+        s->parameters.cpr_uri = g_strdup(params->cpr_uri->u.s);
+    }
+
     if (params->has_cpr_exec_command) {
         qapi_free_strList(s->parameters.cpr_exec_command);
         s->parameters.cpr_exec_command =
@@ -1421,6 +1444,12 @@  void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp)
         params->tls_authz->u.s = strdup("");
     }
 
+    if (params->cpr_uri && params->cpr_uri->type == QTYPE_QNULL) {
+        qobject_unref(params->cpr_uri->u.n);
+        params->cpr_uri->type = QTYPE_QSTRING;
+        params->cpr_uri->u.s = strdup("");
+    }
+
     migrate_params_test_apply(params, &tmp);
 
     if (!migrate_params_check(&tmp, errp)) {
diff --git a/migration/options.h b/migration/options.h
index a239702..7492fcf 100644
--- a/migration/options.h
+++ b/migration/options.h
@@ -85,6 +85,7 @@  const char *migrate_tls_creds(void);
 const char *migrate_tls_hostname(void);
 uint64_t migrate_xbzrle_cache_size(void);
 ZeroPageDetection migrate_zero_page_detection(void);
+const char *migrate_cpr_uri(void);
 
 /* parameters helpers */
 
diff --git a/qapi/migration.json b/qapi/migration.json
index 4e626df..df62456 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -851,6 +851,9 @@ 
 #     only has effect if the @mapped-ram capability is enabled.
 #     (Since 9.1)
 #
+# @cpr-uri: URI for an additional migration channel needed by
+#     @cpr-transfer mode. (Since 9.1)
+#
 # @cpr-exec-command: Command to start the new QEMU process when @mode
 #     is @cpr-exec.  The first list element is the program's filename,
 #     the remainder its arguments. (Since 9.1)
@@ -881,6 +884,7 @@ 
            'mode',
            'zero-page-detection',
            'direct-io',
+           'cpr-uri',
            'cpr-exec-command'] }
 
 ##
@@ -1031,6 +1035,9 @@ 
 #     only has effect if the @mapped-ram capability is enabled.
 #     (Since 9.1)
 #
+# @cpr-uri: URI for an additional migration channel needed by
+#     @cpr-transfer mode. (Since 9.1)
+#
 # @cpr-exec-command: Command to start the new QEMU process when @mode
 #     is @cpr-exec.  The first list element is the program's filename,
 #     the remainder its arguments. (Since 9.1)
@@ -1076,6 +1083,7 @@ 
             '*mode': 'MigMode',
             '*zero-page-detection': 'ZeroPageDetection',
             '*direct-io': 'bool',
+            '*cpr-uri': 'StrOrNull',
             '*cpr-exec-command': [ 'str' ]} }
 
 ##
@@ -1240,6 +1248,9 @@ 
 #     only has effect if the @mapped-ram capability is enabled.
 #     (Since 9.1)
 #
+# @cpr-uri: URI for an additional migration channel needed by
+#     @cpr-transfer mode. (Since 9.1)
+#
 # @cpr-exec-command: Command to start the new QEMU process when @mode
 #     is @cpr-exec.  The first list element is the program's filename,
 #     the remainder its arguments. (Since 9.1)
@@ -1282,6 +1293,7 @@ 
             '*mode': 'MigMode',
             '*zero-page-detection': 'ZeroPageDetection',
             '*direct-io': 'bool',
+            '*cpr-uri': 'str',
             '*cpr-exec-command': [ 'str' ]} }
 
 ##