Re: [Qemu-devel] [PATCH] char: Allow devices to use a single multiplexed chardev.

[Amit Shah <amit.shah@redhat.com>]

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