[libvirt] [PATCH 00/12] qemu: Add support for more migration parameters

QEMU is transforming existing special migration parameters (those which need dedicated QMP commands to be set or queried) into proper parameters handled by query-migrate-parameters and migrate-set-parameters. Even though we may still want to use the existing commands adding support for tha transformed parameters will help us clean all of them before migration and reset them to their original values after a failed migration. Thus we wouldn't need to add more ad-hoc code which resets some of them and ignores some others. Jiri Denemark (12): qemu: Generalize PARSE macro in qemuMonitorJSONGetMigrationParams qemu: Use macro for parsing string migration parameters qemu: Use macro for parsing ull migration parameters qemu: Generalize APPEND macro in qemuMonitorJSONSetMigrationParams qemu: Use macro for setting string migration parameters qemu: Drop giant if statement from qemuMonitorSetMigrationParams qemumonitorjsontest: Rename 1st CHECK macro in migration params test qemumonitorjsontest: Rename 2nd CHECK macro in migration params test qemu: Add support for setting downtime-limit migration parameter qemu: Rename TLS related migration parameters qemu: Add support for max-bandwidth migration parameter qemu: Add support for block-incremental migration parameter src/qemu/qemu_migration.c | 23 +++++----- src/qemu/qemu_monitor.c | 20 +++------ src/qemu/qemu_monitor.h | 10 ++++- src/qemu/qemu_monitor_json.c | 104 +++++++++++++++++++++++++++---------------- tests/qemumonitorjsontest.c | 46 ++++++++++++------- 5 files changed, 123 insertions(+), 80 deletions(-) -- 2.14.3

