[Qemu-devel] [PULL 0/6] spice patch queue

[Qemu-devel] [PULL 0/6] spice patch queue

[Gerd Hoffmann]

  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.

----------------------------------------------------------------
Gerd Hoffmann (3):
      spice: use bottom half instead of refresh timer for cursor updates
      spice: reduce refresh rate in native mode
      spice: rework mirror allocation, add no-resize fast path

Gonglei (1):
      spice: fix memory leak

Marc-André Lureau (2):
      spice: do not require TCP ports
      spice: remove spice-experimental.h include

 hw/display/qxl-render.c    |  2 ++
 hw/display/qxl.c           |  8 ++++----
 include/ui/spice-display.h |  3 ++-
 spice-qemu-char.c          |  1 -
 ui/spice-core.c            | 10 +---------
 ui/spice-display.c         | 45 ++++++++++++++++++++++++++++++++++++---------
 6 files changed, 45 insertions(+), 24 deletions(-)

[Qemu-devel] [PULL 1/6] spice: use bottom half instead of refresh timer for cursor updates

[Gerd Hoffmann]

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

[Qemu-devel] [PULL 2/6] spice: reduce refresh rate in native mode

[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

[Qemu-devel] [PULL 3/6] spice: rework mirror allocation, add no-resize fast path

[Gerd Hoffmann]

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

[Qemu-devel] [PULL 4/6] spice: do not require TCP ports

[Gerd Hoffmann]

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

[Qemu-devel] [PULL 5/6] spice: remove spice-experimental.h include

[Gerd Hoffmann]

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

[Qemu-devel] [PULL 6/6] spice: fix memory leak

[Gerd Hoffmann]

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

Re: [Qemu-devel] [PULL 0/6] spice patch queue

[Peter Maydell]

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