[PATCH 0/2] qemu_migration: set priv->migMaxBandwidth during migration
Kristina Hanicova (2): qemu_migration: set bandwidth in priv during migration qemu_migration: drop unnecessary 'migrate_speed' variable src/qemu/qemu_migration.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) -- 2.31.1
We did not set priv->migMaxBandwidth if '--bandwidth' was specified as an option in the 'migrate' virsh command. This caused in printing the wrong value if virsh command 'migrate-getspeed' was called during the migration. This patch first sets the value to the given bandwidth (if one was specified) and restores the previous value after the migration. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1806856 Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- src/qemu/qemu_migration.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index dd226ea4bc..71edcd5c62 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -4013,6 +4013,7 @@ qemuMigrationSrcRun(virQEMUDriver *driver, qemuMigrationIOThread *iothread = NULL; VIR_AUTOCLOSE fd = -1; unsigned long migrate_speed = resource ? resource : priv->migMaxBandwidth; + unsigned long restore_max_bandwidth = priv->migMaxBandwidth; virErrorPtr orig_err = NULL; unsigned int cookieFlags = 0; bool abort_on_error = !!(flags & VIR_MIGRATE_ABORT_ON_ERROR); @@ -4025,6 +4026,8 @@ qemuMigrationSrcRun(virQEMUDriver *driver, g_autofree char *timestamp = NULL; int rc; + priv->migMaxBandwidth = migrate_speed; + VIR_DEBUG("driver=%p, vm=%p, cookiein=%s, cookieinlen=%d, " "cookieout=%p, cookieoutlen=%p, flags=0x%lx, resource=%lu, " "spec=%p (dest=%d, fwd=%d), dconn=%p, graphicsuri=%s, " @@ -4351,6 +4354,7 @@ qemuMigrationSrcRun(virQEMUDriver *driver, if (events) priv->signalIOError = false; + priv->migMaxBandwidth = restore_max_bandwidth; virErrorRestore(&orig_err); return ret; -- 2.31.1
On Fri, Oct 08, 2021 at 10:19:04 +0200, Kristina Hanicova wrote:
We did not set priv->migMaxBandwidth if '--bandwidth' was specified as an option in the 'migrate' virsh command. This caused in printing the wrong value if virsh command 'migrate-getspeed' was called during the migration. This patch first sets the value to the given bandwidth (if one was specified) and restores the previous value after the migration.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1806856
I was wondering whether we should just read the bandwidth back from QEMU instead when miration is running. But I guess this is good enough. Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
On 10/8/21 12:44 PM, Jiri Denemark wrote:
On Fri, Oct 08, 2021 at 10:19:04 +0200, Kristina Hanicova wrote:
We did not set priv->migMaxBandwidth if '--bandwidth' was specified as an option in the 'migrate' virsh command. This caused in printing the wrong value if virsh command 'migrate-getspeed' was called during the migration. This patch first sets the value to the given bandwidth (if one was specified) and restores the previous value after the migration.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1806856
I was wondering whether we should just read the bandwidth back from QEMU instead when miration is running. But I guess this is good enough.
I was looking into this, but I believe that 'query-migrate' returns the actual bandwidth and not the limit. Or do you mean other command? Michal
On Fri, Oct 08, 2021 at 12:56:24 +0200, Michal Prívozník wrote:
On 10/8/21 12:44 PM, Jiri Denemark wrote:
On Fri, Oct 08, 2021 at 10:19:04 +0200, Kristina Hanicova wrote:
We did not set priv->migMaxBandwidth if '--bandwidth' was specified as an option in the 'migrate' virsh command. This caused in printing the wrong value if virsh command 'migrate-getspeed' was called during the migration. This patch first sets the value to the given bandwidth (if one was specified) and restores the previous value after the migration.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1806856
I was wondering whether we should just read the bandwidth back from QEMU instead when miration is running. But I guess this is good enough.
I was looking into this, but I believe that 'query-migrate' returns the actual bandwidth and not the limit. Or do you mean other command?
Yes, query-migrate-parameters, which is internally mapped to qemuMigrationParamsFetch. Jirka
Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- src/qemu/qemu_migration.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 71edcd5c62..48df080c15 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -4012,7 +4012,6 @@ qemuMigrationSrcRun(virQEMUDriver *driver, g_autofree char *tlsAlias = NULL; qemuMigrationIOThread *iothread = NULL; VIR_AUTOCLOSE fd = -1; - unsigned long migrate_speed = resource ? resource : priv->migMaxBandwidth; unsigned long restore_max_bandwidth = priv->migMaxBandwidth; virErrorPtr orig_err = NULL; unsigned int cookieFlags = 0; @@ -4026,7 +4025,7 @@ qemuMigrationSrcRun(virQEMUDriver *driver, g_autofree char *timestamp = NULL; int rc; - priv->migMaxBandwidth = migrate_speed; + priv->migMaxBandwidth = resource ? resource : priv->migMaxBandwidth; VIR_DEBUG("driver=%p, vm=%p, cookiein=%s, cookieinlen=%d, " "cookieout=%p, cookieoutlen=%p, flags=0x%lx, resource=%lu, " @@ -4119,7 +4118,7 @@ qemuMigrationSrcRun(virQEMUDriver *driver, if (bwParam && qemuMigrationParamsSetULL(migParams, QEMU_MIGRATION_PARAM_MAX_BANDWIDTH, - migrate_speed * 1024 * 1024) < 0) + priv->migMaxBandwidth * 1024 * 1024) < 0) goto error; if (qemuMigrationParamsApply(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT, @@ -4149,7 +4148,7 @@ qemuMigrationSrcRun(virQEMUDriver *driver, if (qemuMigrationSrcNBDStorageCopy(driver, vm, mig, host, - migrate_speed, + priv->migMaxBandwidth, nmigrate_disks, migrate_disks, dconn, tlsAlias, @@ -4197,7 +4196,7 @@ qemuMigrationSrcRun(virQEMUDriver *driver, } if (!bwParam && - qemuMonitorSetMigrationSpeed(priv->mon, migrate_speed) < 0) + qemuMonitorSetMigrationSpeed(priv->mon, priv->migMaxBandwidth) < 0) goto exit_monitor; /* connect to the destination qemu if needed */ -- 2.31.1
On Fri, Oct 08, 2021 at 10:19:05 +0200, Kristina Hanicova wrote:
Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- src/qemu/qemu_migration.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 71edcd5c62..48df080c15 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -4012,7 +4012,6 @@ qemuMigrationSrcRun(virQEMUDriver *driver, g_autofree char *tlsAlias = NULL; qemuMigrationIOThread *iothread = NULL; VIR_AUTOCLOSE fd = -1; - unsigned long migrate_speed = resource ? resource : priv->migMaxBandwidth; unsigned long restore_max_bandwidth = priv->migMaxBandwidth; virErrorPtr orig_err = NULL; unsigned int cookieFlags = 0; @@ -4026,7 +4025,7 @@ qemuMigrationSrcRun(virQEMUDriver *driver, g_autofree char *timestamp = NULL; int rc;
- priv->migMaxBandwidth = migrate_speed; + priv->migMaxBandwidth = resource ? resource : priv->migMaxBandwidth;
Perhaps if (resource > 0) priv->migMaxBandwidth = resource; instead? It's not on a single line, but the else branch is quite redundant. Not a big deal, though. Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
On 10/8/21 10:19 AM, Kristina Hanicova wrote:
Kristina Hanicova (2): qemu_migration: set bandwidth in priv during migration qemu_migration: drop unnecessary 'migrate_speed' variable
src/qemu/qemu_migration.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
Pushed now. Michal
participants (3)
-
Jiri Denemark -
Kristina Hanicova -
Michal Prívozník