[Spice-devel] [PATCH v2] Don't close all but one display during reboot.
Frediano Ziglio
fziglio at redhat.com
Thu May 25 12:14:40 UTC 2017
>
> 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
More information about the Spice-devel
mailing list