Re: [Qemu-devel] [PATCH 0/3] Add new utility function memory_region_allocate_aux_memory()

[Peter Maydell]

On 7 July 2017 at 14:18, Igor Mammedov <address@hidden> wrote:
> On Thu, 6 Jul 2017 17:26:10 +0100
> Peter Maydell <address@hidden> wrote:
>>  * globally rename memory_region_init_ram to memory_region_init_ram_nomigrate
>>    (and document that it is the caller's job to arrange to migrate the RAM)
> it would be a bit weird when MR created as _nomigrate()
> is then registered with vmstate_register_ram() and being migrated.
>
> maybe other way around, places that don't wanna to use explicit
> vmstate_register_ram() could replace 2 calls with helper 
> memory_region_init_ram_automigrate()

I think this is the wrong way round, because it leads to lots of
bugs. Basically it's really not obvious that you (as the author
of board or device code) need to also call a second function
to not just create a RAM region but also make it work correctly
under migration, and migration is often not tested. Working
through the automatic renames here I've found a small pile of
cases where the memory region was just not registered for
migration at all, or where it was registered using _global()
rather than passing the device as owner (which means the device
can only be created once).

Conversely, the cases which really really want to handle the
migration of the contents of the RAM MemoryRegion themselves
are very few in number, the authors of that code generally
know what they're doing, and the fact that they call a function
named foo_nomigrate() provides them with a helpful clue that
they need to think about and handle migration themselves.

This falls I think under item 7 ("the obvious use is (probably)
the correct one") in Rusty Russell's set of hard-to-misuse API
design guidelines:
http://ozlabs.org/~rusty/index.cgi/tech/2008-03-30.html

thanks
-- PMM