[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH] qemu: fix graphics port allocation



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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]