<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">Is it possible to make the max number of monitors something persistent (e.g. set / get that using libvirt), so that the behavior would be consistent between a first boot and a reboot?</div><div class=""> </div><div class="">Christophe</div><div class=""> </div><div><blockquote type="cite" class=""><div class="">On 20 Feb 2017, at 15:49, Jonathon Jongsma <<a href="mailto:jjongsma@redhat.com" class="">jjongsma@redhat.com</a>> wrote:</div> <div class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">On Thu, 2017-02-02 at 10:30 -0600, Jonathon Jongsma wrote:<br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">On Thu, 2017-02-02 at 05:29 -0500, Frediano Ziglio wrote: <blockquote type="cite" class=""><blockquote type="cite" class=""> 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. </blockquote> 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. </blockquote> Well, it could be considered a client issue to some extent, but not an easy one to fix. <blockquote type="cite" class="">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. </blockquote> The initial boot and the reboot are the same, and that's basically why the problem exists. At initial boot, it brings up a single display. And on reboot, it also brings up a single display. The issue is that we want the reboot to behave differently. Initial boot: ------------- - In early boot, the server sends monitors config message when primary surface is created. monitors=1, max-monitors=1 - client only shows a single window, disables menu items for enabling additional displays because the guest doesn't support more than 1 - When QXL driver is loaded and takes over, it takes over sending monitors config messages. monitors=1, max-monitors=4 - client still shows a single window, but now the menu items for enabling additional menus are enabled - client enables a second monitor. Client sends monitors-config request to server - QXL driver enables the second monitor and sends a new monitors- config response to the client. monitors = 2, max-monitors=4 Reboot: ------- - At some point in the reboot process while the QXL driver is still loaded, it disables all but the first monitor, but still claims to support more than one. monitors=1, max-monitors=4 - client keeps both display windows open, but the second one just says "Waiting for display 2..." - eventually the QXL driver gets unloaded, and the RedWorker::driver_cap_monitors_config flag gets set to false - The next time a primary surface is created (during early boot?) the server sends out a new monitors config message to match the size of the primary surface. monitors=1, max-monitors=1 - the client sees that the guest only supports a single monitor, so it closes that inactive second display window - when the qxl driver is loaded and takes over, it takes over sending monitors-config messages. monitors=1, max-monitors=4. Before qemu commit 0a2b5e3a7899b40d05d7c6c1c41eb4e64dd2ed4b, the QXL driver never called _unload() during reboot. This meant that during early boot, the server would never send out monitors-config messages with max-monitors=1. So the client would have never closed its extra inactive display windows. They would have simply remained open with a "Waiting for display n..." message. And as soon as the vdagent was reconnected, the client would have sent a new monitors-config request attempting to enable displays for all open windows. But now that doesn't happen because those windows get closed during reboot. So my fix attempts to retain that behavior (while still fixing the bug that was addressed by 0a2b5e3a7899b40d05d7c6c1c41eb4e64dd2ed4b) by keeping the max-monitors value the same as it had been before rebooting. You could possibly argue that you should instead fix this issue in the client by refusing to close/destroy displays when we recieve a monitors-config message with a max-monitors value that is less than the number of currently-open windows. But the design of the client is such that when we recieve a monitors-config message from the server, we automatically create N Display objects (where N is the value of max- monitors) even if these displays are not yet enabled. The existence of these Display objects determines whether or not the menu for enabling additional displays are enabled, etc. So if we change the client to not destroy extra displays when we receive max-monitors, I fear that we will introduce some additional bugs by violating some hidden assumptions. But we could try it if you think it's a better approach. It would be a more complicated fix, however. </blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Anybody have additional thoughts about this patch?<br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class=""> <blockquote type="cite" class=""> <blockquote type="cite" class="">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- <blockquote type="cite" class="">surfaces[0].context.canvas); </blockquote> - 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) </blockquote> Frediano </blockquote> _______________________________________________ Spice-devel mailing list <a href="mailto:Spice-devel@lists.freedesktop.org" class="">Spice-devel@lists.freedesktop.org</a> https://lists.freedesktop.org/mailman/listinfo/spice-devel </blockquote><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">_______________________________________________<br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Spice-devel mailing list<br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><a href="mailto:Spice-devel@lists.freedesktop.org" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">Spice-devel@lists.freedesktop.org</a><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><a href="https://lists.freedesktop.org/mailman/listinfo/spice-devel" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">https://lists.freedesktop.org/mailman/listinfo/spice-devel</a></div></blockquote></div> </body></html>