Re: [Qemu-devel] [PATCH] qdev: Keep global allocation counter per bus

[Paolo Bonzini]

Il 05/12/2013 12:20, Markus Armbruster ha scritto:
> Paolo Bonzini <address@hidden> writes:
> 
>> Il 05/12/2013 10:44, Markus Armbruster ha scritto:
>>> Incompatible change: device ide-cd moved to a different controller.
>>
>> Yes, it should be stated in the commit message but it's expected as
>> discussed yesterday on IRC.  The solution is not to use "-device" (which
>> was broken) if you care about backwards compatibility; use "-drive if=ide".
> 
> -device is broken for the *other* controller.  It works just fine for
> this one.

It is broken in that "-device bus=ide.0" would access the second
controller, the one corresponding to "-drive if=ide,bus=1", or -hdc/-hdd.

>>> Great fun when you try to live migrate across your patch.
>>>
>>> I'd expect isapc to have the same issue once its crash bug is fixed.
>>>
>>> First law of QEMU hacking: if your patch looks simple, it's probably
>>> wrong ;)
>>
>> Yes, the question is how wrong and how the wrong balances the right.
> 
> Is it really too much bother to change the ide.0 name for the
> controllers that bus=ide.0 doesn't use, and keep it for the one it does
> use?

Yes, for two reasons:

(1) practical reason, the one mentioned above: it would mean that
"-device bus=ide.0" corresponds to "-drive if=ide,bus=1" and similarly
for ide.1/bus=0.  So we would make the cure worse than the disease, in
my opinion.  This IMO is a pretty strong sign that the
backwards-compatibility problem doesn't exist and no one ever used
"-device" for built-in devices on anything other than pc IDE and pseries
SCSI.

(2) technical reason: the two are inverted because bus names currently
have a "last wins" policy.  The policy is implemented by using
QTAILQ_INSERT_HEAD in bus_add_child.  So it is not possible to know the
correct bus names unless you know how many buses you will have (e.g. for
3 buses you'd start giving numbers from ide.2 and go down from there).
And implementing this would probably be really really ugly.

> If yes, the incompatible change needs to be documented much more clearly
> in the commit message.

And in the release notes.

Paolo