On Thu, 29 Nov 2018 at 19:03, Markus Armbruster <arm...@redhat.com> wrote: > > Li Qiang <liq...@163.com> writes: > > > Currently the dc390 device has no unrealize function. This > > can cause memory leak when hotplug/unplug device. Also more > > serious, it will trigger an assert when rehotplug. > [...] > > When a hot-pluggable device doesn't have an ->unrealize() method, unplug > is probably broken. I think we should track down such devices for > review. Any takers?
Add an assert somewhere and catch it with the usual "instantiate everything" qtest? More generally, what is causing dc390 to be hotpluggable? I can't see anything obvious in the class init. If we have APIs where the default is "you get this weird thing you weren't thinking about but it's broken because you weren't thinking about it" then we will have a whole class of bugs that we then need to squash device by device[*]. I think it would be better for devices to have to explicitly opt in to implementing things like hotplug -- that way the failure mode is just "this isn't hotpluggable", we can report that helpfully to the user, and if anybody cares they can add the support. [*] Also currently true for migration support. We should require devices to either provide a VMState or explicitly say they have no state needing migration or explicitly say they don't support migration, and then assert that they do one or the other of those, rather than having the default be "we'll allow migration but not migrate any of the device's state". thanks -- PMM
