[Spice-devel] [PATCH] Don't close all but one display during reboot.

[Snir Sheriber]

Hi,

On 01/31/2017 10:24 PM, Jonathon Jongsma wrote:
> When a guest is rebooted, the QXL driver gets unloaded at some point in
> the reboot process. When the driver is unloaded, the spice server sets a
> single flag to FALSE: RedWorker::driver_cap_monitors_config. This flag
> indicates whether the driver is capable of sending its own monitors config
> messages to the client.
>
> The only place this flag is used is when a new primary surface is created. If
> this flag is true, the server assumes that the driver will send its own
> monitors config very soon after the surface is created. If it's false, the
> server directly sends its own temporary monitors config message to the client
> since the driver cannot.  This temporary monitors config message is based on
> the size of the just-created primary surface and always has a maximum monitor
> count of 1.
>
> This flag is initialized to false at startup so that in early boot (e.g. vga
> text mode), the server will send out these 'temporary' monitor config messages
> to the client whenever the primary surface is destroyed and re-created. This
> causes the client to resize its window to match the size of the guest. When the
> QXL driver is eventually loaded and starts taking over the monitors config
> responsibilities, we set this flag to true and the server stops sending
> monitors config messages out on its own.
>
> If we reboot and set this flag to false, it will result in the server sending a
> monitors config message to the client indicating that the guest now supports a
> maximum of 1 monitor. If the guest happens to have more than one display window
> open, it will destroy those extra windows because they exceed the maximum
> allowed number of monitors. This means that if you reboot a guest with 4
> monitors, after reboot it will only have 1 monitor.
>
> To avoid this, we assume that if we had the ability to support multiple
> monitors at some point, that ability will return at some point. So when the
> server constructs its own temporary monitors config message to send to the
> client (when the driver_cap_monitors_config flag is false), we continue to send
> the previous maximum monitor count instead of always sending a maximum of 1.
>
> Resolves: rhbz#1274447
> ---
>
> NOTE: this fix is a workaround that assumes some things that may not be valid
> in all cases.  The main assumption is that if the QXL driver was used before
> the reboot, it will continue to be used after the reboot. If this assumption is
> violated, the server will continue to report that the guest supports e.g. 4
> displays even though the driver may only support 1. The client would then be
> able to attempt to enable additional displays, which would inevitably fail
> without explanation.  This is not a huge problem, but maybe it's not
> acceptable. If it's not acceptable, I think that fixing this bug would require
> significantly more work and may not be worth the effort.
When i apply your patch, enable 2 displays , reboot & disable qxl (using 
kernel cmd- before booting)
2 displays are still functioning after booting although there is no qxl. 
(i know it's because of vdagent but
can someone shed some light on what exactly is happening? )

Snir.
>   server/display-channel.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/server/display-channel.c b/server/display-channel.c
> index a554bfd..88ee194 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -2459,15 +2459,18 @@ void display_channel_set_monitors_config_to_primary(DisplayChannel *display)
>   {
>       DrawContext *context = &display->priv->surfaces[0].context;
>       QXLHead head = { 0, };
> +    uint16_t old_max = 1;
>   
>       spice_return_if_fail(display->priv->surfaces[0].context.canvas);
>   
> -    if (display->priv->monitors_config)
> +    if (display->priv->monitors_config) {
> +        old_max = display->priv->monitors_config->max_allowed;
>           monitors_config_unref(display->priv->monitors_config);
> +    }
>   
>       head.width = context->width;
>       head.height = context->height;
> -    display->priv->monitors_config = monitors_config_new(&head, 1, 1);
> +    display->priv->monitors_config = monitors_config_new(&head, 1, old_max);
>   }
>   
>   void display_channel_reset_image_cache(DisplayChannel *self)