[PATCH v3 0/2] handling guest multiple displays

[PATCH v3 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 v3:

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

  - Revised commit message
  - Rewrote desription of the new parameter (Markus Armbruster)
  - Replaced unnecessary 'for' loop with 'if' condition
    (Markus Armbruster)

Changes in v2:

* ui/gtk: detach VCS for additional guest displays

  - check if the type of VC is GD_VC_GFX before qemu_console_is_graphic
    (Gerd Hoffman)
  - vc[0] is always primary guest display so we won't need n_gfx_vcs
    (Gerd Hoffmann)
  - making sure detached window's size same as original surface size
    (Daniel P. Berrangé)

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

 qapi/ui.json    |  7 ++++++-
 qemu-options.hx |  2 +-
 ui/gtk.c        | 43 +++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 48 insertions(+), 4 deletions(-)

-- 
2.30.2

[PATCH v3 1/3] 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.

v2: - making sure type of VC is GD_VC_GFX before qemu_console_is_graphic
      (Gerd Hoffman)
    - vc[0] is always primary guest display so we won't need n_gfx_vcs
      (Gerd Hoffmann)
    - making sure detached window's size same as original surface size
      (Daniel P. Berrangé)

Cc: Daniel P. Berrangé <berrange@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
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

Re: [PATCH v3 1/3] ui/gtk: detach VCs for additional guest displays

[Markus Armbruster]

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

> 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.
>
> v2: - making sure type of VC is GD_VC_GFX before qemu_console_is_graphic
>       (Gerd Hoffman)
>     - vc[0] is always primary guest display so we won't need n_gfx_vcs
>       (Gerd Hoffmann)
>     - making sure detached window's size same as original surface size
>       (Daniel P. Berrangé)

Patch history ...

> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> 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>
> ---

... goes here, so it doesn't end up in git.  You can also keep it in the
cover letter instead.

>  ui/gtk.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)

