[libvirt] [PATCH] qemu: Migrate at unlimited speed by default
Previously, qemu did not respond to monitor commands during migration if the limit was too high. This prevented us from raising the limit earlier. The qemu issue seems to be fixed (according to my testing) and we may remove the 32Mb/s limit. --- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_domain.h | 5 ++--- src/qemu/qemu_migration.c | 4 ++-- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 86f0265..a54df2f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -217,7 +217,7 @@ static void *qemuDomainObjPrivateAlloc(void) if (!(priv->cons = virConsoleAlloc())) goto error; - priv->migMaxBandwidth = QEMU_DOMAIN_DEFAULT_MIG_BANDWIDTH_MAX; + priv->migMaxBandwidth = QEMU_DOMAIN_MIG_BANDWIDTH_MAX; return priv; diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 9f9467d..23a5111 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -38,13 +38,12 @@ (1 << VIR_DOMAIN_VIRT_KVM) | \ (1 << VIR_DOMAIN_VIRT_XEN)) -# define QEMU_DOMAIN_DEFAULT_MIG_BANDWIDTH_MAX 32 # if ULONG_MAX == 4294967295 /* Qemu has a 64-bit limit, but we are limited by our historical choice of * representing bandwidth in a long instead of a 64-bit int. */ -# define QEMU_DOMAIN_FILE_MIG_BANDWIDTH_MAX ULONG_MAX +# define QEMU_DOMAIN_MIG_BANDWIDTH_MAX ULONG_MAX # else -# define QEMU_DOMAIN_FILE_MIG_BANDWIDTH_MAX (INT64_MAX / (1024 * 1024)) +# define QEMU_DOMAIN_MIG_BANDWIDTH_MAX (INT64_MAX / (1024 * 1024)) # endif # define JOB_MASK(job) (1 << (job - 1)) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index b792456..6eddce6 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3189,8 +3189,8 @@ qemuMigrationToFile(struct qemud_driver *driver, virDomainObjPtr vm, * Failure to change migration speed is not fatal. */ if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) == 0) { qemuMonitorSetMigrationSpeed(priv->mon, - QEMU_DOMAIN_FILE_MIG_BANDWIDTH_MAX); - priv->migMaxBandwidth = QEMU_DOMAIN_FILE_MIG_BANDWIDTH_MAX; + QEMU_DOMAIN_MIG_BANDWIDTH_MAX); + priv->migMaxBandwidth = QEMU_DOMAIN_MIG_BANDWIDTH_MAX; qemuDomainObjExitMonitorWithDriver(driver, vm); } -- 1.7.11.1
On 08/03/2012 12:16 PM, Jiri Denemark wrote:
Previously, qemu did not respond to monitor commands during migration if the limit was too high. This prevented us from raising the limit earlier. The qemu issue seems to be fixed (according to my testing) and we may remove the 32Mb/s limit. --- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_domain.h | 5 ++--- src/qemu/qemu_migration.c | 4 ++-- 3 files changed, 5 insertions(+), 6 deletions(-)
How does this differ from commit 9f71368? That is, aren't we already supporting unlimited speed since your patch in March? -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Fri, Aug 03, 2012 at 13:06:27 -0600, Eric Blake wrote:
On 08/03/2012 12:16 PM, Jiri Denemark wrote:
Previously, qemu did not respond to monitor commands during migration if the limit was too high. This prevented us from raising the limit earlier. The qemu issue seems to be fixed (according to my testing) and we may remove the 32Mb/s limit. --- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_domain.h | 5 ++--- src/qemu/qemu_migration.c | 4 ++-- 3 files changed, 5 insertions(+), 6 deletions(-)
How does this differ from commit 9f71368? That is, aren't we already supporting unlimited speed since your patch in March?
That commit enabled unlimited speed only when migrating to a file. This patch removes the limit even for over-the-net migrations. Jirka
On 08/03/2012 01:12 PM, Jiri Denemark wrote:
On Fri, Aug 03, 2012 at 13:06:27 -0600, Eric Blake wrote:
On 08/03/2012 12:16 PM, Jiri Denemark wrote:
Previously, qemu did not respond to monitor commands during migration if the limit was too high. This prevented us from raising the limit earlier. The qemu issue seems to be fixed (according to my testing) and we may remove the 32Mb/s limit. --- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_domain.h | 5 ++--- src/qemu/qemu_migration.c | 4 ++-- 3 files changed, 5 insertions(+), 6 deletions(-)
How does this differ from commit 9f71368? That is, aren't we already supporting unlimited speed since your patch in March?
That commit enabled unlimited speed only when migrating to a file. This patch removes the limit even for over-the-net migrations.
Do we need a capability bit, such as a minimum version of qemu at which this works, so that we don't run into problems with older qemu where unlimited still starves the monitor? -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Fri, Aug 03, 2012 at 13:14:21 -0600, Eric Blake wrote:
Do we need a capability bit, such as a minimum version of qemu at which this works, so that we don't run into problems with older qemu where unlimited still starves the monitor?
I was thinking about it but the problem with version is that any distro that might possibly backport those patches would not benefit from this. Jirka
On Fri, Aug 03, 2012 at 08:16:06PM +0200, Jiri Denemark wrote:
Previously, qemu did not respond to monitor commands during migration if the limit was too high. This prevented us from raising the limit earlier. The qemu issue seems to be fixed (according to my testing) and we may remove the 32Mb/s limit.
IIRC, the problem wasn't with QEMU, but rather that libvirt was passing QEMU a file descriptor pointing to a plain file, on which O_NONBLOCK does not work (thanks POSIX). We fixed libvirt to pass a pipe instead to address that. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 08/03/2012 12:16 PM, Jiri Denemark wrote:
Previously, qemu did not respond to monitor commands during migration if the limit was too high. This prevented us from raising the limit earlier. The qemu issue seems to be fixed (according to my testing) and we may remove the 32Mb/s limit. --- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_domain.h | 5 ++--- src/qemu/qemu_migration.c | 4 ++-- 3 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 86f0265..a54df2f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -217,7 +217,7 @@ static void *qemuDomainObjPrivateAlloc(void) if (!(priv->cons = virConsoleAlloc())) goto error;
- priv->migMaxBandwidth = QEMU_DOMAIN_DEFAULT_MIG_BANDWIDTH_MAX; + priv->migMaxBandwidth = QEMU_DOMAIN_MIG_BANDWIDTH_MAX;
This says that new libvirt will unconditionally try to use unlimited bandwidth, instead of the qemu default of 32Mbps-limited bandwidth, in all cases where the user does not additionally provide their own limiting. Is that a backwards-incompatible change? Could there be an existing client relying on the default limiting that will now be swamped by libvirt being faster? Or is it a bug fix, where things will now just operate faster by default, and people that care about limiting are not impacted because they already had an explicit limit? I think it is the latter (a bug fix - no one that cares about limiting should be relying on defaults), so after the discussion we've had in this thread, I'm okay giving: ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Mon, Aug 06, 2012 at 08:10:46 -0600, Eric Blake wrote:
On 08/03/2012 12:16 PM, Jiri Denemark wrote: This says that new libvirt will unconditionally try to use unlimited bandwidth, instead of the qemu default of 32Mbps-limited bandwidth, in all cases where the user does not additionally provide their own limiting.
Is that a backwards-incompatible change? Could there be an existing client relying on the default limiting that will now be swamped by libvirt being faster? Or is it a bug fix, where things will now just operate faster by default, and people that care about limiting are not impacted because they already had an explicit limit?
I think it is the latter (a bug fix - no one that cares about limiting should be relying on defaults), so after the discussion we've had in this thread, I'm okay giving:
Yes, that's how I take it as well.
ACK.
Thanks, pushed. Jirka
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Jiri Denemark