[libvirt] [PATCH 0/8] qemu: Add support for setting post-copy migration bandwidth

This series adds a new VIR_MIGRATE_PARAM_BANDWIDTH_POSTCOPY typed parameter for virDomainMigrate3 and virDomainMigrateToURI3 for setting maximum post-copy migration bandwidth. In case the initial VIR_MIGRATE_PARAM_BANDWIDTH_POSTCOPY value turns out to be suboptimal a new VIR_DOMAIN_MIGRATE_MAX_SPEED_POSTCOPY flag for virDomainMigrateSetMaxSpeed and virDomainMigrateGetMaxSpeed may be used to set/get the maximum post-copy migration bandwidth while migration is already running. Jiri Denemark (8): qemu: Use C99 initializers for qemuMigrationParamsTPMap qemu: Add optional unit to qemuMigrationParamsTPMapItem qemu: Rework qemuDomainMigrateSetMaxSpeed qemu: Make migration params usable outside migration Public API for post-copy migration bandwidth qemu: Implement VIR_MIGRATE_PARAM_BANDWIDTH_POSTCOPY qemu: Implement VIR_DOMAIN_MIGRATE_MAX_SPEED_POSTCOPY flag virsh: Add support for setting post-copy migration bandwidth include/libvirt/libvirt-domain.h | 15 ++++ src/libvirt-domain.c | 11 ++- src/qemu/qemu_driver.c | 118 +++++++++++++++++++++++------- src/qemu/qemu_migration.h | 1 + src/qemu/qemu_migration_params.c | 122 +++++++++++++++++++++++-------- src/qemu/qemu_migration_params.h | 10 +++ tools/virsh-domain.c | 33 ++++++++- tools/virsh.pod | 15 +++- 8 files changed, 260 insertions(+), 65 deletions(-) -- 2.20.1

Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration_params.c | 36 ++++++++++++++++---------------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c index e112e790c2..658e785059 100644 --- a/src/qemu/qemu_migration_params.c +++ b/src/qemu/qemu_migration_params.c @@ -154,29 +154,29 @@ static const qemuMigrationParamsFlagMapItem qemuMigrationParamsFlagMap[] = { /* Translation from VIR_MIGRATE_PARAM_* typed parameters to * qemuMigrationParams. */ static const qemuMigrationParamsTPMapItem qemuMigrationParamsTPMap[] = { - {VIR_MIGRATE_PARAM_AUTO_CONVERGE_INITIAL, - QEMU_MIGRATION_PARAM_THROTTLE_INITIAL, - QEMU_MIGRATION_SOURCE}, + {.typedParam = VIR_MIGRATE_PARAM_AUTO_CONVERGE_INITIAL, + .param = QEMU_MIGRATION_PARAM_THROTTLE_INITIAL, + .party = QEMU_MIGRATION_SOURCE}, - {VIR_MIGRATE_PARAM_AUTO_CONVERGE_INCREMENT, - QEMU_MIGRATION_PARAM_THROTTLE_INCREMENT, - QEMU_MIGRATION_SOURCE}, + {.typedParam = VIR_MIGRATE_PARAM_AUTO_CONVERGE_INCREMENT, + .param = QEMU_MIGRATION_PARAM_THROTTLE_INCREMENT, + .party = QEMU_MIGRATION_SOURCE}, - {VIR_MIGRATE_PARAM_COMPRESSION_MT_LEVEL, - QEMU_MIGRATION_PARAM_COMPRESS_LEVEL, - QEMU_MIGRATION_SOURCE | QEMU_MIGRATION_DESTINATION}, + {.typedParam = VIR_MIGRATE_PARAM_COMPRESSION_MT_LEVEL, + .param = QEMU_MIGRATION_PARAM_COMPRESS_LEVEL, + .party = QEMU_MIGRATION_SOURCE | QEMU_MIGRATION_DESTINATION}, - {VIR_MIGRATE_PARAM_COMPRESSION_MT_THREADS, - QEMU_MIGRATION_PARAM_COMPRESS_THREADS, - QEMU_MIGRATION_SOURCE | QEMU_MIGRATION_DESTINATION}, + {.typedParam = VIR_MIGRATE_PARAM_COMPRESSION_MT_THREADS, + .param = QEMU_MIGRATION_PARAM_COMPRESS_THREADS, + .party = QEMU_MIGRATION_SOURCE | QEMU_MIGRATION_DESTINATION}, - {VIR_MIGRATE_PARAM_COMPRESSION_MT_DTHREADS, - QEMU_MIGRATION_PARAM_DECOMPRESS_THREADS, - QEMU_MIGRATION_SOURCE | QEMU_MIGRATION_DESTINATION}, + {.typedParam = VIR_MIGRATE_PARAM_COMPRESSION_MT_DTHREADS, + .param = QEMU_MIGRATION_PARAM_DECOMPRESS_THREADS, + .party = QEMU_MIGRATION_SOURCE | QEMU_MIGRATION_DESTINATION}, - {VIR_MIGRATE_PARAM_COMPRESSION_XBZRLE_CACHE, - QEMU_MIGRATION_PARAM_XBZRLE_CACHE_SIZE, - QEMU_MIGRATION_SOURCE | QEMU_MIGRATION_DESTINATION}, + {.typedParam = VIR_MIGRATE_PARAM_COMPRESSION_XBZRLE_CACHE, + .param = QEMU_MIGRATION_PARAM_XBZRLE_CACHE_SIZE, + .party = QEMU_MIGRATION_SOURCE | QEMU_MIGRATION_DESTINATION}, }; static const qemuMigrationParamType qemuMigrationParamTypes[] = { -- 2.20.1

On Tue, Feb 05, 2019 at 04:23:04PM +0100, Jiri Denemark wrote:
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration_params.c | 36 ++++++++++++++++---------------- 1 file changed, 18 insertions(+), 18 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Some migration parameters supported by libvirt may use units that differ from the units used by QEMU for the corresponding parameters. For example, libvirt defines migration bandwidth in MiB/s while QEMU expects B/s. Let's add a unit field to qemuMigrationParamsTPMapItem for automatic conversion when translating between libvirt's migration typed parameters and QEMU's migration paramteres. This patch is a preparation for future parameters as the existing VIR_MIGRATE_PARAM_BANDWIDTH parameter is set using "migrate_set_speed" QMP command rather than "migrate-set-parameters" for backward compatibility. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration_params.c | 63 ++++++++++++++++++++++++++------ 1 file changed, 51 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c index 658e785059..c36bed5744 100644 --- a/src/qemu/qemu_migration_params.c +++ b/src/qemu/qemu_migration_params.c @@ -120,6 +120,7 @@ struct _qemuMigrationParamsFlagMapItem { typedef struct _qemuMigrationParamsTPMapItem qemuMigrationParamsTPMapItem; struct _qemuMigrationParamsTPMapItem { const char *typedParam; + unsigned int unit; qemuMigrationParam param; int party; /* bit-wise OR of qemuMigrationParty */ }; @@ -273,7 +274,8 @@ qemuMigrationParamsGetTPInt(qemuMigrationParamsPtr migParams, qemuMigrationParam param, virTypedParameterPtr params, int nparams, - const char *name) + const char *name, + unsigned int unit) { int rc; @@ -287,6 +289,17 @@ qemuMigrationParamsGetTPInt(qemuMigrationParamsPtr migParams, &migParams->params[param].value.i)) < 0) return -1; + if (unit > 0) { + unsigned int max = UINT_MAX / unit; + if (migParams->params[param].value.i > max) { + virReportError(VIR_ERR_OVERFLOW, + _("%s migration parameter must be less than %u"), + name, max + 1); + return -1; + } + migParams->params[param].value.i *= unit; + } + migParams->params[param].set = !!rc; return 0; } @@ -298,16 +311,22 @@ qemuMigrationParamsSetTPInt(qemuMigrationParamsPtr migParams, virTypedParameterPtr *params, int *nparams, int *maxparams, - const char *name) + const char *name, + unsigned int unit) { + int value; + if (qemuMigrationParamsCheckType(param, QEMU_MIGRATION_PARAM_TYPE_INT) < 0) return -1; if (!migParams->params[param].set) return 0; - return virTypedParamsAddInt(params, nparams, maxparams, name, - migParams->params[param].value.i); + value = migParams->params[param].value.i; + if (unit > 0) + value /= unit; + + return virTypedParamsAddInt(params, nparams, maxparams, name, value); } @@ -316,7 +335,8 @@ qemuMigrationParamsGetTPULL(qemuMigrationParamsPtr migParams, qemuMigrationParam param, virTypedParameterPtr params, int nparams, - const char *name) + const char *name, + unsigned int unit) { int rc; @@ -330,6 +350,17 @@ qemuMigrationParamsGetTPULL(qemuMigrationParamsPtr migParams, &migParams->params[param].value.ull)) < 0) return -1; + if (unit > 0) { + unsigned long long max = ULLONG_MAX / unit; + if (migParams->params[param].value.ull > max) { + virReportError(VIR_ERR_OVERFLOW, + _("%s migration parameter must be less than %llu"), + name, max + 1); + return -1; + } + migParams->params[param].value.ull *= unit; + } + migParams->params[param].set = !!rc; return 0; } @@ -341,16 +372,22 @@ qemuMigrationParamsSetTPULL(qemuMigrationParamsPtr migParams, virTypedParameterPtr *params, int *nparams, int *maxparams, - const char *name) + const char *name, + unsigned int unit) { + unsigned long long value; + if (qemuMigrationParamsCheckType(param, QEMU_MIGRATION_PARAM_TYPE_ULL) < 0) return -1; if (!migParams->params[param].set) return 0; - return virTypedParamsAddULLong(params, nparams, maxparams, name, - migParams->params[param].value.ull); + value = migParams->params[param].value.ull; + if (unit > 0) + value /= unit; + + return virTypedParamsAddULLong(params, nparams, maxparams, name, value); } @@ -465,13 +502,15 @@ qemuMigrationParamsFromFlags(virTypedParameterPtr params, switch (qemuMigrationParamTypes[item->param]) { case QEMU_MIGRATION_PARAM_TYPE_INT: if (qemuMigrationParamsGetTPInt(migParams, item->param, params, - nparams, item->typedParam) < 0) + nparams, item->typedParam, + item->unit) < 0) goto error; break; case QEMU_MIGRATION_PARAM_TYPE_ULL: if (qemuMigrationParamsGetTPULL(migParams, item->param, params, - nparams, item->typedParam) < 0) + nparams, item->typedParam, + item->unit) < 0) goto error; break; @@ -533,14 +572,14 @@ qemuMigrationParamsDump(qemuMigrationParamsPtr migParams, case QEMU_MIGRATION_PARAM_TYPE_INT: if (qemuMigrationParamsSetTPInt(migParams, item->param, params, nparams, maxparams, - item->typedParam) < 0) + item->typedParam, item->unit) < 0) return -1; break; case QEMU_MIGRATION_PARAM_TYPE_ULL: if (qemuMigrationParamsSetTPULL(migParams, item->param, params, nparams, maxparams, - item->typedParam) < 0) + item->typedParam, item->unit) < 0) return -1; break; -- 2.20.1

On Tue, Feb 05, 2019 at 04:23:05PM +0100, Jiri Denemark wrote:
Some migration parameters supported by libvirt may use units that differ from the units used by QEMU for the corresponding parameters. For example, libvirt defines migration bandwidth in MiB/s while QEMU expects B/s. Let's add a unit field to qemuMigrationParamsTPMapItem for automatic conversion when translating between libvirt's migration typed parameters and QEMU's migration paramteres.
This patch is a preparation for future parameters as the existing VIR_MIGRATE_PARAM_BANDWIDTH parameter is set using "migrate_set_speed" QMP command rather than "migrate-set-parameters" for backward compatibility.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration_params.c | 63 ++++++++++++++++++++++++++------ 1 file changed, 51 insertions(+), 12 deletions(-)
diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c index 658e785059..c36bed5744 100644 --- a/src/qemu/qemu_migration_params.c +++ b/src/qemu/qemu_migration_params.c @@ -120,6 +120,7 @@ struct _qemuMigrationParamsFlagMapItem { typedef struct _qemuMigrationParamsTPMapItem qemuMigrationParamsTPMapItem; struct _qemuMigrationParamsTPMapItem { const char *typedParam; + unsigned int unit; qemuMigrationParam param; int party; /* bit-wise OR of qemuMigrationParty */ }; @@ -273,7 +274,8 @@ qemuMigrationParamsGetTPInt(qemuMigrationParamsPtr migParams, qemuMigrationParam param, virTypedParameterPtr params, int nparams, - const char *name) + const char *name, + unsigned int unit) { int rc;
@@ -287,6 +289,17 @@ qemuMigrationParamsGetTPInt(qemuMigrationParamsPtr migParams, &migParams->params[param].value.i)) < 0) return -1;
+ if (unit > 0) { + unsigned int max = UINT_MAX / unit; + if (migParams->params[param].value.i > max) { + virReportError(VIR_ERR_OVERFLOW, + _("%s migration parameter must be less than %u"),
Sounds better in my head as: migration parameter '%s' must be... (which should also be easier to translate with the parameter name quoted)
+ name, max + 1); + return -1; + } + migParams->params[param].value.i *= unit; + } + migParams->params[param].set = !!rc; return 0; } @@ -298,16 +311,22 @@ qemuMigrationParamsSetTPInt(qemuMigrationParamsPtr migParams, virTypedParameterPtr *params, int *nparams, int *maxparams, - const char *name) + const char *name, + unsigned int unit) { + int value; + if (qemuMigrationParamsCheckType(param, QEMU_MIGRATION_PARAM_TYPE_INT) < 0) return -1;
if (!migParams->params[param].set) return 0;
- return virTypedParamsAddInt(params, nparams, maxparams, name, - migParams->params[param].value.i); + value = migParams->params[param].value.i; + if (unit > 0) + value /= unit; + + return virTypedParamsAddInt(params, nparams, maxparams, name, value); }
@@ -316,7 +335,8 @@ qemuMigrationParamsGetTPULL(qemuMigrationParamsPtr migParams, qemuMigrationParam param, virTypedParameterPtr params, int nparams, - const char *name) + const char *name, + unsigned int unit) { int rc;
@@ -330,6 +350,17 @@ qemuMigrationParamsGetTPULL(qemuMigrationParamsPtr migParams, &migParams->params[param].value.ull)) < 0) return -1;
+ if (unit > 0) { + unsigned long long max = ULLONG_MAX / unit; + if (migParams->params[param].value.ull > max) { + virReportError(VIR_ERR_OVERFLOW, + _("%s migration parameter must be less than %llu"), + name, max + 1);
Same here.
+ return -1; + } + migParams->params[param].value.ull *= unit; + } + migParams->params[param].set = !!rc; return 0; }
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Let's make the code flow easier to follow and get rid of the ugly endjob label inside if branch. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_driver.c | 41 ++++++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 427c1d02a8..93ec1dacf4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14275,6 +14275,7 @@ qemuDomainMigrateSetMaxSpeed(virDomainPtr dom, virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; qemuDomainObjPrivatePtr priv; + int rc; int ret = -1; virCheckFlags(0, -1); @@ -14294,29 +14295,31 @@ qemuDomainMigrateSetMaxSpeed(virDomainPtr dom, goto cleanup; } - if (virDomainObjIsActive(vm)) { - if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MIGRATION_OP) < 0) - goto cleanup; - - if (virDomainObjCheckActive(vm) < 0) - goto endjob; - - VIR_DEBUG("Setting migration bandwidth to %luMbs", bandwidth); - qemuDomainObjEnterMonitor(driver, vm); - ret = qemuMonitorSetMigrationSpeed(priv->mon, bandwidth); - if (qemuDomainObjExitMonitor(driver, vm) < 0) - ret = -1; - - if (ret == 0) - priv->migMaxBandwidth = bandwidth; - - endjob: - qemuDomainObjEndJob(driver, vm); - } else { + if (!virDomainObjIsActive(vm)) { priv->migMaxBandwidth = bandwidth; ret = 0; + goto cleanup; } + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MIGRATION_OP) < 0) + goto cleanup; + + if (virDomainObjCheckActive(vm) < 0) + goto endjob; + + VIR_DEBUG("Setting migration bandwidth to %luMbs", bandwidth); + qemuDomainObjEnterMonitor(driver, vm); + rc = qemuMonitorSetMigrationSpeed(priv->mon, bandwidth); + if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0) + goto endjob; + + priv->migMaxBandwidth = bandwidth; + + ret = 0; + + endjob: + qemuDomainObjEndJob(driver, vm); + cleanup: virDomainObjEndAPI(&vm); return ret; -- 2.20.1

