From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:56086) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QEZx0-0003fL-Ur for qemu-devel@nongnu.org; Tue, 26 Apr 2011 00:32:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QEZx0-0002MH-1L for qemu-devel@nongnu.org; Tue, 26 Apr 2011 00:32:38 -0400 Received: from mx1.redhat.com ([209.132.183.28]:8261) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QEZwz-0002MC-OQ for qemu-devel@nongnu.org; Tue, 26 Apr 2011 00:32:37 -0400 Date: Tue, 26 Apr 2011 10:02:21 +0530 From: Amit Shah Message-ID: <20110426043221.GC12171@amit-x200.redhat.com> References: <20110422125943.2F24E6FC039@msa105.auone-net.jp> <20110425095720.GA8308@amit-x200.redhat.com> <20110425153028.BEC2C6FC03B@msa105.auone-net.jp> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20110425153028.BEC2C6FC03B@msa105.auone-net.jp> Subject: Re: [Qemu-devel] [PATCH] char: Allow devices to use a single multiplexed chardev. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kusanagi Kouichi Cc: qemu-devel@nongnu.org On (Tue) 26 Apr 2011 [00:30:28], Kusanagi Kouichi wrote: > On 2011-04-25 15:27:20 +0530, Amit Shah wrote: > > On (Fri) 22 Apr 2011 [21:59:42], Kusanagi Kouichi wrote: > > > This fixes regression caused by commit > > > 2d6c1ef40f3678ab47a4d14fb5dadaa486bfcda6 > > > ("char: Prevent multiple devices opening same chardev"). > > > > What's the regression? How do I test it? > > > > -nodefaults -nographic -chardev stdio,id=stdio,mux=on,signal=off -mon stdio -device virtio-serial-pci -device virtconsole,chardev=stdio -device isa-serial,chardev=stdio > fails with > qemu-system-x86_64: -device isa-serial,chardev=stdio: Property 'isa-serial.chardev' can't take value 'stdio', it's in use OK, so please mention it in the commit message :-) > Is this intended? No, it's not. Just one more thing: > > > diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c > > > index 1088a26..0eed712 100644 > > > --- a/hw/qdev-properties.c > > > +++ b/hw/qdev-properties.c > > > @@ -354,10 +354,10 @@ static int parse_chr(DeviceState *dev, Property *prop, const char *str) > > > if (*ptr == NULL) { > > > return -ENOENT; > > > } > > > - if ((*ptr)->assigned) { > > > + if ((*ptr)->avail < 1) { > > > return -EEXIST; > > > } > > > - (*ptr)->assigned = 1; > > > + --(*ptr)->avail; > > > return 0; > > > } 'avail' isn't readily intuitive. Can you use a better name, like 'avail_connections' or something like that? Please CC me on the updated patch. Amit