[libvirt] [PATCH 2/2] graphics: remember graphics not auto allocated ports

[Giuseppe Scrivano]

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]ortAllocated variables keep track of the ports that were
successfully registered (either bound or simply tracked as used) by
the port allocator to allow a clean rollback on errors.

Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1081881

Signed-off-by: Giuseppe Scrivano <gscrivan at redhat.com>
---
 src/conf/domain_conf.h  |  3 ++
 src/qemu/qemu_process.c | 79 +++++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 73 insertions(+), 9 deletions(-)

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 6779a41..7617574 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1385,6 +1385,7 @@ struct _virDomainGraphicsDef {
     union {
         struct {
             int port;
+            bool portAllocated;
             int websocket;
             bool autoport;
             char *keymap;
@@ -1410,6 +1411,8 @@ struct _virDomainGraphicsDef {
         struct {
             int port;
             int tlsPort;
+            bool portAllocated;
+            bool tlsPortAllocated;
             int mousemode;
             char *keymap;
             virDomainGraphicsAuthDef auth;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 0b8155b..06f1e54 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3487,6 +3487,7 @@ qemuProcessVNCAllocatePorts(virQEMUDriverPtr driver,
         if (virPortAllocatorAcquire(driver->remotePorts, &port) < 0)
             return -1;
         graphics->data.vnc.port = port;
+        graphics->data.vnc.portAllocated = true;
     }
 
     if (graphics->data.vnc.websocket == -1) {
@@ -3548,6 +3549,7 @@ qemuProcessSPICEAllocatePorts(virQEMUDriverPtr driver,
             goto error;
 
         graphics->data.spice.port = port;
+        graphics->data.spice.portAllocated = true;
     }
 
     if (needTLSPort || graphics->data.spice.tlsPort == -1) {
@@ -3573,6 +3575,7 @@ qemuProcessSPICEAllocatePorts(virQEMUDriverPtr driver,
                 goto error;
 
             graphics->data.spice.tlsPort = tlsPort;
+            graphics->data.spice.tlsPortAllocated = true;
         }
     }
 
@@ -3803,6 +3806,36 @@ 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.portAllocated = true;
+
+        } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE &&
+                   !graphics->data.spice.autoport) {
+            if (virPortAllocatorSetUsed(driver->remotePorts,
+                                        graphics->data.spice.port,
+                                        true) < 0)
+                goto cleanup;
+
+            graphics->data.spice.portAllocated = true;
+
+            if (virPortAllocatorSetUsed(driver->remotePorts,
+                                        graphics->data.spice.tlsPort,
+                                        true) < 0)
+                goto cleanup;
+
+            graphics->data.spice.tlsPortAllocated = 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;
@@ -4509,19 +4542,47 @@ void qemuProcessStop(virQEMUDriverPtr driver,
     for (i = 0; i < vm->def->ngraphics; ++i) {
         virDomainGraphicsDefPtr graphics = vm->def->graphics[i];
         if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) {
-            if (graphics->data.vnc.autoport) {
-                virPortAllocatorRelease(driver->remotePorts,
-                                        graphics->data.vnc.port);
+            if (graphics->data.vnc.portAllocated) {
+                if (graphics->data.vnc.autoport) {
+                    virPortAllocatorRelease(driver->remotePorts,
+                                            graphics->data.vnc.port);
+                } else {
+                    virPortAllocatorSetUsed(driver->remotePorts,
+                                            graphics->data.vnc.port,
+                                            false);
+                }
+                graphics->data.vnc.portAllocated = 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) {
+                if (graphics->data.spice.portAllocated) {
+                    virPortAllocatorRelease(driver->remotePorts,
+                                            graphics->data.spice.port);
+                    graphics->data.spice.portAllocated = false;
+                }
+                if (graphics->data.spice.tlsPortAllocated) {
+                    virPortAllocatorRelease(driver->remotePorts,
+                                            graphics->data.spice.tlsPort);
+                    graphics->data.spice.tlsPortAllocated = false;
+                }
+            } else {
+                if (graphics->data.spice.portAllocated) {
+                    virPortAllocatorSetUsed(driver->remotePorts,
+                                            graphics->data.spice.port,
+                                            false);
+                    graphics->data.spice.portAllocated = false;
+                }
+                if (graphics->data.spice.tlsPortAllocated) {
+                    virPortAllocatorSetUsed(driver->remotePorts,
+                                            graphics->data.spice.tlsPort,
+                                            false);
+                    graphics->data.spice.tlsPortAllocated = false;
+                }
+            }
         }
     }
 
-- 
1.9.3