[libvirt] [PATCH] qemu: Mark graphics ports used on reconnect

Michal Privoznik posted 1 patch 6 years, 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/d2cacf9e29fc6b9cc98c46c853c2bd294d6704ff.1505742438.git.mprivozn@redhat.com
src/qemu/qemu_process.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
[libvirt] [PATCH] qemu: Mark graphics ports used on reconnect
Posted by Michal Privoznik 6 years, 6 months ago
This is an issue that's bugging me for a long time. I don't know
exactly when and who is to blame but on daemon reconnect we lose
qemu's port allocator internal state. That's okay as we should be
able to rebuild it later. However, now I'm seeing port allocator
biding successfully to ports that are already taken by qemu
(either VNC or Spice). Thus any attempt to start another domain
after daemon is restarted fails because libvirt instructs qemu to
take port 5900 which is already taken.

Now, I don't want to mask the real problem, but one can advocate
that we should be marking graphics ports as already in use on
qemuProcessReconnect anyway, because we already know that they
are taken.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_process.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index d3155e4e7..053aba1a6 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4035,7 +4035,8 @@ qemuProcessStartHook(virQEMUDriverPtr driver,
 
 static int
 qemuProcessGraphicsReservePorts(virQEMUDriverPtr driver,
-                                virDomainGraphicsDefPtr graphics)
+                                virDomainGraphicsDefPtr graphics,
+                                bool reconnect)
 {
     virDomainGraphicsListenDefPtr glisten;
 
@@ -4050,7 +4051,8 @@ qemuProcessGraphicsReservePorts(virQEMUDriverPtr driver,
 
     switch (graphics->type) {
     case VIR_DOMAIN_GRAPHICS_TYPE_VNC:
-        if (!graphics->data.vnc.autoport) {
+        if (!graphics->data.vnc.autoport ||
+            reconnect) {
             if (virPortAllocatorSetUsed(driver->remotePorts,
                                         graphics->data.vnc.port,
                                         true) < 0)
@@ -4065,7 +4067,7 @@ qemuProcessGraphicsReservePorts(virQEMUDriverPtr driver,
         break;
 
     case VIR_DOMAIN_GRAPHICS_TYPE_SPICE:
-        if (graphics->data.spice.autoport)
+        if (graphics->data.spice.autoport && !reconnect)
             return 0;
 
         if (graphics->data.spice.port > 0) {
@@ -4269,7 +4271,7 @@ qemuProcessSetupGraphics(virQEMUDriverPtr driver,
         for (i = 0; i < vm->def->ngraphics; i++) {
             graphics = vm->def->graphics[i];
 
-            if (qemuProcessGraphicsReservePorts(driver, graphics) < 0)
+            if (qemuProcessGraphicsReservePorts(driver, graphics, false) < 0)
                 goto cleanup;
         }
     }
@@ -6881,6 +6883,13 @@ qemuProcessReconnect(void *opaque)
             goto error;
     }
 
+    for (i = 0; i < obj->def->ngraphics; i++) {
+        if (qemuProcessGraphicsReservePorts(driver,
+                                            obj->def->graphics[i],
+                                            true) < 0)
+            goto error;
+    }
+
     if (qemuProcessUpdateState(driver, obj) < 0)
         goto error;
 
-- 
2.13.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Mark graphics ports used on reconnect
Posted by Cole Robinson 6 years, 6 months ago
On 09/18/2017 09:47 AM, Michal Privoznik wrote:
> This is an issue that's bugging me for a long time. I don't know
> exactly when and who is to blame but on daemon reconnect we lose
> qemu's port allocator internal state.

It's the kernel's fault, broken in 4.11+, patches recently posted
upstream but not in any release yet:

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

- Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Mark graphics ports used on reconnect
Posted by Michal Privoznik 6 years, 6 months ago
On 09/18/2017 04:10 PM, Cole Robinson wrote:
> On 09/18/2017 09:47 AM, Michal Privoznik wrote:
>> This is an issue that's bugging me for a long time. I don't know
>> exactly when and who is to blame but on daemon reconnect we lose
>> qemu's port allocator internal state.
> 
> It's the kernel's fault, broken in 4.11+, patches recently posted
> upstream but not in any release yet:
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1432684	
> 

Cool thanks. I'll keep an eye on it. Nevertheless, we should merge this
patch as from control POV it makes no sense to try to bind to ports we
know are taken.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Mark graphics ports used on reconnect
Posted by Martin Kletzander 6 years, 6 months ago
On Mon, Sep 18, 2017 at 04:25:04PM +0200, Michal Privoznik wrote:
>On 09/18/2017 04:10 PM, Cole Robinson wrote:
>> On 09/18/2017 09:47 AM, Michal Privoznik wrote:
>>> This is an issue that's bugging me for a long time. I don't know
>>> exactly when and who is to blame but on daemon reconnect we lose
>>> qemu's port allocator internal state.
>>
>> It's the kernel's fault, broken in 4.11+, patches recently posted
>> upstream but not in any release yet:
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=1432684
>>
>
>Cool thanks. I'll keep an eye on it. Nevertheless, we should merge this
>patch as from control POV it makes no sense to try to bind to ports we
>know are taken.
>

I agree on that (not that it would've been my idea to add this), but
with changed commit message (for example keeping the last bit that has
the same information as the reply above) I'd ACK it.  Consider that done
unless Cole disagrees.

Also if you want to create a unified virPortAllocator instead of many
overlapping ones, be my domain^Wguest =)

>Michal
>
>--
>libvir-list mailing list
>libvir-list@redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Mark graphics ports used on reconnect
Posted by Cole Robinson 6 years, 6 months ago
On 09/19/2017 09:14 AM, Martin Kletzander wrote:
> On Mon, Sep 18, 2017 at 04:25:04PM +0200, Michal Privoznik wrote:
>> On 09/18/2017 04:10 PM, Cole Robinson wrote:
>>> On 09/18/2017 09:47 AM, Michal Privoznik wrote:
>>>> This is an issue that's bugging me for a long time. I don't know
>>>> exactly when and who is to blame but on daemon reconnect we lose
>>>> qemu's port allocator internal state.
>>>
>>> It's the kernel's fault, broken in 4.11+, patches recently posted
>>> upstream but not in any release yet:
>>>
>>> https://bugzilla.redhat.com/show_bug.cgi?id=1432684
>>>
>>
>> Cool thanks. I'll keep an eye on it. Nevertheless, we should merge this
>> patch as from control POV it makes no sense to try to bind to ports we
>> know are taken.
>>
> 
> I agree on that (not that it would've been my idea to add this), but
> with changed commit message (for example keeping the last bit that has
> the same information as the reply above) I'd ACK it.  Consider that done
> unless Cole disagrees.

To clarify I wasn't disagreeing, just wanted to point out the kernel issue

Thanks,
Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list