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

2024-08-16 Thread Steven Sistare 

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 


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

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

2024-08-15 Thread Peter Xu 
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 

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,

-- 
Peter Xu

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

2024-06-30 Thread Steve Sistare 
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 
---
 migration/migration-hmp-cmds.c | 10 ++
 migration/options.c| 29 +
 migration/options.h|  1 +
 qapi/migration.json| 12 
 4 files changed, 52 insertions(+)

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_zer