[libvirt] [PATCH] graphics: Cleanup port policy

[Michal Privoznik]

Even though we say in documentation setting (tls-)port to -1 is legacy
compat style for enabling autoport, we're roughly doing this for VNC.
However, in case of SPICE auto enable autoport iff both port & tlsPort
are equal -1 as documentation says autoport plays with both.
---
 src/conf/domain_conf.c  |   30 ++++++++++++++++++++----------
 src/conf/domain_conf.h  |    5 +++++
 src/qemu/qemu_command.c |    2 +-
 src/qemu/qemu_process.c |   33 ++++++++++++++++++++-------------
 4 files changed, 46 insertions(+), 24 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b185fe7..d142512 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5929,6 +5929,10 @@ virDomainGraphicsDefParseXML(xmlNodePtr node,
                 VIR_FREE(port);
                 goto error;
             }
+            /* Legacy compat syntax, used -1 for auto-port */
+            if (def->data.rdp.port == -1)
+                def->data.rdp.autoport = 1;
+
             VIR_FREE(port);
         } else {
             def->data.rdp.port = 0;
@@ -5936,14 +5940,15 @@ virDomainGraphicsDefParseXML(xmlNodePtr node,
         }
 
         if ((autoport = virXMLPropString(node, "autoport")) != NULL) {
-            if (STREQ(autoport, "yes")) {
-                if (flags & VIR_DOMAIN_XML_INACTIVE)
-                    def->data.rdp.port = 0;
+            if (STREQ(autoport, "yes"))
                 def->data.rdp.autoport = 1;
-            }
+
             VIR_FREE(autoport);
         }
 
+        if (def->data.rdp.autoport && (flags & VIR_DOMAIN_XML_INACTIVE))
+            def->data.rdp.port = 0;
+
         if ((replaceUser = virXMLPropString(node, "replaceUser")) != NULL) {
             if (STREQ(replaceUser, "yes")) {
                 def->data.rdp.replaceUser = 1;
@@ -6009,16 +6014,21 @@ virDomainGraphicsDefParseXML(xmlNodePtr node,
         }
 
         if ((autoport = virXMLPropString(node, "autoport")) != NULL) {
-            if (STREQ(autoport, "yes")) {
-                if (flags & VIR_DOMAIN_XML_INACTIVE) {
-                    def->data.spice.port = 0;
-                    def->data.spice.tlsPort = 0;
-                }
+            if (STREQ(autoport, "yes"))
                 def->data.spice.autoport = 1;
-            }
             VIR_FREE(autoport);
         }
 
+        if (def->data.spice.port == -1 && def->data.spice.tlsPort == -1) {
+            /* Legacy compat syntax, used -1 for auto-port */
+            def->data.spice.autoport = 1;
+        }
+
+        if (def->data.spice.autoport && (flags & VIR_DOMAIN_XML_INACTIVE)) {
+            def->data.spice.port = 0;
+            def->data.spice.tlsPort = 0;
+        }
+
         def->data.spice.keymap = virXMLPropString(node, "keymap");
 
         if (virDomainGraphicsAuthDefParseXML(node, &def->data.spice.auth,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 6fc307e..6da22f4 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1183,6 +1183,11 @@ struct _virDomainGraphicsListenDef {
 };
 
 struct _virDomainGraphicsDef {
+    /* Port value discipline:
+     * Value -1 is legacy syntax indicating that it should be auto-allocated.
+     * Value 0 means port wasn't specified in XML at all.
+     * Positive value is actual port number given in XML.
+     */
     int type;
     union {
         struct {
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 996763c..b6dd1f1 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5375,7 +5375,7 @@ qemuBuildCommandLine(virConnectPtr conn,
 
         virBufferAsprintf(&opt, "port=%u", def->graphics[0]->data.spice.port);
 
-        if (def->graphics[0]->data.spice.tlsPort) {
+        if (def->graphics[0]->data.spice.tlsPort > 0) {
             if (!driver->spiceTLS) {
                 qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                                 _("spice TLS port set in XML configuration,"
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 1ac892f..ef311d1 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3169,28 +3169,35 @@ int qemuProcessStart(virConnectPtr conn,
                 goto cleanup;
             }
             vm->def->graphics[0]->data.vnc.port = port;
-        } else if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE &&
-                   vm->def->graphics[0]->data.spice.autoport) {
-            int port = qemuProcessNextFreePort(driver, QEMU_VNC_PORT_MIN);
-            int tlsPort = -1;
-            if (port < 0) {
-                qemuReportError(VIR_ERR_INTERNAL_ERROR,
-                                "%s", _("Unable to find an unused SPICE port"));
-                goto cleanup;
+        } else if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
+            int port = -1;
+            if (vm->def->graphics[0]->data.spice.autoport ||
+                vm->def->graphics[0]->data.spice.port == -1) {
+                port = qemuProcessNextFreePort(driver, QEMU_VNC_PORT_MIN);
+
+                if (port < 0) {
+                    qemuReportError(VIR_ERR_INTERNAL_ERROR,
+                                    "%s", _("Unable to find an unused SPICE port"));
+                    goto cleanup;
+                }
+
+                vm->def->graphics[0]->data.spice.port = port;
             }
 
-            if (driver->spiceTLS) {
-                tlsPort = qemuProcessNextFreePort(driver, port + 1);
+            if (driver->spiceTLS &&
+                (vm->def->graphics[0]->data.spice.autoport ||
+                 vm->def->graphics[0]->data.spice.tlsPort == -1)) {
+                int tlsPort = qemuProcessNextFreePort(driver,
+                                                      vm->def->graphics[0]->data.spice.port + 1);
                 if (tlsPort < 0) {
                     qemuReportError(VIR_ERR_INTERNAL_ERROR,
                                     "%s", _("Unable to find an unused SPICE TLS port"));
                     qemuProcessReturnPort(driver, port);
                     goto cleanup;
                 }
-            }
 
-            vm->def->graphics[0]->data.spice.port = port;
-            vm->def->graphics[0]->data.spice.tlsPort = tlsPort;
+                vm->def->graphics[0]->data.spice.tlsPort = tlsPort;
+            }
         }
     }
 
-- 
1.7.8.5