[PATCH v4 0/2] handling guest multiple displays

[PATCH v4 0/2] handling guest multiple displays

[Dongwon Kim]

This patch seires is for adding some useful features for the guest os with
multi-displays. First patch is to make all of guest displays visible
when guest os is launched using "detach". Second patch is for providing
a method to assign each guest display to specific physical monitor,
which would be useful if someone wants to directly full-screen individual
guest scanouts to host's physical displays.

Changes in v4:

* ui/gtk: a new array param monitor to specify the target

  - changed "virtual-console" to an official term, "virtual console"
  - made if condition to check only 'full_screen' since 'has_full_screen' won't
    affect the result as 'full_screen' is always false if 'has_full_screen' is false.

Changes in v3:

* ui/gtk: a new array param monitor to specify the target

  - Revised commit message
  - Rewrote desription of the new parameter for clarification
  - 'for' loop that iterates through virtual consoles is actually executed only once
    only if the condition is met so replaced it with 'if' statement

Changes in v2:

* ui/gtk: detach VCS for additional guest displays

  - must check if the type of VC is GD_VC_GFX before qemu_console_is_graphic
  - It is not needed to pre-calculate n_gfx_vcs to determine how many times "detach"
    should be executed (n_gfx_vcs - 1) because the first virtual console (vc[0])
    is always in graphic mode so we can simply detach all other graphic mode
    virtual consoles.
  - making sure detached window's size same as original surface size

Dongwon Kim (2):
  ui/gtk: detach VCS for additional guest displays (v4)
  ui/gtk: a new array param monitor to specify the target displays (v4)

 qapi/ui.json    |  9 ++++++++-
 qemu-options.hx |  3 ++-
 ui/gtk.c        | 41 +++++++++++++++++++++++++++++++++++++++--
 3 files changed, 49 insertions(+), 4 deletions(-)

-- 
2.30.2

[PATCH v4 1/2] ui/gtk: detach VCs for additional guest displays

[Dongwon Kim]

Detaching any addtional guest displays in case multiple displays are
assigned to the guest OS (e.g. max_outputs=n) so that all of them are
visible upon lauching.

Cc: Daniel P. Berrangé <berrange@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
---
 ui/gtk.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index 2a791dd2aa..e6878c3209 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1361,6 +1361,11 @@ static void gd_menu_untabify(GtkMenuItem *item, void *opaque)
 
         g_signal_connect(vc->window, "delete-event",
                          G_CALLBACK(gd_tab_window_close), vc);
+
+        gtk_window_set_default_size(GTK_WINDOW(vc->window),
+                                    surface_width(vc->gfx.ds),
+                                    surface_height(vc->gfx.ds));
+
         gtk_widget_show_all(vc->window);
 
         if (qemu_console_is_graphic(vc->gfx.dcl.con)) {
@@ -2311,6 +2316,7 @@ static void gtk_display_init(DisplayState *ds, DisplayOptions *opts)
     GtkDisplayState *s = g_malloc0(sizeof(*s));
     GdkDisplay *window_display;
     GtkIconTheme *theme;
+    int i;
     char *dir;
 
     if (!gtkinit) {
@@ -2381,7 +2387,12 @@ static void gtk_display_init(DisplayState *ds, DisplayOptions *opts)
     gtk_widget_set_sensitive(s->copy_item,
                              vc && vc->type == GD_VC_VTE);
 #endif
-
+    for (i = 1; i < s->nb_vcs; i++) {
+        if (vc->type == GD_VC_GFX &&
+            qemu_console_is_graphic(s->vc[i].gfx.dcl.con)) {
+            gtk_menu_item_activate(GTK_MENU_ITEM(s->untabify_item));
+        }
+    }
     if (opts->has_full_screen &&
         opts->full_screen) {
         gtk_menu_item_activate(GTK_MENU_ITEM(s->full_screen_item));
-- 
2.30.2

[PATCH v4 2/2] ui/gtk: a new array param monitor to specify the target displays

[Dongwon Kim]

New integer array parameter, 'monitor' is for specifying the target
monitors where individual GTK windows are placed upon launching.

Monitor numbers in the array are associated with virtual consoles
in the order of [VC0, VC1, VC2 ... VCn].

Every GTK window containing each VC will be placed in the region
of corresponding monitors.

Usage: -display gtk,monitor.<id of VC>=<target monitor>,..
       ex)-display gtk,monitor.0=1,monitor.1=0

Cc: Daniel P. Berrangé <berrange@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
---
 qapi/ui.json    |  9 ++++++++-
 qemu-options.hx |  3 ++-
 ui/gtk.c        | 30 ++++++++++++++++++++++++++++--
 3 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/qapi/ui.json b/qapi/ui.json
index 413371d5e8..ee0f9244ef 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -1195,12 +1195,19 @@
 #               assuming the guest will resize the display to match
 #               the window size then.  Otherwise it defaults to "off".
 #               Since 3.1
+# @monitor:     Array of numbers, each of which represents physical
+#               monitor where GTK window containing a given VC will be
+#               placed. Each monitor number in the array will be
+#               associated with a virtual console starting from VC0.
+#
+#               since 7.1
 #
 # Since: 2.12
 ##
 { 'struct'  : 'DisplayGTK',
   'data'    : { '*grab-on-hover' : 'bool',
-                '*zoom-to-fit'   : 'bool'  } }
+                '*zoom-to-fit'   : 'bool',
+                '*monitor'       : ['uint16']  } }
 
 ##
 # @DisplayEGLHeadless:
