From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:34702) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QzDF0-0003FX-FE for qemu-devel@nongnu.org; Thu, 01 Sep 2011 15:47:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QzDEy-0001G8-G5 for qemu-devel@nongnu.org; Thu, 01 Sep 2011 15:47:58 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50523) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QzDEy-0001G4-7s for qemu-devel@nongnu.org; Thu, 01 Sep 2011 15:47:56 -0400 Date: Thu, 1 Sep 2011 20:47:51 +0100 From: "Daniel P. Berrange" Message-ID: <20110901194751.GT14462@redhat.com> References: <20110901163545.71ba1515@doriath> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20110901163545.71ba1515@doriath> Subject: Re: [Qemu-devel] [PATCH] monitor: Protect outbuf from concurrent access Reply-To: "Daniel P. Berrange" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: Marian Krcmarik , Alon Levy , qemu-devel On Thu, Sep 01, 2011 at 04:35:45PM -0300, Luiz Capitulino wrote: > Sometimes, when having lots of VMs running on a RHEV host and the user > attempts to close a SPICE window, libvirt will get corrupted json from > QEMU. > > After some investigation, I found out that the problem is that different > SPICE threads are calling monitor functions (such as > monitor_protocol_event()) in parallel which causes concurrent access > to the monitor's internal buffer outbuf[]. > > This fixes the problem by protecting accesses to outbuf[] with a mutex. > > Honestly speaking, I'm not completely sure this the best thing to do > because the monitor itself and other qemu subsystems are not thread safe, > so having subsystems like SPICE assuming the contrary seems a bit > catastrophic to me... > > Anyways, this commit fixes the problem at hand. IMHO this patch should be applied to stable-0.15 as is, since it is an important fix for SPICE, and this highly targetted mutex lock has low-risk of regressions elsewhere. I'd also apply it for master now, but at the same time perhaps start work on adding broader locking that covers all APIs that monitor.c exposes to internal QEMU code, so we're future proofed against other surprises. > Signed-off-by: Luiz Capitulino Signed-off-by: Daniel P. Berrange Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|