Re: [Qemu-devel] [PATCH 04/14] qdev: don't allow globals to be set by bus name

[Anthony Liguori]

On 05/01/2012 04:47 PM, Paolo Bonzini wrote:
> Il 01/05/2012 22:46, Anthony Liguori ha scritto:
> > > > So I think we can safely break it :-)
> > >
> > > Does this work with compat properties set on a bus?
> >
> > No :-(
> >
> > This is pretty easy to fix though.  The attached should do the trick,
> > I'll update and send out.
> >
> >    Even if it does,
> > > perhaps it's better to avoid the cleverness and wait for my series that
> > > moves bus properties to the appropriate abstract superclass.
> >
> > This series does that FWIW (patch 6/14).
>
> Yeah, it should---modulo bisectability of course.
>
> It's a fairly different approach WRT my series (using
> qdev_add_properties instead of klass->props).  In theory I like it, but
> I fail to see right now whether it breaks "-device foo,?" somehow.  I
> think it does:
>
> $ qemu-system-x86_64 -device rtl8139,?
> rtl8139.mac=macaddr
> rtl8139.vlan=vlan
> rtl8139.netdev=netdev
> rtl8139.bootindex=int32
> rtl8139.addr=pci-devfn<<<  here start bus props
> rtl8139.romfile=string
> rtl8139.rombar=uint32
> rtl8139.multifunction=on/off
> rtl8139.command_serr_enable=on/off
>
> I think it's too late for this series to go into 1.1.

No, this all works as expected (well, almost):

$ qemu-system-x86_64 -device rtl8139,?
rtl8139.vlan=vlan
rtl8139.addr=pci-devfn
rtl8139.romfile=string
rtl8139.multifunction=on/off
rtl8139.command_serr_enable=on/off

But there is stuff missing...

The problem is that qdev_device_help() only prints out legacy properties and 
munges them to look the way they used to.  But when you refactored some of the 
legacy types to be non-legacy, it meant that the legacy- version disappeared and 
those options disappeared from -device <type>,?

So we need to fix this either way.  I think the simple solution is to store 
property info in a list instead of directly printing it.  Then we can walk the 
list and remove non-legacy properties for given legacy properties and munge the 
names in place.

But this is true even with master (but much, much worse):

address@hidden:~/build/qemu$ x86_64-softmmu/qemu-system-x86_64 -device rtl8139,?
<there is no output>

This is because we only look at DeviceClass::props which only contains what 
happens to only contain static properties for rtl8139.  We totally ignore static 
properties in current master.

So my series makes the situation better and I think it's easier to fix the full 
problem.  I also don't view the current bug as a -rc0 blocker (although it's 
obviously a release blocker).  I can send a proper patch later in the week but 
I'd still like to commit the bus changes before -rc0.

We have a month before 1.1.  I think that's plenty of time to address any fall 
out here..

Regards,

Anthony Liguori

> Paolo