On Tue, Oct 15, 2013 at 04:24:14 +0000, Wangyufei (A) wrote: > From f56b290eab36bbb7a9ac53778a55638d473504d1 Mon Sep 17 00:00:00 2001 > From: WangYufei <james.wangyu...@huawei.com> > 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 <james.wangyu...@huawei.com> > --- > 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 libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list