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):

anthony@titi:~/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



Reply via email to