[libvirt] [PATCH 2/2] Return -1 in virPortAllocatorAcquire if all ports are used

[Daniel P. Berrange]

On Thu, Oct 31, 2013 at 01:07:27PM +0100, Ján Tomko wrote:
> 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      | 12 ------------
>  src/util/virportallocator.c  |  7 ++++++-
>  tests/virportallocatortest.c |  6 ++----
>  5 files changed, 10 insertions(+), 36 deletions(-)
> 
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index d4226b8..e4bb75f 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -947,11 +947,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 cb59620..8fb85a8 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -2557,14 +2557,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;
> @@ -2619,14 +2613,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 bdffdf8..daf081d 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -3406,12 +3406,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;
>      }
>  
> @@ -3437,12 +3431,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"));
> -                virPortAllocatorRelease(driver->remotePorts, port);
> -                goto error;
> -            }

Opps, you're loosing the release of 'port' when tlsPort allocation fails.
In fact there was already a broken codepath in this respect.

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 :|