From 8c41794af8db3786577f4a29f29f0a561be2fc6b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A1n=20Tomko?= <jtomko@redhat.com>
Date: Thu, 31 Oct 2013 12:49:04 +0100
Subject: [PATCH] Return -1 in virPortAllocatorAcquire if all ports are used

Report the error in virPortAllocatorAcquire instead
of doing it in every caller.

The error contains the port range name instead of the intended
use for the port, e.g.:
Unable to find an unused port in range 'display' (65534-65535)
instead of:
Unable to find an unused port for SPICE

This also adds error reporting when the QEMU driver could not
find an unused port for VNC, VNC WebSockets or NBD migration.
---
 src/libxl/libxl_conf.c       |  5 -----
 src/qemu/qemu_migration.c    | 16 ++--------------
 src/qemu/qemu_process.c      | 11 -----------
 src/util/virportallocator.c  |  7 ++++++-
 tests/virportallocatortest.c |  6 ++----
 5 files changed, 10 insertions(+), 35 deletions(-)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 79cf2b6cf7..aaeb00e992 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -950,11 +950,6 @@ libxlMakeVfb(libxlDriverPrivatePtr driver,
 
                 if (virPortAllocatorAcquire(driver->reservedVNCPorts, &port) < 0)
                     return -1;
-                if (port == 0) {
-                    virReportError(VIR_ERR_INTERNAL_ERROR,
-                                   "%s", _("Unable to find an unused VNC port"));
-                    return -1;
-                }
                 l_vfb->data.vnc.port = port;
             }
             x_vfb->vnc.display = l_vfb->data.vnc.port - LIBXL_VNC_PORT_MIN;
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 4b5fdba585..e87ea85334 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2561,14 +2561,8 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver,
      * to be a correct hostname which refers to the target machine).
      */
     if (uri_in == NULL) {
-        if (virPortAllocatorAcquire(driver->migrationPorts, &port) < 0) {
+        if (virPortAllocatorAcquire(driver->migrationPorts, &port) < 0)
             goto cleanup;
-        } else if (!port) {
-            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                           _("No migration port available within the "
-                             "configured range"));
-            goto cleanup;
-        }
 
         if ((hostname = virGetHostname()) == NULL)
             goto cleanup;
@@ -2623,14 +2617,8 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver,
         }
 
         if (uri->port == 0) {
-            if (virPortAllocatorAcquire(driver->migrationPorts, &port) < 0) {
+            if (virPortAllocatorAcquire(driver->migrationPorts, &port) < 0)
                 goto cleanup;
-            } else if (!port) {
-                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                               _("No migration port available within the "
-                                 "configured range"));
-                goto cleanup;
-            }
 
             if (well_formed_uri) {
                 uri->port = port;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 22a89a40a2..a26c079de7 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3397,12 +3397,6 @@ qemuProcessSPICEAllocatePorts(virQEMUDriverPtr driver,
         if (virPortAllocatorAcquire(driver->remotePorts, &port) < 0)
             goto error;
 
-        if (port == 0) {
-            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                           _("Unable to find an unused port for SPICE"));
-            goto error;
-        }
-
         graphics->data.spice.port = port;
     }
 
@@ -3428,11 +3422,6 @@ qemuProcessSPICEAllocatePorts(virQEMUDriverPtr driver,
             if (virPortAllocatorAcquire(driver->remotePorts, &tlsPort) < 0)
                 goto error;
 
-            if (tlsPort == 0) {
-                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                               _("Unable to find an unused port for SPICE TLS"));
-                goto error;
-            }
             graphics->data.spice.tlsPort = tlsPort;
         }
     }
diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c
index e653e8c920..694f1916f4 100644
--- a/src/util/virportallocator.c
+++ b/src/util/virportallocator.c
@@ -157,10 +157,15 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa,
                 goto cleanup;
             }
             *port = i;
+            ret = 0;
         }
     }
 
-    ret = 0;
+    if (*port == 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Unable to find an unused port in range '%s' (%d-%d)"),
+                       pa->name, pa->start, pa->end);
+    }
 cleanup:
     virObjectUnlock(pa);
     VIR_FORCE_CLOSE(fd);
diff --git a/tests/virportallocatortest.c b/tests/virportallocatortest.c
index 33de78204c..721356e9d6 100644
--- a/tests/virportallocatortest.c
+++ b/tests/virportallocatortest.c
@@ -118,11 +118,9 @@ static int testAllocAll(const void *args ATTRIBUTE_UNUSED)
         goto cleanup;
     }
 
-    if (virPortAllocatorAcquire(alloc, &p7) < 0)
-        goto cleanup;
-    if (p7 != 0) {
+    if (virPortAllocatorAcquire(alloc, &p7) == 0) {
         if (virTestGetDebug())
-            fprintf(stderr, "Expected 0, got %d", p7);
+            fprintf(stderr, "Expected error, got %d", p7);
         goto cleanup;
     }