Il 22/09/2014 13:43, Markus Armbruster ha scritto: > Paolo Bonzini <pbonz...@redhat.com> writes: > >> Il 22/09/2014 10:34, Michael S. Tsirkin ha scritto: >>> Basically this patch brings back historical HMP behaviour. >>> As far as I could tell, it wasn't changed intentionally. >> >> It was changed intentionally. Or rather, the change was known at the >> time Stefan made it. > > Commit hash?
There are three commits involved. The first is commit f4eb32b (qmp: show QOM properties in device-list-properties, 2014-05-20). It was done in preparation for the change of virtio-blk-pci.drive from qdev property to QOM property, to avoid breaking device-list-properties. The actual change took some more time to review, so it went in one month later. Commit caffdac (virtio-blk: use aliases instead of duplicate qdev properties, 2014-06-18) is what changed virtio-blk-pci.drive from qdev property to QOM property. This changed the type from 'drive' to 'str' in device-list-properties. (Note that this patch fixed an actual bug, it was not just for the sake of cleanup). I cannot find any reference to the change; perhaps it was discussed only on IRC. However, I'm quite certain that I knew about it. Now one thing did slip through; commit caffdac actually dropped virtio-blk-pci.drive from -device virtio-blk-pci,help. Either I misremembered that the first commit fixed command-line help too, or I just assumed that -device foo,help was implemented on top of qmp_device_list_properties. Which wasn't the case, so the third commit (9ef52358, qdev-monitor: include QOM properties in -device FOO, help output, 2014-07-09) did the right thing and brought back virtio-blk-pci.drive into the help message. Now, the cover latter for that patch finally has a hint that the change was known. http://thread.gmane.org/gmane.comp.emulators.qemu/285819 has this text: We simply need to update -device FOO,help code to use both qdev and QOM properties. Note that types change because a 'drive' qdev type is actually a 'str' QOM type. We're moving more and more to QOM properties where the final type for this property would be 'link<Drive>' or similar. It is only in the cover letter and thus not part of any commit message. > I haven't got this part of the code present in my mind at the moment, > but I'm willing to take your assertion of a layering violation for > granted. Is this violation necessary for fixing the regression, or is > it just an artifact of this particular fix? I guess we could always add some ad-hoc mechanism for device-list-properties to get to the "drive" string, and smuggle it as a QOM feature. Then, aliases would just forward that mechanism to the aliased property, which would just work. Of course, with every new feature we would most likely have yet another unfinished transition. In the lack of a clear user complaint (or even of a clear indication that human users ever used -device foo,help...) the tempation to pass the buck is strong. Paolo