From patchwork Tue Sep 18 20:00:31 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Corey Minyard X-Patchwork-Id: 184828 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 07AAA2C007E for ; Wed, 19 Sep 2012 06:01:43 +1000 (EST) Received: from localhost ([::1]:42552 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TE3zI-0006Md-Uo for incoming@patchwork.ozlabs.org; Tue, 18 Sep 2012 16:01:40 -0400 Received: from eggs.gnu.org ([208.118.235.92]:54662) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TE3z4-0006Kr-Je for qemu-devel@nongnu.org; Tue, 18 Sep 2012 16:01:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TE3z2-00049M-Kj for qemu-devel@nongnu.org; Tue, 18 Sep 2012 16:01:26 -0400 Received: from vms173003pub.verizon.net ([206.46.173.3]:55523) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TE3z2-00049B-FW for qemu-devel@nongnu.org; Tue, 18 Sep 2012 16:01:24 -0400 Received: from wf-rch.minyard.home ([unknown] [173.57.151.210]) by vms173003.mailsrvcs.net (Sun Java(tm) System Messaging Server 7u2-7.02 32bit (built Apr 16 2009)) with ESMTPA id <0MAK0045XA9IMSW0@vms173003.mailsrvcs.net> for qemu-devel@nongnu.org; Tue, 18 Sep 2012 15:00:59 -0500 (CDT) Received: from i.minyard.home (i2.minyard.home [192.168.27.116]) by wf-rch.minyard.home (Postfix) with ESMTP id 2396A1F953; Tue, 18 Sep 2012 15:00:50 -0500 (CDT) Received: by i.minyard.home (Postfix, from userid 1000) id DE7FD807B7; Tue, 18 Sep 2012 15:00:49 -0500 (CDT) From: minyard@acm.org To: qemu-devel@nongnu.org Date: Tue, 18 Sep 2012 15:00:31 -0500 Message-id: <1347998443-20599-5-git-send-email-minyard@acm.org> X-Mailer: git-send-email 1.7.4.1 In-reply-to: <1347998443-20599-1-git-send-email-minyard@acm.org> References: <1347998443-20599-1-git-send-email-minyard@acm.org> X-detected-operating-system: by eggs.gnu.org: Solaris 10 (1203?) X-Received-From: 206.46.173.3 Cc: Corey Minyard Subject: [Qemu-devel] [PATCH 04/16] qemu-char: Fix a race reporting opens and closes X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org From: Corey Minyard When an open event is reported, it is done through a bh. But close events are reported immediately. So if an open event is in the bh and a close occurs, an extraneous open happens, which can confuse a user. To fix this, this patch sets the "opened" flag immediately instead of in the bh handler and checks to make sure the opened flag is set before reporting an open event. This also modifies the spice code to call qemu_chr_generic_open to report an open, to keep things consistent. Signed-off-by: Corey Minyard --- qemu-char.c | 18 ++++++++++++++++-- spice-qemu-char.c | 7 +++++-- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/qemu-char.c b/qemu-char.c index f6a671b..9e3ac40 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -111,10 +111,13 @@ void qemu_chr_be_event(CharDriverState *s, int event) /* Keep track if the char device is open */ switch (event) { case CHR_EVENT_OPENED: + /* + * See the comment in qemu_chr_generic_open_bh() for why + * 's->opened = 1' is not here. + */ if (s->recon_timer) { qemu_del_timer(s->recon_timer); } - s->opened = 1; break; case CHR_EVENT_CLOSED: if (s->recon_timer) { @@ -134,13 +137,24 @@ void qemu_chr_be_event(CharDriverState *s, int event) static void qemu_chr_generic_open_bh(void *opaque) { CharDriverState *s = opaque; - qemu_chr_be_event(s, CHR_EVENT_OPENED); + /* + * Since the "close" event doesn't go through a bh, there is a + * possible race condition if a close comes in after an open, but + * the open is in the bh queue. So we double-check here, and we + * set opened in qemu_chr_generic_open() instead of + * qemu_chr_be_event(). + */ + if (s->opened) { + qemu_chr_be_event(s, CHR_EVENT_OPENED); + } qemu_bh_delete(s->bh); s->bh = NULL; } void qemu_chr_generic_open(CharDriverState *s) { + /* See the comment in qemu_chr_generic_open_bh() for why this is here */ + s->opened = 1; if (s->bh == NULL) { s->bh = qemu_bh_new(qemu_chr_generic_open_bh, s); qemu_bh_schedule(s->bh); diff --git a/spice-qemu-char.c b/spice-qemu-char.c index 2fb8a10..d7516bd 100644 --- a/spice-qemu-char.c +++ b/spice-qemu-char.c @@ -93,8 +93,11 @@ static void vmc_state(SpiceCharDeviceInstance *sin, int connected) return; } - qemu_chr_be_event(scd->chr, - connected ? CHR_EVENT_OPENED : CHR_EVENT_CLOSED); + if (connected) { + qemu_chr_generic_open(scd->chr); + } else { + qemu_chr_be_event(scd->chr, CHR_EVENT_CLOSED); + } } static SpiceCharDeviceInterface vmc_interface = {