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

Reply via email to