diff --git a/qemu-options.hx b/qemu-options.hx
index 377d22fbd8..aabdfb0636 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1938,7 +1938,8 @@ DEF("display", HAS_ARG, QEMU_OPTION_display,
 #endif
 #if defined(CONFIG_GTK)
     "-display gtk[,full-screen=on|off][,gl=on|off][,grab-on-hover=on|off]\n"
-    "            [,show-cursor=on|off][,window-close=on|off]\n"
+    "            [,monitor.<id of VC>=<monitor number>][,show-cursor=on|off]"
+    "            [,window-close=on|off]\n"
 #endif
 #if defined(CONFIG_VNC)
     "-display vnc=<display>[,<optargs>]\n"
diff --git a/ui/gtk.c b/ui/gtk.c
index e6878c3209..598ab4970f 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -2316,6 +2316,10 @@ static void gtk_display_init(DisplayState *ds, DisplayOptions *opts)
     GtkDisplayState *s = g_malloc0(sizeof(*s));
     GdkDisplay *window_display;
     GtkIconTheme *theme;
+    GtkWidget *win;
+    GdkRectangle dest;
+    uint16List *mon;
+    int n_mon;
     int i;
     char *dir;
 
@@ -2393,10 +2397,32 @@ static void gtk_display_init(DisplayState *ds, DisplayOptions *opts)
             gtk_menu_item_activate(GTK_MENU_ITEM(s->untabify_item));
         }
     }
