On 8 June 2017 at 08:54, KONRAD Frederic <fred.kon...@greensocs.com> wrote: > Le 06/06/2017 à 17:18, Peter Maydell a écrit : >> Some top-level review:
>> * qemu_clk_device_add_clock() does a g_strdup, but when is this freed? >> (consider devices which are hot-unpluggable) >> * similarly, what if a device with a clock with a lot of child nodes >> is destroyed? how are the ClkList structs freed? >> * interaction with migration -- how is the "this clock is at this rate" >> state intended to be migrated? > Nice thanks for the feedback! > > Actually the hot unpluggable is a good question what can we do for that? > Is that reasonable for a device which produce a clock to be hot > unpluggable? I don't know if it's very likely (but you might have a PCI device for instance which uses clocks internally). But I think for a bit of low-level infrastructure like clocktree it's worth getting the details of the lifecycle right from the start. > For the migration maybe we can refresh the whole clock tree at the end > of the migration. Is that a good idea? That seems kind of awkward -- where would this code that did a clock tree refresh be? Also you're then reliant on all the callback functions registered to not actually do anything that affects device state when the refresh happens. It would be cleaner if the qemu-clk objects managed their own internal state migration (but can we do this without having to make them be Device objects rather than just objects ?). Cc'd Paolo who might have an opinion on these -- my opinion currently is mostly "this doesn't really look right" rather than knowing what the right approach is. thanks -- PMM