[PATCH v3 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

v3: - Revised commit message
    - Rewrote desription of the new parameter (Markus Armbruster)
    - Replaced unnecessary 'for' loop with 'if' condition
      (Markus Armbruster)

Cc: Daniel P. Berrangé <berrange@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
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        | 31 +++++++++++++++++++++++++++++--
 3 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/qapi/ui.json b/qapi/ui.json
index 413371d5e8..7b4c098bb4 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..935176e614 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,33 @@ 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->has_full_screen && 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->has_full_screen &&
+               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.20.1

Re: [PATCH v3 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
>
> v3: - Revised commit message
>     - Rewrote desription of the new parameter (Markus Armbruster)
>     - Replaced unnecessary 'for' loop with 'if' condition
>       (Markus Armbruster)

Again, patch history ...
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> 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>
> ---

... goes here.

>  qapi/ui.json    |  9 ++++++++-
>  qemu-options.hx |  3 ++-
>  ui/gtk.c        | 31 +++++++++++++++++++++++++++++--
>  3 files changed, 39 insertions(+), 4 deletions(-)
>
> diff --git a/qapi/ui.json b/qapi/ui.json
> index 413371d5e8..7b4c098bb4 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.

Drop the hyphen in "virtual-console".

Is the term "virtual console" obvious?  Gerd?

> +#
> +#               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..935176e614 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,33 @@ 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->has_full_screen && 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->has_full_screen &&
> +               opts->full_screen) {

I'd join these two lines, like

       } else if (opts->has_full_screen && opts->full_screen) {

or even exploit the fact that a opts->full_screen is false when absent

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

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

[Gerd Hoffmann]

  Hi,

> > +# @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.
> 
> Drop the hyphen in "virtual-console".
> 
> Is the term "virtual console" obvious?  Gerd?

I think so, same term is used elsewhere too for the same concept.

take care,
  Gerd

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

[Dongwon Kim]

Thanks, yeah, I will remove '-' and resubmitt the patch.

On Fri, Jul 01, 2022 at 11:58:48AM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> > > +# @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.
> > 
> > Drop the hyphen in "virtual-console".
> > 
> > Is the term "virtual console" obvious?  Gerd?
> 
> I think so, same term is used elsewhere too for the same concept.
> 
> take care,
>   Gerd
> 

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

[Dongwon Kim]

On Thu, Jun 30, 2022 at 05:19:26PM +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
> >
> > v3: - Revised commit message
> >     - Rewrote desription of the new parameter (Markus Armbruster)
> >     - Replaced unnecessary 'for' loop with 'if' condition
> >       (Markus Armbruster)
> 
> Again, patch history ...
> > Cc: Daniel P. Berrangé <berrange@redhat.com>
> > Cc: Markus Armbruster <armbru@redhat.com>
> > Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> > 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>
> > ---
> 
> ... goes here.

No problem moving down the version history but may I ask you if that
is current rule? We don't want to include the history anymore in the
git history?

And FYI, the cover letter has the whole history already. I guess I can
simply remove the history from individual patches then?

Thanks!!

> 
> >  qapi/ui.json    |  9 ++++++++-
> >  qemu-options.hx |  3 ++-
> >  ui/gtk.c        | 31 +++++++++++++++++++++++++++++--
> >  3 files changed, 39 insertions(+), 4 deletions(-)
> >
> > diff --git a/qapi/ui.json b/qapi/ui.json
> > index 413371d5e8..7b4c098bb4 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.
> 
> Drop the hyphen in "virtual-console".
> 
> Is the term "virtual console" obvious?  Gerd?
> 

I will do so.

> > +#
> > +#               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..935176e614 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,33 @@ 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->has_full_screen && 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->has_full_screen &&
> > +               opts->full_screen) {
> 
> I'd join these two lines, like
> 
>        } else if (opts->has_full_screen && opts->full_screen) {
> 
> or even exploit the fact that a opts->full_screen is false when absent
> 
>        } else if (opts->full_screen) {

This is simpler. I will go with this.

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

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

[Markus Armbruster]

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

> On Thu, Jun 30, 2022 at 05:19:26PM +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
>> >
>> > v3: - Revised commit message
>> >     - Rewrote desription of the new parameter (Markus Armbruster)
>> >     - Replaced unnecessary 'for' loop with 'if' condition
>> >       (Markus Armbruster)
>> 
>> Again, patch history ...
>>
>> > Cc: Daniel P. Berrangé <berrange@redhat.com>
>> > Cc: Markus Armbruster <armbru@redhat.com>
>> > Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
>> > 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>
>> > ---
>> 
>> ... goes here.
>
> No problem moving down the version history but may I ask you if that
> is current rule? We don't want to include the history anymore in the
> git history?

Patch history is really valuable for reviewers, but once the patch is
done, it's rarely interesting anymore, so we keep it out of Git.

Sometimes, bits of history are still useful to understand why the patch
is done the way it is.  Such bits should be worked into the commit
message.

Don't:

    v2: A replaced by B [J. Reviewer]

Do:

    I initially tried A, but it turned out to be a bad idea because X,
    so I did B instead.

Makes sense?

> And FYI, the cover letter has the whole history already. I guess I can
> simply remove the history from individual patches then?

I like to keep detailed history in the cover letter.

Others like to keep it in each patch.

Still others like to keep an overview in the cover letter and details in
each patch.

All fine.

> Thanks!!
>
>> 
>> >  qapi/ui.json    |  9 ++++++++-
>> >  qemu-options.hx |  3 ++-
>> >  ui/gtk.c        | 31 +++++++++++++++++++++++++++++--
>> >  3 files changed, 39 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/qapi/ui.json b/qapi/ui.json
>> > index 413371d5e8..7b4c098bb4 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.
>> 
>> Drop the hyphen in "virtual-console".
>> 
>> Is the term "virtual console" obvious?  Gerd?
>> 
>
> I will do so.

Replace it by space, of course.

[...]