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

[Juan Quintela <quintela@redhat.com>]

Peter Lieven <pl@kamp.de> 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: qemu-stable@nongnu.org
> Signed-off-by: Peter Lieven <pl@kamp.de>

.....

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) {
        }


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?

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?

PD. No, I haven't investigated right now how RAM_SAVE_FLAG_HOOK works
with all of this.

Later, Juan.