Re: [PATCH 1/2] ui/gtk: detach_all option for making all VCs detached upon starting

[Dongwon Kim]

On Tue, Jul 20, 2021 at 03:42:16PM +0200, Thomas Huth wrote:
> On 19/07/2021 23.41, Dongwon Kim wrote:
> > With "detach-all=on" for display, all VCs are detached from the beginning.
> > This is useful when there are multiple displays assigned to a guest OS.
> 
> Can you elaborate? (i.e. why is it useful? Do you just want to avoid having
> multiple things opened at startup? Or is there a different reason?)
Hi,

The original motivation is related to an use-case with a guest with
multi-displays. In that use case, we wanted to have all guest displays
placed side by side from beginning. Virtual consoles other than guest
displays (e.g. virtio-gpu-pci) are not actually needed but I found doing
"detach-all" is the simplest way.

> 
> > Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
> > Signed-off-by: Khairul Anuar Romli <khairul.anuar.romli@intel.com>
> > ---
> >   qapi/ui.json | 4 +++-
> >   ui/gtk.c     | 7 +++++++
> >   2 files changed, 10 insertions(+), 1 deletion(-)
> > 
> > diff --git a/qapi/ui.json b/qapi/ui.json
> > index 1052ca9c38..ff14bb2f46 100644
> > --- a/qapi/ui.json
> > +++ b/qapi/ui.json
> > @@ -1141,6 +1141,7 @@
> >   # @show-cursor:   Force showing the mouse cursor (default: off).
> >   #                 (since: 5.0)
> >   # @gl:            Enable OpenGL support (default: off).
> > +# @detach-all:    Detatch all VirtualConsoles from beginning (default: 
> > off).
> 
> Needs a comment à la "(since: 6.2)" at the end (like the one after
> "show-cursor" some lines earlier.
> 
> >   #
> >   # Since: 2.12
> >   #
> > @@ -1150,7 +1151,8 @@
> >                   '*full-screen'   : 'bool',
> >                   '*window-close'  : 'bool',
> >                   '*show-cursor'   : 'bool',
> > -                '*gl'            : 'DisplayGLMode' },
> > +                '*gl'            : 'DisplayGLMode',
> > +                '*detach-all'    : 'bool' },
> 
> If this is for GTK only, shouldn't this rather go into DisplayGTK instead?
> Or will this be also useful for other display types later?

This option might not be that useful for other use cases.. but at the
same time, I'm pretty sure this will work universally (won't break
anything..) but for now, I think it's good idea to limit this to GTK.

-DW

> 
>  Thomas
> 
> 
> >     'discriminator' : 'type',
> >     'data'    : { 'gtk'            : 'DisplayGTK',
> >                   'curses'         : 'DisplayCurses',
> > diff --git a/ui/gtk.c b/ui/gtk.c
> > index ce885d2ca3..a07e5a049e 100644
> > --- a/ui/gtk.c
> > +++ b/ui/gtk.c
> > @@ -2211,6 +2211,7 @@ static void gtk_display_init(DisplayState *ds, 
> > DisplayOptions *opts)
> >       GdkDisplay *window_display;
> >       GtkIconTheme *theme;
> >       char *dir;
> > +    int i;
> >       if (!gtkinit) {
> >           fprintf(stderr, "gtk initialization failed\n");
> > @@ -2290,6 +2291,12 @@ static void gtk_display_init(DisplayState *ds, 
> > DisplayOptions *opts)
> >           gtk_menu_item_activate(GTK_MENU_ITEM(s->grab_on_hover_item));
> >       }
> >       gd_clipboard_init(s);
> > +
> > +    if (opts->detach_all) {
> > +        for (i = 0; i < s->nb_vcs - 1; i++) {
> > +            gtk_menu_item_activate(GTK_MENU_ITEM(s->untabify_item));
> > +        }
> > +    }
> >   }
> >   static void early_gtk_display_init(DisplayOptions *opts)
> > 
>