On Tue, Feb 05, 2019 at 04:23:06PM +0100, Jiri Denemark wrote:
Let's make the code flow easier to follow and get rid of the ugly endjob label inside if branch.
Thank you.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_driver.c | 41 ++++++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 19 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

So far migration parameters were changed only at the beginning of migration mostly via an automatic translation from flags and typed parameters. We need to export a few more functions to support APIs which may set migration parameters while migration is already running. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration_params.c | 16 +++++++++++++++- src/qemu/qemu_migration_params.h | 9 +++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c index c36bed5744..117ae0e998 100644 --- a/src/qemu/qemu_migration_params.c +++ b/src/qemu/qemu_migration_params.c @@ -216,7 +216,7 @@ qemuMigrationParamsGetAlwaysOnCaps(qemuMigrationParty party) } -static qemuMigrationParamsPtr +qemuMigrationParamsPtr qemuMigrationParamsNew(void) { qemuMigrationParamsPtr params; @@ -1039,6 +1039,20 @@ qemuMigrationParamsFetch(virQEMUDriverPtr driver, } +int +qemuMigrationParamsSetULL(qemuMigrationParamsPtr migParams, + qemuMigrationParam param, + unsigned long long value) +{ + if (qemuMigrationParamsCheckType(param, QEMU_MIGRATION_PARAM_TYPE_ULL) < 0) + return -1; + + migParams->params[param].value.ull = value; + migParams->params[param].set = true; + return 0; +} + + /** * Returns -1 on error, * 0 on success, diff --git a/src/qemu/qemu_migration_params.h b/src/qemu/qemu_migration_params.h index 709b818dcf..db0745ec28 100644 --- a/src/qemu/qemu_migration_params.h +++ b/src/qemu/qemu_migration_params.h @@ -84,8 +84,12 @@ qemuMigrationParamsDump(qemuMigrationParamsPtr migParams, int *maxparams, unsigned long *flags); +qemuMigrationParamsPtr +qemuMigrationParamsNew(void); + void qemuMigrationParamsFree(qemuMigrationParamsPtr migParams); +VIR_DEFINE_AUTOPTR_FUNC(qemuMigrationParams, qemuMigrationParamsFree) int qemuMigrationParamsApply(virQEMUDriverPtr driver, @@ -112,6 +116,11 @@ qemuMigrationParamsFetch(virQEMUDriverPtr driver, int asyncJob, qemuMigrationParamsPtr *migParams); +int +qemuMigrationParamsSetULL(qemuMigrationParamsPtr migParams, + qemuMigrationParam param, + unsigned long long value); + int qemuMigrationParamsGetULL(qemuMigrationParamsPtr migParams, qemuMigrationParam param, -- 2.20.1

On Tue, Feb 05, 2019 at 04:23:07PM +0100, Jiri Denemark wrote:
So far migration parameters were changed only at the beginning of migration mostly via an automatic translation from flags and typed parameters. We need to export a few more functions to support APIs which may set migration parameters while migration is already running.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration_params.c | 16 +++++++++++++++- src/qemu/qemu_migration_params.h | 9 +++++++++ 2 files changed, 24 insertions(+), 1 deletion(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

This patch adds a new VIR_MIGRATE_PARAM_BANDWIDTH_POSTCOPY typed parameter for virDomainMigrate3 and virDomainMigrateToURI3 for setting maximum post-copy migration bandwidth. In case the initial VIR_MIGRATE_PARAM_BANDWIDTH_POSTCOPY value turns out to be suboptimal a new VIR_DOMAIN_MIGRATE_MAX_SPEED_POSTCOPY flag for virDomainMigrateSetMaxSpeed and virDomainMigrateGetMaxSpeed may be used to set/get the maximum post-copy migration bandwidth while migration is already running. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- include/libvirt/libvirt-domain.h | 15 +++++++++++++++ src/libvirt-domain.c | 11 ++++++++--- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 0388911ded..d99f9b690b 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -903,6 +903,15 @@ typedef enum { */ # define VIR_MIGRATE_PARAM_BANDWIDTH "bandwidth" +/** + * VIR_MIGRATE_PARAM_BANDWIDTH_POSTCOPY: + * + * virDomainMigrate* params field: the maximum bandwidth (in MiB/s) that will + * be used for post-copy phase of a migration as VIR_TYPED_PARAM_ULLONG. If set + * to 0 or omitted, post-copy migration speed will not be limited. + */ +# define VIR_MIGRATE_PARAM_BANDWIDTH_POSTCOPY "bandwidth.postcopy" + /** * VIR_MIGRATE_PARAM_GRAPHICS_URI: * @@ -1062,6 +1071,12 @@ int virDomainMigrateSetCompressionCache(virDomainPtr domain, unsigned long long cacheSize, unsigned int flags); +/* Domain migration speed flags. */ +typedef enum { + /* Set or get maximum speed of postcopy migration. */ + VIR_DOMAIN_MIGRATE_MAX_SPEED_POSTCOPY = (1 << 0), +} virDomainMigrateMaxSpeedFlags; + int virDomainMigrateSetMaxSpeed(virDomainPtr domain, unsigned long bandwidth, unsigned int flags); diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 58b4863c8f..7b87b3716a 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -9036,11 +9036,13 @@ virDomainMigrateSetCompressionCache(virDomainPtr domain, * virDomainMigrateSetMaxSpeed: * @domain: a domain object * @bandwidth: migration bandwidth limit in MiB/s - * @flags: extra flags; not used yet, so callers should always pass 0 + * @flags: bitwise-OR of virDomainMigrateMaxSpeedFlags * * The maximum bandwidth (in MiB/s) that will be used to do migration * can be specified with the bandwidth parameter. Not all hypervisors - * will support a bandwidth cap + * will support a bandwidth cap. When VIR_DOMAIN_MIGRATE_MAX_SPEED_POSTCOPY + * is set in @flags, this API sets the maximum bandwidth for the postcopy + * phase of the migration. * * Returns 0 in case of success, -1 otherwise. */ @@ -9077,10 +9079,13 @@ virDomainMigrateSetMaxSpeed(virDomainPtr domain, * virDomainMigrateGetMaxSpeed: * @domain: a domain object * @bandwidth: return value of current migration bandwidth limit in MiB/s - * @flags: extra flags; not used yet, so callers should always pass 0 + * @flags: bitwise-OR of virDomainMigrateMaxSpeedFlags * * Get the current maximum bandwidth (in MiB/s) that will be used if the * domain is migrated. Not all hypervisors will support a bandwidth limit. + * When VIR_DOMAIN_MIGRATE_MAX_SPEED_POSTCOPY is set in @flags, this API + * gets the current maximum bandwidth for the postcopy phase of the + * migration. * * Returns 0 in case of success, -1 otherwise. */ -- 2.20.1

On Tue, Feb 05, 2019 at 04:23:08PM +0100, Jiri Denemark wrote:
This patch adds a new VIR_MIGRATE_PARAM_BANDWIDTH_POSTCOPY typed parameter for virDomainMigrate3 and virDomainMigrateToURI3 for setting maximum post-copy migration bandwidth.
In case the initial VIR_MIGRATE_PARAM_BANDWIDTH_POSTCOPY value turns out to be suboptimal a new VIR_DOMAIN_MIGRATE_MAX_SPEED_POSTCOPY flag for virDomainMigrateSetMaxSpeed and virDomainMigrateGetMaxSpeed may be used to set/get the maximum post-copy migration bandwidth while migration is already running.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- include/libvirt/libvirt-domain.h | 15 +++++++++++++++ src/libvirt-domain.c | 11 ++++++++--- 2 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 0388911ded..d99f9b690b 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -903,6 +903,15 @@ typedef enum { */ # define VIR_MIGRATE_PARAM_BANDWIDTH "bandwidth"
+/** + * VIR_MIGRATE_PARAM_BANDWIDTH_POSTCOPY: + * + * virDomainMigrate* params field: the maximum bandwidth (in MiB/s) that will
I guess granularity < 1 MiB/s is not worth the trouble of creating an inconsistent API, is it?
+ * be used for post-copy phase of a migration as VIR_TYPED_PARAM_ULLONG. If set + * to 0 or omitted, post-copy migration speed will not be limited. + */ +# define VIR_MIGRATE_PARAM_BANDWIDTH_POSTCOPY "bandwidth.postcopy" + /** * VIR_MIGRATE_PARAM_GRAPHICS_URI: * @@ -1062,6 +1071,12 @@ int virDomainMigrateSetCompressionCache(virDomainPtr domain, unsigned long long cacheSize, unsigned int flags);
+/* Domain migration speed flags. */ +typedef enum { + /* Set or get maximum speed of postcopy migration. */
post-copy is the spelling used for human-readable strings
+ VIR_DOMAIN_MIGRATE_MAX_SPEED_POSTCOPY = (1 << 0), +} virDomainMigrateMaxSpeedFlags; + int virDomainMigrateSetMaxSpeed(virDomainPtr domain, unsigned long bandwidth, unsigned int flags); diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 58b4863c8f..7b87b3716a 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -9036,11 +9036,13 @@ virDomainMigrateSetCompressionCache(virDomainPtr domain, * virDomainMigrateSetMaxSpeed: * @domain: a domain object * @bandwidth: migration bandwidth limit in MiB/s - * @flags: extra flags; not used yet, so callers should always pass 0 + * @flags: bitwise-OR of virDomainMigrateMaxSpeedFlags * * The maximum bandwidth (in MiB/s) that will be used to do migration * can be specified with the bandwidth parameter. Not all hypervisors - * will support a bandwidth cap + * will support a bandwidth cap. When VIR_DOMAIN_MIGRATE_MAX_SPEED_POSTCOPY + * is set in @flags, this API sets the maximum bandwidth for the postcopy
post-copy
+ * phase of the migration. * * Returns 0 in case of success, -1 otherwise. */ @@ -9077,10 +9079,13 @@ virDomainMigrateSetMaxSpeed(virDomainPtr domain, * virDomainMigrateGetMaxSpeed: * @domain: a domain object * @bandwidth: return value of current migration bandwidth limit in MiB/s - * @flags: extra flags; not used yet, so callers should always pass 0 + * @flags: bitwise-OR of virDomainMigrateMaxSpeedFlags * * Get the current maximum bandwidth (in MiB/s) that will be used if the * domain is migrated. Not all hypervisors will support a bandwidth limit. + * When VIR_DOMAIN_MIGRATE_MAX_SPEED_POSTCOPY is set in @flags, this API + * gets the current maximum bandwidth for the postcopy phase of the
post-copy
+ * migration. *
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

This typed parameter for virDomainMigrate3 and virDomainMigrateToURI3 APIs may be used for setting maximum post-copy migration bandwidth. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration.h | 1 + src/qemu/qemu_migration_params.c | 7 +++++++ src/qemu/qemu_migration_params.h | 1 + 3 files changed, 9 insertions(+) diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index 976a723d21..ca73d3e467 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -79,6 +79,7 @@ VIR_MIGRATE_PARAM_PERSIST_XML, VIR_TYPED_PARAM_STRING, \ VIR_MIGRATE_PARAM_AUTO_CONVERGE_INITIAL, VIR_TYPED_PARAM_INT, \ VIR_MIGRATE_PARAM_AUTO_CONVERGE_INCREMENT, VIR_TYPED_PARAM_INT, \ + VIR_MIGRATE_PARAM_BANDWIDTH_POSTCOPY, VIR_TYPED_PARAM_ULLONG, \ NULL diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c index 117ae0e998..67070b9d08 100644 --- a/src/qemu/qemu_migration_params.c +++ b/src/qemu/qemu_migration_params.c @@ -102,6 +102,7 @@ VIR_ENUM_IMPL(qemuMigrationParam, QEMU_MIGRATION_PARAM_LAST, "downtime-limit", "block-incremental", "xbzrle-cache-size", + "max-postcopy-bandwidth", ); typedef struct _qemuMigrationParamsAlwaysOnItem qemuMigrationParamsAlwaysOnItem; @@ -178,6 +179,11 @@ static const qemuMigrationParamsTPMapItem qemuMigrationParamsTPMap[] = { {.typedParam = VIR_MIGRATE_PARAM_COMPRESSION_XBZRLE_CACHE, .param = QEMU_MIGRATION_PARAM_XBZRLE_CACHE_SIZE, .party = QEMU_MIGRATION_SOURCE | QEMU_MIGRATION_DESTINATION}, + + {.typedParam = VIR_MIGRATE_PARAM_BANDWIDTH_POSTCOPY, + .unit = 1024 * 1024, /* MB/s */ + .param = QEMU_MIGRATION_PARAM_MAX_POSTCOPY_BANDWIDTH, + .party = QEMU_MIGRATION_SOURCE | QEMU_MIGRATION_DESTINATION}, }; static const qemuMigrationParamType qemuMigrationParamTypes[] = { @@ -192,6 +198,7 @@ static const qemuMigrationParamType qemuMigrationParamTypes[] = { [QEMU_MIGRATION_PARAM_DOWNTIME_LIMIT] = QEMU_MIGRATION_PARAM_TYPE_ULL, [QEMU_MIGRATION_PARAM_BLOCK_INCREMENTAL] = QEMU_MIGRATION_PARAM_TYPE_BOOL, [QEMU_MIGRATION_PARAM_XBZRLE_CACHE_SIZE] = QEMU_MIGRATION_PARAM_TYPE_ULL, + [QEMU_MIGRATION_PARAM_MAX_POSTCOPY_BANDWIDTH] = QEMU_MIGRATION_PARAM_TYPE_ULL, }; verify(ARRAY_CARDINALITY(qemuMigrationParamTypes) == QEMU_MIGRATION_PARAM_LAST); diff --git a/src/qemu/qemu_migration_params.h b/src/qemu/qemu_migration_params.h index db0745ec28..2460684a00 100644 --- a/src/qemu/qemu_migration_params.h +++ b/src/qemu/qemu_migration_params.h @@ -55,6 +55,7 @@ typedef enum { QEMU_MIGRATION_PARAM_DOWNTIME_LIMIT, QEMU_MIGRATION_PARAM_BLOCK_INCREMENTAL, QEMU_MIGRATION_PARAM_XBZRLE_CACHE_SIZE, + QEMU_MIGRATION_PARAM_MAX_POSTCOPY_BANDWIDTH, QEMU_MIGRATION_PARAM_LAST } qemuMigrationParam; -- 2.20.1

On Tue, Feb 05, 2019 at 04:23:09PM +0100, Jiri Denemark wrote:
This typed parameter for virDomainMigrate3 and virDomainMigrateToURI3 APIs may be used for setting maximum post-copy migration bandwidth.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration.h | 1 + src/qemu/qemu_migration_params.c | 7 +++++++ src/qemu/qemu_migration_params.h | 1 + 3 files changed, 9 insertions(+)
diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c index 117ae0e998..67070b9d08 100644 --- a/src/qemu/qemu_migration_params.c +++ b/src/qemu/qemu_migration_params.c @@ -178,6 +179,11 @@ static const qemuMigrationParamsTPMapItem qemuMigrationParamsTPMap[] = { {.typedParam = VIR_MIGRATE_PARAM_COMPRESSION_XBZRLE_CACHE, .param = QEMU_MIGRATION_PARAM_XBZRLE_CACHE_SIZE, .party = QEMU_MIGRATION_SOURCE | QEMU_MIGRATION_DESTINATION}, + + {.typedParam = VIR_MIGRATE_PARAM_BANDWIDTH_POSTCOPY, + .unit = 1024 * 1024, /* MB/s */
MiB/s
+ .param = QEMU_MIGRATION_PARAM_MAX_POSTCOPY_BANDWIDTH, + .party = QEMU_MIGRATION_SOURCE | QEMU_MIGRATION_DESTINATION}, };
static const qemuMigrationParamType qemuMigrationParamTypes[] = {
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

This flag tells virDomainMigrateSetMaxSpeed and virDomainMigrateGetMaxSpeed APIs to work on post-copy migration bandwidth. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_driver.c | 91 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 78 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 93ec1dacf4..06a0560a35 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14275,10 +14275,12 @@ qemuDomainMigrateSetMaxSpeed(virDomainPtr dom, virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; qemuDomainObjPrivatePtr priv; - int rc; + bool postcopy = !!(flags & VIR_DOMAIN_MIGRATE_MAX_SPEED_POSTCOPY); + VIR_AUTOPTR(qemuMigrationParams) migParams = NULL; + unsigned long long max; int ret = -1; - virCheckFlags(0, -1); + virCheckFlags(VIR_DOMAIN_MIGRATE_MAX_SPEED_POSTCOPY, -1); if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; @@ -14288,14 +14290,18 @@ qemuDomainMigrateSetMaxSpeed(virDomainPtr dom, if (virDomainMigrateSetMaxSpeedEnsureACL(dom->conn, vm->def) < 0) goto cleanup; - if (bandwidth > QEMU_DOMAIN_MIG_BANDWIDTH_MAX) { + if (postcopy) + max = ULLONG_MAX / 1024 / 1024; + else + max = QEMU_DOMAIN_MIG_BANDWIDTH_MAX; + + if (bandwidth > max) { virReportError(VIR_ERR_OVERFLOW, - _("bandwidth must be less than %llu"), - QEMU_DOMAIN_MIG_BANDWIDTH_MAX + 1ULL); + _("bandwidth must be less than %llu"), max + 1); goto cleanup; } - if (!virDomainObjIsActive(vm)) { + if (!postcopy && !virDomainObjIsActive(vm)) { priv->migMaxBandwidth = bandwidth; ret = 0; goto cleanup; @@ -14308,12 +14314,29 @@ qemuDomainMigrateSetMaxSpeed(virDomainPtr dom, goto endjob; VIR_DEBUG("Setting migration bandwidth to %luMbs", bandwidth); - qemuDomainObjEnterMonitor(driver, vm); - rc = qemuMonitorSetMigrationSpeed(priv->mon, bandwidth); - if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0) - goto endjob; - priv->migMaxBandwidth = bandwidth; + if (postcopy) { + if (!(migParams = qemuMigrationParamsNew())) + goto endjob; + + if (qemuMigrationParamsSetULL(migParams, + QEMU_MIGRATION_PARAM_MAX_POSTCOPY_BANDWIDTH, + bandwidth * 1024 * 1024) < 0) + goto endjob; + + if (qemuMigrationParamsApply(driver, vm, QEMU_ASYNC_JOB_NONE, + migParams) < 0) + goto endjob; + } else { + int rc; + + qemuDomainObjEnterMonitor(driver, vm); + rc = qemuMonitorSetMigrationSpeed(priv->mon, bandwidth); + if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0) + goto endjob; + + priv->migMaxBandwidth = bandwidth; + } ret = 0; @@ -14330,11 +14353,13 @@ qemuDomainMigrateGetMaxSpeed(virDomainPtr dom, unsigned long *bandwidth, unsigned int flags) { + virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; qemuDomainObjPrivatePtr priv; + bool postcopy = !!(flags & VIR_DOMAIN_MIGRATE_MAX_SPEED_POSTCOPY); int ret = -1; - virCheckFlags(0, -1); + virCheckFlags(VIR_DOMAIN_MIGRATE_MAX_SPEED_POSTCOPY, -1); if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; @@ -14344,7 +14369,47 @@ qemuDomainMigrateGetMaxSpeed(virDomainPtr dom, if (virDomainMigrateGetMaxSpeedEnsureACL(dom->conn, vm->def) < 0) goto cleanup; - *bandwidth = priv->migMaxBandwidth; + if (postcopy) { + VIR_AUTOPTR(qemuMigrationParams) migParams = NULL; + unsigned long long bw; + int rc = -1; + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) + goto cleanup; + + if (virDomainObjCheckActive(vm) == 0 && + qemuMigrationParamsFetch(driver, vm, QEMU_ASYNC_JOB_NONE, + &migParams) == 0) { + rc = qemuMigrationParamsGetULL(migParams, + QEMU_MIGRATION_PARAM_MAX_POSTCOPY_BANDWIDTH, + &bw); + + /* QEMU reports B/s while we use MiB/s */ + bw /= 1024 * 1024; + } + + qemuDomainObjEndJob(driver, vm); + + if (rc < 0) { + goto cleanup; + } else if (rc == 1) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("querying maximum post-copy migration speed is " + "not supported by QEMU binary")); + goto cleanup; + } if (bw > ULONG_MAX) { + virReportError(VIR_ERR_OVERFLOW, + _("bandwidth %llu is greater than %lu which is the " + "maximum value supported by this API"), + bw, ULONG_MAX); + goto cleanup; + } + + *bandwidth = bw; + } else { + *bandwidth = priv->migMaxBandwidth; + } + ret = 0; cleanup: -- 2.20.1

On Tue, Feb 05, 2019 at 04:23:10PM +0100, Jiri Denemark wrote:
This flag tells virDomainMigrateSetMaxSpeed and virDomainMigrateGetMaxSpeed APIs to work on post-copy migration bandwidth.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_driver.c | 91 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 78 insertions(+), 13 deletions(-)
@@ -14344,7 +14369,47 @@ qemuDomainMigrateGetMaxSpeed(virDomainPtr dom, if (virDomainMigrateGetMaxSpeedEnsureACL(dom->conn, vm->def) < 0) goto cleanup;
- *bandwidth = priv->migMaxBandwidth; + if (postcopy) {
This whole branch looks like a good candidate for a helper function.
+ VIR_AUTOPTR(qemuMigrationParams) migParams = NULL; + unsigned long long bw; + int rc = -1; + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) + goto cleanup; + + if (virDomainObjCheckActive(vm) == 0 && + qemuMigrationParamsFetch(driver, vm, QEMU_ASYNC_JOB_NONE, + &migParams) == 0) { + rc = qemuMigrationParamsGetULL(migParams, + QEMU_MIGRATION_PARAM_MAX_POSTCOPY_BANDWIDTH, + &bw); + + /* QEMU reports B/s while we use MiB/s */ + bw /= 1024 * 1024; + }
You coould put an 'endjob' label here.
+ + qemuDomainObjEndJob(driver, vm); + + if (rc < 0) { + goto cleanup; + } else if (rc == 1) {
No need for 'else' after goto.
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("querying maximum post-copy migration speed is " + "not supported by QEMU binary")); + goto cleanup; + } if (bw > ULONG_MAX) {
Missing newline. Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

This flag tells virDomainMigrateSetMaxSpeed and virDomainMigrateGetMaxSpeed APIs to work on post-copy migration bandwidth. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- Notes: Version 2: - if (postcopy) branch from qemuDomainMigrateGetMaxSpeed separated into a helper function src/qemu/qemu_driver.c | 110 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 97 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0c0ed4fab9..7106f6d553 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14275,10 +14275,12 @@ qemuDomainMigrateSetMaxSpeed(virDomainPtr dom, virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; qemuDomainObjPrivatePtr priv; - int rc; + bool postcopy = !!(flags & VIR_DOMAIN_MIGRATE_MAX_SPEED_POSTCOPY); + VIR_AUTOPTR(qemuMigrationParams) migParams = NULL; + unsigned long long max; int ret = -1; - virCheckFlags(0, -1); + virCheckFlags(VIR_DOMAIN_MIGRATE_MAX_SPEED_POSTCOPY, -1); if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; @@ -14288,14 +14290,18 @@ qemuDomainMigrateSetMaxSpeed(virDomainPtr dom, if (virDomainMigrateSetMaxSpeedEnsureACL(dom->conn, vm->def) < 0) goto cleanup; - if (bandwidth > QEMU_DOMAIN_MIG_BANDWIDTH_MAX) { + if (postcopy) + max = ULLONG_MAX / 1024 / 1024; + else + max = QEMU_DOMAIN_MIG_BANDWIDTH_MAX; + + if (bandwidth > max) { virReportError(VIR_ERR_OVERFLOW, - _("bandwidth must be less than %llu"), - QEMU_DOMAIN_MIG_BANDWIDTH_MAX + 1ULL); + _("bandwidth must be less than %llu"), max + 1); goto cleanup; } - if (!virDomainObjIsActive(vm)) { + if (!postcopy && !virDomainObjIsActive(vm)) { priv->migMaxBandwidth = bandwidth; ret = 0; goto cleanup; @@ -14308,12 +14314,29 @@ qemuDomainMigrateSetMaxSpeed(virDomainPtr dom, goto endjob; VIR_DEBUG("Setting migration bandwidth to %luMbs", bandwidth); - qemuDomainObjEnterMonitor(driver, vm); - rc = qemuMonitorSetMigrationSpeed(priv->mon, bandwidth); - if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0) - goto endjob; - priv->migMaxBandwidth = bandwidth; + if (postcopy) { + if (!(migParams = qemuMigrationParamsNew())) + goto endjob; + + if (qemuMigrationParamsSetULL(migParams, + QEMU_MIGRATION_PARAM_MAX_POSTCOPY_BANDWIDTH, + bandwidth * 1024 * 1024) < 0) + goto endjob; + + if (qemuMigrationParamsApply(driver, vm, QEMU_ASYNC_JOB_NONE, + migParams) < 0) + goto endjob; + } else { + int rc; + + qemuDomainObjEnterMonitor(driver, vm); + rc = qemuMonitorSetMigrationSpeed(priv->mon, bandwidth); + if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0) + goto endjob; + + priv->migMaxBandwidth = bandwidth; + } ret = 0; @@ -14325,16 +14348,71 @@ qemuDomainMigrateSetMaxSpeed(virDomainPtr dom, return ret; } + +static int +qemuDomainMigrationGetPostcopyBandwidth(virQEMUDriverPtr driver, + virDomainObjPtr vm, + unsigned long *bandwidth) +{ + VIR_AUTOPTR(qemuMigrationParams) migParams = NULL; + unsigned long long bw; + int rc; + int ret = -1; + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) + return -1; + + if (virDomainObjCheckActive(vm) < 0) + goto cleanup; + + if (qemuMigrationParamsFetch(driver, vm, QEMU_ASYNC_JOB_NONE, + &migParams) < 0) + goto cleanup; + + if ((rc = qemuMigrationParamsGetULL(migParams, + QEMU_MIGRATION_PARAM_MAX_POSTCOPY_BANDWIDTH, + &bw)) < 0) + goto cleanup; + + if (rc == 1) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("querying maximum post-copy migration speed is " + "not supported by QEMU binary")); + goto cleanup; + } + + /* QEMU reports B/s while we use MiB/s */ + bw /= 1024 * 1024; + + if (bw > ULONG_MAX) { + virReportError(VIR_ERR_OVERFLOW, + _("bandwidth %llu is greater than %lu which is the " + "maximum value supported by this API"), + bw, ULONG_MAX); + goto cleanup; + } + + *bandwidth = bw; + ret = 0; + + cleanup: + qemuDomainObjEndJob(driver, vm); + return ret; +} + + static int qemuDomainMigrateGetMaxSpeed(virDomainPtr dom, unsigned long *bandwidth, unsigned int flags) { + virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; qemuDomainObjPrivatePtr priv; + bool postcopy = !!(flags & VIR_DOMAIN_MIGRATE_MAX_SPEED_POSTCOPY); int ret = -1; - virCheckFlags(0, -1); + virCheckFlags(VIR_DOMAIN_MIGRATE_MAX_SPEED_POSTCOPY, -1); if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; @@ -14344,7 +14422,13 @@ qemuDomainMigrateGetMaxSpeed(virDomainPtr dom, if (virDomainMigrateGetMaxSpeedEnsureACL(dom->conn, vm->def) < 0) goto cleanup; - *bandwidth = priv->migMaxBandwidth; + if (postcopy) { + if (qemuDomainMigrationGetPostcopyBandwidth(driver, vm, bandwidth) < 0) + goto cleanup; + } else { + *bandwidth = priv->migMaxBandwidth; + } + ret = 0; cleanup: -- 2.20.1

On Thu, Feb 07, 2019 at 04:11:46PM +0100, Jiri Denemark wrote:
This flag tells virDomainMigrateSetMaxSpeed and virDomainMigrateGetMaxSpeed APIs to work on post-copy migration bandwidth.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> ---
Notes: Version 2: - if (postcopy) branch from qemuDomainMigrateGetMaxSpeed separated into a helper function
src/qemu/qemu_driver.c | 110 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 97 insertions(+), 13 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

On 2/7/19 10:11 AM, Jiri Denemark wrote:
This flag tells virDomainMigrateSetMaxSpeed and virDomainMigrateGetMaxSpeed APIs to work on post-copy migration bandwidth.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> ---
Notes: Version 2: - if (postcopy) branch from qemuDomainMigrateGetMaxSpeed separated into a helper function
[...]
+ +static int +qemuDomainMigrationGetPostcopyBandwidth(virQEMUDriverPtr driver, + virDomainObjPtr vm, + unsigned long *bandwidth) +{ + VIR_AUTOPTR(qemuMigrationParams) migParams = NULL; + unsigned long long bw; + int rc; + int ret = -1; + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) + return -1; + + if (virDomainObjCheckActive(vm) < 0) + goto cleanup; + + if (qemuMigrationParamsFetch(driver, vm, QEMU_ASYNC_JOB_NONE, + &migParams) < 0) + goto cleanup; + + if ((rc = qemuMigrationParamsGetULL(migParams, + QEMU_MIGRATION_PARAM_MAX_POSTCOPY_BANDWIDTH, + &bw)) < 0) + goto cleanup; + + if (rc == 1) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("querying maximum post-copy migration speed is " + "not supported by QEMU binary")); + goto cleanup; + } + + /* QEMU reports B/s while we use MiB/s */ + bw /= 1024 * 1024; + + if (bw > ULONG_MAX) {
FWIW: Coverity generates a complaint here: 1) Event result_independent_of_operands: "bw > 18446744073709551615ULL /* 9223372036854775807L * 2UL + 1UL */" is always false regardless of the values of its operands. This occurs as the logical operand of "if". John
+ virReportError(VIR_ERR_OVERFLOW, + _("bandwidth %llu is greater than %lu which is the " + "maximum value supported by this API"), + bw, ULONG_MAX); + goto cleanup; + } + + *bandwidth = bw; + ret = 0; + + cleanup: + qemuDomainObjEndJob(driver, vm); + return ret; +} + +
[...]

Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- tools/virsh-domain.c | 33 +++++++++++++++++++++++++++++++-- tools/virsh.pod | 15 +++++++++++---- 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 275ac0c318..db0d5d4dcc 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -10557,6 +10557,10 @@ static const vshCmdOptDef opts_migrate[] = { .type = VSH_OT_BOOL, .help = N_("use TLS for migration") }, + {.name = "postcopy-bandwidth", + .type = VSH_OT_INT, + .help = N_("post-copy migration bandwidth limit in MiB/s") + }, {.name = NULL} }; @@ -10753,6 +10757,15 @@ doMigrate(void *opaque) goto save_error; } + if ((rv = vshCommandOptULongLong(ctl, cmd, "postcopy-bandwidth", &ullOpt)) < 0) { + goto out; + } else if (rv > 0) { + if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams, + VIR_MIGRATE_PARAM_BANDWIDTH_POSTCOPY, + ullOpt) < 0) + goto save_error; + } + if (vshCommandOptBool(cmd, "live")) flags |= VIR_MIGRATE_LIVE; if (vshCommandOptBool(cmd, "p2p")) @@ -11150,6 +11163,10 @@ static const vshCmdOptDef opts_migrate_setspeed[] = { .flags = VSH_OFLAG_REQ, .help = N_("migration bandwidth limit in MiB/s") }, + {.name = "postcopy", + .type = VSH_OT_BOOL, + .help = N_("set postcopy migration bandwidth") + }, {.name = NULL} }; @@ -11158,6 +11175,7 @@ cmdMigrateSetMaxSpeed(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom = NULL; unsigned long bandwidth = 0; + unsigned int flags = 0; bool ret = false; if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) @@ -11166,7 +11184,10 @@ cmdMigrateSetMaxSpeed(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptULWrap(ctl, cmd, "bandwidth", &bandwidth) < 0) goto done; - if (virDomainMigrateSetMaxSpeed(dom, bandwidth, 0) < 0) + if (vshCommandOptBool(cmd, "postcopy")) + flags |= VIR_DOMAIN_MIGRATE_MAX_SPEED_POSTCOPY; + + if (virDomainMigrateSetMaxSpeed(dom, bandwidth, flags) < 0) goto done; ret = true; @@ -11191,6 +11212,10 @@ static const vshCmdInfo info_migrate_getspeed[] = { static const vshCmdOptDef opts_migrate_getspeed[] = { VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), + {.name = "postcopy", + .type = VSH_OT_BOOL, + .help = N_("get postcopy migration bandwidth") + }, {.name = NULL} }; @@ -11199,12 +11224,16 @@ cmdMigrateGetMaxSpeed(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom = NULL; unsigned long bandwidth; + unsigned int flags = 0; bool ret = false; if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; - if (virDomainMigrateGetMaxSpeed(dom, &bandwidth, 0) < 0) + if (vshCommandOptBool(cmd, "postcopy")) + flags |= VIR_DOMAIN_MIGRATE_MAX_SPEED_POSTCOPY; + + if (virDomainMigrateGetMaxSpeed(dom, &bandwidth, flags) < 0) goto done; vshPrint(ctl, "%lu\n", bandwidth); diff --git a/tools/virsh.pod b/tools/virsh.pod index 59a5900162..67edb57b14 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1903,6 +1903,7 @@ I<domain> I<desturi> [I<migrateuri>] [I<graphicsuri>] [I<listen-address>] [I<dna [I<--comp-mt-level>] [I<--comp-mt-threads>] [I<--comp-mt-dthreads>] [I<--comp-xbzrle-cache>] [I<--auto-converge>] [I<auto-converge-initial>] [I<auto-converge-increment>] [I<--persistent-xml> B<file>] [I<--tls>] +[I<--postcopy-bandwidth> B<bandwidth>] Migrate domain to another host. Add I<--live> for live migration; <--p2p> for peer-2-peer migration; I<--direct> for direct migration; or I<--tunnelled> @@ -1933,6 +1934,8 @@ Once migration is running, the user may switch to post-copy using the B<migrate-postcopy> command sent from another virsh instance or use I<--postcopy-after-precopy> along with I<--postcopy> to let libvirt automatically switch to post-copy after the first pass of pre-copy is finished. +The maximum bandwidth consumed during the post-copy phase may be limited using +I<--postcopy-bandwidth>. I<--auto-converge> forces convergence during live migration. The initial guest CPU throttling rate can be set with I<auto-converge-initial>. If the @@ -2098,17 +2101,21 @@ is supposed to be used while the domain is being live-migrated as a reaction to migration progress and increasing number of compression cache misses obtained from domjobinfo. -=item B<migrate-setspeed> I<domain> I<bandwidth> +=item B<migrate-setspeed> I<domain> I<bandwidth> [I<--postcopy>] Set the maximum migration bandwidth (in MiB/s) for a domain which is being migrated to another host. I<bandwidth> is interpreted as an unsigned long long value. Specifying a negative value results in an essentially unlimited value being provided to the hypervisor. The hypervisor can choose whether to -reject the value or convert it to the maximum value allowed. +reject the value or convert it to the maximum value allowed. If the +I<--postcopy> option is specified, the command will set the maximum bandwidth +allowed during a post-copy migration phase. -=item B<migrate-getspeed> I<domain> +=item B<migrate-getspeed> I<domain> [I<--postcopy>] -Get the maximum migration bandwidth (in MiB/s) for a domain. +Get the maximum migration bandwidth (in MiB/s) for a domain. If the +I<--postcopy> option is specified, the command will get the maximum bandwidth +allowed during a post-copy migration phase. =item B<migrate-postcopy> I<domain> -- 2.20.1

On Tue, Feb 05, 2019 at 04:23:11PM +0100, Jiri Denemark wrote:
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- tools/virsh-domain.c | 33 +++++++++++++++++++++++++++++++-- tools/virsh.pod | 15 +++++++++++---- 2 files changed, 42 insertions(+), 6 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 275ac0c318..db0d5d4dcc 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -11150,6 +11163,10 @@ static const vshCmdOptDef opts_migrate_setspeed[] = { .flags = VSH_OFLAG_REQ, .help = N_("migration bandwidth limit in MiB/s") }, + {.name = "postcopy", + .type = VSH_OT_BOOL, + .help = N_("set postcopy migration bandwidth")
post-copy
+ }, {.name = NULL} };
@@ -11191,6 +11212,10 @@ static const vshCmdInfo info_migrate_getspeed[] = {
static const vshCmdOptDef opts_migrate_getspeed[] = { VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), + {.name = "postcopy", + .type = VSH_OT_BOOL, + .help = N_("get postcopy migration bandwidth")
post-copy
+ }, {.name = NULL} };
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (3)
-
Jiri Denemark
-
John Ferlan
-
Ján Tomko