[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCHv4] migration: catch unknown flags in ram_load
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCHv4] migration: catch unknown flags in ram_load |
Date: |
Tue, 10 Jun 2014 14:15:11 +0100 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
* Peter Lieven (address@hidden) wrote:
> On 10.06.2014 15:00, Eric Blake wrote:
> >On 06/10/2014 06:55 AM, Eric Blake 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.
> >>>
> >>> } 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?
> >For that matter, would it be better to change the if-tree into a switch,
> >so that the default case catches unsupported combinations?
> >
> >switch (flags) {
> > ...
> > case RAM_SAVE_FLAG_HOOK: ...
> > case RAM_SAVE_FLAG_EOS: ...
> > default: report unsupported flags value
> >}
> >
> The RAM_SAVE_FLAG_HOOK is the only real flag. It seems that the
> flag value is used at least somewhere in the code of RDMA.
There's also RAM_SAVE_FLAG_CONTINUE that's used as a tweak to
make for smaller headers.
Dave
> For that matter, we could handle the hook separately and everything
> else in the switch statement. This would immediately solve the issue
> of the very restricted space for the flags as we could use everything
> below RAM_SAVE_FLAG_HOOK as counter immediately.
>
> Looking at the code I further see that the hook function is made to return
> an error code which is not checked at the moment.
>
> Peter
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK