* [Qemu-devel] [PULL 1/6] spice: use bottom half instead of refresh timer for cursor updates
2014-12-16 14:13 [Qemu-devel] [PULL 0/6] spice patch queue Gerd Hoffmann
@ 2014-12-16 14:13 ` Gerd Hoffmann
2014-12-16 14:13 ` [Qemu-devel] [PULL 2/6] spice: reduce refresh rate in native mode Gerd Hoffmann
` (5 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Gerd Hoffmann @ 2014-12-16 14:13 UTC (permalink / raw)
To: qemu-devel; +Cc: Marc-André Lureau, Gerd Hoffmann, Anthony Liguori
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 e812ddd..a542087 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 b540dd6..5151bac 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 4252ab8..53883a1 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 def7b52..5d03340 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) {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Qemu-devel] [PULL 2/6] spice: reduce refresh rate in native mode
2014-12-16 14:13 [Qemu-devel] [PULL 0/6] spice patch queue Gerd Hoffmann
2014-12-16 14:13 ` [Qemu-devel] [PULL 1/6] spice: use bottom half instead of refresh timer for cursor updates Gerd Hoffmann
@ 2014-12-16 14:13 ` Gerd Hoffmann
2014-12-16 14:13 ` [Qemu-devel] [PULL 3/6] spice: rework mirror allocation, add no-resize fast path Gerd Hoffmann
` (4 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Gerd Hoffmann @ 2014-12-16 14:13 UTC (permalink / raw)
To: qemu-devel; +Cc: Marc-André Lureau, Gerd Hoffmann
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 5151bac..61df477 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);
}
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Qemu-devel] [PULL 3/6] spice: rework mirror allocation, add no-resize fast path
2014-12-16 14:13 [Qemu-devel] [PULL 0/6] spice patch queue Gerd Hoffmann
2014-12-16 14:13 ` [Qemu-devel] [PULL 1/6] spice: use bottom half instead of refresh timer for cursor updates Gerd Hoffmann
2014-12-16 14:13 ` [Qemu-devel] [PULL 2/6] spice: reduce refresh rate in native mode Gerd Hoffmann
@ 2014-12-16 14:13 ` Gerd Hoffmann
2014-12-16 14:13 ` [Qemu-devel] [PULL 4/6] spice: do not require TCP ports Gerd Hoffmann
` (3 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Gerd Hoffmann @ 2014-12-16 14:13 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Anthony Liguori
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 5d03340..d2e3793 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);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Qemu-devel] [PULL 4/6] spice: do not require TCP ports
2014-12-16 14:13 [Qemu-devel] [PULL 0/6] spice patch queue Gerd Hoffmann
` (2 preceding siblings ...)
2014-12-16 14:13 ` [Qemu-devel] [PULL 3/6] spice: rework mirror allocation, add no-resize fast path Gerd Hoffmann
@ 2014-12-16 14:13 ` Gerd Hoffmann
2014-12-16 14:13 ` [Qemu-devel] [PULL 5/6] spice: remove spice-experimental.h include Gerd Hoffmann
` (2 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Gerd Hoffmann @ 2014-12-16 14:13 UTC (permalink / raw)
To: qemu-devel; +Cc: Marc-André Lureau, Gerd Hoffmann, Anthony Liguori
From: Marc-André Lureau <marcandre.lureau@gmail.com>
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 6467fa4..e8d4bae 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);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Qemu-devel] [PULL 5/6] spice: remove spice-experimental.h include
2014-12-16 14:13 [Qemu-devel] [PULL 0/6] spice patch queue Gerd Hoffmann
` (3 preceding siblings ...)
2014-12-16 14:13 ` [Qemu-devel] [PULL 4/6] spice: do not require TCP ports Gerd Hoffmann
@ 2014-12-16 14:13 ` Gerd Hoffmann
2014-12-16 14:13 ` [Qemu-devel] [PULL 6/6] spice: fix memory leak Gerd Hoffmann
2014-12-17 16:23 ` [Qemu-devel] [PULL 0/6] spice patch queue Peter Maydell
6 siblings, 0 replies; 9+ messages in thread
From: Gerd Hoffmann @ 2014-12-16 14:13 UTC (permalink / raw)
To: qemu-devel; +Cc: Marc-André Lureau, Gerd Hoffmann, Anthony Liguori
From: Marc-André Lureau <marcandre.lureau@gmail.com>
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 8106e06..7e0d300 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 e8d4bae..497670c 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"
--
1.8.3.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Qemu-devel] [PULL 6/6] spice: fix memory leak
2014-12-16 14:13 [Qemu-devel] [PULL 0/6] spice patch queue Gerd Hoffmann
` (4 preceding siblings ...)
2014-12-16 14:13 ` [Qemu-devel] [PULL 5/6] spice: remove spice-experimental.h include Gerd Hoffmann
@ 2014-12-16 14:13 ` Gerd Hoffmann
2014-12-17 16:23 ` [Qemu-devel] [PULL 0/6] spice patch queue Peter Maydell
6 siblings, 0 replies; 9+ messages in thread
From: Gerd Hoffmann @ 2014-12-16 14:13 UTC (permalink / raw)
To: qemu-devel; +Cc: Gonglei, Gerd Hoffmann, Anthony Liguori
From: Gonglei <arei.gonglei@huawei.com>
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 497670c..fe705c1 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));
--
1.8.3.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PULL 0/6] spice patch queue
2014-12-16 14:13 [Qemu-devel] [PULL 0/6] spice patch queue Gerd Hoffmann
` (5 preceding siblings ...)
2014-12-16 14:13 ` [Qemu-devel] [PULL 6/6] spice: fix memory leak Gerd Hoffmann
@ 2014-12-17 16:23 ` Peter Maydell
6 siblings, 0 replies; 9+ messages in thread
From: Peter Maydell @ 2014-12-17 16:23 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: QEMU Developers
On 16 December 2014 at 14:13, Gerd Hoffmann <kraxel@redhat.com> wrote:
> Hi,
>
> Collection of misc spice patches, piled up during the 2.2 freeze.
>
> please pull,
> Gerd
>
> The following changes since commit dfa9c2a0f4d0a0c8b2c1449ecdbb1297427e1560:
>
> Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2014-12-15 16:43:42 +0000)
>
> are available in the git repository at:
>
>
> git://anongit.freedesktop.org/spice/qemu tags/pull-spice-20141216-1
>
> for you to fetch changes up to a41642708a5d1cbe8ad966227bbee1ed5eb421ad:
>
> spice: fix memory leak (2014-12-16 14:15:29 +0100)
>
> ----------------------------------------------------------------
> misc spice updates.
>
Applied, thanks.
-- PMM
^ permalink raw reply [flat|nested] 9+ messages in thread