[Qemu-devel] inconsistency between device traversal in qdev and legacy

[Qemu-devel] inconsistency between device traversal in qdev and legacy

[Peter Maydell]

If you have a model with more than one USB bus, and you
create a USB device on the command line without specifying
which bus to plug it into, QEMU will choose a different bus
depending on whether you use the legacy "-usbdevice keyboard"
or the qdev "-device usb-kbd".

This is because the legacy option finds the USB bus with
usb_bus_find(), which searches the 'busses' list, which is
populated by usb_bus_new(), with each new bus added to the
tail of the list. The qdev option looks for the USB bus
with qbus_find_recursive(), which walks the qdev tree.
Since qbus_create_inplace() adds each new child bus to
the front of the parent's child_bus list, this means that
qbus_find_recursive() will encounter the last-added bus
first, whereas usb_bus_find() will get the first-added bus.

I assume this is likely to apply to other bus types as well.

Is there anything we can do to fix this inconsistency [*],
or are we tied to the existing enumeration orders in both
cases for compatibility with users with currently-working
command lines or configurations?

[*] the actual code changes are simple enough, obviously

-- PMM

Re: [Qemu-devel] inconsistency between device traversal in qdev and legacy

[Gerd Hoffmann]

   Hi,

> Since qbus_create_inplace() adds each new child bus to
> the front of the parent's child_bus list, this means that
> qbus_find_recursive() will encounter the last-added bus
> first, whereas usb_bus_find() will get the first-added bus.

Quite a while back I've tried to switch qdev from QLIST to QTAILQ 
exactly to allow adding stuff to the tail of the lists(s), because that 
feels more natural to me than the current ordering.  "info qtree" is 
upside-down too ;)

Gave up after resending it one or two times, the forgot about it, wasn't 
*that* important to me.

> Is there anything we can do to fix this inconsistency [*],
> or are we tied to the existing enumeration orders in both
> cases for compatibility with users with currently-working
> command lines or configurations?

Could be we break something.  I think it is unlikely though.  Multiple 
busses of the same type are pretty uncommon, and any examples with 
multiple lsi adapters (for example) advertise explicitly assign devices 
via bus=.  libvirt uses bus= everywhere too.

cheers,
   Gerd

Re: [Qemu-devel] inconsistency between device traversal in qdev and legacy

[Andreas Färber]

Hi,

Am 08.06.2011 um 19:00 schrieb Gerd Hoffmann:

>> Since qbus_create_inplace() adds each new child bus to
>> the front of the parent's child_bus list, this means that
>> qbus_find_recursive() will encounter the last-added bus
>> first, whereas usb_bus_find() will get the first-added bus.
>
> Quite a while back I've tried to switch qdev from QLIST to QTAILQ  
> exactly to allow adding stuff to the tail of the lists(s), because  
> that feels more natural to me than the current ordering.  "info  
> qtree" is upside-down too ;)

>> Is there anything we can do to fix this inconsistency [*],
>> or are we tied to the existing enumeration orders in both
>> cases for compatibility with users with currently-working
>> command lines or configurations?
>
> Could be we break something.  I think it is unlikely though.   
> Multiple busses of the same type are pretty uncommon, and any  
> examples with multiple lsi adapters (for example) advertise  
> explicitly assign devices via bus=.  libvirt uses bus= everywhere too.

 From Peter's description I believe this could be the root cause of  
Rob's reversed -net /dev/ethX devices as well? That supposedly broke  
with 0.14. So for setups earlier than 0.14 reversing would be a  
bugfix, but if people adapted to 0.14 behavior changing it back would  
break things. Difficult call.

Andreas

Re: [Qemu-devel] inconsistency between device traversal in qdev and legacy

[Peter Maydell]

On 8 June 2011 18:37, Andreas Färber <andreas.faerber@web.de> wrote:
> From Peter's description I believe this could be the root cause of Rob's
> reversed -net /dev/ethX devices as well? That supposedly broke with 0.14. So
> for setups earlier than 0.14 reversing would be a bugfix, but if people
> adapted to 0.14 behavior changing it back would break things. Difficult
> call.

OTOH if not many people complained about the 0.13->0.14 transition then
presumably not very many will complain if we put it back? :-)

-- PMM

Re: [Qemu-devel] inconsistency between device traversal in qdev and legacy

[Rob Landley]

On 06/08/2011 12:39 PM, Peter Maydell wrote:
> On 8 June 2011 18:37, Andreas Färber <andreas.faerber@web.de> wrote:
>> From Peter's description I believe this could be the root cause of Rob's
>> reversed -net /dev/ethX devices as well? That supposedly broke with 0.14. So
>> for setups earlier than 0.14 reversing would be a bugfix, but if people
>> adapted to 0.14 behavior changing it back would break things. Difficult
>> call.
> 
> OTOH if not many people complained about the 0.13->0.14 transition then
> presumably not very many will complain if we put it back? :-)
> 
> -- PMM

