Only release ports that have been allocated before. This fixes these issues: * trying to release ports when qemuProcessStart fails before port allocation * trying to release the SPICE TLS port if spice_tls is 0 * failing to release SPICE port with autoport=off (when only one of them is -1) --- v1: https://www.redhat.com/archives/libvir-list/2013-February/msg01464.html Use a pair of booleans in domain private data instead of a new field in the domain definition. src/qemu/qemu_domain.h | 5 +++++ src/qemu/qemu_process.c | 28 +++++++++++++++++++--------- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 30e6b97..bac4d8b 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -163,6 +163,11 @@ struct _qemuDomainObjPrivate { qemuDomainCleanupCallback *cleanupCallbacks; size_t ncleanupCallbacks; size_t ncleanupCallbacks_max; + + /* Track port allocation state for SPICE + * (only one device is allowed per domain) */ + bool spicePortAllocated; + bool spiceTlsPortAllocated; }; struct qemuDomainWatchdogEvent diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index b9fdcd2..1442cc7 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3633,6 +3633,7 @@ int qemuProcessStart(virConnectPtr conn, } graphics->data.spice.port = port; + priv->spicePortAllocated = true; } if (cfg->spiceTLS && (graphics->data.spice.autoport || @@ -3645,10 +3646,12 @@ int qemuProcessStart(virConnectPtr conn, virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to find an unused port for SPICE TLS")); virPortAllocatorRelease(driver->remotePorts, port); + priv->spicePortAllocated = false; goto cleanup; } graphics->data.spice.tlsPort = tlsPort; + priv->spiceTlsPortAllocated = true; } } @@ -4311,22 +4314,29 @@ retry: qemuProcessRemoveDomainStatus(driver, vm); - /* Remove VNC and Spice ports from port reservation bitmap, but only if - they were reserved by the driver (autoport=yes) + /* Remove VNC and SPICE ports from port reservation bitmap, but only if + they were reserved by the driver: + autoport = yes and non-zero port for VNC, + priv->spice(Tls)PortAllocated for SPICE. */ 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) { + graphics->data.vnc.autoport && graphics->data.vnc.port) { ignore_value(virPortAllocatorRelease(driver->remotePorts, graphics->data.vnc.port)); } - if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE && - graphics->data.spice.autoport) { - ignore_value(virPortAllocatorRelease(driver->remotePorts, - graphics->data.spice.port)); - ignore_value(virPortAllocatorRelease(driver->remotePorts, - graphics->data.spice.tlsPort)); + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { + if (priv->spicePortAllocated) { + ignore_value(virPortAllocatorRelease(driver->remotePorts, + graphics->data.spice.port)); + priv->spicePortAllocated = false; + } + if (priv->spiceTlsPortAllocated) { + ignore_value(virPortAllocatorRelease(driver->remotePorts, + graphics->data.spice.tlsPort)); + priv->spiceTlsPortAllocated = false; + } } } -- 1.7.12.4