Re: [Qemu-devel] [PATCHv4] migration: catch unknown flags in ram_load

[Juan Quintela]

Eric Blake <address@hidden> wrote:
> On 06/10/2014 03:29 AM, Peter Lieven wrote:
>> if a saved vm has unknown flags in the memory data qemu
>> currently simply ignores this flag and continues which
>> yields in an unpredictable result.
>> 
>> This patch catches all unknown flags and aborts the
>> loading of the vm. Additionally error reports are thrown
>> if the migration aborts abnormally.
>> 
>> Signed-off-by: Peter Lieven <address@hidden>
>> ---
>
>> +    while (!ret) {
>>          addr = qemu_get_be64(f);
>>  
>>          flags = addr & ~TARGET_PAGE_MASK;
>
>> -
>> -        if (flags & RAM_SAVE_FLAG_COMPRESS) {
>> +        } else if (flags & RAM_SAVE_FLAG_COMPRESS) {
>>              void *host;
>
>>          } else if (flags & RAM_SAVE_FLAG_XBZRLE) {
>
>>          } else if (flags & RAM_SAVE_FLAG_HOOK) {
>>              ram_control_load_hook(f, flags);
>> +        } else if (flags & RAM_SAVE_FLAG_EOS) {
>
> Umm, is the migration format specifically documented as having at most
> one flag per operation, or is it valid to send two flags at once?  That
> is, can I send RAM_SAVE_FLAG_XBZRLE | RAM_SAVE_FLAG_HOOK on a single
> packet?  Should we be flagging streams that send unexpected flag
> combinations as invalid, even when each flag is in isolation okay,
> rather than the current behavior of silently prioritizing one flag and
> ignoring the other?

Migration Format?  Documented? O:-)

Now, more seriously, current code don't allows for more than one flag to
be enabled at the same time.  RAM_SAVE_FLAG_HOOK is always sent alone by
itself.

And old code just accepted silently unknown flags, this change makes
unknown flags to just fail the migration.  What is the only sensible
thing to do IMHO O:-)

Thanks, Juan.