On 25.09.2017 19:59, Eduardo Habkost wrote: > On Mon, Sep 25, 2017 at 07:42:13PM +0200, Thomas Huth wrote: >> On 25.09.2017 17:26, Peter Maydell wrote: >>> On 25 September 2017 at 16:19, Thomas Huth <th...@redhat.com> wrote: >>>> Not sure whether this works for the virtio-xxx-device devices, >>>> though, since they are marked as user_creatable = true currently... >>> >>> That's deliberate -- for the arm boards with virtio-mmio >>> the board creates a bunch of virtio-mmio transports and the >>> virtio-foo-device can be user created to plug into those. >> >> Yes, I know ... I'm just wondering whether the virtio-xxx-device devices >> should be non-user_creatable on the non-ARM targets, since they >> apparently can't be used with "-device" there...? > > I wouldn't like to make DeviceClass fields depend on the target. > Being user-creatable doesn't mean they will work on all machines, > anyway. If user/management need more specific info, they need > something like the query-device-slots command I've proposed some > time ago. > >> >>> If overall trying to do the 'right thing' is tricky here >>> then perhaps we can start by flipping the default to >>> not-hotpluggable and marking some devices hotpluggable >>> that in theory shouldn't be with a comment about why. >> >> Yes, if Eduardo's idea to move the test to qmp_device_add() does not >> work out (I'll try that next), your suggestion is certainly the best >> thing we can do right now. > > I think it would work, but we would lose the feature Peter > mentions below: > >> >>> Incidentally I think there's still some advantage in the >>> "created as part of some other device" devices having >>> to be explicitly marked hotpluggable, because those >>> devices do still need some specific code in them to >>> handle it (ie code to release the resources that are >>> created in the device's realize method). >> >> Ok ... by the way, does anybody know more devices like >> virtio-xxx-device, i.e. devices that are indirectly plugged when adding >> other devices? > > "make check" found a few candidates: > https://travis-ci.org/ehabkost/qemu/jobs/278743999 > > Initialization of device dpcd failed: Device 'dpcd' does not support > hotplugging > [...] > Initialization of device nand failed: Device 'nand' does not support > hotplugging
I've now had a look at those, and I now feel like the whole "hotpluggable = false by default" idea was either just wrong or there are other smart ideas necessary to solve this issue: These devices are created during the instance_init() function of another device, e.g. "dpcd" is created in the xlnx_dp_init() function, which is the instance_init of TYPE_XLNX_DP. Now, instance_init() can be called at any time during runtime, even without really adding the device, e.g. for parameter introspection (try "device_add xlnx.v-dp,help" at the HMP prompt for example). But just setting "hotpluggable = true" for a device (e.g. "dpcd") which could be created during instance_init also does not sound very attractive to me... Not sure about any good alternative way to tackle this right now, maybe I've eventually got to check user_creatable in device_set_realized, too, or move the hotpluggable checks to qmp_device_add() instead... I'll think about that for a while... suggestions welcome! Thomas
