The libxl driver reconnects to all running VMs when libvirtd is restarted,
but it failed to mark auto-allocated graphics ports as set in the port
allocator. If many VMs are running that use port auto-allocation and
libvirtd is restarted, the port allocator is likely to hand out a port
already in use when a new VM is created that uses auto-allocation. VM
creation will fail due to the port clash.

When reconnecting to running VMs after a libvirtd restart, let the port
allocator know about previously allocated ports.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---

Like 31e937fb3b, another item unaccounted for when reconnecting to VMs
after a daemon restart.

 src/libxl/libxl_driver.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 2d9385654c..97965aaf1d 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -393,6 +393,7 @@ libxlReconnectDomain(virDomainObj *vm,
     virHostdevManager *hostdev_mgr = driver->hostdevMgr;
     unsigned int hostdev_flags = VIR_HOSTDEV_SP_PCI;
     int ret = -1;
+    size_t i;
 
     hostdev_flags |= VIR_HOSTDEV_SP_USB;
 
@@ -447,6 +448,28 @@ libxlReconnectDomain(virDomainObj *vm,
 
     libxlReconnectNotifyNets(vm->def);
 
+    /* Set any auto-allocated graphics ports to used */
+    for (i = 0; i < vm->def->ngraphics; i++) {
+        virDomainGraphicsDef *graphics = vm->def->graphics[i];
+
+         switch (graphics->type) {
+         case VIR_DOMAIN_GRAPHICS_TYPE_VNC:
+             if (graphics->data.vnc.autoport)
+                 virPortAllocatorSetUsed(graphics->data.vnc.port);
+             break;
+         case VIR_DOMAIN_GRAPHICS_TYPE_SPICE:
+             if (graphics->data.spice.autoport)
+                 virPortAllocatorSetUsed(graphics->data.spice.port);
+             break;
+         case VIR_DOMAIN_GRAPHICS_TYPE_SDL:
+         case VIR_DOMAIN_GRAPHICS_TYPE_RDP:
+         case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP:
+         case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS:
+         case VIR_DOMAIN_GRAPHICS_TYPE_LAST:
+             break;
+         }
+    }
+
     if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0)
         VIR_WARN("Cannot update XML for running Xen guest %s", vm->def->name);
 
-- 
2.34.1

On a Friday in 2022, Jim Fehlig wrote:
>The libxl driver reconnects to all running VMs when libvirtd is restarted,
>but it failed to mark auto-allocated graphics ports as set in the port
>allocator. If many VMs are running that use port auto-allocation and
>libvirtd is restarted, the port allocator is likely to hand out a port
>already in use when a new VM is created that uses auto-allocation. VM
>creation will fail due to the port clash.
>

In the QEMU driver, we also mark user-specified ports in the allocator,
to let it now it should skip them. But I suppose people worried about
clashes do not use those.

>When reconnecting to running VMs after a libvirtd restart, let the port
>allocator know about previously allocated ports.
>
>Signed-off-by: Jim Fehlig <jfehlig@suse.com>
>---
>
>Like 31e937fb3b, another item unaccounted for when reconnecting to VMs
>after a daemon restart.
>
> src/libxl/libxl_driver.c | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

However, I did not find a virPortAllocatorRelease call for the spice
port in src/libxl (for VNC it's done in libxlDomainCleanup )

Jano

On 2/7/22 06:38, Ján Tomko wrote:
> On a Friday in 2022, Jim Fehlig wrote:
>> The libxl driver reconnects to all running VMs when libvirtd is restarted,
>> but it failed to mark auto-allocated graphics ports as set in the port
>> allocator. If many VMs are running that use port auto-allocation and
>> libvirtd is restarted, the port allocator is likely to hand out a port
>> already in use when a new VM is created that uses auto-allocation. VM
>> creation will fail due to the port clash.
>>
> 
> In the QEMU driver, we also mark user-specified ports in the allocator,
> to let it now it should skip them. But I suppose people worried about
> clashes do not use those.

On one hand I dislike the allocator keeping track of stuff it didn't allocated, 
but on the other it is a resiliency improvement to consider.

>> When reconnecting to running VMs after a libvirtd restart, let the port
>> allocator know about previously allocated ports.
>>
>> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
>> ---
>>
>> Like 31e937fb3b, another item unaccounted for when reconnecting to VMs
>> after a daemon restart.
>>
>> src/libxl/libxl_driver.c | 23 +++++++++++++++++++++++
>> 1 file changed, 23 insertions(+)
>>
> 
> Reviewed-by: Ján Tomko <jtomko@redhat.com>
> 
> However, I did not find a virPortAllocatorRelease call for the spice
> port in src/libxl (for VNC it's done in libxlDomainCleanup )

Thanks a lot for checking and bringing it to my attention! I've sent a patch to 
improve the port release logic

https://listman.redhat.com/archives/libvir-list/2022-February/msg00252.html

Regards,
Jim