virPortAllocatorSetUsed permits to set a port as already used and prevent the port allocator to use it without any attempt to bind it. Signed-off-by: Giuseppe Scrivano <gscrivan(a)redhat.com&gt; --- src/libvirt_private.syms | 1 + src/util/virportallocator.c | 44 +++++++++++++++++++++++++++++++++++++++++++- src/util/virportallocator.h | 4 ++++ 3 files changed, 48 insertions(+), 1 deletion(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 2a2b9c0..95a45aa 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1770,6 +1770,7 @@ virPidFileWritePath; virPortAllocatorAcquire; virPortAllocatorNew; virPortAllocatorRelease; +virPortAllocatorSetUsed; # util/virprocess.h diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c index b68133a..226f386 100644 --- a/src/util/virportallocator.c +++ b/src/util/virportallocator.c @@ -1,7 +1,7 @@ /* * virportallocator.c: Allocate & track TCP port allocations * - * Copyright (C) 2013 Red Hat, Inc. + * Copyright (C) 2013, 2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -250,3 +250,45 @@ int virPortAllocatorRelease(virPortAllocatorPtr pa, virObjectUnlock(pa); return ret; } + +int virPortAllocatorSetUsed(virPortAllocatorPtr pa, + unsigned short port, + bool value) +{ + int ret = -1; + + virObjectLock(pa); + + if (port < pa->start || + port > pa->end) { + ret = 0; + goto cleanup; + } + + if (value) { + bool used = false; + if (virBitmapGetBit(pa->bitmap, port - pa->start, &used) < 0) + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to query port %d"), port); + + if (used || virBitmapSetBit(pa->bitmap, port - pa->start) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to reserve port %d"), port); + goto cleanup; + } + } + else { + if (virBitmapClearBit(pa->bitmap, + port - pa->start) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to release port %d"), + port); + goto cleanup; + } + } + + ret = 0; + cleanup: + virObjectUnlock(pa); + return ret; +} diff --git a/src/util/virportallocator.h b/src/util/virportallocator.h index c8aa6de..e5ee56d 100644 --- a/src/util/virportallocator.h +++ b/src/util/virportallocator.h @@ -38,4 +38,8 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa, int virPortAllocatorRelease(virPortAllocatorPtr pa, unsigned short port); +int virPortAllocatorSetUsed(virPortAllocatorPtr pa, + unsigned short port, + bool value); + #endif /* __VIR_PORT_ALLOCATOR_H__ */ -- 1.9.3

Michal Privoznik <mprivozn(a)redhat.com&gt; writes: I've no write access to the repo. Could these fixes be amended before pushing the patches or shall I go for v3? Thanks, Giuseppe

When looking for a port to allocate, the port allocator didn't take in consideration ports that are statically set by the user. Defining these two graphics elements in the XML would cause an error, as the port allocator would try to use the same port for the spice graphics element: <graphics type='spice' autoport='yes'/> <graphics type='vnc' port='5900' autoport='no'/> The new *[pP]ortReserved variables keep track of the ports that were successfully tracked as used by the port allocator but that weren't bound. Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1081881 Signed-off-by: Giuseppe Scrivano <gscrivan(a)redhat.com&gt; --- src/conf/domain_conf.h | 3 +++ src/qemu/qemu_process.c | 68 ++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 65 insertions(+), 6 deletions(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 6779a41..1122eb2 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1385,6 +1385,7 @@ struct _virDomainGraphicsDef { union { struct { int port; + bool portReserved; int websocket; bool autoport; char *keymap; @@ -1410,6 +1411,8 @@ struct _virDomainGraphicsDef { struct { int port; int tlsPort; + bool portReserved; + bool tlsPortReserved; int mousemode; char *keymap; virDomainGraphicsAuthDef auth; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 0b8155b..561e37e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3803,6 +3803,41 @@ int qemuProcessStart(virConnectPtr conn, for (i = 0; i < vm->def->ngraphics; ++i) { virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && + !graphics->data.vnc.autoport) { + if (virPortAllocatorSetUsed(driver->remotePorts, + graphics->data.vnc.port, + true) < 0) { + goto cleanup; + } + + graphics->data.vnc.portReserved = true; + + } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE && + !graphics->data.spice.autoport) { + + if (graphics->data.spice.port > 0) { + if (virPortAllocatorSetUsed(driver->remotePorts, + graphics->data.spice.port, + true) < 0) + goto cleanup; + + graphics->data.spice.portReserved = true; + } + + if (graphics->data.spice.tlsPort > 0) { + if (virPortAllocatorSetUsed(driver->remotePorts, + graphics->data.spice.tlsPort, + true) < 0) + goto cleanup; + + graphics->data.spice.tlsPortReserved = true; + } + } + } + + for (i = 0; i < vm->def->ngraphics; ++i) { + virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { if (qemuProcessVNCAllocatePorts(driver, graphics) < 0) goto cleanup; @@ -4513,15 +4548,36 @@ void qemuProcessStop(virQEMUDriverPtr driver, virPortAllocatorRelease(driver->remotePorts, graphics->data.vnc.port); } + else if (graphics->data.vnc.portReserved) { + virPortAllocatorSetUsed(driver->remotePorts, + graphics->data.spice.port, + false); + graphics->data.vnc.portReserved = false; + } virPortAllocatorRelease(driver->webSocketPorts, graphics->data.vnc.websocket); } - if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE && - graphics->data.spice.autoport) { - virPortAllocatorRelease(driver->remotePorts, - graphics->data.spice.port); - virPortAllocatorRelease(driver->remotePorts, - graphics->data.spice.tlsPort); + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { + if (graphics->data.spice.autoport) { + virPortAllocatorRelease(driver->remotePorts, + graphics->data.spice.port); + virPortAllocatorRelease(driver->remotePorts, + graphics->data.spice.tlsPort); + } else { + if (graphics->data.spice.portReserved) { + virPortAllocatorSetUsed(driver->remotePorts, + graphics->data.spice.port, + false); + graphics->data.spice.portReserved = false; + } + + if (graphics->data.spice.tlsPortReserved) { + virPortAllocatorSetUsed(driver->remotePorts, + graphics->data.spice.tlsPort, + false); + graphics->data.spice.tlsPortReserved = false; + } + } } } -- 1.9.3