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

[Ján Tomko]

On 11/13/2013 05:39 PM, Daniel P. Berrange wrote:
> 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.
> 

'port' is released on the error: label. Should I move the double-release
removal into a separate commit to make it more obvious?

Jan


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20131113/8e9f9c2b/attachment-0001.sig>