7:58 a.m.
Right now, we allocate a port or a TLS port for SPICE if it's set to -1,
even if autoport is off. But we only free them if autoport is on.
With autoport on, we only allocate TLS port if cfg->spiceTLS is set, but
we free it even if it's not, leading to an error message.
This patch separates the autoport=yes|no XML option into autoport and
tlsAutoport in virDomainGraphicsDef:
if autoport is yes, we set them both to 1 (and vice versa)
if either of the port numbers is -1, the corresponding autoport is set
to 1 and it's used as a condition for acquiring/releasing the port.
For TLS ports, cfg->spiceTLS is checked before releasing the port as
well.
Also check the port number before trying to release it - the vnc port
will be 0, the spice port will be -1 or 0 if it hasn't been allocated
yet (if qemuProcessStart fails before port allocation).
---
src/conf/domain_conf.c | 17 +++++++++++------
src/conf/domain_conf.h | 1 +
src/qemu/qemu_hotplug.c | 3 ++-
src/qemu/qemu_process.c | 26 ++++++++++++++------------
4 files changed, 28 insertions(+), 19 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b65e52a..cb27f82 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7083,8 +7083,10 @@ virDomainGraphicsDefParseXML(xmlNodePtr node,
}
if ((autoport = virXMLPropString(node, "autoport")) != NULL) {
- if (STREQ(autoport, "yes"))
+ if (STREQ(autoport, "yes")) {
def->data.spice.autoport = 1;
+ def->data.spice.tlsAutoport = 1;
+ }
VIR_FREE(autoport);
}
@@ -7102,12 +7104,14 @@ virDomainGraphicsDefParseXML(xmlNodePtr node,
VIR_FREE(defaultMode);
}
- if (def->data.spice.port == -1 && def->data.spice.tlsPort == -1) {
- /* Legacy compat syntax, used -1 for auto-port */
+ /* Legacy compat syntax, used -1 for auto-port */
+ if (def->data.spice.port == -1)
def->data.spice.autoport = 1;
- }
+ if (def->data.spice.tlsPort == -1)
+ def->data.spice.tlsAutoport = 1;
- if (def->data.spice.autoport && (flags & VIR_DOMAIN_XML_INACTIVE))
{
+ if (def->data.spice.autoport && def->data.spice.tlsAutoport
&&
+ (flags & VIR_DOMAIN_XML_INACTIVE)) {
def->data.spice.port = 0;
def->data.spice.tlsPort = 0;
}
@@ -14201,7 +14205,8 @@ virDomainGraphicsDefFormat(virBufferPtr buf,
def->data.spice.tlsPort);
virBufferAsprintf(buf, " autoport='%s'",
- def->data.spice.autoport ? "yes" :
"no");
+ (def->data.spice.autoport &&
+ def->data.spice.tlsAutoport) ? "yes" :
"no");
if (listenAddr)
virBufferAsprintf(buf, " listen='%s'", listenAddr);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 0828954..cc67716 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1356,6 +1356,7 @@ struct _virDomainGraphicsDef {
char *keymap;
virDomainGraphicsAuthDef auth;
unsigned int autoport :1;
+ unsigned int tlsAutoport :1;
int channels[VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST];
int defaultMode; /* enum virDomainGraphicsSpiceChannelMode */
int image;
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 78961a7..d8f9007 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1879,9 +1879,10 @@ qemuDomainChangeGraphics(virQEMUDriverPtr driver,
case VIR_DOMAIN_GRAPHICS_TYPE_SPICE:
if ((olddev->data.spice.autoport != dev->data.spice.autoport) ||
+ (olddev->data.spice.tlsAutoport != dev->data.spice.tlsAutoport) ||
(!dev->data.spice.autoport &&
(olddev->data.spice.port != dev->data.spice.port)) ||
- (!dev->data.spice.autoport &&
+ (!dev->data.spice.tlsAutoport &&
(olddev->data.spice.tlsPort != dev->data.spice.tlsPort))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("cannot change port settings on spice
graphics"));
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 732964f..7c2a54e 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3622,8 +3622,7 @@ int qemuProcessStart(virConnectPtr conn,
graphics->data.vnc.port = port;
} else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
unsigned short port = 0;
- if (graphics->data.spice.autoport ||
- graphics->data.spice.port == -1) {
+ if (graphics->data.spice.autoport) {
if (virPortAllocatorAcquire(driver->remotePorts, &port) < 0)
goto cleanup;
@@ -3635,9 +3634,7 @@ int qemuProcessStart(virConnectPtr conn,
graphics->data.spice.port = port;
}
- if (cfg->spiceTLS &&
- (graphics->data.spice.autoport ||
- graphics->data.spice.tlsPort == -1)) {
+ if (cfg->spiceTLS && graphics->data.spice.tlsAutoport) {
unsigned short tlsPort;
if (virPortAllocatorAcquire(driver->remotePorts, &tlsPort) <
0)
goto cleanup;
@@ -4318,16 +4315,21 @@ retry:
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 (graphics->data.spice.autoport &&
+ graphics->data.spice.port > 0) {
+ ignore_value(virPortAllocatorRelease(driver->remotePorts,
+ graphics->data.spice.port));
+ }
+ if (cfg->spiceTLS && graphics->data.spice.tlsAutoport
&&
+ graphics->data.spice.tlsPort > 0) {
+ ignore_value(virPortAllocatorRelease(driver->remotePorts,
+ graphics->data.spice.tlsPort));
+ }
}
}
--
1.7.12.4