Re: [Qemu-devel] [PATCH] migration: discard RAMBlocks of type ram_device

[Peter Maydell]

On 12 April 2018 at 14:41, Cédric Le Goater <address@hidden> wrote:
> On 04/12/2018 02:08 PM, Peter Maydell wrote:
>> On 12 April 2018 at 12:53, Dr. David Alan Gilbert <address@hidden> wrote:
>>> * Peter Maydell (address@hidden) wrote:
>>>> David suggested on IRC that we would want a flag on the ramblock
>>>> for "not migratable", because there are other uses for "don't
>>>> migrate this" than just "is this a ram device".
>>>
>>> My original suggestion to your series was with a flag, but I'd forgotten
>>> about that by the time I'd made the suggestion to Cédric.
>>> In your case would just adding an extra term to the
>>> ram_block_is_migratable function work, or do you really need a flag?
>>
>> I don't see how else you would identify the ram block that needs
>> to be skipped. Also I think it's just better design to decouple
>> the decision about "should we migrate this ram block" from the
>> migration code itself, and push it up to the code layer that knows
>> it's creating ram blocks that shouldn't be migrated.
>
> Do you mean adding a new RAMBlock flag RAM_NON_MIGRATABLE
> in exec.c ? That would require to add an extra bool to the
> following functions :
>
>         memory_region_init_ram_ptr()
>         qemu_ram_alloc_from_ptr()
>         qemu_ram_alloc_internal()
>
> Would that be ok ?

Paolo may have an opinion what the API here should be, but
at the MemoryRegion level we already have a mix of functions
memory_region_init_foo_nomigrate() and memory_region_init_foo(),
which at the moment just control whether we call
vmstate_register_ram() or not. Is it possible to hook into that,
so that we default to marking RAMBlocks as not-migrated, and
when vmstate_register_ram() is called we set the flag to
"migrated"?

NB: this probably requires careful thought to make sure we
don't accidentally break migration compat, but it seems like
the cleaner approach. At the moment we do weird things if you
migrate a RAM block that hasn't had its ID string set, IIRC,
so just not migrating those RAM blocks seems like a better
plan than mishandling them. It would also mean that the
vmstate_register/unregister_ram() functions did what they
claim to do based on the function name, I think.

thanks
-- PMM