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

[Peter Lieven]

Am 12.05.2014 12:25, schrieb Peter Lieven:
> Am 12.05.2014 12:19, schrieb Juan Quintela:
>> Peter Lieven <address@hidden> 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.
>>>
>>> CC: address@hidden
>>> Signed-off-by: Peter Lieven <address@hidden>
>> .....
>>
>> Once here, shouldn't be better to do this as:
>>
>> change do {} while ()   for while (true) {}
>>
>>>  
>>> @@ -1121,6 +1119,9 @@ static int ram_load(QEMUFile *f, void *opaque, int 
>>> version_id)
>>>              }
>>>          } else if (flags & RAM_SAVE_FLAG_HOOK) {
>>>              ram_control_load_hook(f, flags);
>>> +        } else if (!(flags & RAM_SAVE_FLAG_EOS)) {
>>> +            ret = -EINVAL;
>>> +            goto done;
>>>          }
>>>          error = qemu_file_get_error(f);
>>>          if (error) {
>>         } else if (flags & RAM_SAVE_FLAG_HOOK) {
>>             ram_control_load_hook(f, flags);
>> +       } else if (flags & RAM_SAVE_FLAG_EOS) {
>> +           break;
>> +       } else {
>> +           ret = -EINVAL;
>> +           goto done;
>>         }
>>           error = qemu_file_get_error(f);
>>           if (error) {

we can also drop the error variable I think and change the loop to

while (!ret) {}

>>         }
>>
>>
>> This way, we are checking RAM_SAVE_FLAG_EOS the same way than any other
>> flag?  And we don't have to duplicate the FLAG_NAME?
> Ok, I will send a v2.
>
>> Unrelated to this patch, all the flags are a bitmap, but really, the
>> ones that can be together are RAM_SAVE_FLAG_CONTINUE and the rest, all
>> the others need to be alone.  I am telling this because we have used
>> already 8 flags, and we are using the low bits of offset to save the
>> flags, we have 10 flags?  Perhaps changing the last flag to mean that
>> the low bits pass to be a counter?
> Some better encoding would indeed be useful. I already thought
> that we might run out of flags soon. We have 11 flags I think,
> but there is not much space left. Reserving the last flag to indicate
> that the lower 10 bits a are counter might be a good option.
>
> Peter
>
>> PD. No, I haven't investigated right now how RAM_SAVE_FLAG_HOOK works
>> with all of this.
>>
>> Later, Juan.
>>