[Spice-devel] [PATCH] 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
> 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.
> 

After reading this paragraph and the bug I don't see the issue.
I mean, if on my laptop I reboot when I reboot I get a single monitor
till the OS and other stuff kick in. After a reboot the firmware use
one monitor or if capable do the mirroring but always the same way.

I think the issue is that on first boot the guest activate the
additional monitors and the client do the same while on second
boot (reboot) not so to me looks like more an issue of the client
instead of the server.

I would try to get a network capture to look at DisplayChannel
messages if they are more or less the same for first boot and
reboot. If they are the same I would look at the client state
to check that while booting it's the same.

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

Frediano