[libvirt] [PULL 08/12] spice-display: fix qemu_spice_cursor_refresh_bh locking
From: Paolo Bonzini
spice-display should not call the ui/console.c functions dpy_cursor_define
and dpy_moues_set with the SimpleSpiceDisplay lock taken. That will cause
a deadlock, because the DisplayChangeListener callbacks will take the lock
again. It is also in general a bad idea to invoke generic callbacks with a
lock taken, because it can cause AB-BA deadlocks in the long run. The only
thing that requires care is that the cursor may disappear as soon as the
mutex is released, so you need an extra cursor_get/cursor_put pair.
Signed-off-by: Paolo Bonzini
Reviewed-by: Marc-André Lureau
Message-id: 20180720063109.4631-3-pbonz...@redhat.com
[ kraxel: fix dpy_cursor_define() call ]
Signed-off-by: Gerd Hoffmann
---
ui/spice-display.c | 30 ++
1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/ui/spice-display.c b/ui/spice-display.c
index 46df673cd7..7b230987dc 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -450,29 +450,35 @@ void qemu_spice_display_switch(SimpleSpiceDisplay *ssd,
qemu_mutex_unlock(&ssd->lock);
}
-static void qemu_spice_cursor_refresh_unlocked(SimpleSpiceDisplay *ssd)
+void qemu_spice_cursor_refresh_bh(void *opaque)
{
+SimpleSpiceDisplay *ssd = opaque;
+
+qemu_mutex_lock(&ssd->lock);
if (ssd->cursor) {
+QEMUCursor *c = ssd->cursor;
assert(ssd->dcl.con);
-dpy_cursor_define(ssd->dcl.con, ssd->cursor);
+cursor_get(c);
+qemu_mutex_unlock(&ssd->lock);
+dpy_cursor_define(ssd->dcl.con, c);
+qemu_mutex_lock(&ssd->lock);
+cursor_put(c);
}
+
if (ssd->mouse_x != -1 && ssd->mouse_y != -1) {
+int x, y;
assert(ssd->dcl.con);
-dpy_mouse_set(ssd->dcl.con, ssd->mouse_x, ssd->mouse_y, 1);
+x = ssd->mouse_x;
+y = ssd->mouse_y;
ssd->mouse_x = -1;
ssd->mouse_y = -1;
+qemu_mutex_unlock(&ssd->lock);
+dpy_mouse_set(ssd->dcl.con, x, y, 1);
+} else {
+qemu_mutex_unlock(&ssd->lock);
}
}
-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)
{
graphic_hw_update(ssd->dcl.con);
--
2.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PULL 08/12] spice-display: fix qemu_spice_cursor_refresh_bh locking
From: Paolo Bonzini
spice-display should not call the ui/console.c functions dpy_cursor_define
and dpy_moues_set with the SimpleSpiceDisplay lock taken. That will cause
a deadlock, because the DisplayChangeListener callbacks will take the lock
again. It is also in general a bad idea to invoke generic callbacks with a
lock taken, because it can cause AB-BA deadlocks in the long run. The only
thing that requires care is that the cursor may disappear as soon as the
mutex is released, so you need an extra cursor_get/cursor_put pair.
Signed-off-by: Paolo Bonzini
Reviewed-by: Marc-André Lureau
Message-id: 20180720063109.4631-3-pbonz...@redhat.com
[ kraxel: fix dpy_cursor_define() call ]
Signed-off-by: Gerd Hoffmann
---
ui/spice-display.c | 30 ++
1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/ui/spice-display.c b/ui/spice-display.c
index 46df673cd7..7b230987dc 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -450,29 +450,35 @@ void qemu_spice_display_switch(SimpleSpiceDisplay *ssd,
qemu_mutex_unlock(&ssd->lock);
}
-static void qemu_spice_cursor_refresh_unlocked(SimpleSpiceDisplay *ssd)
+void qemu_spice_cursor_refresh_bh(void *opaque)
{
+SimpleSpiceDisplay *ssd = opaque;
+
+qemu_mutex_lock(&ssd->lock);
if (ssd->cursor) {
+QEMUCursor *c = ssd->cursor;
assert(ssd->dcl.con);
-dpy_cursor_define(ssd->dcl.con, ssd->cursor);
+cursor_get(c);
+qemu_mutex_unlock(&ssd->lock);
+dpy_cursor_define(ssd->dcl.con, c);
+qemu_mutex_lock(&ssd->lock);
+cursor_put(c);
}
+
if (ssd->mouse_x != -1 && ssd->mouse_y != -1) {
+int x, y;
assert(ssd->dcl.con);
-dpy_mouse_set(ssd->dcl.con, ssd->mouse_x, ssd->mouse_y, 1);
+x = ssd->mouse_x;
+y = ssd->mouse_y;
ssd->mouse_x = -1;
ssd->mouse_y = -1;
+qemu_mutex_unlock(&ssd->lock);
+dpy_mouse_set(ssd->dcl.con, x, y, 1);
+} else {
+qemu_mutex_unlock(&ssd->lock);
}
}
-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)
{
graphic_hw_update(ssd->dcl.con);
--
2.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PULL 08/12] spice-display: fix qemu_spice_cursor_refresh_bh locking
From: Paolo Bonzini
spice-display should not call the ui/console.c functions dpy_cursor_define
and dpy_moues_set with the SimpleSpiceDisplay lock taken. That will cause
a deadlock, because the DisplayChangeListener callbacks will take the lock
again. It is also in general a bad idea to invoke generic callbacks with a
lock taken, because it can cause AB-BA deadlocks in the long run. The only
thing that requires care is that the cursor may disappear as soon as the
mutex is released, so you need an extra cursor_get/cursor_put pair.
Signed-off-by: Paolo Bonzini
Reviewed-by: Marc-André Lureau
Message-id: 20180720063109.4631-3-pbonz...@redhat.com
[ kraxel: fix dpy_cursor_define() call ]
Signed-off-by: Gerd Hoffmann
---
ui/spice-display.c | 30 ++
1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/ui/spice-display.c b/ui/spice-display.c
index 46df673cd7..7b230987dc 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -450,29 +450,35 @@ void qemu_spice_display_switch(SimpleSpiceDisplay *ssd,
qemu_mutex_unlock(&ssd->lock);
}
-static void qemu_spice_cursor_refresh_unlocked(SimpleSpiceDisplay *ssd)
+void qemu_spice_cursor_refresh_bh(void *opaque)
{
+SimpleSpiceDisplay *ssd = opaque;
+
+qemu_mutex_lock(&ssd->lock);
if (ssd->cursor) {
+QEMUCursor *c = ssd->cursor;
assert(ssd->dcl.con);
-dpy_cursor_define(ssd->dcl.con, ssd->cursor);
+cursor_get(c);
+qemu_mutex_unlock(&ssd->lock);
+dpy_cursor_define(ssd->dcl.con, c);
+qemu_mutex_lock(&ssd->lock);
+cursor_put(c);
}
+
if (ssd->mouse_x != -1 && ssd->mouse_y != -1) {
+int x, y;
assert(ssd->dcl.con);
-dpy_mouse_set(ssd->dcl.con, ssd->mouse_x, ssd->mouse_y, 1);
+x = ssd->mouse_x;
+y = ssd->mouse_y;
ssd->mouse_x = -1;
ssd->mouse_y = -1;
+qemu_mutex_unlock(&ssd->lock);
+dpy_mouse_set(ssd->dcl.con, x, y, 1);
+} else {
+qemu_mutex_unlock(&ssd->lock);
}
}
-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)
{
graphic_hw_update(ssd->dcl.con);
--
2.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PULL 08/12] spice-display: fix qemu_spice_cursor_refresh_bh locking
From: Paolo Bonzini
spice-display should not call the ui/console.c functions dpy_cursor_define
and dpy_moues_set with the SimpleSpiceDisplay lock taken. That will cause
a deadlock, because the DisplayChangeListener callbacks will take the lock
again. It is also in general a bad idea to invoke generic callbacks with a
lock taken, because it can cause AB-BA deadlocks in the long run. The only
thing that requires care is that the cursor may disappear as soon as the
mutex is released, so you need an extra cursor_get/cursor_put pair.
Signed-off-by: Paolo Bonzini
Reviewed-by: Marc-André Lureau
Message-id: 20180720063109.4631-3-pbonz...@redhat.com
Signed-off-by: Gerd Hoffmann
---
ui/spice-display.c | 28 +---
1 file changed, 17 insertions(+), 11 deletions(-)
diff --git a/ui/spice-display.c b/ui/spice-display.c
index 46df673cd7..f1d341091a 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -450,29 +450,35 @@ void qemu_spice_display_switch(SimpleSpiceDisplay *ssd,
qemu_mutex_unlock(&ssd->lock);
}
-static void qemu_spice_cursor_refresh_unlocked(SimpleSpiceDisplay *ssd)
+void qemu_spice_cursor_refresh_bh(void *opaque)
{
+SimpleSpiceDisplay *ssd = opaque;
+
+qemu_mutex_lock(&ssd->lock);
if (ssd->cursor) {
+QEMUCursor *c = ssd->cursor;
assert(ssd->dcl.con);
+cursor_get(c);
+qemu_mutex_unlock(&ssd->lock);
dpy_cursor_define(ssd->dcl.con, ssd->cursor);
+qemu_mutex_lock(&ssd->lock);
+cursor_put(c);
}
+
if (ssd->mouse_x != -1 && ssd->mouse_y != -1) {
+int x, y;
assert(ssd->dcl.con);
-dpy_mouse_set(ssd->dcl.con, ssd->mouse_x, ssd->mouse_y, 1);
+x = ssd->mouse_x;
+y = ssd->mouse_y;
ssd->mouse_x = -1;
ssd->mouse_y = -1;
+qemu_mutex_unlock(&ssd->lock);
+dpy_mouse_set(ssd->dcl.con, x, y, 1);
+} else {
+qemu_mutex_unlock(&ssd->lock);
}
}
-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)
{
graphic_hw_update(ssd->dcl.con);
--
2.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
