From 5cf857175d724e399b53d9ce29d09fa7aef5bf36 Mon Sep 17 00:00:00 2001

From: Christophe Fergeau <cfergeau@redhat.com>

Date: Thu, 9 Jan 2014 17:41:07 +0100

Subject: [PATCH] randr: Make resolution changing more robust

When running a guest using a KMS QXL driver on a RHEL6 hypervisor,

spice_vdagent would crash on resolution changes with:

X Error of failed request:  BadValue (integer parameter out of range for operation)

  Major opcode of failed request:  139 (RANDR)

  Minor opcode of failed request:  21 (RRSetCrtcConfig)

  Value in failed request:  0x0

  Serial number of failed request:  75

  Current serial number in output stream:  75

(if using a newer QEMU, this will not crash as the resolution codepath is

totally different, see red_dispatcher_use_client_monitors_config() in

spice-server, when using the UMS QXL driver, the crash will not happen

because of

http://cgit.freedesktop.org/xorg/driver/xf86-video-qxl/commit/?id=907d0ff0

).

This crash happens in vdagent_x11_set_monitor_config() because the X server

rejects the request because we are trying to set a CRTC to a size which is

bigger than the current screen (using 'screen' and 'CRTC' as in the RandR

spec at http://www.x.org/releases/X11R7.5/doc/randrproto/randrproto.txt ).

If we resize the screen before changing the CRTCs resolution, then this

error will no longer happen. However, if we first call XRRSetScreenSize()

and then XRRSetCrtcConfig(), the call to XRRSetScreeSize() will fail when

we try to make the screen smaller as there will be active CRTCs which would

be bigger than the screen.

In order to solve these issues, we follow the same process as what mutter

does in meta_monitor_manager_xrandr_apply_configuration()

https://git.gnome.org/browse/mutter/tree/src/core/monitor-xrandr.c#n689:

1. we disable the CRTCs which are off and the ones which are bigger than the

   size we want to set the screen to.

2. we resize the screen with a call to XRRSetScreenSize().

3. we set each CRTCs to their new resolution.

https://bugzilla.redhat.com/show_bug.cgi?id=1066422

(cherry picked from commit 9ae51f3702b1fc0d2747e44474c87a818d1e8ec3)

---

 src/vdagent-x11-randr.c | 61 +++++++++++++++++++++++++++++++++++++------------

 1 file changed, 47 insertions(+), 14 deletions(-)

diff --git a/src/vdagent-x11-randr.c b/src/vdagent-x11-randr.c

index 5223f88..5fcfcc8 100644

--- a/src/vdagent-x11-randr.c

+++ b/src/vdagent-x11-randr.c

@@ -688,8 +688,6 @@ void vdagent_x11_set_monitor_config(struct vdagent_x11 *x11,

                                     VDAgentMonitorsConfig *mon_config,

                                     int fallback)

 {

-    int width, height;

-    int x, y;

     int primary_w, primary_h;

     int i, real_num_of_monitors = 0;

     VDAgentMonitorsConfig *curr = NULL;

@@ -748,24 +746,38 @@ void vdagent_x11_set_monitor_config(struct vdagent_x11 *x11,

     for (i = mon_config->num_of_monitors; i < x11->randr.res->noutput; i++)

         xrandr_disable_output(x11, i);

+    /* First, disable disabled CRTCs... */

     for (i = 0; i < mon_config->num_of_monitors; ++i) {

         if (!monitor_enabled(&mon_config->monitors[i])) {

             xrandr_disable_output(x11, i);

-            continue;

         }

-        /* Try to create the requested resolution */

-        width = mon_config->monitors[i].width;

-        height = mon_config->monitors[i].height;

-        x = mon_config->monitors[i].x;

-        y = mon_config->monitors[i].y;

-        if (!xrandr_add_and_set(x11, i, x, y, width, height) &&

-                enabled_monitors(mon_config) == 1) {

-            set_screen_to_best_size(x11, width, height,

-                                    &primary_w, &primary_h);

-            goto update;

+    }

+

+    /* ... and disable the ones that would be bigger than

+     * the new RandR screen once it is resized. If they are enabled the

+     * XRRSetScreenSize call will fail with BadMatch. They will be

+     * reenabled after hanging the screen size.

+     */

+    for (i = 0; i < curr->num_of_monitors; ++i) {

+        int width, height;

+        int x, y;

+

+        width = curr->monitors[i].width;

+        height = curr->monitors[i].height;

+        x = curr->monitors[i].x;

+        y = curr->monitors[i].y;

+

+        if ((x + width > primary_w) || (y + height > primary_h)) {

+            if (x11->debug)

+                syslog(LOG_DEBUG, "Disabling monitor %d: "

+                       "%dx%d+%d+%d > (%d,%d)",

+                       i, width, height, x, y, primary_w, primary_h);

+

+            xrandr_disable_output(x11, i);

         }

     }

+    /* Then we can resize the RandR screen. */

     if (primary_w != x11->width[0] || primary_h != x11->height[0]) {

         if (x11->debug)

             syslog(LOG_DEBUG, "Changing screen size to %dx%d",

@@ -793,7 +805,28 @@ void vdagent_x11_set_monitor_config(struct vdagent_x11 *x11,

         }

     }

-update:

+    /* Finally, we set the new resolutions on RandR CRTCs now that the

+     * RandR screen is big enough to hold these.  */

+    for (i = 0; i < mon_config->num_of_monitors; ++i) {

+        int width, height;

+        int x, y;

+

+        if (!monitor_enabled(&mon_config->monitors[i])) {

+            continue;

+        }

+        /* Try to create the requested resolution */

+        width = mon_config->monitors[i].width;

+        height = mon_config->monitors[i].height;

+        x = mon_config->monitors[i].x;

+        y = mon_config->monitors[i].y;

+        if (!xrandr_add_and_set(x11, i, x, y, width, height) &&

+                enabled_monitors(mon_config) == 1) {

+            set_screen_to_best_size(x11, width, height,

+                                    &primary_w, &primary_h);

+            break;

+        }

+    }

+

     update_randr_res(x11,

         x11->randr.num_monitors != enabled_monitors(mon_config));

     x11->width[0] = primary_w;