The macro (now called PARSE_SET) is now usable for any type which needs a *_set bool for indicating a valid value. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_monitor_json.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 05cc634d2..16554d5b2 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2679,20 +2679,23 @@ qemuMonitorJSONGetMigrationParams(qemuMonitorPtr mon, result = virJSONValueObjectGet(reply, "return"); -#define PARSE(VAR, FIELD) \ +#define PARSE_SET(API, VAR, FIELD) \ do { \ - if (virJSONValueObjectGetNumberInt(result, FIELD, \ - ¶ms->VAR) == 0) \ + if (API(result, FIELD, ¶ms->VAR) == 0) \ params->VAR ## _set = true; \ } while (0) - PARSE(compressLevel, "compress-level"); - PARSE(compressThreads, "compress-threads"); - PARSE(decompressThreads, "decompress-threads"); - PARSE(cpuThrottleInitial, "cpu-throttle-initial"); - PARSE(cpuThrottleIncrement, "cpu-throttle-increment"); +#define PARSE_INT(VAR, FIELD) \ + PARSE_SET(virJSONValueObjectGetNumberInt, VAR, FIELD) -#undef PARSE + PARSE_INT(compressLevel, "compress-level"); + PARSE_INT(compressThreads, "compress-threads"); + PARSE_INT(decompressThreads, "decompress-threads"); + PARSE_INT(cpuThrottleInitial, "cpu-throttle-initial"); + PARSE_INT(cpuThrottleIncrement, "cpu-throttle-increment"); + +#undef PARSE_SET +#undef PARSE_INT if (virJSONValueObjectGetNumberUlong(result, "downtime-limit", ¶ms->downtimeLimit) == 0) -- 2.14.3

Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_monitor_json.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 16554d5b2..cb0bb0d0d 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2659,7 +2659,6 @@ qemuMonitorJSONGetMigrationParams(qemuMonitorPtr mon, virJSONValuePtr result; virJSONValuePtr cmd = NULL; virJSONValuePtr reply = NULL; - const char *tlsStr = NULL; memset(params, 0, sizeof(*params)); @@ -2688,29 +2687,31 @@ qemuMonitorJSONGetMigrationParams(qemuMonitorPtr mon, #define PARSE_INT(VAR, FIELD) \ PARSE_SET(virJSONValueObjectGetNumberInt, VAR, FIELD) +#define PARSE_STR(VAR, FIELD) \ + do { \ + const char *str; \ + if ((str = virJSONValueObjectGetString(result, FIELD))) { \ + if (VIR_STRDUP(params->VAR, str) < 0) \ + goto cleanup; \ + } \ + } while (0) + PARSE_INT(compressLevel, "compress-level"); PARSE_INT(compressThreads, "compress-threads"); PARSE_INT(decompressThreads, "decompress-threads"); PARSE_INT(cpuThrottleInitial, "cpu-throttle-initial"); PARSE_INT(cpuThrottleIncrement, "cpu-throttle-increment"); + PARSE_STR(migrateTLSAlias, "tls-creds"); + PARSE_STR(migrateTLSHostname, "tls-hostname"); #undef PARSE_SET #undef PARSE_INT +#undef PARSE_STR if (virJSONValueObjectGetNumberUlong(result, "downtime-limit", ¶ms->downtimeLimit) == 0) params->downtimeLimit_set = true; - if ((tlsStr = virJSONValueObjectGetString(result, "tls-creds"))) { - if (VIR_STRDUP(params->migrateTLSAlias, tlsStr) < 0) - goto cleanup; - } - - if ((tlsStr = virJSONValueObjectGetString(result, "tls-hostname"))) { - if (VIR_STRDUP(params->migrateTLSHostname, tlsStr) < 0) - goto cleanup; - } - ret = 0; cleanup: virJSONValueFree(cmd); -- 2.14.3

Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_monitor_json.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index cb0bb0d0d..76fc84ed0 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2687,6 +2687,9 @@ qemuMonitorJSONGetMigrationParams(qemuMonitorPtr mon, #define PARSE_INT(VAR, FIELD) \ PARSE_SET(virJSONValueObjectGetNumberInt, VAR, FIELD) +#define PARSE_ULONG(VAR, FIELD) \ + PARSE_SET(virJSONValueObjectGetNumberUlong, VAR, FIELD) + #define PARSE_STR(VAR, FIELD) \ do { \ const char *str; \ @@ -2703,15 +2706,13 @@ qemuMonitorJSONGetMigrationParams(qemuMonitorPtr mon, PARSE_INT(cpuThrottleIncrement, "cpu-throttle-increment"); PARSE_STR(migrateTLSAlias, "tls-creds"); PARSE_STR(migrateTLSHostname, "tls-hostname"); + PARSE_ULONG(downtimeLimit, "downtime-limit"); #undef PARSE_SET #undef PARSE_INT +#undef PARSE_ULONG #undef PARSE_STR - if (virJSONValueObjectGetNumberUlong(result, "downtime-limit", - ¶ms->downtimeLimit) == 0) - params->downtimeLimit_set = true; - ret = 0; cleanup: virJSONValueFree(cmd); -- 2.14.3

The APPEND macro is now be usable for any type. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_monitor_json.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 76fc84ed0..218bbd8bd 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2739,21 +2739,24 @@ qemuMonitorJSONSetMigrationParams(qemuMonitorPtr mon, if (!(args = virJSONValueNewObject())) goto cleanup; -#define APPEND(VAR, FIELD) \ +#define APPEND(VALID, API, VAR, FIELD) \ do { \ - if (params->VAR ## _set && \ - virJSONValueObjectAppendNumberInt(args, FIELD, \ - params->VAR) < 0) \ + if (VALID && API(args, FIELD, params->VAR) < 0) \ goto cleanup; \ } while (0) - APPEND(compressLevel, "compress-level"); - APPEND(compressThreads, "compress-threads"); - APPEND(decompressThreads, "decompress-threads"); - APPEND(cpuThrottleInitial, "cpu-throttle-initial"); - APPEND(cpuThrottleIncrement, "cpu-throttle-increment"); +#define APPEND_INT(VAR, FIELD) \ + APPEND(params->VAR ## _set, \ + virJSONValueObjectAppendNumberInt, VAR, FIELD) + + APPEND_INT(compressLevel, "compress-level"); + APPEND_INT(compressThreads, "compress-threads"); + APPEND_INT(decompressThreads, "decompress-threads"); + APPEND_INT(cpuThrottleInitial, "cpu-throttle-initial"); + APPEND_INT(cpuThrottleIncrement, "cpu-throttle-increment"); #undef APPEND +#undef APPEND_INT if (params->migrateTLSAlias && virJSONValueObjectAppendString(args, "tls-creds", -- 2.14.3

Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_monitor_json.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 218bbd8bd..826133543 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2749,24 +2749,21 @@ qemuMonitorJSONSetMigrationParams(qemuMonitorPtr mon, APPEND(params->VAR ## _set, \ virJSONValueObjectAppendNumberInt, VAR, FIELD) +#define APPEND_STR(VAR, FIELD) \ + APPEND(params->VAR, \ + virJSONValueObjectAppendString, VAR, FIELD) + APPEND_INT(compressLevel, "compress-level"); APPEND_INT(compressThreads, "compress-threads"); APPEND_INT(decompressThreads, "decompress-threads"); APPEND_INT(cpuThrottleInitial, "cpu-throttle-initial"); APPEND_INT(cpuThrottleIncrement, "cpu-throttle-increment"); + APPEND_STR(migrateTLSAlias, "tls-creds"); + APPEND_STR(migrateTLSHostname, "tls-hostname"); #undef APPEND #undef APPEND_INT - - if (params->migrateTLSAlias && - virJSONValueObjectAppendString(args, "tls-creds", - params->migrateTLSAlias) < 0) - goto cleanup; - - if (params->migrateTLSHostname && - virJSONValueObjectAppendString(args, "tls-hostname", - params->migrateTLSHostname) < 0) - goto cleanup; +#undef APPEND_STR if (virJSONValueObjectAppend(cmd, "arguments", args) < 0) goto cleanup; -- 2.14.3

The check can be easily replaced with a simple test in the JSON implementation and we don't need to update it every time a new parameter is added. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_monitor.c | 9 --------- src/qemu/qemu_monitor_json.c | 5 +++++ 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index dd9d64a20..71069827e 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2618,15 +2618,6 @@ qemuMonitorSetMigrationParams(qemuMonitorPtr mon, QEMU_CHECK_MONITOR_JSON(mon); - if (!params->compressLevel_set && - !params->compressThreads_set && - !params->decompressThreads_set && - !params->cpuThrottleInitial_set && - !params->cpuThrottleIncrement_set && - !params->migrateTLSAlias && - !params->migrateTLSHostname) - return 0; - return qemuMonitorJSONSetMigrationParams(mon, params); } diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 826133543..d3c37ded8 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2765,6 +2765,11 @@ qemuMonitorJSONSetMigrationParams(qemuMonitorPtr mon, #undef APPEND_INT #undef APPEND_STR + if (virJSONValueObjectKeysNumber(args) == 0) { + ret = 0; + goto cleanup; + } + if (virJSONValueObjectAppend(cmd, "arguments", args) < 0) goto cleanup; args = NULL; -- 2.14.3

The first CHECK macro in the test is used for checking integer values. Let's make it a bit more generic to be usable for any numeric type and use it for a new CHECK_INT macro. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- tests/qemumonitorjsontest.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 4d3b738e5..1dcff6672 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1813,7 +1813,7 @@ testQemuMonitorJSONqemuMonitorJSONGetMigrationParams(const void *data) ¶ms) < 0) goto cleanup; -#define CHECK(VAR, FIELD, VALUE) \ +#define CHECK_NUM(VAR, FIELD, VALUE, FMT) \ do { \ if (!params.VAR ## _set) { \ virReportError(VIR_ERR_INTERNAL_ERROR, "%s is not set", FIELD); \ @@ -1821,19 +1821,23 @@ testQemuMonitorJSONqemuMonitorJSONGetMigrationParams(const void *data) } \ if (params.VAR != VALUE) { \ virReportError(VIR_ERR_INTERNAL_ERROR, \ - "Invalid %s: %d, expected %d", \ + "Invalid %s: " FMT ", expected " FMT, \ FIELD, params.VAR, VALUE); \ goto cleanup; \ } \ } while (0) - CHECK(compressLevel, "compress-level", 1); - CHECK(compressThreads, "compress-threads", 8); - CHECK(decompressThreads, "decompress-threads", 2); - CHECK(cpuThrottleInitial, "cpu-throttle-initial", 20); - CHECK(cpuThrottleIncrement, "cpu-throttle-increment", 10); +#define CHECK_INT(VAR, FIELD, VALUE) \ + CHECK_NUM(VAR, FIELD, VALUE, "%d") -#undef CHECK + CHECK_INT(compressLevel, "compress-level", 1); + CHECK_INT(compressThreads, "compress-threads", 8); + CHECK_INT(decompressThreads, "decompress-threads", 2); + CHECK_INT(cpuThrottleInitial, "cpu-throttle-initial", 20); + CHECK_INT(cpuThrottleIncrement, "cpu-throttle-increment", 10); + +#undef CHECK_NUM +#undef CHECK_INT #define CHECK(VAR, FIELD, VALUE) \ do { \ -- 2.14.3

The second CHECK macro was used for string parameters. Let's rename it to CHECK_STR and move it up to have all checks in one place. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- tests/qemumonitorjsontest.c | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 1dcff6672..2cefdcac9 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1830,16 +1830,7 @@ testQemuMonitorJSONqemuMonitorJSONGetMigrationParams(const void *data) #define CHECK_INT(VAR, FIELD, VALUE) \ CHECK_NUM(VAR, FIELD, VALUE, "%d") - CHECK_INT(compressLevel, "compress-level", 1); - CHECK_INT(compressThreads, "compress-threads", 8); - CHECK_INT(decompressThreads, "decompress-threads", 2); - CHECK_INT(cpuThrottleInitial, "cpu-throttle-initial", 20); - CHECK_INT(cpuThrottleIncrement, "cpu-throttle-increment", 10); - -#undef CHECK_NUM -#undef CHECK_INT - -#define CHECK(VAR, FIELD, VALUE) \ +#define CHECK_STR(VAR, FIELD, VALUE) \ do { \ if (!params.VAR) { \ virReportError(VIR_ERR_INTERNAL_ERROR, "%s is not set", FIELD); \ @@ -1853,10 +1844,17 @@ testQemuMonitorJSONqemuMonitorJSONGetMigrationParams(const void *data) } \ } while (0) - CHECK(migrateTLSAlias, "tls-creds", "tls0"); - CHECK(migrateTLSHostname, "tls-hostname", ""); + CHECK_INT(compressLevel, "compress-level", 1); + CHECK_INT(compressThreads, "compress-threads", 8); + CHECK_INT(decompressThreads, "decompress-threads", 2); + CHECK_INT(cpuThrottleInitial, "cpu-throttle-initial", 20); + CHECK_INT(cpuThrottleIncrement, "cpu-throttle-increment", 10); + CHECK_STR(migrateTLSAlias, "tls-creds", "tls0"); + CHECK_STR(migrateTLSHostname, "tls-hostname", ""); -#undef CHECK +#undef CHECK_NUM +#undef CHECK_INT +#undef CHECK_STR ret = 0; -- 2.14.3

We already support setting the maximum downtime with a dedicated virDomainMigrateSetMaxDowntime API. This patch does not implement another way of setting the downtime by adding a new public migration parameter. It just makes sure any parameter we are able to get from a QEMU monitor by query-migrate-parameters can be passed back to QEMU via migrate-set-parameters. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_monitor.c | 5 +++-- src/qemu/qemu_monitor_json.c | 6 ++++++ tests/qemumonitorjsontest.c | 8 +++++++- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 71069827e..c88726735 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2607,14 +2607,15 @@ qemuMonitorSetMigrationParams(qemuMonitorPtr mon, VIR_DEBUG("compressLevel=%d:%d compressThreads=%d:%d " "decompressThreads=%d:%d cpuThrottleInitial=%d:%d " "cpuThrottleIncrement=%d:%d tlsAlias=%s " - "tlsHostname=%s", + "tlsHostname=%s downtimeLimit=%d:%llu", params->compressLevel_set, params->compressLevel, params->compressThreads_set, params->compressThreads, params->decompressThreads_set, params->decompressThreads, params->cpuThrottleInitial_set, params->cpuThrottleInitial, params->cpuThrottleIncrement_set, params->cpuThrottleIncrement, NULLSTR(params->migrateTLSAlias), - NULLSTR(params->migrateTLSHostname)); + NULLSTR(params->migrateTLSHostname), + params->downtimeLimit_set, params->downtimeLimit); QEMU_CHECK_MONITOR_JSON(mon); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index d3c37ded8..a2f3e3317 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2753,6 +2753,10 @@ qemuMonitorJSONSetMigrationParams(qemuMonitorPtr mon, APPEND(params->VAR, \ virJSONValueObjectAppendString, VAR, FIELD) +#define APPEND_ULONG(VAR, FIELD) \ + APPEND(params->VAR ## _set, \ + virJSONValueObjectAppendNumberUlong, VAR, FIELD) + APPEND_INT(compressLevel, "compress-level"); APPEND_INT(compressThreads, "compress-threads"); APPEND_INT(decompressThreads, "decompress-threads"); @@ -2760,10 +2764,12 @@ qemuMonitorJSONSetMigrationParams(qemuMonitorPtr mon, APPEND_INT(cpuThrottleIncrement, "cpu-throttle-increment"); APPEND_STR(migrateTLSAlias, "tls-creds"); APPEND_STR(migrateTLSHostname, "tls-hostname"); + APPEND_ULONG(downtimeLimit, "downtime-limit"); #undef APPEND #undef APPEND_INT #undef APPEND_STR +#undef APPEND_ULONG if (virJSONValueObjectKeysNumber(args) == 0) { ret = 0; diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 2cefdcac9..cc55b0c43 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1803,7 +1803,8 @@ testQemuMonitorJSONqemuMonitorJSONGetMigrationParams(const void *data) " \"compress-level\": 1," " \"cpu-throttle-initial\": 20," " \"tls-creds\": \"tls0\"," - " \"tls-hostname\": \"\"" + " \"tls-hostname\": \"\"," + " \"downtime-limit\": 500" " }" "}") < 0) { goto cleanup; @@ -1830,6 +1831,9 @@ testQemuMonitorJSONqemuMonitorJSONGetMigrationParams(const void *data) #define CHECK_INT(VAR, FIELD, VALUE) \ CHECK_NUM(VAR, FIELD, VALUE, "%d") +#define CHECK_ULONG(VAR, FIELD, VALUE) \ + CHECK_NUM(VAR, FIELD, VALUE, "%llu") + #define CHECK_STR(VAR, FIELD, VALUE) \ do { \ if (!params.VAR) { \ @@ -1851,9 +1855,11 @@ testQemuMonitorJSONqemuMonitorJSONGetMigrationParams(const void *data) CHECK_INT(cpuThrottleIncrement, "cpu-throttle-increment", 10); CHECK_STR(migrateTLSAlias, "tls-creds", "tls0"); CHECK_STR(migrateTLSHostname, "tls-hostname", ""); + CHECK_ULONG(downtimeLimit, "downtime-limit", 500ULL); #undef CHECK_NUM #undef CHECK_INT +#undef CHECK_ULONG #undef CHECK_STR ret = 0; -- 2.14.3

The parameters used "migrate" prefix which is pretty redundant and qemuMonitorMigrationParams structure is our internal representation of QEMU migration parameters and it is supposed to use names which match QEMU names. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration.c | 23 +++++++++++------------ src/qemu/qemu_monitor.c | 7 +++---- src/qemu/qemu_monitor.h | 4 ++-- src/qemu/qemu_monitor_json.c | 8 ++++---- tests/qemumonitorjsontest.c | 8 ++++---- 5 files changed, 24 insertions(+), 26 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index af744661f..e4e9e79cc 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -114,7 +114,7 @@ qemuMigrationCheckTLSCreds(virQEMUDriverPtr driver, goto cleanup; /* NB: Could steal NULL pointer too! Let caller decide what to do. */ - VIR_STEAL_PTR(priv->migTLSAlias, migParams.migrateTLSAlias); + VIR_STEAL_PTR(priv->migTLSAlias, migParams.tlsCreds); ret = 0; @@ -225,7 +225,7 @@ qemuMigrationAddTLSObjects(virQEMUDriverPtr driver, *tlsAlias, &tlsProps) < 0) goto error; - if (VIR_STRDUP(migParams->migrateTLSAlias, *tlsAlias) < 0) + if (VIR_STRDUP(migParams->tlsCreds, *tlsAlias) < 0) goto error; return 0; @@ -2349,8 +2349,8 @@ qemuMigrationParamsClear(qemuMonitorMigrationParamsPtr migParams) if (!migParams) return; - VIR_FREE(migParams->migrateTLSAlias); - VIR_FREE(migParams->migrateTLSHostname); + VIR_FREE(migParams->tlsCreds); + VIR_FREE(migParams->tlsHostname); } @@ -2391,8 +2391,8 @@ qemuMigrationSetEmptyTLSParams(virQEMUDriverPtr driver, if (!priv->migTLSAlias) return 0; - if (VIR_STRDUP(migParams->migrateTLSAlias, "") < 0 || - VIR_STRDUP(migParams->migrateTLSHostname, "") < 0) + if (VIR_STRDUP(migParams->tlsCreds, "") < 0 || + VIR_STRDUP(migParams->tlsHostname, "") < 0) return -1; return 0; @@ -2508,8 +2508,8 @@ qemuMigrationResetTLS(virQEMUDriverPtr driver, qemuDomainDelTLSObjects(driver, vm, asyncJob, secAlias, tlsAlias); qemuDomainSecretInfoFree(&priv->migSecinfo); - if (VIR_STRDUP(migParams.migrateTLSAlias, "") < 0 || - VIR_STRDUP(migParams.migrateTLSHostname, "") < 0 || + if (VIR_STRDUP(migParams.tlsCreds, "") < 0 || + VIR_STRDUP(migParams.tlsHostname, "") < 0 || qemuMigrationSetParams(driver, vm, asyncJob, &migParams) < 0) goto cleanup; @@ -2774,7 +2774,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, goto stopjob; /* Force reset of 'tls-hostname', it's a source only parameter */ - if (VIR_STRDUP(migParams.migrateTLSHostname, "") < 0) + if (VIR_STRDUP(migParams.tlsHostname, "") < 0) goto stopjob; } else { @@ -3737,12 +3737,11 @@ qemuMigrationRun(virQEMUDriverPtr driver, * connect directly to the destination. */ if (spec->destType == MIGRATION_DEST_CONNECT_HOST || spec->destType == MIGRATION_DEST_FD) { - if (VIR_STRDUP(migParams->migrateTLSHostname, - spec->dest.host.name) < 0) + if (VIR_STRDUP(migParams->tlsHostname, spec->dest.host.name) < 0) goto error; } else { /* Be sure there's nothing from a previous migration */ - if (VIR_STRDUP(migParams->migrateTLSHostname, "") < 0) + if (VIR_STRDUP(migParams->tlsHostname, "") < 0) goto error; } } else { diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index c88726735..3e2c69a9a 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2606,15 +2606,14 @@ qemuMonitorSetMigrationParams(qemuMonitorPtr mon, { VIR_DEBUG("compressLevel=%d:%d compressThreads=%d:%d " "decompressThreads=%d:%d cpuThrottleInitial=%d:%d " - "cpuThrottleIncrement=%d:%d tlsAlias=%s " - "tlsHostname=%s downtimeLimit=%d:%llu", + "cpuThrottleIncrement=%d:%d tlsCreds=%s tlsHostname=%s " + "downtimeLimit=%d:%llu", params->compressLevel_set, params->compressLevel, params->compressThreads_set, params->compressThreads, params->decompressThreads_set, params->decompressThreads, params->cpuThrottleInitial_set, params->cpuThrottleInitial, params->cpuThrottleIncrement_set, params->cpuThrottleIncrement, - NULLSTR(params->migrateTLSAlias), - NULLSTR(params->migrateTLSHostname), + NULLSTR(params->tlsCreds), NULLSTR(params->tlsHostname), params->downtimeLimit_set, params->downtimeLimit); QEMU_CHECK_MONITOR_JSON(mon); diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index bc8494fae..e123baaae 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -625,8 +625,8 @@ struct _qemuMonitorMigrationParams { /* Value is either NULL, "", or some string. NULL indicates no support; * whereas, some string value indicates we can support setting/clearing */ - char *migrateTLSAlias; - char *migrateTLSHostname; + char *tlsCreds; + char *tlsHostname; bool downtimeLimit_set; unsigned long long downtimeLimit; diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index a2f3e3317..9f238bc30 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2704,8 +2704,8 @@ qemuMonitorJSONGetMigrationParams(qemuMonitorPtr mon, PARSE_INT(decompressThreads, "decompress-threads"); PARSE_INT(cpuThrottleInitial, "cpu-throttle-initial"); PARSE_INT(cpuThrottleIncrement, "cpu-throttle-increment"); - PARSE_STR(migrateTLSAlias, "tls-creds"); - PARSE_STR(migrateTLSHostname, "tls-hostname"); + PARSE_STR(tlsCreds, "tls-creds"); + PARSE_STR(tlsHostname, "tls-hostname"); PARSE_ULONG(downtimeLimit, "downtime-limit"); #undef PARSE_SET @@ -2762,8 +2762,8 @@ qemuMonitorJSONSetMigrationParams(qemuMonitorPtr mon, APPEND_INT(decompressThreads, "decompress-threads"); APPEND_INT(cpuThrottleInitial, "cpu-throttle-initial"); APPEND_INT(cpuThrottleIncrement, "cpu-throttle-increment"); - APPEND_STR(migrateTLSAlias, "tls-creds"); - APPEND_STR(migrateTLSHostname, "tls-hostname"); + APPEND_STR(tlsCreds, "tls-creds"); + APPEND_STR(tlsHostname, "tls-hostname"); APPEND_ULONG(downtimeLimit, "downtime-limit"); #undef APPEND diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index cc55b0c43..aa5da8be9 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1853,8 +1853,8 @@ testQemuMonitorJSONqemuMonitorJSONGetMigrationParams(const void *data) CHECK_INT(decompressThreads, "decompress-threads", 2); CHECK_INT(cpuThrottleInitial, "cpu-throttle-initial", 20); CHECK_INT(cpuThrottleIncrement, "cpu-throttle-increment", 10); - CHECK_STR(migrateTLSAlias, "tls-creds", "tls0"); - CHECK_STR(migrateTLSHostname, "tls-hostname", ""); + CHECK_STR(tlsCreds, "tls-creds", "tls0"); + CHECK_STR(tlsHostname, "tls-hostname", ""); CHECK_ULONG(downtimeLimit, "downtime-limit", 500ULL); #undef CHECK_NUM @@ -1865,8 +1865,8 @@ testQemuMonitorJSONqemuMonitorJSONGetMigrationParams(const void *data) ret = 0; cleanup: - VIR_FREE(params.migrateTLSAlias); - VIR_FREE(params.migrateTLSHostname); + VIR_FREE(params.tlsCreds); + VIR_FREE(params.tlsHostname); qemuMonitorTestFree(test); return ret; } -- 2.14.3

We already support several ways of setting migration bandwidth and this is not adding another one. With this patch we are able to read and write this parameter using query-migrate-parameters and migrate-set-parameters in one call with all other parameters. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_monitor.c | 3 ++- src/qemu/qemu_monitor.h | 3 +++ src/qemu/qemu_monitor_json.c | 2 ++ tests/qemumonitorjsontest.c | 2 ++ 4 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 3e2c69a9a..04b18baf9 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2607,13 +2607,14 @@ qemuMonitorSetMigrationParams(qemuMonitorPtr mon, VIR_DEBUG("compressLevel=%d:%d compressThreads=%d:%d " "decompressThreads=%d:%d cpuThrottleInitial=%d:%d " "cpuThrottleIncrement=%d:%d tlsCreds=%s tlsHostname=%s " - "downtimeLimit=%d:%llu", + "maxBandwidth=%d:%llu downtimeLimit=%d:%llu", params->compressLevel_set, params->compressLevel, params->compressThreads_set, params->compressThreads, params->decompressThreads_set, params->decompressThreads, params->cpuThrottleInitial_set, params->cpuThrottleInitial, params->cpuThrottleIncrement_set, params->cpuThrottleIncrement, NULLSTR(params->tlsCreds), NULLSTR(params->tlsHostname), + params->maxBandwidth_set, params->maxBandwidth, params->downtimeLimit_set, params->downtimeLimit); QEMU_CHECK_MONITOR_JSON(mon); diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index e123baaae..7836dd332 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -628,6 +628,9 @@ struct _qemuMonitorMigrationParams { char *tlsCreds; char *tlsHostname; + bool maxBandwidth_set; + unsigned long long maxBandwidth; + bool downtimeLimit_set; unsigned long long downtimeLimit; }; diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 9f238bc30..115610e50 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2706,6 +2706,7 @@ qemuMonitorJSONGetMigrationParams(qemuMonitorPtr mon, PARSE_INT(cpuThrottleIncrement, "cpu-throttle-increment"); PARSE_STR(tlsCreds, "tls-creds"); PARSE_STR(tlsHostname, "tls-hostname"); + PARSE_ULONG(maxBandwidth, "max-bandwidth"); PARSE_ULONG(downtimeLimit, "downtime-limit"); #undef PARSE_SET @@ -2764,6 +2765,7 @@ qemuMonitorJSONSetMigrationParams(qemuMonitorPtr mon, APPEND_INT(cpuThrottleIncrement, "cpu-throttle-increment"); APPEND_STR(tlsCreds, "tls-creds"); APPEND_STR(tlsHostname, "tls-hostname"); + APPEND_ULONG(maxBandwidth, "max-bandwidth"); APPEND_ULONG(downtimeLimit, "downtime-limit"); #undef APPEND diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index aa5da8be9..488c79cc3 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1804,6 +1804,7 @@ testQemuMonitorJSONqemuMonitorJSONGetMigrationParams(const void *data) " \"cpu-throttle-initial\": 20," " \"tls-creds\": \"tls0\"," " \"tls-hostname\": \"\"," + " \"max-bandwidth\": 1234567890," " \"downtime-limit\": 500" " }" "}") < 0) { @@ -1855,6 +1856,7 @@ testQemuMonitorJSONqemuMonitorJSONGetMigrationParams(const void *data) CHECK_INT(cpuThrottleIncrement, "cpu-throttle-increment", 10); CHECK_STR(tlsCreds, "tls-creds", "tls0"); CHECK_STR(tlsHostname, "tls-hostname", ""); + CHECK_ULONG(maxBandwidth, "max-bandwidth", 1234567890ULL); CHECK_ULONG(downtimeLimit, "downtime-limit", 500ULL); #undef CHECK_NUM -- 2.14.3

We handle incremental storage migration in a different way. The support for this new (as of QEMU 2.10) parameter is only needed for full coverage of migration parameters used by QEMU. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_monitor.c | 6 ++++-- src/qemu/qemu_monitor.h | 3 +++ src/qemu/qemu_monitor_json.c | 10 ++++++++++ tests/qemumonitorjsontest.c | 8 +++++++- 4 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 04b18baf9..611876ff8 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2607,7 +2607,8 @@ qemuMonitorSetMigrationParams(qemuMonitorPtr mon, VIR_DEBUG("compressLevel=%d:%d compressThreads=%d:%d " "decompressThreads=%d:%d cpuThrottleInitial=%d:%d " "cpuThrottleIncrement=%d:%d tlsCreds=%s tlsHostname=%s " - "maxBandwidth=%d:%llu downtimeLimit=%d:%llu", + "maxBandwidth=%d:%llu downtimeLimit=%d:%llu " + "blockIncremental=%d:%d", params->compressLevel_set, params->compressLevel, params->compressThreads_set, params->compressThreads, params->decompressThreads_set, params->decompressThreads, @@ -2615,7 +2616,8 @@ qemuMonitorSetMigrationParams(qemuMonitorPtr mon, params->cpuThrottleIncrement_set, params->cpuThrottleIncrement, NULLSTR(params->tlsCreds), NULLSTR(params->tlsHostname), params->maxBandwidth_set, params->maxBandwidth, - params->downtimeLimit_set, params->downtimeLimit); + params->downtimeLimit_set, params->downtimeLimit, + params->blockIncremental_set, params->blockIncremental); QEMU_CHECK_MONITOR_JSON(mon); diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 7836dd332..f81fb7f2a 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -633,6 +633,9 @@ struct _qemuMonitorMigrationParams { bool downtimeLimit_set; unsigned long long downtimeLimit; + + bool blockIncremental_set; + bool blockIncremental; }; int qemuMonitorGetMigrationParams(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 115610e50..aa2599209 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2690,6 +2690,9 @@ qemuMonitorJSONGetMigrationParams(qemuMonitorPtr mon, #define PARSE_ULONG(VAR, FIELD) \ PARSE_SET(virJSONValueObjectGetNumberUlong, VAR, FIELD) +#define PARSE_BOOL(VAR, FIELD) \ + PARSE_SET(virJSONValueObjectGetBoolean, VAR, FIELD) + #define PARSE_STR(VAR, FIELD) \ do { \ const char *str; \ @@ -2708,10 +2711,12 @@ qemuMonitorJSONGetMigrationParams(qemuMonitorPtr mon, PARSE_STR(tlsHostname, "tls-hostname"); PARSE_ULONG(maxBandwidth, "max-bandwidth"); PARSE_ULONG(downtimeLimit, "downtime-limit"); + PARSE_BOOL(blockIncremental, "block-incremental"); #undef PARSE_SET #undef PARSE_INT #undef PARSE_ULONG +#undef PARSE_BOOL #undef PARSE_STR ret = 0; @@ -2758,6 +2763,10 @@ qemuMonitorJSONSetMigrationParams(qemuMonitorPtr mon, APPEND(params->VAR ## _set, \ virJSONValueObjectAppendNumberUlong, VAR, FIELD) +#define APPEND_BOOL(VAR, FIELD) \ + APPEND(params->VAR ## _set, \ + virJSONValueObjectAppendBoolean, VAR, FIELD) + APPEND_INT(compressLevel, "compress-level"); APPEND_INT(compressThreads, "compress-threads"); APPEND_INT(decompressThreads, "decompress-threads"); @@ -2767,6 +2776,7 @@ qemuMonitorJSONSetMigrationParams(qemuMonitorPtr mon, APPEND_STR(tlsHostname, "tls-hostname"); APPEND_ULONG(maxBandwidth, "max-bandwidth"); APPEND_ULONG(downtimeLimit, "downtime-limit"); + APPEND_BOOL(blockIncremental, "block-incremental"); #undef APPEND #undef APPEND_INT diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 488c79cc3..aa2f67947 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1805,7 +1805,8 @@ testQemuMonitorJSONqemuMonitorJSONGetMigrationParams(const void *data) " \"tls-creds\": \"tls0\"," " \"tls-hostname\": \"\"," " \"max-bandwidth\": 1234567890," - " \"downtime-limit\": 500" + " \"downtime-limit\": 500," + " \"block-incremental\": true" " }" "}") < 0) { goto cleanup; @@ -1835,6 +1836,9 @@ testQemuMonitorJSONqemuMonitorJSONGetMigrationParams(const void *data) #define CHECK_ULONG(VAR, FIELD, VALUE) \ CHECK_NUM(VAR, FIELD, VALUE, "%llu") +#define CHECK_BOOL(VAR, FIELD, VALUE) \ + CHECK_NUM(VAR, FIELD, VALUE, "%d") + #define CHECK_STR(VAR, FIELD, VALUE) \ do { \ if (!params.VAR) { \ @@ -1858,10 +1862,12 @@ testQemuMonitorJSONqemuMonitorJSONGetMigrationParams(const void *data) CHECK_STR(tlsHostname, "tls-hostname", ""); CHECK_ULONG(maxBandwidth, "max-bandwidth", 1234567890ULL); CHECK_ULONG(downtimeLimit, "downtime-limit", 500ULL); + CHECK_BOOL(blockIncremental, "block-incremental", true); #undef CHECK_NUM #undef CHECK_INT #undef CHECK_ULONG +#undef CHECK_BOOL #undef CHECK_STR ret = 0; -- 2.14.3

On 10/26/2017 06:03 PM, Jiri Denemark wrote:
QEMU is transforming existing special migration parameters (those which need dedicated QMP commands to be set or queried) into proper parameters handled by query-migrate-parameters and migrate-set-parameters. Even though we may still want to use the existing commands adding support for tha transformed parameters will help us clean all of them before migration and reset them to their original values after a failed migration. Thus we wouldn't need to add more ad-hoc code which resets some of them and ignores some others.
Jiri Denemark (12): qemu: Generalize PARSE macro in qemuMonitorJSONGetMigrationParams qemu: Use macro for parsing string migration parameters qemu: Use macro for parsing ull migration parameters qemu: Generalize APPEND macro in qemuMonitorJSONSetMigrationParams qemu: Use macro for setting string migration parameters qemu: Drop giant if statement from qemuMonitorSetMigrationParams qemumonitorjsontest: Rename 1st CHECK macro in migration params test qemumonitorjsontest: Rename 2nd CHECK macro in migration params test qemu: Add support for setting downtime-limit migration parameter qemu: Rename TLS related migration parameters qemu: Add support for max-bandwidth migration parameter qemu: Add support for block-incremental migration parameter
src/qemu/qemu_migration.c | 23 +++++----- src/qemu/qemu_monitor.c | 20 +++------ src/qemu/qemu_monitor.h | 10 ++++- src/qemu/qemu_monitor_json.c | 104 +++++++++++++++++++++++++++---------------- tests/qemumonitorjsontest.c | 46 ++++++++++++------- 5 files changed, 123 insertions(+), 80 deletions(-)
As long as you add an update to docs/news.xml to describe the basics at least and the addition of parameters for downtime-limit, max-bandwidth, and block-incremental Reviewed-by: John Ferlan <jferlan@redhat.com> (series) John

On Thu, Nov 02, 2017 at 16:12:34 -0400, John Ferlan wrote:
On 10/26/2017 06:03 PM, Jiri Denemark wrote:
QEMU is transforming existing special migration parameters (those which need dedicated QMP commands to be set or queried) into proper parameters handled by query-migrate-parameters and migrate-set-parameters. Even though we may still want to use the existing commands adding support for tha transformed parameters will help us clean all of them before migration and reset them to their original values after a failed migration. Thus we wouldn't need to add more ad-hoc code which resets some of them and ignores some others.
Jiri Denemark (12): qemu: Generalize PARSE macro in qemuMonitorJSONGetMigrationParams qemu: Use macro for parsing string migration parameters qemu: Use macro for parsing ull migration parameters qemu: Generalize APPEND macro in qemuMonitorJSONSetMigrationParams qemu: Use macro for setting string migration parameters qemu: Drop giant if statement from qemuMonitorSetMigrationParams qemumonitorjsontest: Rename 1st CHECK macro in migration params test qemumonitorjsontest: Rename 2nd CHECK macro in migration params test qemu: Add support for setting downtime-limit migration parameter qemu: Rename TLS related migration parameters qemu: Add support for max-bandwidth migration parameter qemu: Add support for block-incremental migration parameter
src/qemu/qemu_migration.c | 23 +++++----- src/qemu/qemu_monitor.c | 20 +++------ src/qemu/qemu_monitor.h | 10 ++++- src/qemu/qemu_monitor_json.c | 104 +++++++++++++++++++++++++++---------------- tests/qemumonitorjsontest.c | 46 ++++++++++++------- 5 files changed, 123 insertions(+), 80 deletions(-)
As long as you add an update to docs/news.xml to describe the basics at least and the addition of parameters for downtime-limit, max-bandwidth, and block-incremental
Nothing to be added to news.xml really. These are all internal changes with no visible effect to a user. This will change once we use all this to properly reset all parameters before and after migration to make sure we migrate with a clean environment. It will deserve a news item, but we're not there yet.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Thanks. Jirka
participants (2)
-
Jiri Denemark
-
John Ferlan