[virt-tools-list] [PATCH virt-viewer 0/4] Don't open blank window for a disabled first display

[Fabiano Fidêncio]

Hey Jonathon,

On Wed, 2014-08-27 at 16:38 -0500, Jonathon Jongsma wrote:
> On Wed, 2014-08-27 at 03:32 +0200, Fabiano Fidêncio wrote:
> > Hey Jonathon,
> > 
> > 
> > On Fri, Aug 22, 2014 at 4:59 PM, Jonathon Jongsma
> > <jjongsma at redhat.com> wrote:
> >         This is a patch series that attempts to fix rhbz#1032939. It's
> >         a pretty
> >         significant re-factoring of some of the multi-monitor handling
> >         code, so it
> >         requires careful review and testing.
> >         
> >         The current design of virt-viewer is that we have a
> >         VirtViewerWindow object
> >         (the "main window") that is shown at startup in order to
> >         communicate
> >         session-level connection status to the user. This window is
> >         stored in a hash
> >         table with a key being the ID of the display (#0, the first
> >         display). When we
> >         receive a new spice message indicating the maximum number of
> >         monitors available
> >         (e.g. 4 in the case of a linux guest with QXL), we create
> >         display objects for
> >         each of these remote displays, and signal that a new display
> >         was added. In
> >         resonse to this signal, we look up the window associated with
> >         this display's
> >         ID. If the window exists (as it does with display #0), we
> >         associate the display
> >         with that window.  Otherwise, we create a new window, and
> >         insert it into the
> >         hash table with the display's ID.
> >         
> >         Usually the first display on the guest is enabled, so this
> >         works out OK. But
> >         in cases where the first display is disabled on the guest, we
> >         end up with a
> >         "main window" that is blank and simply displays the message
> >         "Waiting for
> >         display 1...".
> >         
> >         The approach taken in this patch series is to remove the
> >         explicit association
> >         of an ID with a particular VirtViewerWindow. In other words,
> >         we no longer
> >         maintain a hash table of windows that associate a fixed id
> >         with the window
> >         object. Instead, we have a simple list of windows, and the ID
> >         of that window is
> >         determined by the ID of the VirtViewerDisplay object that it
> >         contains. In
> >         addition, we only create a VirtViewerWindow object for the
> >         display when the
> >         remote display becomes ready. Disabled remote displays do not
> >         have an
> >         associated VirtViewerWindow object until they become enabled.
> >         This means that
> >         the initial window (the "main window") can become any ID: it
> >         gets its ID from
> >         the first display that becomes enabled.
> >         
> >         Kiosk mode and Fullscreen mode require some additional
> >         handling, but I've
> >         tested both of them and they seem to work with this patch
> >         series.
> >         
> >         
> >         Jonathon Jongsma (4):
> >           App: keep hash table of displays
> >           VirtViewerDisplay: add convenience API for getting nth
> >           VirtViewerApp: store windows in a list
> >           Create windows on demand, not at startup
> >         
> >          src/remote-viewer.c       |  14 +--
> >          src/virt-viewer-app.c     | 296
> >         +++++++++++++++++++++++++---------------------
> >          src/virt-viewer-app.h     |   2 +-
> >          src/virt-viewer-display.c |   6 +
> >          src/virt-viewer-display.h |   1 +
> >          5 files changed, 176 insertions(+), 143 deletions(-)
> >         
> >         --
> >         1.9.3
> >         
> >         _______________________________________________
> >         virt-tools-list mailing list
> >         virt-tools-list at redhat.com
> >         https://www.redhat.com/mailman/listinfo/virt-tools-list
> > 
> > Doing a simple test here I found a regression. The first opened window
> > is always a really small window (400 x something) and I have to resize
> > it to a useful size manually. Can you reproduce it?
> 
> 
> Yes, I can reproduce this regression.  Will investigate.
> 
> Jonathon

According to my simple tests, the changes work as expected.
I've added a some comments on each patch of the series.

> 
> 
> _______________________________________________
> virt-tools-list mailing list
> virt-tools-list at redhat.com
> https://www.redhat.com/mailman/listinfo/virt-tools-list

Best Regards,
--
Fabiano Fidêncio