On Tue, Oct 15, 2013 at 04:24:14 +0000, Wangyufei (A) wrote:
> From f56b290eab36bbb7a9ac53778a55638d473504d1 Mon Sep 17 00:00:00 2001
> From: WangYufei <[email protected]>
> Date: Fri, 11 Oct 2013 11:27:13 +0800
> Subject: [PATCH] qemu_migrate: Fix assign the same port when migrating
> concurrently
>
> When we migrate vms concurrently, there's a chance that libvirtd on
> destination assign the same port for different migrations, which will lead to
> migration failed during migration prepare phase on destination. So we use
> virPortAllocator here to solve the problem.
>
> Signed-off-by: WangYufei <[email protected]>
> ---
> src/qemu/qemu_command.h | 3 +++
> src/qemu/qemu_conf.h | 6 +++---
> src/qemu/qemu_domain.h | 1 +
> src/qemu/qemu_driver.c | 6 ++++++
> src/qemu/qemu_migration.c | 28 +++++++++++++++++++++-------
> 5 files changed, 34 insertions(+), 10 deletions(-)
>
...
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 3a1aab7..93ae237 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
...
> @@ -2297,6 +2300,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
>
> *def = NULL;
> priv = vm->privateData;
> + priv->migrationPort = port;
> if (VIR_STRDUP(priv->origname, origname) < 0)
> goto cleanup;
>
> @@ -2415,6 +2419,11 @@ cleanup:
> VIR_FREE(xmlout);
> VIR_FORCE_CLOSE(dataFD[0]);
> VIR_FORCE_CLOSE(dataFD[1]);
> + if (ret < 0) {
> + virPortAllocatorRelease(driver->migrationPorts, port);
> + if (priv)
> + priv->migrationPort = 0;
> + }
This would possibly release a port that was not acquired from port
allocator but was supplied via migration URI.
> if (vm) {
> if (ret >= 0 || vm->persistent)
> virObjectUnlock(vm);
...
> @@ -2600,8 +2609,11 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver,
> cleanup:
> virURIFree(uri);
> VIR_FREE(hostname);
> - if (ret != 0)
> + if (ret != 0) {
> VIR_FREE(*uri_out);
> + virPortAllocatorRelease(driver->migrationPorts,
> + (unsigned short)this_port);
> + }
And this may result in the port being released twice (once in
qemuMigrationPrepareAny and once here).
> return ret;
> }
>
In other words, I think we need the following changes to be squashed in
(I'll send the complete patch with the suggested changes squashed).
Jirka
diff --git i/src/qemu/qemu_migration.c w/src/qemu/qemu_migration.c
index f8315da..0f17d0e 100644
--- i/src/qemu/qemu_migration.c
+++ w/src/qemu/qemu_migration.c
@@ -2159,7 +2159,8 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
virDomainDefPtr *def,
const char *origname,
virStreamPtr st,
- unsigned int port,
+ unsigned short port,
+ bool autoPort,
const char *listenAddress,
unsigned long flags)
{
@@ -2328,7 +2329,6 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
*def = NULL;
priv = vm->privateData;
- priv->migrationPort = port;
if (VIR_STRDUP(priv->origname, origname) < 0)
goto cleanup;
@@ -2440,6 +2440,8 @@ done:
goto cleanup;
}
+ if (autoPort)
+ priv->migrationPort = port;
ret = 0;
cleanup:
@@ -2447,11 +2449,6 @@ cleanup:
VIR_FREE(xmlout);
VIR_FORCE_CLOSE(dataFD[0]);
VIR_FORCE_CLOSE(dataFD[1]);
- if (ret < 0) {
- virPortAllocatorRelease(driver->migrationPorts, port);
- if (priv)
- priv->migrationPort = 0;
- }
if (vm) {
if (ret >= 0 || vm->persistent)
virObjectUnlock(vm);
@@ -2531,7 +2528,8 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver,
const char *listenAddress,
unsigned long flags)
{
- int this_port;
+ unsigned short port = 0;
+ bool autoPort = true;
char *hostname = NULL;
const char *p;
char *uri_str = NULL;
@@ -2558,8 +2556,7 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver,
* to be a correct hostname which refers to the target machine).
*/
if (uri_in == NULL) {
- if (virPortAllocatorAcquire(driver->migrationPorts,
- (unsigned short *)&this_port) < 0)
+ if (virPortAllocatorAcquire(driver->migrationPorts, &port) < 0)
goto cleanup;
/* Get hostname */
@@ -2616,8 +2613,7 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver,
if (uri->port == 0) {
/* Generate a port */
- if (virPortAllocatorAcquire(driver->migrationPorts,
- (unsigned short *)&this_port) < 0)
+ if (virPortAllocatorAcquire(driver->migrationPorts, &port) < 0)
goto cleanup;
/* Caller frees */
@@ -2625,7 +2621,8 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver,
goto cleanup;
} else {
- this_port = uri->port;
+ port = uri->port;
+ autoPort = false;
}
}
@@ -2634,14 +2631,14 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver,
ret = qemuMigrationPrepareAny(driver, dconn, cookiein, cookieinlen,
cookieout, cookieoutlen, def, origname,
- NULL, this_port, listenAddress, flags);
+ NULL, port, autoPort, listenAddress, flags);
cleanup:
virURIFree(uri);
VIR_FREE(hostname);
if (ret != 0) {
VIR_FREE(*uri_out);
- virPortAllocatorRelease(driver->migrationPorts,
- (unsigned short)this_port);
+ if (autoPort)
+ virPortAllocatorRelease(driver->migrationPorts, port);
}
return ret;
}
--
libvir-list mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/libvir-list