[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 6/6] migration: catch unknown flags in ram_load
From: |
Peter Lieven |
Subject: |
Re: [Qemu-devel] [PATCH 6/6] migration: catch unknown flags in ram_load |
Date: |
Tue, 17 Jun 2014 11:31:27 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.5.0 |
On 16.06.2014 19:31, Eric Blake wrote:
On 06/15/2014 09:15 PM, Juan Quintela wrote:
From: Peter Lieven <address@hidden>
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.
This patch is a strict improvement, so I'm glad it went in. However, I
still feel that we aren't doing a good job of silently ignoring
unexpected combinations of flag bits, and had suggestions in the
original thread on further followups that are worth having before the
2.1 release.
-
- if (flags & RAM_SAVE_FLAG_COMPRESS) {
+ } else if (flags & RAM_SAVE_FLAG_COMPRESS) {
void *host;
Among other things, switching from a chain of if-else to a switch might
make it easier to document explicit supported combinations of flags and
reject other values from an invalid stream.
Is this what you have in mind?
diff --git a/arch_init.c b/arch_init.c
index 8ddaf35..925cc66 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -1039,7 +1039,7 @@ void ram_handle_compressed(void *host, uint8_t ch,
uint64_t size)
static int ram_load(QEMUFile *f, void *opaque, int version_id)
{
ram_addr_t addr;
- int flags, ret = 0;
+ int flags = 0, ret = 0;
static uint64_t seq_iter;
seq_iter++;
@@ -1048,97 +1048,96 @@ static int ram_load(QEMUFile *f, void *opaque, int
version_id)
ret = -EINVAL;
}
- while (!ret) {
+ while (!ret && !(flags & RAM_SAVE_FLAG_EOS)) {
addr = qemu_get_be64(f);
flags = addr & ~TARGET_PAGE_MASK;
addr &= TARGET_PAGE_MASK;
- if (flags & RAM_SAVE_FLAG_MEM_SIZE) {
- /* Synchronize RAM block list */
- char id[256];
- ram_addr_t length;
- ram_addr_t total_ram_bytes = addr;
-
- while (total_ram_bytes) {
- RAMBlock *block;
- uint8_t len;
-
- len = qemu_get_byte(f);
- qemu_get_buffer(f, (uint8_t *)id, len);
- id[len] = 0;
- length = qemu_get_be64(f);
-
- QTAILQ_FOREACH(block, &ram_list.blocks, next) {
- if (!strncmp(id, block->idstr, sizeof(id))) {
- if (block->length != length) {
- error_report("Length mismatch: %s: " RAM_ADDR_FMT
- " in != " RAM_ADDR_FMT, id, length,
- block->length);
- ret = -EINVAL;
+ if (flags & RAM_SAVE_FLAG_HOOK) {
+ ram_control_load_hook(f, flags);
+ } else {
+ ram_addr_t total_ram_bytes;
+ void *host;
+ uint8_t ch;
+ switch (flags & ~RAM_SAVE_FLAG_CONTINUE) {
+ case RAM_SAVE_FLAG_MEM_SIZE:
+ /* Synchronize RAM block list */
+ total_ram_bytes = addr;
+ while (total_ram_bytes) {
+ RAMBlock *block;
+ uint8_t len;
+ char id[256];
+ ram_addr_t length;
+
+ len = qemu_get_byte(f);
+ qemu_get_buffer(f, (uint8_t *)id, len);
+ id[len] = 0;
+ length = qemu_get_be64(f);
+
+ QTAILQ_FOREACH(block, &ram_list.blocks, next) {
+ if (!strncmp(id, block->idstr, sizeof(id))) {
+ if (block->length != length) {
+ error_report("Length mismatch: %s: "
RAM_ADDR_FMT
+ " in != " RAM_ADDR_FMT, id,
length,
+ block->length);
+ ret = -EINVAL;
+ }
+ continue;
}
- break;
}
+ if (!block) {
+ error_report("Unknown ramblock \"%s\", cannot "
+ "accept migration", id);
+ ret = -EINVAL;
+ }
+ if (ret) {
+ continue;
+ }
+ total_ram_bytes -= length;
}
-
- if (!block) {
- error_report("Unknown ramblock \"%s\", cannot "
- "accept migration", id);
+ break;
+ case RAM_SAVE_FLAG_COMPRESS:
+ host = host_from_stream_offset(f, addr, flags);
+ if (!host) {
+ error_report("Illegal RAM offset " RAM_ADDR_FMT, addr);
ret = -EINVAL;
+ continue;
}
- if (ret) {
- break;
- }
-
- total_ram_bytes -= length;
- }
- } else if (flags & RAM_SAVE_FLAG_COMPRESS) {
- void *host;
- uint8_t ch;
-
- host = host_from_stream_offset(f, addr, flags);
- if (!host) {
- error_report("Illegal RAM offset " RAM_ADDR_FMT, addr);
- ret = -EINVAL;
+ ch = qemu_get_byte(f);
+ ram_handle_compressed(host, ch, TARGET_PAGE_SIZE);
break;
- }
-
- ch = qemu_get_byte(f);
- ram_handle_compressed(host, ch, TARGET_PAGE_SIZE);
- } else if (flags & RAM_SAVE_FLAG_PAGE) {
- void *host;
-
- host = host_from_stream_offset(f, addr, flags);
- if (!host) {
- error_report("Illegal RAM offset " RAM_ADDR_FMT, addr);
- ret = -EINVAL;
+ case RAM_SAVE_FLAG_PAGE:
+ host = host_from_stream_offset(f, addr, flags);
+ if (!host) {
+ error_report("Illegal RAM offset " RAM_ADDR_FMT, addr);
+ ret = -EINVAL;
+ continue;
+ }
+ qemu_get_buffer(f, host, TARGET_PAGE_SIZE);
break;
- }
-
- qemu_get_buffer(f, host, TARGET_PAGE_SIZE);
- } else if (flags & RAM_SAVE_FLAG_XBZRLE) {
- void *host = host_from_stream_offset(f, addr, flags);
- if (!host) {
- error_report("Illegal RAM offset " RAM_ADDR_FMT, addr);
- ret = -EINVAL;
+ case RAM_SAVE_FLAG_XBZRLE:
+ host = host_from_stream_offset(f, addr, flags);
+ if (!host) {
+ error_report("Illegal RAM offset " RAM_ADDR_FMT, addr);
+ ret = -EINVAL;
+ continue;
+ }
+ if (load_xbzrle(f, addr, host) < 0) {
+ error_report("Failed to decompress XBZRLE page at "
+ RAM_ADDR_FMT, addr);
+ ret = -EINVAL;
+ continue;
+ }
break;
- }
-
- if (load_xbzrle(f, addr, host) < 0) {
- error_report("Failed to decompress XBZRLE page at "
- RAM_ADDR_FMT, addr);
+ case RAM_SAVE_FLAG_EOS:
+ /* normal exit */
+ continue;
+ default:
+ error_report("Unknown migration flags: %#x", flags);
ret = -EINVAL;
- break;
+ continue;
}
- } else if (flags & RAM_SAVE_FLAG_HOOK) {
- ram_control_load_hook(f, flags);
- } else if (flags & RAM_SAVE_FLAG_EOS) {
- /* normal exit */
- break;
- } else {
- error_report("Unknown migration flags: %#x", flags);
- ret = -EINVAL;
- break;
}
ret = qemu_file_get_error(f);
}
- [Qemu-devel] [PULL 0/6] migration queue, Juan Quintela, 2014/06/15
- [Qemu-devel] [PATCH 1/6] savevm: Remove all the unneeded version_minimum_id_old (ppc), Juan Quintela, 2014/06/15
- [Qemu-devel] [PATCH 2/6] savevm: Remove all the unneeded version_minimum_id_old (x86), Juan Quintela, 2014/06/15
- [Qemu-devel] [PATCH 4/6] migration: Increase default max_downtime from 30ms to 300ms, Juan Quintela, 2014/06/15
- [Qemu-devel] [PATCH 3/6] vmstate: Refactor opening of files, Juan Quintela, 2014/06/15
- [Qemu-devel] [PATCH 6/6] migration: catch unknown flags in ram_load, Juan Quintela, 2014/06/15
- [Qemu-devel] [PATCH 5/6] rdma: Fix block during rdma migration, Juan Quintela, 2014/06/15
- Re: [Qemu-devel] [PULL 0/6] migration queue, Peter Maydell, 2014/06/16