-    if (opts->has_full_screen &&
-        opts->full_screen) {
+
+    if (opts->u.gtk.has_monitor) {
+        i = 0;
+        n_mon = gdk_display_get_n_monitors(window_display);
+        for (mon = opts->u.gtk.monitor; mon; mon = mon->next) {
+            if (mon->value < n_mon && i < s->nb_vcs) {
+                win = s->vc[i].window ? s->vc[i].window : s->window;
+                if (opts->full_screen) {
+                    gtk_window_fullscreen_on_monitor(
+                        GTK_WINDOW(win),
+                        gdk_display_get_default_screen(window_display),
+                        mon->value);
+                } else {
+                    gdk_monitor_get_geometry(
+                        gdk_display_get_monitor(window_display, mon->value),
+                        &dest);
+                    gtk_window_move(GTK_WINDOW(win),
+                                    dest.x, dest.y);
+                }
+                i++;
+            }
+        }
+    } else if (opts->full_screen) {
         gtk_menu_item_activate(GTK_MENU_ITEM(s->full_screen_item));
     }
+
     if (opts->u.gtk.has_grab_on_hover &&
         opts->u.gtk.grab_on_hover) {
         gtk_menu_item_activate(GTK_MENU_ITEM(s->grab_on_hover_item));
-- 
2.30.2

Re: [PATCH v4 2/2] ui/gtk: a new array param monitor to specify the target displays

[Markus Armbruster]

Dongwon Kim <dongwon.kim@intel.com> writes:

> New integer array parameter, 'monitor' is for specifying the target
> monitors where individual GTK windows are placed upon launching.
>
> Monitor numbers in the array are associated with virtual consoles
> in the order of [VC0, VC1, VC2 ... VCn].
>
> Every GTK window containing each VC will be placed in the region
> of corresponding monitors.
>
> Usage: -display gtk,monitor.<id of VC>=<target monitor>,..
>        ex)-display gtk,monitor.0=1,monitor.1=0
>
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
> Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
> ---
>  qapi/ui.json    |  9 ++++++++-
>  qemu-options.hx |  3 ++-
>  ui/gtk.c        | 30 ++++++++++++++++++++++++++++--
>  3 files changed, 38 insertions(+), 4 deletions(-)
>
> diff --git a/qapi/ui.json b/qapi/ui.json
> index 413371d5e8..ee0f9244ef 100644
> --- a/qapi/ui.json
> +++ b/qapi/ui.json
> @@ -1195,12 +1195,19 @@
>  #               assuming the guest will resize the display to match
>  #               the window size then.  Otherwise it defaults to "off".
>  #               Since 3.1
> +# @monitor:     Array of numbers, each of which represents physical
> +#               monitor where GTK window containing a given VC will be
> +#               placed. Each monitor number in the array will be
> +#               associated with a virtual console starting from VC0.
> +#
> +#               since 7.1

I dislike repeating the type (here: array of numbers) in the
description.

Suggest something like

   # @monitor:     List of physical monitor numbers where the GTK windows
   #               containing the virtual consoles VC0, VC1, ... are to be
   #               placed.  (Since 7.1)

Missing: what happens when there are more VCs than list elements.  Can
you tell us?

>  #
>  # Since: 2.12
>  ##
>  { 'struct'  : 'DisplayGTK',
>    'data'    : { '*grab-on-hover' : 'bool',
> -                '*zoom-to-fit'   : 'bool'  } }
> +                '*zoom-to-fit'   : 'bool',
> +                '*monitor'       : ['uint16']  } }
>  
>  ##
>  # @DisplayEGLHeadless:

[...]

Re: [PATCH v4 2/2] ui/gtk: a new array param monitor to specify the target displays

[Dongwon Kim]

On Tue, Jul 12, 2022 at 08:11:08AM +0200, Markus Armbruster wrote:
> Dongwon Kim <dongwon.kim@intel.com> writes:
> 
> > New integer array parameter, 'monitor' is for specifying the target
> > monitors where individual GTK windows are placed upon launching.
> >
> > Monitor numbers in the array are associated with virtual consoles
> > in the order of [VC0, VC1, VC2 ... VCn].
> >
> > Every GTK window containing each VC will be placed in the region
> > of corresponding monitors.
> >
> > Usage: -display gtk,monitor.<id of VC>=<target monitor>,..
> >        ex)-display gtk,monitor.0=1,monitor.1=0
> >
> > Cc: Daniel P. Berrangé <berrange@redhat.com>
> > Cc: Markus Armbruster <armbru@redhat.com>
> > Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
> > Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
> > ---
> >  qapi/ui.json    |  9 ++++++++-
> >  qemu-options.hx |  3 ++-
> >  ui/gtk.c        | 30 ++++++++++++++++++++++++++++--
> >  3 files changed, 38 insertions(+), 4 deletions(-)
> >
> > diff --git a/qapi/ui.json b/qapi/ui.json
> > index 413371d5e8..ee0f9244ef 100644
> > --- a/qapi/ui.json
> > +++ b/qapi/ui.json
> > @@ -1195,12 +1195,19 @@
> >  #               assuming the guest will resize the display to match
> >  #               the window size then.  Otherwise it defaults to "off".
> >  #               Since 3.1
> > +# @monitor:     Array of numbers, each of which represents physical
> > +#               monitor where GTK window containing a given VC will be
> > +#               placed. Each monitor number in the array will be
> > +#               associated with a virtual console starting from VC0.
> > +#
> > +#               since 7.1
> 
> I dislike repeating the type (here: array of numbers) in the
> description.
> 
> Suggest something like
> 
>    # @monitor:     List of physical monitor numbers where the GTK windows
>    #               containing the virtual consoles VC0, VC1, ... are to be
>    #               placed.  (Since 7.1)
> 
> Missing: what happens when there are more VCs than list elements.  Can
> you tell us?

    # @monitor:     List of physical monitor numbers where the GTK windows
    #               containing the virtual consoles VC0, VC1, ... are to be
    #               placed. If a mapping exists for a VC, then it'd be
    #               placed on that specific physical monitor; otherwise,
    #               it'd default to the monitor from where it was launched
    #               since 7.1

