From 0b2824e5e48a787be3edbfc897244b4621e5bd61 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Tue, 4 Nov 2014 13:59:59 +0100
Subject: [PATCH 1/6] spice: use bottom half instead of refresh timer for
cursor updates
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Calling directly doesn't work due to the qxl-render code running in
spice server thread context. Meanwhile bottom half scheduling is
thread-safe though, so we can use that to kick a cursor update in
main i/o thread context.
Cc: Marc-André Lureau <marcandre.lureau@gmail.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/display/qxl-render.c | 2 ++
hw/display/qxl.c | 5 +----
include/ui/spice-display.h | 3 ++-
ui/spice-display.c | 12 ++++++++++--
4 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
index e812ddd6e7..a542087fcc 100644
--- a/hw/display/qxl-render.c
+++ b/hw/display/qxl-render.c
@@ -283,12 +283,14 @@ int qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt *ext)
qxl->ssd.mouse_x = cmd->u.set.position.x;
qxl->ssd.mouse_y = cmd->u.set.position.y;
qemu_mutex_unlock(&qxl->ssd.lock);
+ qemu_bh_schedule(qxl->ssd.cursor_bh);
break;
case QXL_CURSOR_MOVE:
qemu_mutex_lock(&qxl->ssd.lock);
qxl->ssd.mouse_x = cmd->u.position.x;
qxl->ssd.mouse_y = cmd->u.position.y;
qemu_mutex_unlock(&qxl->ssd.lock);
+ qemu_bh_schedule(qxl->ssd.cursor_bh);
break;
}
return 0;
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index b540dd656c..5151bac32d 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -1861,10 +1861,6 @@ static void display_refresh(DisplayChangeListener *dcl)
if (qxl->mode == QXL_MODE_VGA) {
qemu_spice_display_refresh(&qxl->ssd);
- } else {
- qemu_mutex_lock(&qxl->ssd.lock);
- qemu_spice_cursor_refresh_unlocked(&qxl->ssd);
- qemu_mutex_unlock(&qxl->ssd.lock);
}
}
@@ -2025,6 +2021,7 @@ static int qxl_init_common(PCIQXLDevice *qxl)
qxl_reset_state(qxl);
qxl->update_area_bh = qemu_bh_new(qxl_render_update_area_bh, qxl);
+ qxl->ssd.cursor_bh = qemu_bh_new(qemu_spice_cursor_refresh_bh, &qxl->ssd);
return 0;
}
diff --git a/include/ui/spice-display.h b/include/ui/spice-display.h
index 4252ab856f..53883a17fc 100644
--- a/include/ui/spice-display.h
+++ b/include/ui/spice-display.h
@@ -102,6 +102,7 @@ struct SimpleSpiceDisplay {
/* cursor (with qxl): qxl local renderer -> displaychangelistener */
QEMUCursor *cursor;
int mouse_x, mouse_y;
+ QEMUBH *cursor_bh;
};
struct SimpleSpiceUpdate {
@@ -134,7 +135,7 @@ void qemu_spice_display_update(SimpleSpiceDisplay *ssd,
void qemu_spice_display_switch(SimpleSpiceDisplay *ssd,
DisplaySurface *surface);
void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd);
-void qemu_spice_cursor_refresh_unlocked(SimpleSpiceDisplay *ssd);
+void qemu_spice_cursor_refresh_bh(void *opaque);
void qemu_spice_add_memslot(SimpleSpiceDisplay *ssd, QXLDevMemSlot *memslot,
qxl_async_io async);
diff --git a/ui/spice-display.c b/ui/spice-display.c
index def7b52e9c..5d033406ec 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -438,7 +438,7 @@ void qemu_spice_display_switch(SimpleSpiceDisplay *ssd,
ssd->notify++;
}
-void qemu_spice_cursor_refresh_unlocked(SimpleSpiceDisplay *ssd)
+static void qemu_spice_cursor_refresh_unlocked(SimpleSpiceDisplay *ssd)
{
if (ssd->cursor) {
assert(ssd->dcl.con);
@@ -454,6 +454,15 @@ void qemu_spice_cursor_refresh_unlocked(SimpleSpiceDisplay *ssd)
}
}
+void qemu_spice_cursor_refresh_bh(void *opaque)
+{
+ SimpleSpiceDisplay *ssd = opaque;
+
+ qemu_mutex_lock(&ssd->lock);
+ qemu_spice_cursor_refresh_unlocked(ssd);
+ qemu_mutex_unlock(&ssd->lock);
+}
+
void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd)
{
dprint(3, "%s/%d:\n", __func__, ssd->qxl.id);
@@ -464,7 +473,6 @@ void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd)
qemu_spice_create_update(ssd);
ssd->notify++;
}
- qemu_spice_cursor_refresh_unlocked(ssd);
qemu_mutex_unlock(&ssd->lock);
if (ssd->notify) {
From 3dcadce5076d4b42fa395c39662d65e050b77784 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Tue, 4 Nov 2014 14:16:12 +0100
Subject: [PATCH 2/6] spice: reduce refresh rate in native mode
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Now that cursor updates are out of the way qxl needs the refresh timer
only when when running in vga mode, for dirty bitmap checking. In
native qxl mode the guest will notify us, so we don't need to poll and
can use the idle interval (one refresh wakeup every few seconds).
Cc: Marc-André Lureau <marcandre.lureau@gmail.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/display/qxl.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index 5151bac32d..61df477264 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -1092,6 +1092,7 @@ static void qxl_enter_vga_mode(PCIQXLDevice *d)
spice_qxl_driver_unload(&d->ssd.qxl);
#endif
graphic_console_set_hwops(d->ssd.dcl.con, d->vga.hw_ops, &d->vga);
+ update_displaychangelistener(&d->ssd.dcl, GUI_REFRESH_INTERVAL_DEFAULT);
qemu_spice_create_host_primary(&d->ssd);
d->mode = QXL_MODE_VGA;
vga_dirty_log_start(&d->vga);
@@ -1105,6 +1106,7 @@ static void qxl_exit_vga_mode(PCIQXLDevice *d)
}
trace_qxl_exit_vga_mode(d->id);
graphic_console_set_hwops(d->ssd.dcl.con, &qxl_ops, d);
+ update_displaychangelistener(&d->ssd.dcl, GUI_REFRESH_INTERVAL_IDLE);
vga_dirty_log_stop(&d->vga);
qxl_destroy_primary(d, QXL_SYNC);
}
@@ -1153,6 +1155,7 @@ static void qxl_soft_reset(PCIQXLDevice *d)
qxl_enter_vga_mode(d);
} else {
d->mode = QXL_MODE_UNDEFINED;
+ update_displaychangelistener(&d->ssd.dcl, GUI_REFRESH_INTERVAL_IDLE);
}
}
From 555e72f2d02125766601db52c6380357b3820fcb Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Sat, 8 Nov 2014 08:56:34 +0100
Subject: [PATCH 3/6] spice: rework mirror allocation, add no-resize fast path
Add fast path to qemu_spice_display_switch in case old and new
displaysurface have identical size (happens with display panning
and page flipping). We just swap the backing store then and don't
go through the whole process of deleting and creating the primary
surface.
To simplify the code a bit move mirror surface allocation to
qemu_spice_display_switch().
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
ui/spice-display.c | 33 ++++++++++++++++++++++++++-------
1 file changed, 26 insertions(+), 7 deletions(-)
diff --git a/ui/spice-display.c b/ui/spice-display.c
index 5d033406ec..d2e379379f 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -207,12 +207,6 @@ static void qemu_spice_create_update(SimpleSpiceDisplay *ssd)
return;
};
- if (ssd->surface == NULL) {
- ssd->surface = pixman_image_ref(ssd->ds->image);
- ssd->mirror = qemu_pixman_mirror_create(ssd->ds->format,
- ssd->ds->image);
- }
-
for (blk = 0; blk < blocks; blk++) {
dirty_top[blk] = -1;
}
@@ -409,7 +403,29 @@ void qemu_spice_display_switch(SimpleSpiceDisplay *ssd,
SimpleSpiceUpdate *update;
bool need_destroy;
- dprint(1, "%s/%d:\n", __func__, ssd->qxl.id);
+ if (surface && ssd->surface &&
+ surface_width(surface) == pixman_image_get_width(ssd->surface) &&
+ surface_height(surface) == pixman_image_get_height(ssd->surface)) {
+ /* no-resize fast path: just swap backing store */
+ dprint(1, "%s/%d: fast (%dx%d)\n", __func__, ssd->qxl.id,
+ surface_width(surface), surface_height(surface));
+ qemu_mutex_lock(&ssd->lock);
+ ssd->ds = surface;
+ pixman_image_unref(ssd->surface);
+ ssd->surface = pixman_image_ref(ssd->ds->image);
+ qemu_mutex_unlock(&ssd->lock);
+ qemu_spice_display_update(ssd, 0, 0,
+ surface_width(surface),
+ surface_height(surface));
+ return;
+ }
+
+ /* full mode switch */
+ dprint(1, "%s/%d: full (%dx%d -> %dx%d)\n", __func__, ssd->qxl.id,
+ ssd->surface ? pixman_image_get_width(ssd->surface) : 0,
+ ssd->surface ? pixman_image_get_height(ssd->surface) : 0,
+ surface ? surface_width(surface) : 0,
+ surface ? surface_height(surface) : 0);
memset(&ssd->dirty, 0, sizeof(ssd->dirty));
if (ssd->surface) {
@@ -422,6 +438,9 @@ void qemu_spice_display_switch(SimpleSpiceDisplay *ssd,
qemu_mutex_lock(&ssd->lock);
need_destroy = (ssd->ds != NULL);
ssd->ds = surface;
+ ssd->surface = pixman_image_ref(ssd->ds->image);
+ ssd->mirror = qemu_pixman_mirror_create(ssd->ds->format,
+ ssd->ds->image);
while ((update = QTAILQ_FIRST(&ssd->updates)) != NULL) {
QTAILQ_REMOVE(&ssd->updates, update, next);
qemu_spice_destroy_update(ssd, update);
From cf7856adefebe86e0cd50302d93b3045e3111690 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= <marcandre.lureau@gmail.com>
Date: Tue, 11 Nov 2014 13:39:19 +0100
Subject: [PATCH 4/6] spice: do not require TCP ports
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
It is possible to use Spice server without TCP port. On local VM,
qemu (and libvirt) can add new clients thanks to QMP add_client command.
Signed-off-by: Marc-André Lureau <marcandre.lureau@gmail.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
ui/spice-core.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 6467fa4776..e8d4baedd8 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -661,10 +661,6 @@ void qemu_spice_init(void)
}
port = qemu_opt_get_number(opts, "port", 0);
tls_port = qemu_opt_get_number(opts, "tls-port", 0);
- if (!port && !tls_port) {
- error_report("neither port nor tls-port specified for spice");
- exit(1);
- }
if (port < 0 || port > 65535) {
error_report("spice port is out of range");
exit(1);
From e0883e2de0ef36f254acc274e80ddeac13a2a8f6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= <marcandre.lureau@gmail.com>
Date: Mon, 17 Nov 2014 16:52:49 +0100
Subject: [PATCH 5/6] spice: remove spice-experimental.h include
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Nothing seems to be using functions from spice-experimental.h (better
that way). Let's remove its inclusion.
Signed-off-by: Marc-André Lureau <marcandre.lureau@gmail.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
spice-qemu-char.c | 1 -
ui/spice-core.c | 1 -
2 files changed, 2 deletions(-)
diff --git a/spice-qemu-char.c b/spice-qemu-char.c
index 8106e063c0..7e0d300777 100644
--- a/spice-qemu-char.c
+++ b/spice-qemu-char.c
@@ -3,7 +3,6 @@
#include "ui/qemu-spice.h"
#include "sysemu/char.h"
#include <spice.h>
-#include <spice-experimental.h>
#include <spice/protocol.h>
#include "qemu/osdep.h"
diff --git a/ui/spice-core.c b/ui/spice-core.c
index e8d4baedd8..497670c4d5 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -16,7 +16,6 @@
*/
#include <spice.h>
-#include <spice-experimental.h>
#include <netdb.h>
#include "sysemu/sysemu.h"
From a41642708a5d1cbe8ad966227bbee1ed5eb421ad Mon Sep 17 00:00:00 2001
From: Gonglei <arei.gonglei@huawei.com>
Date: Fri, 5 Dec 2014 16:30:10 +0800
Subject: [PATCH 6/6] spice: fix memory leak
If errors happen for middle items of channel_list,
qmp_query_spice_channels() returns NULL, and the variable
cur_item going out of scope leaks the storage it points to.
The flag is a compatibility thing for older spice-server
versions. Meanwhile our minimum spice version requirement is
new enough that we should never ever see this error, and if we
do something went very seriously wrong. Let's using assert()
instead of returning NULL to avoid a memory leak.
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
ui/spice-core.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 497670c4d5..fe705c1ae2 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -385,10 +385,7 @@ static SpiceChannelList *qmp_query_spice_channels(void)
struct sockaddr *paddr;
socklen_t plen;
- if (!(item->info->flags & SPICE_CHANNEL_EVENT_FLAG_ADDR_EXT)) {
- error_report("invalid channel event");
- return NULL;
- }
+ assert(item->info->flags & SPICE_CHANNEL_EVENT_FLAG_ADDR_EXT);
chan = g_malloc0(sizeof(*chan));
chan->value = g_malloc0(sizeof(*chan->value));