[RFC PATCH] virt-manager: console disconnected after rebooting virtual machine in Xen mode

[Cole Robinson]

On 10/13/21 3:30 PM, Charles Arnold wrote:
> Running with virt-manager 3.2.0, not a problem in 2.2.1.
> 
> When rebooting an HVM guest on Xen, virt-manager gets a disconnected
> event which closes the viewer. See
> self._viewer.connect("disconnected", self._viewer_disconnected).
> Running on KVM, this event does not seem to occur on VM reboot.
> 
> Both Xen and KVM get a state changed event,
> self.vm.connect("state-changed", self._vm_state_changed_cb)
> but for Xen, the viewer is gone due to the prior disconnect and when
> calling into _activate_default_console_page(), the viewer does not
> exist nor does it get created.
> The VM comes up fine but you need to close the console window and
> open it again to see the running VM.
> 
> This patch resolves the issue for me.
> 
> diff --git a/virtManager/details/console.py
> b/virtManager/details/console.py
> index fafb7202..f8a005ee 100644
> --- a/virtManager/details/console.py
> +++ b/virtManager/details/console.py
> @@ -913,6 +913,8 @@ class vmmConsolePages(vmmGObjectUI):
>              return
> 
>          cpage = self.widget("console-pages").get_current_page()
> +        if cpage == _CONSOLE_PAGE_GRAPHICS:
> +            self._init_viewer()
>          if cpage != _CONSOLE_PAGE_UNAVAILABLE:
>              return
> 
> 

Sorry for the long delayed response. Thanks for analyzing this. I think
I've seen some similar presenting issue with qemu but it's been hard to
reproduce.

The flow in console.py is confusing despite my previous cleanup
attempts. There's two error pages: _CONSOLE_PAGE_GRAPHICS with subpage
_GFX_PAGE_UNAVAILABLE, and _CONSOLE_PAGE_UNAVAILABLE

The former is meant to be for fatal connection errors, which should
block simple attempts to reconnect. So the error is preserved on screen
unless the dialog is closed+reopened. Otherwise an error may come in but
quickly get wiped out by a VM XML change event, which would attempt to
reconnect to the VM, possibly asking for auth, etc.

_CONSOLE_PAGE_UNAVAILABLE is lighter weight. This is the 'guest not
running' screen, and a few others. If the VM is running and a state
change event comes in, and we are on this page, we attempt to connect to
the viewer (like when you click 'run' on an open details window).

Unfortunately your patch makes the two cases act identical, which we
don't want for the current code IMO

The issue in this case is that viewer-disconnected is overloaded, since
it can trigger for error conditions (like ssh tunnel problems), or for
normal disconnects like this reboot case. The former should block
console reconnection, the latter shouldn't since we have no useful error
feedback to report.

Can you try this patch and see if it fixes your case?

diff --git a/virtManager/details/console.py b/virtManager/details/console.py
index fafb7202..9b1e5db5 100644
--- a/virtManager/details/console.py
+++ b/virtManager/details/console.py
@@ -814,14 +814,23 @@ class vmmConsolePages(vmmGObjectUI):
             return

         msg = _("Viewer was disconnected.")
+        errmsg = ""
         if errdetails:
-            msg += "\n" + errdetails
+            errmsg += "\n" + errdetails
         if ssherr:
             log.debug("SSH tunnel error output: %s", ssherr)
-            msg += "\n\n"
-            msg += _("SSH tunnel error output: %s") % ssherr
+            errmsg += "\n\n"
+            errmsg += _("SSH tunnel error output: %s") % ssherr

-        self._activate_gfx_unavailable_page(msg)
+        if errmsg:
+            self._activate_gfx_unavailable_page(msg + errmsg)
+            return
+
+        # If no error message was reported, this isn't a clear graphics
+        # error that should block reconnecting. So use the top level
+        # 'VM unavailable' page which makes it easier for the user to
+        # reconnect.
+        self._activate_vm_unavailable_page(msg)

     def _viewer_disconnected(self, ignore, errdetails, ssherr):
         self._activate_gfx_unavailable_page(_("Viewer disconnected."))



Thanks,
Cole