On Thu, 2017-05-25 at 08:14 -0400, Frediano Ziglio 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
> > 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.
The problem is, i'm not sure if there *is* a proper fix for this.
Perhaps you could change the client so that it doesn't close displays
that are currently opened when we receive a new monitors config message
with max-monitors < n_monitors. But that was an intentional choice in
the design of the client and changing it would potentially cause other
issues. I'm not sure it's worth changing.
>
> Either way,
>
> Acked-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
>
> >
> > 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
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel