Il 05/12/2013 12:20, Markus Armbruster ha scritto: > Paolo Bonzini <pbonz...@redhat.com> 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
