From patchwork Fri Mar 22 21:39:04 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Luiz Capitulino X-Patchwork-Id: 230268 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 9968F2C00AA for ; Sat, 23 Mar 2013 08:39:31 +1100 (EST) Received: from localhost ([::1]:42288 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UJ9gP-0006lD-SX for incoming@patchwork.ozlabs.org; Fri, 22 Mar 2013 17:39:29 -0400 Received: from eggs.gnu.org ([208.118.235.92]:35576) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UJ9g7-0006kR-EB for qemu-devel@nongnu.org; Fri, 22 Mar 2013 17:39:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UJ9g6-0006ZS-4y for qemu-devel@nongnu.org; Fri, 22 Mar 2013 17:39:11 -0400 Received: from mx1.redhat.com ([209.132.183.28]:11926) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UJ9g5-0006ZM-S2 for qemu-devel@nongnu.org; Fri, 22 Mar 2013 17:39:10 -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 r2MLd7WT019649 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 22 Mar 2013 17:39:07 -0400 Received: from doriath (ovpn-113-108.phx2.redhat.com [10.3.113.108]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r2MLd51n023711; Fri, 22 Mar 2013 17:39:05 -0400 Date: Fri, 22 Mar 2013 17:39:04 -0400 From: Luiz Capitulino To: Luiz Capitulino Message-ID: <20130322173904.66d2f5ce@doriath> In-Reply-To: <20130322165039.32aae1fb@doriath> References: <514C21C6.3070800@greensocs.com> <20130322165039.32aae1fb@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: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: Anthony Liguori , kraxel@redhat.com, qemu-devel , KONRAD =?UTF-8?B?RnLDqWTDqXJpYw==?= Subject: Re: [Qemu-devel] Abort in monitor_puts. 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 On Fri, 22 Mar 2013 16:50:39 -0400 Luiz Capitulino wrote: > On Fri, 22 Mar 2013 10:17:58 +0100 > KONRAD Frédéric wrote: > > > Hi, > > > > Seems there is an issue with the current git (found by toddf on IRC). > > > > To reproduce: > > > > ./qemu-system-x86_64 --monitor stdio --nographic > > > > and put "?" it should abort. > > > > Here is the backtrace: > > > > #0 0x00007f77cd347935 in raise () from /lib64/libc.so.6 > > #1 0x00007f77cd3490e8 in abort () from /lib64/libc.so.6 > > #2 0x00007f77cd3406a2 in __assert_fail_base () from /lib64/libc.so.6 > > #3 0x00007f77cd340752 in __assert_fail () from /lib64/libc.so.6 > > #4 0x00007f77d1c1f226 in monitor_puts (mon=, > > str=) at > > Yes, it's easy to reproduce. Bisect says: > > f628926bb423fa8a7e0b114511400ea9df38b76a is the first bad commit > commit f628926bb423fa8a7e0b114511400ea9df38b76a > Author: Gerd Hoffmann > Date: Tue Mar 19 10:57:56 2013 +0100 > > fix monitor > > chardev flow control broke monitor, fix it by adding watch support. > > Signed-off-by: Anthony Liguori > > My impression is that monitor_puts() in being called in parallel. Not all. What's happening is that qemu_chr_fe_write() is returning < 0, mon->outbuf_index is not reset and is full, this causes the assert in monitor_puts() to trig. The previous version of monitor_flush() ignores errors, and everything works, so doing the same thing here fixes the problem :) For some reason I'm unable to see what the error code is. Gerd, do you think the patch below is reasonable? If it's not, how should we handle errors here? diff --git a/monitor.c b/monitor.c index cfb5d64..ecfe97c 100644 --- a/monitor.c +++ b/monitor.c @@ -274,12 +274,11 @@ void monitor_flush(Monitor *mon) if (mon && mon->outbuf_index != 0 && !mon->mux_out) { rc = qemu_chr_fe_write(mon->chr, mon->outbuf, mon->outbuf_index); - if (rc == mon->outbuf_index) { + if (rc == mon->outbuf_index || rc < 0) { /* all flushed */ mon->outbuf_index = 0; return; - } - if (rc > 0) { + } else { /* partinal write */ memmove(mon->outbuf, mon->outbuf + rc, mon->outbuf_index - rc); mon->outbuf_index -= rc;