[libvirt] [PATCH] qemu: Mark graphics ports used on reconnect
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
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
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:
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
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:
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
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:
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
participants (3)
-
Cole Robinson -
Martin Kletzander -
Michal Privoznik