How does this look?
> 
> >  #
> >  # Since: 2.12
> >  ##
> >  { 'struct'  : 'DisplayGTK',
> >    'data'    : { '*grab-on-hover' : 'bool',
> > -                '*zoom-to-fit'   : 'bool'  } }
> > +                '*zoom-to-fit'   : 'bool',
> > +                '*monitor'       : ['uint16']  } }
> >  
> >  ##
> >  # @DisplayEGLHeadless:
> 
> [...]
> 

Re: [PATCH v4 2/2] ui/gtk: a new array param monitor to specify the target displays

[Markus Armbruster]

Dongwon Kim <dongwon.kim@intel.com> writes:

> On Tue, Jul 12, 2022 at 08:11:08AM +0200, Markus Armbruster wrote:
>> Dongwon Kim <dongwon.kim@intel.com> writes:
>> 
>> > New integer array parameter, 'monitor' is for specifying the target
>> > monitors where individual GTK windows are placed upon launching.
>> >
>> > Monitor numbers in the array are associated with virtual consoles
>> > in the order of [VC0, VC1, VC2 ... VCn].
>> >
>> > Every GTK window containing each VC will be placed in the region
>> > of corresponding monitors.
>> >
>> > Usage: -display gtk,monitor.<id of VC>=<target monitor>,..
>> >        ex)-display gtk,monitor.0=1,monitor.1=0
>> >
>> > Cc: Daniel P. Berrangé <berrange@redhat.com>
>> > Cc: Markus Armbruster <armbru@redhat.com>
>> > Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> > Cc: Paolo Bonzini <pbonzini@redhat.com>
>> > Cc: Gerd Hoffmann <kraxel@redhat.com>
>> > Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
>> > Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
>> > ---
>> >  qapi/ui.json    |  9 ++++++++-
>> >  qemu-options.hx |  3 ++-
>> >  ui/gtk.c        | 30 ++++++++++++++++++++++++++++--
>> >  3 files changed, 38 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/qapi/ui.json b/qapi/ui.json
>> > index 413371d5e8..ee0f9244ef 100644
>> > --- a/qapi/ui.json
>> > +++ b/qapi/ui.json
>> > @@ -1195,12 +1195,19 @@
>> >  #               assuming the guest will resize the display to match
>> >  #               the window size then.  Otherwise it defaults to "off".
>> >  #               Since 3.1
>> > +# @monitor:     Array of numbers, each of which represents physical
>> > +#               monitor where GTK window containing a given VC will be
>> > +#               placed. Each monitor number in the array will be
>> > +#               associated with a virtual console starting from VC0.
>> > +#
>> > +#               since 7.1
>> 
>> I dislike repeating the type (here: array of numbers) in the
>> description.
>> 
>> Suggest something like
>> 
>>    # @monitor:     List of physical monitor numbers where the GTK windows
>>    #               containing the virtual consoles VC0, VC1, ... are to be
>>    #               placed.  (Since 7.1)
>> 
>> Missing: what happens when there are more VCs than list elements.  Can
>> you tell us?
>
>     # @monitor:     List of physical monitor numbers where the GTK windows
>     #               containing the virtual consoles VC0, VC1, ... are to be
>     #               placed. If a mapping exists for a VC, then it'd be
>     #               placed on that specific physical monitor; otherwise,
>     #               it'd default to the monitor from where it was launched
>     #               since 7.1
>
> How does this look?

Pretty good!

Nitpicks: replace "id'd" by "it will" or "it is to be", end your second
sentence with a period, and format "since" like we do elsewhere.
Together:

      # @monitor:     List of physical monitor numbers where the GTK windows
      #               containing the virtual consoles VC0, VC1, ... are to be
      #               placed. If a mapping exists for a VC, then it is to be
      #               placed on that specific physical monitor; otherwise,
      #               it defaults to the monitor from where it was launched.
      #               (Since 7.1)

>> 
>> >  #
>> >  # Since: 2.12
>> >  ##
>> >  { 'struct'  : 'DisplayGTK',
>> >    'data'    : { '*grab-on-hover' : 'bool',
>> > -                '*zoom-to-fit'   : 'bool'  } }
>> > +                '*zoom-to-fit'   : 'bool',
>> > +                '*monitor'       : ['uint16']  } }
>> >  
>> >  ##
>> >  # @DisplayEGLHeadless:
>> 
>> [...]
>>