From patchwork Thu Sep 1 19:35:45 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Luiz Capitulino X-Patchwork-Id: 112968 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [140.186.70.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 1CA95B6F93 for ; Fri, 2 Sep 2011 05:36:01 +1000 (EST) Received: from localhost ([::1]:43066 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QzD3O-0006N9-Cl for incoming@patchwork.ozlabs.org; Thu, 01 Sep 2011 15:35:58 -0400 Received: from eggs.gnu.org ([140.186.70.92]:40174) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QzD3I-0006EH-0A for qemu-devel@nongnu.org; Thu, 01 Sep 2011 15:35:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QzD3G-0007XZ-Mx for qemu-devel@nongnu.org; Thu, 01 Sep 2011 15:35:51 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38297) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QzD3G-0007XO-Dt for qemu-devel@nongnu.org; Thu, 01 Sep 2011 15:35:50 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p81JZmW0005457 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 1 Sep 2011 15:35:49 -0400 Received: from doriath (ovpn-113-117.phx2.redhat.com [10.3.113.117]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id p81JZkVZ024329; Thu, 1 Sep 2011 15:35:47 -0400 Date: Thu, 1 Sep 2011 16:35:45 -0300 From: Luiz Capitulino To: qemu-devel Message-ID: <20110901163545.71ba1515@doriath> Organization: Red Hat Mime-Version: 1.0 X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 209.132.183.28 Cc: Marian Krcmarik , Alon Levy Subject: [Qemu-devel] [PATCH] monitor: Protect outbuf from concurrent access 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 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. Signed-off-by: Luiz Capitulino --- monitor.c | 16 +++++++++++++++- 1 files changed, 15 insertions(+), 1 deletions(-) diff --git a/monitor.c b/monitor.c index 04f465a..61d4d93 100644 --- a/monitor.c +++ b/monitor.c @@ -57,6 +57,7 @@ #include "json-parser.h" #include "osdep.h" #include "cpu.h" +#include "qemu-thread.h" #ifdef CONFIG_SIMPLE_TRACE #include "trace.h" #endif @@ -144,6 +145,7 @@ struct Monitor { int suspend_cnt; uint8_t outbuf[1024]; int outbuf_index; + QemuMutex mutex; ReadLineState *rs; MonitorControl *mc; CPUState *mon_cpu; @@ -246,10 +248,14 @@ static int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func, void monitor_flush(Monitor *mon) { + qemu_mutex_lock(&mon->mutex); + if (mon && mon->outbuf_index != 0 && !mon->mux_out) { qemu_chr_fe_write(mon->chr, mon->outbuf, mon->outbuf_index); mon->outbuf_index = 0; } + + qemu_mutex_unlock(&mon->mutex); } /* flush at every end of line or if the buffer is full */ @@ -257,6 +263,8 @@ static void monitor_puts(Monitor *mon, const char *str) { char c; + qemu_mutex_lock(&mon->mutex); + for(;;) { c = *str++; if (c == '\0') @@ -265,9 +273,14 @@ static void monitor_puts(Monitor *mon, const char *str) mon->outbuf[mon->outbuf_index++] = '\r'; mon->outbuf[mon->outbuf_index++] = c; if (mon->outbuf_index >= (sizeof(mon->outbuf) - 1) - || c == '\n') + || c == '\n') { + qemu_mutex_unlock(&mon->mutex); monitor_flush(mon); + qemu_mutex_lock(&mon->mutex); + } } + + qemu_mutex_unlock(&mon->mutex); } void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap) @@ -5275,6 +5288,7 @@ void monitor_init(CharDriverState *chr, int flags) mon = g_malloc0(sizeof(*mon)); + qemu_mutex_init(&mon->mutex); mon->chr = chr; mon->flags = flags; if (flags & MONITOR_USE_READLINE) {