[PULL 00/11] Ui 20210115 patches

[PULL 00/11] Ui 20210115 patches

[Gerd Hoffmann]

The following changes since commit 45240eed4f064576d589ea60ebadf3c11d7ab891:

  Merge remote-tracking branch 'remotes/armbru/tags/pull-yank-2021-01-13' int=
o staging (2021-01-13 14:19:24 +0000)

are available in the Git repository at:

  git://git.kraxel.org/qemu tags/ui-20210115-pull-request

for you to fetch changes up to 763deea7e906321f8ba048c359f168f60d51c14e:

  vnc: add support for extended desktop resize (2021-01-15 11:22:43 +0100)

----------------------------------------------------------------
ui/gtk: refresh rate fixes.
ui/vnc: add support for desktop resize and power contol.
ui/vnc: misc bugfixes.

----------------------------------------------------------------

Alex Chen (1):
  vnc: Fix a memleak in vnc_display_connect()

Daniel P. Berrang=C3=A9 (1):
  ui: add support for remote power control to VNC server

Gerd Hoffmann (3):
  vnc: move check into vnc_cursor_define
  vnc: move initialization to framebuffer_update_request
  vnc: add support for extended desktop resize

Nikola Pavlica (2):
  ui/gtk: expose gd_monitor_update_interval
  ui/gtk: update monitor interval on egl displays

Volker R=C3=BCmelin (3):
  ui/gtk: don't try to redefine SI prefixes
  ui/gtk: rename variable window to widget
  ui/gtk: limit virtual console max update interval

Zihao Chang (1):
  vnc: fix unfinalized tlscreds for VncDisplay

 include/ui/gtk.h |   3 +-
 ui/vnc.h         |  15 +++++
 ui/gtk-egl.c     |   3 +
 ui/gtk.c         |  25 ++++----
 ui/vnc.c         | 147 ++++++++++++++++++++++++++++++++++++++++++-----
 qemu-options.hx  |   4 ++
 6 files changed, 169 insertions(+), 28 deletions(-)

--=20
2.29.2

[PULL 01/11] ui/gtk: don't try to redefine SI prefixes

[Gerd Hoffmann]

From: Volker Rümelin <vr_qemu@t-online.de>

Redefining SI prefixes is always wrong. 1s has per definition
1000ms. Remove the misnamed named constant and replace it with
a comment explaining the frequency to period conversion in two
simple steps. Now you can cancel out the unit mHz in the comment
with the implicit unit mHz in refresh_rate_millihz and see why
the implicit unit ms for update_interval remains.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
Message-Id: <20201213165724.13418-1-vr_qemu@t-online.de>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/ui/gtk.h | 2 --
 ui/gtk.c         | 3 ++-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/include/ui/gtk.h b/include/ui/gtk.h
index eaeb450f913e..80851fb4c7e1 100644
--- a/include/ui/gtk.h
+++ b/include/ui/gtk.h
@@ -24,8 +24,6 @@
 #include "ui/egl-context.h"
 #endif
 
