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

[Frediano Ziglio]

> 
> 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
> based on the size of the just-created primary surface. This temporary monitors
> config message always has a maximum monitor count of 1.
> 
> This flag is set 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 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 thsi flag to false, it will result in the server sending a

typo: thsi -> this

> 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 will return at some point. So when the server
> constructs its own monitors config message to send to the client (when the
> driver_cap_monitors_config flag is false), we send the previous maximum monitor
> count instead of always sending a maximum of 1.
> 
> Resolves: rhbz#1274447
> ---
> 
> I'm simply re-sending this patch to see if anybody has additional opinions on
> this patch. It is clearly a workaround, and there was some skepticism about the
> approach when I initially proposed it. I did answer several concerns voiced by
> people but the discussion sort of petered out without a conclusion. I'm hoping
> to get a firm ACK or NACK on the patch.
> 

Maybe as workaround we should open a bug to remember to fix this.

Either way,

Acked-by: Frediano Ziglio <fziglio at redhat.com>

> 
>  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 bfff413..3864419 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -2447,15 +2447,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)

Frediano