On 23/07/2016 15:18, Corey Minyard wrote: > On 07/23/2016 02:46 AM, Paolo Bonzini wrote: >> >> On 22/07/2016 21:50, miny...@acm.org wrote: >>> This has kind of opened a can of worms for me, though. Looking >>> at a lot of the devices, there is no unrealize function and that >>> can leave a lot of things hanging. And for ISA bus devices, there >>> is no way to unregister ports. >> Right, this is because they aren't hotpluggable. >> >> I should dig out the huge patchset I had to make timers statically >> allocated... >> >> Paolo > Am I correct in saying, then, that instead of adding a finalize > function to the IPMI BMC, we should instead make it not hot > pluggable? And then the rest of my patches are not really > relevant. I already have a function to set hotpluggable to > false for the BMCs, I can post that.
If they are ISA devices they should already not be hot-unpluggable, because none of the ISA bridges implements HotplugHandler. Because that's just the way the bus works, it shouldn't be an issue. > From what I have seen, you can unrealize devices using the > API, even if they are not hot pluggable, by setting the realized > bool. Is that ok? It's not great, but it's not a big deal either. The original idea behind "realize" was to have it as a sort of Vcc pin where a false/true pulse would work as a reset, but this never materialized. Now the true->false transition on realize is really only used as part of a full guest-triggered hot-unplug sequence, which is guest->hotplug_handler_unplug->(method call)->object_unparent. Because all HotplugHandlers call object_unparent, which in turn ends up freeing the object, a false->true->false transition on realized (and thus the timer leak) is not guest-triggerable. There are various fixes, including: - making the device non-hotpluggable - moving the timer_new and timer_free respectively to instance_init and instance_finalize - making the timer static, which requires some small changes in the timer API. Most of the last bullet is scriptable with Coccinelle. Right now I'd just do #2 or don't bother. Paolo