-#define MILLISEC_PER_SEC 1000000
-
 typedef struct GtkDisplayState GtkDisplayState;
 
 typedef struct VirtualGfxConsole {
diff --git a/ui/gtk.c b/ui/gtk.c
index e8474456df88..a83c8c3785e1 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -798,7 +798,8 @@ static gboolean gd_draw_event(GtkWidget *widget, cairo_t *cr, void *opaque)
     refresh_rate_millihz = gd_refresh_rate_millihz(vc->window ?
                                                    vc->window : s->window);
     if (refresh_rate_millihz) {
-        vc->gfx.dcl.update_interval = MILLISEC_PER_SEC / refresh_rate_millihz;
+        /* T = 1 / f = 1 [s*Hz] / f = 1000*1000 [ms*mHz] / f */
+        vc->gfx.dcl.update_interval = 1000 * 1000 / refresh_rate_millihz;
     }
 
     fbw = surface_width(vc->gfx.ds);
-- 
2.29.2

[PULL 02/11] ui/gtk: rename variable window to widget

[Gerd Hoffmann]

From: Volker Rümelin <vr_qemu@t-online.de>

The type of the variable window is GtkWidget. Rename the variable
from window to widget, because windows and widgets are different
things.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
Message-Id: <20201213165724.13418-2-vr_qemu@t-online.de>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/gtk.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index a83c8c3785e1..439c1e949fd9 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -752,13 +752,13 @@ static void gd_resize_event(GtkGLArea *area,
  * If available, return the refresh rate of the display in milli-Hertz,
  * else return 0.
  */
-static int gd_refresh_rate_millihz(GtkWidget *window)
+static int gd_refresh_rate_millihz(GtkWidget *widget)
 {
 #ifdef GDK_VERSION_3_22
-    GdkWindow *win = gtk_widget_get_window(window);
+    GdkWindow *win = gtk_widget_get_window(widget);
 
     if (win) {
-        GdkDisplay *dpy = gtk_widget_get_display(window);
+        GdkDisplay *dpy = gtk_widget_get_display(widget);
         GdkMonitor *monitor = gdk_display_get_monitor_at_window(dpy, win);
 
         return gdk_monitor_get_refresh_rate(monitor);
-- 
2.29.2

[PULL 03/11] ui/gtk: limit virtual console max update interval

[Gerd Hoffmann]

From: Volker Rümelin <vr_qemu@t-online.de>

Limit the virtual console maximum update interval to
GUI_REFRESH_INTERVAL_DEFAULT. This papers over a integer
overflow bug in gtk3 on Windows where the reported monitor
refresh frequency can be much smaller than the real refresh
frequency.

The gtk bug report can be found here:
https://gitlab.gnome.org/GNOME/gtk/-/issues/3394

On my Windows 10 system gtk reports a monitor refresh rate of
1.511Hz instead of 60.031Hz and slows down the screen update
rate in qemu to a crawl. Provided you are affected by the gtk
bug on Windows, these are the steps to reproduce the issue:

Start qemu with -display gtk and activate all qemu virtual
consoles and notice the reduced qemu refresh rate. Activating
all virtual consoles is necessary, because gui_update() in
ui/console.c uses the minimum of all display change listeners
update interval and not yet activated virtual consoles report
the default update interval (30ms).

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
Message-Id: <20201213165724.13418-3-vr_qemu@t-online.de>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/gtk.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index 439c1e949fd9..d2004a4dc162 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -749,10 +749,10 @@ static void gd_resize_event(GtkGLArea *area,
 #endif
 
 /*
- * If available, return the refresh rate of the display in milli-Hertz,
- * else return 0.
+ * If available, return the update interval of the monitor in ms,
+ * else return 0 (the default update interval).
  */
-static int gd_refresh_rate_millihz(GtkWidget *widget)
+static int gd_monitor_update_interval(GtkWidget *widget)
 {
 #ifdef GDK_VERSION_3_22
     GdkWindow *win = gtk_widget_get_window(widget);
@@ -760,8 +760,13 @@ static int gd_refresh_rate_millihz(GtkWidget *widget)
     if (win) {
         GdkDisplay *dpy = gtk_widget_get_display(widget);
         GdkMonitor *monitor = gdk_display_get_monitor_at_window(dpy, win);
+        int refresh_rate = gdk_monitor_get_refresh_rate(monitor); /* [mHz] */
 
-        return gdk_monitor_get_refresh_rate(monitor);
+        if (refresh_rate) {
+            /* T = 1 / f = 1 [s*Hz] / f = 1000*1000 [ms*mHz] / f */
+            return MIN(1000 * 1000 / refresh_rate,
+                       GUI_REFRESH_INTERVAL_DEFAULT);
+        }
     }
 #endif
     return 0;
@@ -774,7 +779,6 @@ static gboolean gd_draw_event(GtkWidget *widget, cairo_t *cr, void *opaque)
     int mx, my;
     int ww, wh;
     int fbw, fbh;
-    int refresh_rate_millihz;
 
 #if defined(CONFIG_OPENGL)
     if (vc->gfx.gls) {
@@ -795,12 +799,8 @@ static gboolean gd_draw_event(GtkWidget *widget, cairo_t *cr, void *opaque)
         return FALSE;
     }
 
-    refresh_rate_millihz = gd_refresh_rate_millihz(vc->window ?
-                                                   vc->window : s->window);
-    if (refresh_rate_millihz) {
-        /* T = 1 / f = 1 [s*Hz] / f = 1000*1000 [ms*mHz] / f */
-        vc->gfx.dcl.update_interval = 1000 * 1000 / refresh_rate_millihz;
-    }
+    vc->gfx.dcl.update_interval =
+        gd_monitor_update_interval(vc->window ? vc->window : s->window);
 
     fbw = surface_width(vc->gfx.ds);
     fbh = surface_height(vc->gfx.ds);
-- 
2.29.2

[PULL 04/11] ui/gtk: expose gd_monitor_update_interval

[Gerd Hoffmann]

From: Nikola Pavlica <pavlica.nikola@gmail.com>

The gd_egl_refresh function, as the name suggests, is responsible for
refreshing displays when using EGL graphics with QEMU's GTK UI. This is
a perfect candidate for a function to update the refresh rate in.

Since gd_monitor_update_interval is inaccessible from the gd_egl_refresh
function, we need to expose/globalize it in the include/ui/gtk.h file.

Signed-off-by: Nikola Pavlica <pavlica.nikola@gmail.com>
Message-Id: <20210114140153.301473-2-pavlica.nikola@gmail.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/ui/gtk.h | 1 +
 ui/gtk.c         | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/ui/gtk.h b/include/ui/gtk.h
index 80851fb4c7e1..3f395d7f943b 100644
--- a/include/ui/gtk.h
+++ b/include/ui/gtk.h
@@ -86,6 +86,7 @@ extern bool gtk_use_gl_area;
 
 /* ui/gtk.c */
 void gd_update_windowsize(VirtualConsole *vc);
+int gd_monitor_update_interval(GtkWidget *widget);
 
 /* ui/gtk-egl.c */
 void gd_egl_init(VirtualConsole *vc);
diff --git a/ui/gtk.c b/ui/gtk.c
index d2004a4dc162..26665cd2e657 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -752,7 +752,7 @@ static void gd_resize_event(GtkGLArea *area,
  * If available, return the update interval of the monitor in ms,
  * else return 0 (the default update interval).
  */
-static int gd_monitor_update_interval(GtkWidget *widget)
+int gd_monitor_update_interval(GtkWidget *widget)
 {
 #ifdef GDK_VERSION_3_22
     GdkWindow *win = gtk_widget_get_window(widget);
-- 
2.29.2

[PULL 05/11] ui/gtk: update monitor interval on egl displays

[Gerd Hoffmann]

From: Nikola Pavlica <pavlica.nikola@gmail.com>

When running QEMU's GTK UI without EGL or OGL, the
gd_monitor_update_interval function gets executed and the display refresh
rate gets updated accordingly. However, when using EGL or just regular
OGL, the function never gets executed.

Which is why I decided that the function should be in gd_egl_refresh
where the display output gets updated, in the same vain as how it's done
for normal GTK UIs (aka. those without EGL) - in it's display refresh
function.

Since the gd_monitor_update_interval function now is exposed, we are
going to use it to update the refresh rate.

Signed-off-by: Nikola Pavlica <pavlica.nikola@gmail.com>
Message-Id: <20210114140153.301473-3-pavlica.nikola@gmail.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/gtk-egl.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
index 99231a3597f5..71c3d698b400 100644
--- a/ui/gtk-egl.c
+++ b/ui/gtk-egl.c
@@ -113,6 +113,9 @@ void gd_egl_refresh(DisplayChangeListener *dcl)
 {
     VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
 
+    vc->gfx.dcl.update_interval = gd_monitor_update_interval(
+            vc->window ? vc->window : vc->gfx.drawing_area);
+
     if (!vc->gfx.esurface) {
         gd_egl_init(vc);
         if (!vc->gfx.esurface) {
-- 
2.29.2

[PULL 06/11] vnc: fix unfinalized tlscreds for VncDisplay

[Gerd Hoffmann]

From: Zihao Chang <changzihao1@huawei.com>

In vnc_display_open(), if tls-creds is enabled, do object_ref(object
ref 1->2) for tls-creds. While in vnc_display_close(), object_unparent
sets object ref to 1(2->1) and  unparent the object for root.
Problem:
1. the object can not be found from the objects_root, while the object
is not finalized.
2. the qemu_opts of tls-creds(id: creds0) is not deleted, so new tls
object with the same id(creds0) can not be delete & add.

Signed-off-by: Zihao Chang <changzihao1@huawei.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20210111131911.805-1-changzihao1@huawei.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/vnc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 7452ac7df2ce..69e92b1ef361 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3234,7 +3234,7 @@ static void vnc_display_close(VncDisplay *vd)
     vd->auth = VNC_AUTH_INVALID;
     vd->subauth = VNC_AUTH_INVALID;
     if (vd->tlscreds) {
-        object_unparent(OBJECT(vd->tlscreds));
+        object_unref(OBJECT(vd->tlscreds));
         vd->tlscreds = NULL;
     }
     if (vd->tlsauthz) {
-- 
2.29.2

[PULL 07/11] ui: add support for remote power control to VNC server

[Gerd Hoffmann]

From: Daniel P. Berrangé <berrange@redhat.com>

The "XVP" (Xen VNC Proxy) extension defines a mechanism for a VNC client
to issue power control requests to trigger graceful shutdown, reboot, or
hard reset.

This option is not enabled by default, since we cannot assume that users
with VNC access implicitly have administrator access to the guest OS.

Thus is it enabled with a boolean "power-control" option e.g.

   -vnc :1,power-control=on

While, QEMU can easily support shutdown and reset, there's no easy way
to wire up reboot support at this time. In theory it could be done by
issuing a shutdown, followed by a reset, but there's no convenient
wiring for such a pairing in QEMU. It also isn't possible to have the
VNC server directly talk to QEMU guest agent, since the agent chardev is
typically owned by an external mgmt app.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

[ kraxel: rebase to master  ]
[ kraxel: add missing break ]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/vnc.h        | 13 +++++++++++
 ui/vnc.c        | 59 +++++++++++++++++++++++++++++++++++++++++++++++++
 qemu-options.hx |  4 ++++
 3 files changed, 76 insertions(+)

diff --git a/ui/vnc.h b/ui/vnc.h
index c8d3ad9ec496..5feeef9df08c 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -176,6 +176,7 @@ struct VncDisplay
     int ws_subauth; /* Used by websockets */
     bool lossy;
     bool non_adaptive;
+    bool power_control;
     QCryptoTLSCreds *tlscreds;
     QAuthZ *tlsauthz;
     char *tlsauthzid;
@@ -412,6 +413,7 @@ enum {
 #define VNC_ENCODING_TIGHT_PNG            0xFFFFFEFC /* -260 */
 #define VNC_ENCODING_LED_STATE            0XFFFFFEFB /* -261 */
 #define VNC_ENCODING_DESKTOP_RESIZE_EXT   0XFFFFFECC /* -308 */
+#define VNC_ENCODING_XVP                  0XFFFFFECB /* -309 */
 #define VNC_ENCODING_ALPHA_CURSOR         0XFFFFFEC6 /* -314 */
 #define VNC_ENCODING_WMVi                 0x574D5669
 
@@ -453,6 +455,7 @@ enum VncFeatures {
     VNC_FEATURE_ZRLE,
     VNC_FEATURE_ZYWRLE,
     VNC_FEATURE_LED_STATE,
+    VNC_FEATURE_XVP,
 };
 
 #define VNC_FEATURE_RESIZE_MASK              (1 << VNC_FEATURE_RESIZE)
@@ -467,6 +470,7 @@ enum VncFeatures {
 #define VNC_FEATURE_ZRLE_MASK                (1 << VNC_FEATURE_ZRLE)
 #define VNC_FEATURE_ZYWRLE_MASK              (1 << VNC_FEATURE_ZYWRLE)
 #define VNC_FEATURE_LED_STATE_MASK           (1 << VNC_FEATURE_LED_STATE)
+#define VNC_FEATURE_XVP_MASK                 (1 << VNC_FEATURE_XVP)
 
 
 /* Client -> Server message IDs */
@@ -519,6 +523,15 @@ enum VncFeatures {
 #define VNC_MSG_SERVER_QEMU_AUDIO_BEGIN           1
 #define VNC_MSG_SERVER_QEMU_AUDIO_DATA            2
 
+/* XVP server -> client status code */
+#define VNC_XVP_CODE_FAIL 0
+#define VNC_XVP_CODE_INIT 1
+
+/* XVP client -> server action request  */
+#define VNC_XVP_ACTION_SHUTDOWN 2
+#define VNC_XVP_ACTION_REBOOT 3
+#define VNC_XVP_ACTION_RESET 4
+
 
 /*****************************************************************************
  *
diff --git a/ui/vnc.c b/ui/vnc.c
index 69e92b1ef361..a0bf750767a2 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -30,6 +30,7 @@
 #include "trace.h"
 #include "hw/qdev-core.h"
 #include "sysemu/sysemu.h"
+#include "sysemu/runstate.h"
 #include "qemu/error-report.h"
 #include "qemu/main-loop.h"
 #include "qemu/module.h"
@@ -2042,6 +2043,17 @@ static void send_ext_audio_ack(VncState *vs)
     vnc_flush(vs);
 }
 
+static void send_xvp_message(VncState *vs, int code)
+{
+    vnc_lock_output(vs);
+    vnc_write_u8(vs, VNC_MSG_SERVER_XVP);
+    vnc_write_u8(vs, 0); /* pad */
+    vnc_write_u8(vs, 1); /* version */
+    vnc_write_u8(vs, code);
+    vnc_unlock_output(vs);
+    vnc_flush(vs);
+}
+
 static void set_encodings(VncState *vs, int32_t *encodings, size_t n_encodings)
 {
     int i;
@@ -2121,6 +2133,12 @@ static void set_encodings(VncState *vs, int32_t *encodings, size_t n_encodings)
         case VNC_ENCODING_LED_STATE:
             vs->features |= VNC_FEATURE_LED_STATE_MASK;
             break;
+        case VNC_ENCODING_XVP:
+            if (vs->vd->power_control) {
+                vs->features |= VNC_FEATURE_XVP;
+                send_xvp_message(vs, VNC_XVP_CODE_INIT);
+            }
+            break;
         case VNC_ENCODING_COMPRESSLEVEL0 ... VNC_ENCODING_COMPRESSLEVEL0 + 9:
             vs->tight->compression = (enc & 0x0F);
             break;
@@ -2353,6 +2371,42 @@ static int protocol_client_msg(VncState *vs, uint8_t *data, size_t len)
 
         client_cut_text(vs, read_u32(data, 4), data + 8);
         break;
+    case VNC_MSG_CLIENT_XVP:
+        if (!(vs->features & VNC_FEATURE_XVP)) {
+            error_report("vnc: xvp client message while disabled");
+            vnc_client_error(vs);
+            break;
+        }
+        if (len == 1) {
+            return 4;
+        }
+        if (len == 4) {
+            uint8_t version = read_u8(data, 2);
+            uint8_t action = read_u8(data, 3);
+
+            if (version != 1) {
+                error_report("vnc: xvp client message version %d != 1",
+                             version);
+                vnc_client_error(vs);
+                break;
+            }
+
+            switch (action) {
+            case VNC_XVP_ACTION_SHUTDOWN:
+                qemu_system_powerdown_request();
+                break;
+            case VNC_XVP_ACTION_REBOOT:
+                send_xvp_message(vs, VNC_XVP_CODE_FAIL);
+                break;
+            case VNC_XVP_ACTION_RESET:
+                qemu_system_reset_request(SHUTDOWN_CAUSE_HOST_QMP_SYSTEM_RESET);
+                break;
+            default:
+                send_xvp_message(vs, VNC_XVP_CODE_FAIL);
+                break;
+            }
+        }
+        break;
     case VNC_MSG_CLIENT_QEMU:
         if (len == 1)
             return 2;
@@ -3379,6 +3433,9 @@ static QemuOptsList qemu_vnc_opts = {
         },{
             .name = "audiodev",
             .type = QEMU_OPT_STRING,
+        },{
+            .name = "power-control",
+            .type = QEMU_OPT_BOOL,
         },
         { /* end of list */ }
     },
@@ -3942,6 +3999,8 @@ void vnc_display_open(const char *id, Error **errp)
         vd->non_adaptive = true;
     }
 
+    vd->power_control = qemu_opt_get_bool(opts, "power-control", false);
+
     if (tlsauthz) {
         vd->tlsauthzid = g_strdup(tlsauthz);
     } else if (acl) {
diff --git a/qemu-options.hx b/qemu-options.hx
index 1698a0c751ff..05fe35ceb6f8 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2222,6 +2222,10 @@ SRST
         transmission. When not using an -audiodev argument, this option
         must be omitted, otherwise is must be present and specify a
         valid audiodev.
+
+    ``power-control``
+        Permit the remote client to issue shutdown, reboot or reset power
+        control requests.
 ERST
 
 ARCHHEADING(, QEMU_ARCH_I386)
-- 
2.29.2

[PULL 08/11] vnc: Fix a memleak in vnc_display_connect()

[Gerd Hoffmann]

From: Alex Chen <alex.chen@huawei.com>

Free the 'sioc' when the qio_channel_socket_connect_sync() fails.

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Alex Chen <alex.chen@huawei.com>
Reviewed-by: Li Qiang <liq3ea@gmail.com>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Message-Id: <20201126065702.35095-1-alex.chen@huawei.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/vnc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ui/vnc.c b/ui/vnc.c
index a0bf750767a2..d4854d351bac 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3805,6 +3805,7 @@ static int vnc_display_connect(VncDisplay *vd,
     sioc = qio_channel_socket_new();
     qio_channel_set_name(QIO_CHANNEL(sioc), "vnc-reverse");
     if (qio_channel_socket_connect_sync(sioc, saddr[0], errp) < 0) {
+        object_unref(OBJECT(sioc));
         return -1;
     }
     vnc_connect(vd, sioc, false, false);
-- 
2.29.2

[PULL 09/11] vnc: move check into vnc_cursor_define

[Gerd Hoffmann]

Move the check whenever a cursor exists into the vnc_cursor_define()
function so callers don't have to do it.

Suggested-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-id: 20210112134120.2031837-2-kraxel@redhat.com
---
 ui/vnc.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index d4854d351bac..0f01481cac57 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -793,9 +793,7 @@ static void vnc_dpy_switch(DisplayChangeListener *dcl,
     QTAILQ_FOREACH(vs, &vd->clients, next) {
         vnc_colordepth(vs);
         vnc_desktop_resize(vs);
-        if (vs->vd->cursor) {
-            vnc_cursor_define(vs);
-        }
+        vnc_cursor_define(vs);
         memset(vs->dirty, 0x00, sizeof(vs->dirty));
         vnc_set_area_dirty(vs->dirty, vd, 0, 0,
                            vnc_width(vd),
@@ -929,6 +927,10 @@ static int vnc_cursor_define(VncState *vs)
     QEMUCursor *c = vs->vd->cursor;
     int isize;
 
+    if (!vs->vd->cursor) {
+        return -1;
+    }
+
     if (vnc_has_feature(vs, VNC_FEATURE_ALPHA_CURSOR)) {
         vnc_lock_output(vs);
         vnc_write_u8(vs,  VNC_MSG_SERVER_FRAMEBUFFER_UPDATE);
@@ -2155,9 +2157,7 @@ static void set_encodings(VncState *vs, int32_t *encodings, size_t n_encodings)
     vnc_desktop_resize(vs);
     check_pointer_type_change(&vs->mouse_mode_notifier, NULL);
     vnc_led_state_change(vs);
-    if (vs->vd->cursor) {
-        vnc_cursor_define(vs);
-    }
+    vnc_cursor_define(vs);
 }
 
 static void set_pixel_conversion(VncState *vs)
-- 
2.29.2

[PULL 10/11] vnc: move initialization to framebuffer_update_request

[Gerd Hoffmann]

qemu sends various state info like current cursor shape to newly connected
clients in response to a set_encoding message.  This is not correct according
to the rfb spec.  Send that information in response to a full (incremental=0)
framebuffer update request instead.  Also send the resize information
unconditionally, not only in case of an actual server-side change.

This makes the qemu vnc server conform to the spec and allows clients to
request the complete vnc server state without reconnect.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-id: 20210112134120.2031837-3-kraxel@redhat.com
---
 ui/vnc.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 0f01481cac57..b4e98cf647f5 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -661,10 +661,6 @@ static void vnc_desktop_resize(VncState *vs)
     if (vs->ioc == NULL || !vnc_has_feature(vs, VNC_FEATURE_RESIZE)) {
         return;
     }
-    if (vs->client_width == pixman_image_get_width(vs->vd->server) &&
-        vs->client_height == pixman_image_get_height(vs->vd->server)) {
-        return;
-    }
 
     assert(pixman_image_get_width(vs->vd->server) < 65536 &&
            pixman_image_get_width(vs->vd->server) >= 0);
@@ -2014,6 +2010,10 @@ static void framebuffer_update_request(VncState *vs, int incremental,
     } else {
         vs->update = VNC_STATE_UPDATE_FORCE;
         vnc_set_area_dirty(vs->dirty, vs->vd, x, y, w, h);
+        vnc_colordepth(vs);
+        vnc_desktop_resize(vs);
+        vnc_led_state_change(vs);
+        vnc_cursor_define(vs);
     }
 }
 
@@ -2154,10 +2154,7 @@ static void set_encodings(VncState *vs, int32_t *encodings, size_t n_encodings)
             break;
         }
     }
-    vnc_desktop_resize(vs);
     check_pointer_type_change(&vs->mouse_mode_notifier, NULL);
-    vnc_led_state_change(vs);
-    vnc_cursor_define(vs);
 }
 
 static void set_pixel_conversion(VncState *vs)
-- 
2.29.2

[PULL 11/11] vnc: add support for extended desktop resize

[Gerd Hoffmann]

The extended desktop resize encoding adds support for (a) clients
sending resize requests to the server, and (b) multihead support.

This patch implements (a).  All resize requests are rejected by qemu.
Qemu can't resize the framebuffer on its own, this is in the hands of
the guest, so all qemu can do is forward the request to the guest.
Should the guest actually resize the framebuffer we can notify the vnc
client later with a separate message.

This requires support in the display device.  Works with virtio-gpu.

https://github.com/rfbproto/rfbproto/blob/master/rfbproto.rst#extendeddesktopsize-pseudo-encoding

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-id: 20210112134120.2031837-4-kraxel@redhat.com
---
 ui/vnc.h |  2 ++
 ui/vnc.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/ui/vnc.h b/ui/vnc.h
index 5feeef9df08c..116463d5f099 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -444,6 +444,7 @@ enum {
 
 enum VncFeatures {
     VNC_FEATURE_RESIZE,
+    VNC_FEATURE_RESIZE_EXT,
     VNC_FEATURE_HEXTILE,
     VNC_FEATURE_POINTER_TYPE_CHANGE,
     VNC_FEATURE_WMVI,
@@ -459,6 +460,7 @@ enum VncFeatures {
 };
 
 #define VNC_FEATURE_RESIZE_MASK              (1 << VNC_FEATURE_RESIZE)
+#define VNC_FEATURE_RESIZE_EXT_MASK          (1 << VNC_FEATURE_RESIZE_EXT)
 #define VNC_FEATURE_HEXTILE_MASK             (1 << VNC_FEATURE_HEXTILE)
 #define VNC_FEATURE_POINTER_TYPE_CHANGE_MASK (1 << VNC_FEATURE_POINTER_TYPE_CHANGE)
 #define VNC_FEATURE_WMVI_MASK                (1 << VNC_FEATURE_WMVI)
diff --git a/ui/vnc.c b/ui/vnc.c
index b4e98cf647f5..d429bfee5a65 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -655,10 +655,35 @@ void vnc_framebuffer_update(VncState *vs, int x, int y, int w, int h,
     vnc_write_s32(vs, encoding);
 }
 
+static void vnc_desktop_resize_ext(VncState *vs, int reject_reason)
+{
+    vnc_lock_output(vs);
+    vnc_write_u8(vs, VNC_MSG_SERVER_FRAMEBUFFER_UPDATE);
+    vnc_write_u8(vs, 0);
+    vnc_write_u16(vs, 1); /* number of rects */
+    vnc_framebuffer_update(vs,
+                           reject_reason ? 1 : 0,
+                           reject_reason,
+                           vs->client_width, vs->client_height,
+                           VNC_ENCODING_DESKTOP_RESIZE_EXT);
+    vnc_write_u8(vs, 1);  /* number of screens */
+    vnc_write_u8(vs, 0);  /* padding */
+    vnc_write_u8(vs, 0);  /* padding */
+    vnc_write_u8(vs, 0);  /* padding */
+    vnc_write_u32(vs, 0); /* screen id */
+    vnc_write_u16(vs, 0); /* screen x-pos */
+    vnc_write_u16(vs, 0); /* screen y-pos */
+    vnc_write_u16(vs, vs->client_width);
+    vnc_write_u16(vs, vs->client_height);
+    vnc_write_u32(vs, 0); /* screen flags */
+    vnc_unlock_output(vs);
+    vnc_flush(vs);
+}
 
 static void vnc_desktop_resize(VncState *vs)
 {
-    if (vs->ioc == NULL || !vnc_has_feature(vs, VNC_FEATURE_RESIZE)) {
+    if (vs->ioc == NULL || (!vnc_has_feature(vs, VNC_FEATURE_RESIZE) &&
+                            !vnc_has_feature(vs, VNC_FEATURE_RESIZE_EXT))) {
         return;
     }
 
@@ -668,6 +693,12 @@ static void vnc_desktop_resize(VncState *vs)
            pixman_image_get_height(vs->vd->server) >= 0);
     vs->client_width = pixman_image_get_width(vs->vd->server);
     vs->client_height = pixman_image_get_height(vs->vd->server);
+
+    if (vnc_has_feature(vs, VNC_FEATURE_RESIZE_EXT)) {
+        vnc_desktop_resize_ext(vs, 0);
+        return;
+    }
+
     vnc_lock_output(vs);
     vnc_write_u8(vs, VNC_MSG_SERVER_FRAMEBUFFER_UPDATE);
     vnc_write_u8(vs, 0);
@@ -2114,6 +2145,9 @@ static void set_encodings(VncState *vs, int32_t *encodings, size_t n_encodings)
         case VNC_ENCODING_DESKTOPRESIZE:
             vs->features |= VNC_FEATURE_RESIZE_MASK;
             break;
+        case VNC_ENCODING_DESKTOP_RESIZE_EXT:
+            vs->features |= VNC_FEATURE_RESIZE_EXT_MASK;
+            break;
         case VNC_ENCODING_POINTER_TYPE_CHANGE:
             vs->features |= VNC_FEATURE_POINTER_TYPE_CHANGE_MASK;
             break;
@@ -2474,6 +2508,34 @@ static int protocol_client_msg(VncState *vs, uint8_t *data, size_t len)
             break;
         }
         break;
+    case VNC_MSG_CLIENT_SET_DESKTOP_SIZE:
+    {
+        size_t size;
+        uint8_t screens;
+
+        if (len < 8) {
+            return 8;
+        }
+
+        screens = read_u8(data, 6);
+        size    = 8 + screens * 16;
+        if (len < size) {
+            return size;
+        }
+
+        if (dpy_ui_info_supported(vs->vd->dcl.con)) {
+            QemuUIInfo info;
+            memset(&info, 0, sizeof(info));
+            info.width = read_u16(data, 2);
+            info.height = read_u16(data, 4);
+            dpy_set_ui_info(vs->vd->dcl.con, &info);
+            vnc_desktop_resize_ext(vs, 4 /* Request forwarded */);
+        } else {
+            vnc_desktop_resize_ext(vs, 3 /* Invalid screen layout */);
+        }
+
+        break;
+    }
     default:
         VNC_DEBUG("Msg: %d\n", data[0]);
         vnc_client_error(vs);
-- 
2.29.2

Re: [PULL 00/11] Ui 20210115 patches

[Peter Maydell]

On Fri, 15 Jan 2021 at 10:28, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> The following changes since commit 45240eed4f064576d589ea60ebadf3c11d7ab891:
>
>   Merge remote-tracking branch 'remotes/armbru/tags/pull-yank-2021-01-13' int=
> o staging (2021-01-13 14:19:24 +0000)
>
> are available in the Git repository at:
>
>   git://git.kraxel.org/qemu tags/ui-20210115-pull-request
>
> for you to fetch changes up to 763deea7e906321f8ba048c359f168f60d51c14e:
>
>   vnc: add support for extended desktop resize (2021-01-15 11:22:43 +0100)
>
> ----------------------------------------------------------------
> ui/gtk: refresh rate fixes.
> ui/vnc: add support for desktop resize and power contol.
> ui/vnc: misc bugfixes.


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/6.0
for any user-visible changes.

-- PMM

Re: [PULL 10/11] vnc: move initialization to framebuffer_update_request

[Laszlo Ersek]

On 01/15/21 11:24, Gerd Hoffmann wrote:
> qemu sends various state info like current cursor shape to newly connected
> clients in response to a set_encoding message.  This is not correct according
> to the rfb spec.  Send that information in response to a full (incremental=0)
> framebuffer update request instead.  Also send the resize information
> unconditionally, not only in case of an actual server-side change.
>
> This makes the qemu vnc server conform to the spec and allows clients to
> request the complete vnc server state without reconnect.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Message-id: 20210112134120.2031837-3-kraxel@redhat.com
> ---
>  ui/vnc.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)

This patch breaks QEMU for me.

Bisection log below.

(I started the bisection from commit facf7c60ee60 ("vl: initialize
displays _after_ exiting preconfiguration", 2021-01-02), which is the
fix for the previous display regression. I noticed the regression after
pulling master today, at fef80ea073c4.)

> git bisect start
> # good: [facf7c60ee60aab7d73b204ee8c86b90fbc6b3db] vl: initialize displays _after_ exiting preconfiguration
> git bisect good facf7c60ee60aab7d73b204ee8c86b90fbc6b3db
> # bad: [fef80ea073c4862bc9eaddb6ddb0ed970b8ad7c4] Merge remote-tracking branch 'remotes/ericb/tags/pull-nbd-2021-01-20' into staging
> git bisect bad fef80ea073c4862bc9eaddb6ddb0ed970b8ad7c4
> # good: [88d4005b098427638d7551aa04ebde4fdd06835b] tcg: Use tcg_constant_{i32,i64,vec} with gvec expanders
> git bisect good 88d4005b098427638d7551aa04ebde4fdd06835b
> # bad: [1eaada8ae15f10f7a7f1e2505bd77dbb11a8be85] hw/riscv: sifive_u: Use SIFIVE_U_CPU for mc->default_cpu_type
> git bisect bad 1eaada8ae15f10f7a7f1e2505bd77dbb11a8be85
> # good: [c7a9ef75173f090616328d6870f71e8da2b6bd50] target/mips: Introduce decode tree bindings for MSA ASE
> git bisect good c7a9ef75173f090616328d6870f71e8da2b6bd50
> # bad: [7cb6b97300f0405b4c6856c49bdc33fa3265852f] Merge remote-tracking branch 'remotes/kraxel/tags/ui-20210115-pull-request' into staging
> git bisect bad 7cb6b97300f0405b4c6856c49bdc33fa3265852f
> # good: [eaca85763bcd94ddac3fa11f8cc20e974dc11102] target/mips: Remove vendor specific CPU definitions
> git bisect good eaca85763bcd94ddac3fa11f8cc20e974dc11102
> # good: [5f8679fe46d78acfa5fc43a3fd6b3fe95525d9bd] vnc: Fix a memleak in vnc_display_connect()
> git bisect good 5f8679fe46d78acfa5fc43a3fd6b3fe95525d9bd
> # good: [a968a38005bf2568605cac7f86b9fba7fc089726] Merge remote-tracking branch 'remotes/gkurz-gitlab/tags/9p-next-2021-01-15' into staging
> git bisect good a968a38005bf2568605cac7f86b9fba7fc089726
> # bad: [9e1632ad07ca49de99da4bb231e9e2f22f2d8df5] vnc: move initialization to framebuffer_update_request
> git bisect bad 9e1632ad07ca49de99da4bb231e9e2f22f2d8df5
> # good: [b3c2de9cd5bc0023901e7a4d568dfc5152b6cc4a] vnc: move check into vnc_cursor_define
> git bisect good b3c2de9cd5bc0023901e7a4d568dfc5152b6cc4a
> # first bad commit: [9e1632ad07ca49de99da4bb231e9e2f22f2d8df5] vnc: move initialization to framebuffer_update_request

The symptom is the following: in virt-manager, the display remains dead
(black), when I start an OVMF guest. At the same time, unusually high
CPU load can be seen on the host; it makes me think that virt-manager is
trying, in a busy loop, to complete the VNC handshake, or some such.
Ultimately, although the guest boots up fine, virt-manager gives up, and
the display window says "Viewer was disconnected".

Versions (apart from qemu):

- gtk-vnc2-0.7.0-3.el7.x86_64
- gvnc-0.7.0-3.el7.x86_64
- libvirt-daemon-6.6.0-8 (rebuilt from RHEL8 to RHEL7)
- spice-gtk3-0.35-5.el7_9.1.x86_64 (in case it matters...)
- virt-manager-1.5.0-7.el7.noarch

The domain config is:

>     <graphics type='vnc' port='-1' autoport='yes'>
>       <listen type='address'/>
>     </graphics>
>     <video>
>       <model type='qxl' ram='65536' vram='65536' vgamem='16384' heads='1' primary='yes'/>
>       <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
>     </video>

Reverting

- 763deea7e906 ("vnc: add support for extended desktop resize",
  2021-01-15), and

- 9e1632ad07ca ("vnc: move initialization to
  framebuffer_update_request", 2021-01-15),

in this order, returns QEMU to working state.

Thanks
Laszlo

>
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 0f01481cac57..b4e98cf647f5 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -661,10 +661,6 @@ static void vnc_desktop_resize(VncState *vs)
>      if (vs->ioc == NULL || !vnc_has_feature(vs, VNC_FEATURE_RESIZE)) {
>          return;
>      }
> -    if (vs->client_width == pixman_image_get_width(vs->vd->server) &&
> -        vs->client_height == pixman_image_get_height(vs->vd->server)) {
> -        return;
> -    }
>
>      assert(pixman_image_get_width(vs->vd->server) < 65536 &&
>             pixman_image_get_width(vs->vd->server) >= 0);
> @@ -2014,6 +2010,10 @@ static void framebuffer_update_request(VncState *vs, int incremental,
>      } else {
>          vs->update = VNC_STATE_UPDATE_FORCE;
>          vnc_set_area_dirty(vs->dirty, vs->vd, x, y, w, h);
> +        vnc_colordepth(vs);
> +        vnc_desktop_resize(vs);
> +        vnc_led_state_change(vs);
> +        vnc_cursor_define(vs);
>      }
>  }
>
> @@ -2154,10 +2154,7 @@ static void set_encodings(VncState *vs, int32_t *encodings, size_t n_encodings)
>              break;
>          }
>      }
> -    vnc_desktop_resize(vs);
>      check_pointer_type_change(&vs->mouse_mode_notifier, NULL);
> -    vnc_led_state_change(vs);
> -    vnc_cursor_define(vs);
>  }
>
>  static void set_pixel_conversion(VncState *vs)
>

Re: [PULL 10/11] vnc: move initialization to framebuffer_update_request

[Laszlo Ersek]

On 01/21/21 22:22, Laszlo Ersek wrote:
> On 01/15/21 11:24, Gerd Hoffmann wrote:
>> qemu sends various state info like current cursor shape to newly connected
>> clients in response to a set_encoding message.  This is not correct according
>> to the rfb spec.  Send that information in response to a full (incremental=0)
>> framebuffer update request instead.  Also send the resize information
>> unconditionally, not only in case of an actual server-side change.
>>
>> This makes the qemu vnc server conform to the spec and allows clients to
>> request the complete vnc server state without reconnect.
>>
>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>> Message-id: 20210112134120.2031837-3-kraxel@redhat.com
>> ---
>>  ui/vnc.c | 11 ++++-------
>>  1 file changed, 4 insertions(+), 7 deletions(-)
>
> This patch breaks QEMU for me.
>
> Bisection log below.
>
> (I started the bisection from commit facf7c60ee60 ("vl: initialize
> displays _after_ exiting preconfiguration", 2021-01-02), which is the
> fix for the previous display regression. I noticed the regression after
> pulling master today, at fef80ea073c4.)
>
>> git bisect start
>> # good: [facf7c60ee60aab7d73b204ee8c86b90fbc6b3db] vl: initialize displays _after_ exiting preconfiguration
>> git bisect good facf7c60ee60aab7d73b204ee8c86b90fbc6b3db
>> # bad: [fef80ea073c4862bc9eaddb6ddb0ed970b8ad7c4] Merge remote-tracking branch 'remotes/ericb/tags/pull-nbd-2021-01-20' into staging
>> git bisect bad fef80ea073c4862bc9eaddb6ddb0ed970b8ad7c4
>> # good: [88d4005b098427638d7551aa04ebde4fdd06835b] tcg: Use tcg_constant_{i32,i64,vec} with gvec expanders
>> git bisect good 88d4005b098427638d7551aa04ebde4fdd06835b
>> # bad: [1eaada8ae15f10f7a7f1e2505bd77dbb11a8be85] hw/riscv: sifive_u: Use SIFIVE_U_CPU for mc->default_cpu_type
>> git bisect bad 1eaada8ae15f10f7a7f1e2505bd77dbb11a8be85
>> # good: [c7a9ef75173f090616328d6870f71e8da2b6bd50] target/mips: Introduce decode tree bindings for MSA ASE
>> git bisect good c7a9ef75173f090616328d6870f71e8da2b6bd50
>> # bad: [7cb6b97300f0405b4c6856c49bdc33fa3265852f] Merge remote-tracking branch 'remotes/kraxel/tags/ui-20210115-pull-request' into staging
>> git bisect bad 7cb6b97300f0405b4c6856c49bdc33fa3265852f
>> # good: [eaca85763bcd94ddac3fa11f8cc20e974dc11102] target/mips: Remove vendor specific CPU definitions
>> git bisect good eaca85763bcd94ddac3fa11f8cc20e974dc11102
>> # good: [5f8679fe46d78acfa5fc43a3fd6b3fe95525d9bd] vnc: Fix a memleak in vnc_display_connect()
>> git bisect good 5f8679fe46d78acfa5fc43a3fd6b3fe95525d9bd
>> # good: [a968a38005bf2568605cac7f86b9fba7fc089726] Merge remote-tracking branch 'remotes/gkurz-gitlab/tags/9p-next-2021-01-15' into staging
>> git bisect good a968a38005bf2568605cac7f86b9fba7fc089726
>> # bad: [9e1632ad07ca49de99da4bb231e9e2f22f2d8df5] vnc: move initialization to framebuffer_update_request
>> git bisect bad 9e1632ad07ca49de99da4bb231e9e2f22f2d8df5
>> # good: [b3c2de9cd5bc0023901e7a4d568dfc5152b6cc4a] vnc: move check into vnc_cursor_define
>> git bisect good b3c2de9cd5bc0023901e7a4d568dfc5152b6cc4a
>> # first bad commit: [9e1632ad07ca49de99da4bb231e9e2f22f2d8df5] vnc: move initialization to framebuffer_update_request
>
> The symptom is the following: in virt-manager, the display remains dead
> (black), when I start an OVMF guest. At the same time, unusually high
> CPU load can be seen on the host; it makes me think that virt-manager is
> trying, in a busy loop, to complete the VNC handshake, or some such.
> Ultimately, although the guest boots up fine, virt-manager gives up, and
> the display window says "Viewer was disconnected".
>
> Versions (apart from qemu):
>
> - gtk-vnc2-0.7.0-3.el7.x86_64
> - gvnc-0.7.0-3.el7.x86_64
> - libvirt-daemon-6.6.0-8 (rebuilt from RHEL8 to RHEL7)
> - spice-gtk3-0.35-5.el7_9.1.x86_64 (in case it matters...)
> - virt-manager-1.5.0-7.el7.noarch
>
> The domain config is:
>
>>     <graphics type='vnc' port='-1' autoport='yes'>
>>       <listen type='address'/>
>>     </graphics>
>>     <video>
>>       <model type='qxl' ram='65536' vram='65536' vgamem='16384' heads='1' primary='yes'/>
>>       <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
>>     </video>
>
> Reverting
>
> - 763deea7e906 ("vnc: add support for extended desktop resize",
>   2021-01-15), and
>
> - 9e1632ad07ca ("vnc: move initialization to
>   framebuffer_update_request", 2021-01-15),
>
> in this order, returns QEMU to working state.

The patch also breaks QEMU for the "examples/gvncviewer" utility, built
from the gtk-vnc project at commit c7942ba5499a ("src: remove use of
obsolete thread APIs", 2021-01-07). (This commit is the last one that I
can easily build on RHEL7, as the next one would be f79a54e8b692
("build: bump min versions of dependancies to RHEL8 vintage",
2021-01-07).)

The symptom is that gncviewer @ c7942ba5499a displays an initial image,
but no updates -- instead, it is spinning:

> Connected to server
> Remote desktop size changed to 640x480
> Connection initialized
> [here]

and then after a while, it prints:

> Error: Failed to flush data
> Disconnected from server

If I revert the above-mentioned two QEMU patches (the one after this
patch first, for a clean context, and then this one), then the same
gncviewer binary (built at c7942ba5499a) works perfectly with QEMU.

(The vncviewer utility from tigervnc-1.8.0-22.el7.x86_64 works fine both
with this QEMU patch in place, and with it reverted.)

So I think that this QEMU change exercises a part of the RFB spec that
gtk-vnc used to be incompatible with, and that incompatibility must have
been fixed only recently, somewhere in the commit range
c7942ba5499a..6046a8b4bfd9 (the latter being the current HEAD of the
master branch):

 1  f79a54e8b692 build: bump min versions of dependancies to RHEL8 vintage
 2  ddff6d5b1b8f build: enable glib/gtk API version usage checking
 3  80b73802cf96 build: change with-vala and introspection options into features
 4  8b79e4023777 build: disable introspection in mingw builds
 5  27e73b8b0f6c src: add minimal support for extended desktop resize
 6  faacea8b016e src: block non-incremental updates after extended desktop resize
 7  b884d72881db src: add API for requesting a desktop resize
 8  4821aeb05e1b src: add APIs to control whether desktop resizing is allowed
 9  8e86bc76bca7 src: implement dynamic resize of remote desktop on widget resize
10  6046a8b4bfd9 examples: add menu option to enable desktop resizing

I can't perform a reverse bisection on this range (for finding the fix
in gtk-vnc) because commit#1 prevents building gtk-vnc on RHEL7 (bumps
the minimum libgcrypt version from 1.5.0 to 1.8.0).

Thanks
Laszlo

>> diff --git a/ui/vnc.c b/ui/vnc.c
>> index 0f01481cac57..b4e98cf647f5 100644
>> --- a/ui/vnc.c
>> +++ b/ui/vnc.c
>> @@ -661,10 +661,6 @@ static void vnc_desktop_resize(VncState *vs)
>>      if (vs->ioc == NULL || !vnc_has_feature(vs, VNC_FEATURE_RESIZE)) {
>>          return;
>>      }
>> -    if (vs->client_width == pixman_image_get_width(vs->vd->server) &&
>> -        vs->client_height == pixman_image_get_height(vs->vd->server)) {
>> -        return;
>> -    }
>>
>>      assert(pixman_image_get_width(vs->vd->server) < 65536 &&
>>             pixman_image_get_width(vs->vd->server) >= 0);
>> @@ -2014,6 +2010,10 @@ static void framebuffer_update_request(VncState *vs, int incremental,
>>      } else {
>>          vs->update = VNC_STATE_UPDATE_FORCE;
>>          vnc_set_area_dirty(vs->dirty, vs->vd, x, y, w, h);
>> +        vnc_colordepth(vs);
>> +        vnc_desktop_resize(vs);
>> +        vnc_led_state_change(vs);
>> +        vnc_cursor_define(vs);
>>      }
>>  }
>>
>> @@ -2154,10 +2154,7 @@ static void set_encodings(VncState *vs, int32_t *encodings, size_t n_encodings)
>>              break;
>>          }
>>      }
>> -    vnc_desktop_resize(vs);
>>      check_pointer_type_change(&vs->mouse_mode_notifier, NULL);
>> -    vnc_led_state_change(vs);
>> -    vnc_cursor_define(vs);
>>  }
>>
>>  static void set_pixel_conversion(VncState *vs)
>>
>

Re: [PULL 10/11] vnc: move initialization to framebuffer_update_request

[Gerd Hoffmann]

> This patch breaks QEMU for me.

> The symptom is the following: in virt-manager, the display remains dead
> (black), when I start an OVMF guest. At the same time, unusually high
> CPU load can be seen on the host; it makes me think that virt-manager is
> trying, in a busy loop, to complete the VNC handshake, or some such.
> Ultimately, although the guest boots up fine, virt-manager gives up, and
> the display window says "Viewer was disconnected".

It is the vnc_colordepth() call. Seems gtk-vnc sends a update request
with incremental=0 as response to the VNC_ENCODING_WMVi message.  So
sending that as response to an incremental=0 update request creates an
endless loop ...

take care,
  Gerd

diff --git a/ui/vnc.c b/ui/vnc.c
index d429bfee5a65..0a3e2f4aa98c 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2041,7 +2041,6 @@ static void framebuffer_update_request(VncState *vs, int incremental,
     } else {
         vs->update = VNC_STATE_UPDATE_FORCE;
         vnc_set_area_dirty(vs->dirty, vs->vd, x, y, w, h);
-        vnc_colordepth(vs);
         vnc_desktop_resize(vs);
         vnc_led_state_change(vs);
         vnc_cursor_define(vs);

Re: [PULL 10/11] vnc: move initialization to framebuffer_update_request

[Laszlo Ersek]

On 01/22/21 09:46, Gerd Hoffmann wrote:
>> This patch breaks QEMU for me.
> 
>> The symptom is the following: in virt-manager, the display remains dead
>> (black), when I start an OVMF guest. At the same time, unusually high
>> CPU load can be seen on the host; it makes me think that virt-manager is
>> trying, in a busy loop, to complete the VNC handshake, or some such.
>> Ultimately, although the guest boots up fine, virt-manager gives up, and
>> the display window says "Viewer was disconnected".
> 
> It is the vnc_colordepth() call. Seems gtk-vnc sends a update request
> with incremental=0 as response to the VNC_ENCODING_WMVi message.  So
> sending that as response to an incremental=0 update request creates an
> endless loop ...

Interesting; I saw that commit 9e1632ad07ca *added* (as opposed to
*moving*) the vnc_colordepth() call; I thought it was a relatively
insignificant bit...

I'll report back on your separately posted patch.

Thanks!
Laszlo


> 
> take care,
>   Gerd
> 
> diff --git a/ui/vnc.c b/ui/vnc.c
> index d429bfee5a65..0a3e2f4aa98c 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -2041,7 +2041,6 @@ static void framebuffer_update_request(VncState *vs, int incremental,
>      } else {
>          vs->update = VNC_STATE_UPDATE_FORCE;
>          vnc_set_area_dirty(vs->dirty, vs->vd, x, y, w, h);
> -        vnc_colordepth(vs);
>          vnc_desktop_resize(vs);
>          vnc_led_state_change(vs);
>          vnc_cursor_define(vs);
> 

Re: [PULL 10/11] vnc: move initialization to framebuffer_update_request

[Gerd Hoffmann]

On Fri, Jan 22, 2021 at 01:49:38PM +0100, Laszlo Ersek wrote:
> On 01/22/21 09:46, Gerd Hoffmann wrote:
> >> This patch breaks QEMU for me.
> > 
> >> The symptom is the following: in virt-manager, the display remains dead
> >> (black), when I start an OVMF guest. At the same time, unusually high
> >> CPU load can be seen on the host; it makes me think that virt-manager is
> >> trying, in a busy loop, to complete the VNC handshake, or some such.
> >> Ultimately, although the guest boots up fine, virt-manager gives up, and
> >> the display window says "Viewer was disconnected".
> > 
> > It is the vnc_colordepth() call. Seems gtk-vnc sends a update request
> > with incremental=0 as response to the VNC_ENCODING_WMVi message.  So
> > sending that as response to an incremental=0 update request creates an
> > endless loop ...
> 
> Interesting; I saw that commit 9e1632ad07ca *added* (as opposed to
> *moving*) the vnc_colordepth() call; I thought it was a relatively
> insignificant bit...

/me too.

Some discussions in the resize changes indicated that the idea of a
non-incremetal update request is that the server sends the *full*
server-side state, so the client can render the screen properly without
remembering old state.

So I thought ok, lets send the colordepth info too, no big deal ...

take care,
  Gerd