I suspect most people emulating boards only feed one interface to the
thing, so it probably doesn't come up much.

Either way, I have a workaround now, but consistency would be nice.

Rob

Re: [Qemu-devel] inconsistency between device traversal in qdev and legacy

[Andreas Färber]

Am 08.06.2011 um 19:39 schrieb Peter Maydell:

> On 8 June 2011 18:37, Andreas Färber <andreas.faerber@web.de> wrote:
>> From Peter's description I believe this could be the root cause of  
>> Rob's
>> reversed -net /dev/ethX devices as well? That supposedly broke with  
>> 0.14. So
>> for setups earlier than 0.14 reversing would be a bugfix, but if  
>> people
>> adapted to 0.14 behavior changing it back would break things.  
>> Difficult
>> call.
>
> OTOH if not many people complained about the 0.13->0.14 transition  
> then
> presumably not very many will complain if we put it back? :-)

Well, I'm all in favor of a consistent, logical FIFO order. I see no  
strong reason for O(1) there. :)

Andreas

Re: [Qemu-devel] inconsistency between device traversal in qdev and legacy

[Markus Armbruster]

Gerd Hoffmann <kraxel@redhat.com> writes:

>   Hi,
>
> Peter Maydell <peter.maydell@linaro.org> writes:
> 
>> If you have a model with more than one USB bus, and you
>> create a USB device on the command line without specifying
>> which bus to plug it into, QEMU will choose a different bus
>> depending on whether you use the legacy "-usbdevice keyboard"
>> or the qdev "-device usb-kbd".
>> 
>> This is because the legacy option finds the USB bus with
>> usb_bus_find(), which searches the 'busses' list, which is
>> populated by usb_bus_new(), with each new bus added to the
>> tail of the list. The qdev option looks for the USB bus
>> with qbus_find_recursive(), which walks the qdev tree.
>> Since qbus_create_inplace() adds each new child bus to
>> the front of the parent's child_bus list, this means that
>> qbus_find_recursive() will encounter the last-added bus
>> first, whereas usb_bus_find() will get the first-added bus.

The fact that we duplicate qtree information in a separate list "busses"
either means we've been too lazy to garbage collect busses, or we've
failed to make working with the qtree as easy as it should be.

> Quite a while back I've tried to switch qdev from QLIST to QTAILQ
> exactly to allow adding stuff to the tail of the lists(s), because
> that feels more natural to me than the current ordering.  "info qtree"
> is upside-down too ;)

Has always annoyed me.  Let's bite the bullet and change it.

> Gave up after resending it one or two times, the forgot about it,
> wasn't *that* important to me.

Still got a pointer to the patch?

qdev is too important to continue much longer without a maintainer.

>> I assume this is likely to apply to other bus types as well.
>>
>> Is there anything we can do to fix this inconsistency [*],
>> or are we tied to the existing enumeration orders in both
>> cases for compatibility with users with currently-working
>> command lines or configurations?
>
> Could be we break something.  I think it is unlikely though.  Multiple
> busses of the same type are pretty uncommon, and any examples with
> multiple lsi adapters (for example) advertise explicitly assign
> devices via bus=.  libvirt uses bus= everywhere too.

The sooner we fix up the order, the less stuff we'll break.

>> [*] the actual code changes are simple enough, obviously

Care to cook up a patch?

Re: [Qemu-devel] inconsistency between device traversal in qdev and legacy

[Gerd Hoffmann]

   Hi,

>> Gave up after resending it one or two times, the forgot about it,
>> wasn't *that* important to me.
>
> Still got a pointer to the patch?

Not in my git tree any more.  Google found me this instead:

http://permalink.gmane.org/gmane.comp.emulators.qemu/74360

Looks like I'm not the only one who tried ...

cheers,
   Gerd

Re: [Qemu-devel] inconsistency between device traversal in qdev and legacy

[Peter Maydell]

On 9 June 2011 14:59, Markus Armbruster <armbru@redhat.com> wrote:
> The fact that we duplicate qtree information in a separate list "busses"
> either means we've been too lazy to garbage collect busses, or we've
> failed to make working with the qtree as easy as it should be.

If you get rid of busses you need to update usb-musb.c to
be more qdev-ish (so it has a parent device it can pass to
usb_bus_new()). Otherwise the USB bus it creates won't appear
anywhere (at the moment it at least appears on the busses list
so it's usable with the legacy syntax if not with -device.)

(There's code in the meego/qemu-linaro trees that does this, but
it's a bit tangled up with the other patches at the moment.)

-- PMM