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

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

[Dongwon Kim]

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

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

[PATCH 2/2] ui/gtk: specify detached window's size and location

[Dongwon Kim]

Specify location and size of detached window based on top level window's
location and size info when detachment happens.

Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
---
 ui/gtk.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/ui/gtk.c b/ui/gtk.c
index a07e5a049e..9b4e85624a 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1274,6 +1274,8 @@ static void gd_menu_untabify(GtkMenuItem *item, void *opaque)
 {
     GtkDisplayState *s = opaque;
     VirtualConsole *vc = gd_vc_find_current(s);
+    gint x, y, w, h;
+    int i;
 
     if (vc->type == GD_VC_GFX &&
         qemu_console_is_graphic(vc->gfx.dcl.con)) {
@@ -1284,6 +1286,18 @@ static void gd_menu_untabify(GtkMenuItem *item, void *opaque)
         gtk_widget_set_sensitive(vc->menu_item, false);
         vc->window = gtk_window_new(GTK_WINDOW_TOPLEVEL);
         gd_widget_reparent(s->notebook, vc->window, vc->tab_item);
+        gtk_window_get_position(GTK_WINDOW(s->window), &x, &y);
+        gtk_window_get_size(GTK_WINDOW(s->window), &w, &h);
+
+        for (i = 0; i < s->nb_vcs; i++) {
+            if (vc == &s->vc[i]) {
+                break;
+            }
+        }
+
+        gtk_window_move(GTK_WINDOW(vc->window),
+                        x + w * (i % (s->nb_vcs/2) + 1), y + h * (i / (s->nb_vcs/2)));
+        gtk_window_resize(GTK_WINDOW(vc->window), w, h);
 
         g_signal_connect(vc->window, "delete-event",
                          G_CALLBACK(gd_tab_window_close), vc);
-- 
2.17.1

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

[Thomas Huth]

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

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

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

[Romli, Khairul Anuar]

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

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

[Dongwon Kim]

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.

v2: Move "detach-all" option to under DisplayGTK as it's GTK specific
    (Thomas Huth)

Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
Signed-off-by: Khairul Anuar Romli <khairul.anuar.romli@intel.com>
---
 qapi/ui.json | 5 ++++-
 ui/gtk.c     | 7 +++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/qapi/ui.json b/qapi/ui.json
index 1052ca9c38..e37af91683 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -1035,13 +1035,16 @@
 #               assuming the guest will resize the display to match
 #               the window size then.  Otherwise it defaults to "off".
 #               Since 3.1
+# @detach-all:  Detach all VirtualConsoles when starting Qemu (default: off).
+#               Since 6.0
 #
 # Since: 2.12
 #
 ##
 { 'struct'  : 'DisplayGTK',
   'data'    : { '*grab-on-hover' : 'bool',
-                '*zoom-to-fit'   : 'bool'  } }
+                '*zoom-to-fit'   : 'bool',
+                '*detach-all'    : 'bool'  } }
 
 ##
 # @DisplayEGLHeadless:
diff --git a/ui/gtk.c b/ui/gtk.c
index 98046f577b..48c1fff54f 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -2189,6 +2189,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");
@@ -2268,6 +2269,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->u.gtk.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)
-- 
2.17.1