Re: [Qemu-devel] [PATCH 4/4] spice: drop obsolete iothread locking

[Bing Bu Cao <caobingbu@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3835 bytes --]

Hi,Gerd

I found the lock has been revert in qemu-kvm repository and
qemu-kvm on RHEL6.3 with spice/qxl has one problem:


Doing some display configuration (such as changing resolution) in guest 
will involve in deadlock in guest,meanwhile
which affect the host hang for a long while.


I wonder whether this patch should be revert back in qemu-kvm again?
Actually, I have no idea why added in QEMU but not in qemu-kvm? :)


On 04/29/2011 05:38 PM, Gerd Hoffmann wrote:
> We don't use qemu internals from spice server context any more.
> Thus we don't also need to grab the iothread mutex from spice
> server context.  And we don't have to temporarely release the
> lock to avoid deadlocks.  Drop all the calls.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>   hw/qxl.c           |    8 --------
>   ui/spice-display.c |    6 ------
>   2 files changed, 0 insertions(+), 14 deletions(-)
>
> diff --git a/hw/qxl.c b/hw/qxl.c
> index 4dfddf0..2bb36c6 100644
> --- a/hw/qxl.c
> +++ b/hw/qxl.c
> @@ -666,10 +666,8 @@ static void qxl_hard_reset(PCIQXLDevice *d, int loadvm)
>       dprint(d, 1, "%s: start%s\n", __FUNCTION__,
>              loadvm ? " (loadvm)" : "");
>   
> -    qemu_mutex_unlock_iothread();
>       d->ssd.worker->reset_cursor(d->ssd.worker);
>       d->ssd.worker->reset_image_cache(d->ssd.worker);
> -    qemu_mutex_lock_iothread();
>       qxl_reset_surfaces(d);
>       qxl_reset_memslots(d);
>   
> @@ -799,9 +797,7 @@ static void qxl_reset_surfaces(PCIQXLDevice *d)
>   {
>       dprint(d, 1, "%s:\n", __FUNCTION__);
>       d->mode = QXL_MODE_UNDEFINED;
> -    qemu_mutex_unlock_iothread();
>       d->ssd.worker->destroy_surfaces(d->ssd.worker);
> -    qemu_mutex_lock_iothread();
>       memset(&d->guest_surfaces.cmds, 0, sizeof(d->guest_surfaces.cmds));
>   }
>   
> @@ -870,9 +866,7 @@ static void qxl_destroy_primary(PCIQXLDevice *d)
>       dprint(d, 1, "%s\n", __FUNCTION__);
>   
>       d->mode = QXL_MODE_UNDEFINED;
> -    qemu_mutex_unlock_iothread();
>       d->ssd.worker->destroy_primary_surface(d->ssd.worker, 0);
> -    qemu_mutex_lock_iothread();
>   }
>   
>   static void qxl_set_mode(PCIQXLDevice *d, int modenr, int loadvm)
> @@ -942,10 +936,8 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
>       case QXL_IO_UPDATE_AREA:
>       {
>           QXLRect update = d->ram->update_area;
> -        qemu_mutex_unlock_iothread();
>           d->ssd.worker->update_area(d->ssd.worker, d->ram->update_surface,
>                                      &update, NULL, 0, 0);
> -        qemu_mutex_lock_iothread();
>           break;
>       }
>       case QXL_IO_NOTIFY_CMD:
> diff --git a/ui/spice-display.c b/ui/spice-display.c
> index 1e1a35d..34201cd 100644
> --- a/ui/spice-display.c
> +++ b/ui/spice-display.c
> @@ -176,18 +176,14 @@ void qemu_spice_create_host_primary(SimpleSpiceDisplay *ssd)
>       surface.mem        = (intptr_t)ssd->buf;
>       surface.group_id   = MEMSLOT_GROUP_HOST;
>   
> -    qemu_mutex_unlock_iothread();
>       ssd->worker->create_primary_surface(ssd->worker, 0, &surface);
> -    qemu_mutex_lock_iothread();
>   }
>   
>   void qemu_spice_destroy_host_primary(SimpleSpiceDisplay *ssd)
>   {
>       dprint(1, "%s:\n", __FUNCTION__);
>   
> -    qemu_mutex_unlock_iothread();
>       ssd->worker->destroy_primary_surface(ssd->worker, 0);
> -    qemu_mutex_lock_iothread();
>   }
>   
>   void qemu_spice_vm_change_state_handler(void *opaque, int running, int reason)
> @@ -197,9 +193,7 @@ void qemu_spice_vm_change_state_handler(void *opaque, int running, int reason)
>       if (running) {
>           ssd->worker->start(ssd->worker);
>       } else {
> -        qemu_mutex_unlock_iothread();
>           ssd->worker->stop(ssd->worker);
> -        qemu_mutex_lock_iothread();
>       }
>       ssd->running = running;
>   }


[-- Attachment #2: Type: text/html, Size: 4410 bytes --]