diff mbox series

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

Message ID 20210719214157.5863-1-dongwon.kim@intel.com (mailing list archive)
State New, archived
Headers show
Series [1/2] ui/gtk: detach_all option for making all VCs detached upon starting | expand

Commit Message

Kim, Dongwon July 19, 2021, 9:41 p.m. UTC
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.

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(-)

Comments

Thomas Huth July 20, 2021, 1:42 p.m. UTC | #1
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?)

> 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?

  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)
>
Kim, Dongwon July 20, 2021, 10:16 p.m. UTC | #2
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)
> > 
>
Romli, Khairul Anuar July 21, 2021, 1:54 a.m. UTC | #3
I've tried and this patch is able to detach all the virtual console after we launch the qemu. However, I think we need to filter out other terminal that are not related to view such as compatmonitor(), serial and parallel. 

Also, I think we can have the detach specific to virtio-pci view without the need to have new parameters.

> -----Original Message-----
> From: Kim, Dongwon <dongwon.kim@intel.com>
> Sent: Wednesday, July 21, 2021 6:17 AM
> To: Thomas Huth <thuth@redhat.com>
> Cc: qemu-devel@nongnu.org; Romli, Khairul Anuar
> <khairul.anuar.romli@intel.com>
> Subject: Re: [PATCH 1/2] ui/gtk: detach_all option for making all VCs
> detached upon starting
> 
> 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++) {

[Romli, Khairul Anuar]  We can a conditional check here to only detech virtio-pci view rather than "everything". Also, we may want to consider not to detach the primary view and keep it remain in qemu primary window.

> > > +            gtk_menu_item_activate(GTK_MENU_ITEM(s->untabify_item));
> > > +        }
> > > +    }
> > >   }
> > >   static void early_gtk_display_init(DisplayOptions *opts)
> > >
> >
diff mbox series

Patch

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).
 #
 # Since: 2.12
 #
@@ -1150,7 +1151,8 @@ 
                 '*full-screen'   : 'bool',
                 '*window-close'  : 'bool',
                 '*show-cursor'   : 'bool',
-                '*gl'            : 'DisplayGLMode' },
+                '*gl'            : 'DisplayGLMode',
+                '*detach-all'    : 'bool' },
   '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)