From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([140.186.70.92]:53657)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <alevy@redhat.com>) id 1R5CS5-0007ci-Hp
	for qemu-devel@nongnu.org; Sun, 18 Sep 2011 04:10:14 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <alevy@redhat.com>) id 1R5CS4-0005w6-Lp
	for qemu-devel@nongnu.org; Sun, 18 Sep 2011 04:10:13 -0400
Received: from mx1.redhat.com ([209.132.183.28]:31509)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <alevy@redhat.com>) id 1R5CS4-0005w1-Dg
	for qemu-devel@nongnu.org; Sun, 18 Sep 2011 04:10:12 -0400
Received: from int-mx11.intmail.prod.int.phx2.redhat.com
	(int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24])
	by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p8I8A77E027302
	(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=FAIL)
	for <qemu-devel@nongnu.org>; Sun, 18 Sep 2011 04:10:11 -0400
Date: Sun, 18 Sep 2011 11:07:28 +0300
From: Alon Levy <alevy@redhat.com>
Message-ID: <20110918080728.GE2685@bow.tlv.redhat.com>
References: <1315233590-11142-1-git-send-email-yhalperi@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1315233590-11142-1-git-send-email-yhalperi@redhat.com>
Subject: Re: [Qemu-devel] [PATCH] spice: set qxl->ssd.running=true before
 telling spice to start, RHBZ #733993
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Yonit Halperin <yhalperi@redhat.com>
Cc: qemu-devel@nongnu.org, kraxel@redhat.com

On Mon, Sep 05, 2011 at 05:39:50PM +0300, Yonit Halperin wrote:
> If qxl->ssd.running=true is set after telling spice to start, the spice server
> thread can call qxl_send_events while qxl->ssd.running is still false. This leads to
> assert(d->ssd.running).
> 

Looks good.

Acked-by: Alon Levy <alevy@redhat.com>

> Signed-off-by: Yonit Halperin <yhalperi@redhat.com>
> ---
> Since it looks like the purpose of the assert in qxl_send_event is preventing changes
> in the guest when the vm is stopped, I think it is not necessary for ssd.running to be
> exactly synchronized with the spice server status, but just be true before
> the spice worker starts.
> 
>  ui/spice-display.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/ui/spice-display.c b/ui/spice-display.c
> index 683d454..3224f99 100644
> --- a/ui/spice-display.c
> +++ b/ui/spice-display.c
> @@ -260,11 +260,12 @@ void qemu_spice_vm_change_state_handler(void *opaque, int running, int reason)
>      SimpleSpiceDisplay *ssd = opaque;
>  
>      if (running) {
> +        ssd->running = true;
>          qemu_spice_start(ssd);
>      } else {
>          qemu_spice_stop(ssd);
> +        ssd->running = false;
>      }
> -    ssd->running = running;
>  }
>  
>  void qemu_spice_display_init_common(SimpleSpiceDisplay *ssd, DisplayState *ds)
> -- 
> 1.7.4.4
> 
>