* Re: [Qemu-devel] [PATCH RESEND] migration: catch unknown flags in ram_load
[not found] <1401485884-25500-1-git-send-email-pl@kamp.de>
@ 2014-06-03 17:27 ` Dr. David Alan Gilbert
2014-06-06 12:25 ` Amit Shah
2014-06-10 7:31 ` Amit Shah
0 siblings, 2 replies; 4+ messages in thread
From: Dr. David Alan Gilbert @ 2014-06-03 17:27 UTC (permalink / raw)
To: Peter Lieven; +Cc: pbonzini, qemu-stable, qemu-devel, quintela
* 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.
Yes, I think that's quite nice - the only thing I'd add which would help
a little is an error_report() in the case where you have invalid flags
to let you know that's the reason for the EINVAL.
(Having spent the day tracking down migration bugs...)
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>
> CC: qemu-stable@nongnu.org
> Signed-off-by: Peter Lieven <pl@kamp.de>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
> arch_init.c | 35 ++++++++++++++++-------------------
> 1 file changed, 16 insertions(+), 19 deletions(-)
>
> diff --git a/arch_init.c b/arch_init.c
> index 995f56d..322d129 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -1030,17 +1030,15 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
> {
> ram_addr_t addr;
> int flags, ret = 0;
> - int error;
> static uint64_t seq_iter;
>
> seq_iter++;
>
> if (version_id != 4) {
> ret = -EINVAL;
> - goto done;
> }
>
> - do {
> + while (!ret) {
> addr = qemu_get_be64(f);
>
> flags = addr & ~TARGET_PAGE_MASK;
> @@ -1069,7 +1067,6 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
> " in != " RAM_ADDR_FMT "\n", id, length,
> block->length);
> ret = -EINVAL;
> - goto done;
> }
> break;
> }
> @@ -1079,21 +1076,21 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
> fprintf(stderr, "Unknown ramblock \"%s\", cannot "
> "accept migration\n", id);
> ret = -EINVAL;
> - goto done;
> + }
> + if (ret) {
> + break;
> }
>
> total_ram_bytes -= length;
> }
> - }
> -
> - if (flags & RAM_SAVE_FLAG_COMPRESS) {
> + } else if (flags & RAM_SAVE_FLAG_COMPRESS) {
> void *host;
> uint8_t ch;
>
> host = host_from_stream_offset(f, addr, flags);
> if (!host) {
> ret = -EINVAL;
> - goto done;
> + break;
> }
>
> ch = qemu_get_byte(f);
> @@ -1104,7 +1101,7 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
> host = host_from_stream_offset(f, addr, flags);
> if (!host) {
> ret = -EINVAL;
> - goto done;
> + break;
> }
>
> qemu_get_buffer(f, host, TARGET_PAGE_SIZE);
> @@ -1112,24 +1109,24 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
> void *host = host_from_stream_offset(f, addr, flags);
> if (!host) {
> ret = -EINVAL;
> - goto done;
> + break;
> }
>
> if (load_xbzrle(f, addr, host) < 0) {
> ret = -EINVAL;
> - goto done;
> + break;
> }
> } else if (flags & RAM_SAVE_FLAG_HOOK) {
> ram_control_load_hook(f, flags);
> + } else if (flags & RAM_SAVE_FLAG_EOS) {
> + break;
> + } else {
> + ret = -EINVAL;
> + break;
> }
> - error = qemu_file_get_error(f);
> - if (error) {
> - ret = error;
> - goto done;
> - }
> - } while (!(flags & RAM_SAVE_FLAG_EOS));
> + ret = qemu_file_get_error(f);
> + }
>
> -done:
> DPRINTF("Completed load of VM with exit code %d seq iteration "
> "%" PRIu64 "\n", ret, seq_iter);
> return ret;
> --
> 1.7.9.5
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND] migration: catch unknown flags in ram_load
2014-06-03 17:27 ` [Qemu-devel] [PATCH RESEND] migration: catch unknown flags in ram_load Dr. David Alan Gilbert
@ 2014-06-06 12:25 ` Amit Shah
2014-06-10 7:31 ` Amit Shah
1 sibling, 0 replies; 4+ messages in thread
From: Amit Shah @ 2014-06-06 12:25 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: pbonzini, quintela, Peter Lieven, qemu-stable, qemu-devel
On (Tue) 03 Jun 2014 [18:27:15], Dr. David Alan Gilbert wrote:
> * 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.
>
> Yes, I think that's quite nice - the only thing I'd add which would help
> a little is an error_report() in the case where you have invalid flags
> to let you know that's the reason for the EINVAL.
Agreed.
Amit
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND] migration: catch unknown flags in ram_load
2014-06-03 17:27 ` [Qemu-devel] [PATCH RESEND] migration: catch unknown flags in ram_load Dr. David Alan Gilbert
2014-06-06 12:25 ` Amit Shah
@ 2014-06-10 7:31 ` Amit Shah
2014-06-10 8:04 ` Peter Lieven
1 sibling, 1 reply; 4+ messages in thread
From: Amit Shah @ 2014-06-10 7:31 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: pbonzini, quintela, Peter Lieven, qemu-stable, qemu-devel
On (Tue) 03 Jun 2014 [18:27:15], Dr. David Alan Gilbert wrote:
> * 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.
>
> Yes, I think that's quite nice - the only thing I'd add which would help
> a little is an error_report() in the case where you have invalid flags
> to let you know that's the reason for the EINVAL.
>
> (Having spent the day tracking down migration bugs...)
Peter, can you please add these before we pick the patch up?
Thanks,
Amit
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH RESEND] migration: catch unknown flags in ram_load
2014-06-10 7:31 ` Amit Shah
@ 2014-06-10 8:04 ` Peter Lieven
0 siblings, 0 replies; 4+ messages in thread
From: Peter Lieven @ 2014-06-10 8:04 UTC (permalink / raw)
To: Amit Shah, Dr. David Alan Gilbert
Cc: pbonzini, quintela, qemu-stable, qemu-devel
On 10.06.2014 09:31, Amit Shah wrote:
> On (Tue) 03 Jun 2014 [18:27:15], Dr. David Alan Gilbert wrote:
>> * 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.
>> Yes, I think that's quite nice - the only thing I'd add which would help
>> a little is an error_report() in the case where you have invalid flags
>> to let you know that's the reason for the EINVAL.
>>
>> (Having spent the day tracking down migration bugs...)
> Peter, can you please add these before we pick the patch up?
On its way.
Peter
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-06-10 8:05 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <1401485884-25500-1-git-send-email-pl@kamp.de>
2014-06-03 17:27 ` [Qemu-devel] [PATCH RESEND] migration: catch unknown flags in ram_load Dr. David Alan Gilbert
2014-06-06 12:25 ` Amit Shah
2014-06-10 7:31 ` Amit Shah
2014-06-10 8:04 ` Peter Lieven
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.