Re: [RFC PATCH 6/9] ui/gtk: Add a new parameter to assign connectors/monitors to GFX VCs

[Markus Armbruster]

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

> From: Vivek Kasireddy <vivek.kasireddy@intel.com>
>
> The new parameter named "connector" can be used to assign physical
> monitors/connectors to individual GFX VCs such that when the monitor
> is connected or hotplugged, the associated GTK window would be
> moved to it. If the monitor is disconnected or unplugged, the
> associated GTK window would be hidden and a relevant disconnect
> event would be sent to the Guest.
>
> Usage: -device virtio-gpu-pci,max_outputs=2,blob=true,...
>        -display gtk,gl=on,connectors.0=eDP-1,connectors.1=DP-1.....
>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>

[...]

> --- a/qapi/ui.json
> +++ b/qapi/ui.json
> @@ -1315,13 +1315,22 @@
>  # @show-menubar: Display the main window menubar.  Defaults to "on".
>  #     (Since 8.0)
>  #
> +# @connectors:  List of physical monitor/connector names where the GTK
> +#               windows containing the respective graphics virtual consoles
> +#               (VCs) are to be placed. If a mapping exists for a VC, it
> +#               will be moved to that specific monitor or else it would
> +#               not be displayed anywhere and would appear disconnected
> +#               to the guest.
> +#               (Since 8.1)

Please format like

   # @connectors: List of physical monitor/connector names where the GTK
   #     windows containing the respective graphics virtual consoles
   #     (VCs) are to be placed.  If a mapping exists for a VC, it will
   #     be moved to that specific monitor or else it would not be
   #     displayed anywhere and would appear disconnected to the guest.
   #     (Since 8.1)

to blend in with recent commit a937b6aa739 (qapi: Reformat doc comments
to conform to current conventions).

The meaning of @connectors is less than clear.  The phrase "If a mapping
exists for a VC" suggests it is a mapping of sorts.  "List of physical
monitor/connector names" indicates it maps to physical monitor /
connector name.  What does it map from?  VC number?  How are VCs
numbered?  Is it the same number we use in QOM /backend/console[NUM]?

Using a list for the mapping means the mapping must be dense, e.g. I
can't map #0 and #2 but not #1.  Is this what we want?

The sentence "If a mapping exists" confusing has a dangling else
ambiguity of sorts.  I can interpret it as

    If a mapping exists for a VC:
        the window will be moved to that specific monitor
        or else it would not be displayed anywhere and would appear ...

or as

    If a mapping exists for a VC:
        the window will be moved to that specific monitor
    or else it would not be displayed anywhere and would appear ...

I think we have three cases:

0. No mapping provided

1. Mapping provided, and the named monitor / connector exists

2. Mapping provided, and the named monitor / connector does not exist

We can go from case 1 to 2 (disconnect) and vice versa (connect) at any
time.

Please spell out behavior for each case, and for the transitions between
case 1 and 2.

> +#
>  # Since: 2.12
>  ##
>  { 'struct'  : 'DisplayGTK',
>    'data'    : { '*grab-on-hover' : 'bool',
>                  '*zoom-to-fit'   : 'bool',
>                  '*show-tabs'     : 'bool',
> -                '*show-menubar'  : 'bool'  } }
> +                '*show-menubar'  : 'bool',
> +                '*connectors'    : ['str'] } }
>  
>  ##
>  # @DisplayEGLHeadless:

[...]