Re: [PATCH 1/4] migration/savevm: Add a compatibility check for capabilities

[Fabiano Rosas <farosas@suse.de>]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Thu, Mar 27, 2025 at 11:39:31AM -0300, Fabiano Rosas wrote:
>> It has always been possible to enable arbitrary migration capabilities
>> and attempt to take a snapshot of the VM with the savevm/loadvm
>> commands as well as their QMP counterparts
>> snapshot-save/snapshot-load.
>> 
>> Most migration capabilities are not meant to be used with snapshots
>> and there's a risk of crashing QEMU or producing incorrect
>> behavior. Ideally, every migration capability would either be
>> implemented for savevm or explicitly rejected.
>
> IMHO, this a prime example of why migration config shouldn't be held
> as global state, and instead passed as parameters to the commands
> that need them.  The snapshot-save/load commands would then only
> be able to accept what few settings are actually relevant, instead
> of inheriting any/all global migration state.
>

Right, I remember we got caught around the fact that some migration
options are needed during runtime as well... but I don't remember the
details, let try to find that thread.

>> Add a compatibility check routine and reject the snapshot command if
>> an incompatible capability is enabled. For now only act on the the two
>> that actually cause a crash: multifd and mapped-ram.
>> 
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2818
>
> Issue is 2881 not 2818                                   ^^^^^^^
>

It seem that was your own C-t =)

>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>> ---
>>  migration/options.c | 26 ++++++++++++++++++++++++++
>>  migration/options.h |  1 +
>>  migration/savevm.c  |  8 ++++++++
>>  3 files changed, 35 insertions(+)
>
> With regards,
> Daniel