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

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/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.