qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-devel] [PATCH] migration/savevm.c: do not fail when len > MAX_VM_C


From: Daniel Henrique Barboza
Subject: [Qemu-devel] [PATCH] migration/savevm.c: do not fail when len > MAX_VM_CMD_PACKAGED_SIZE
Date: Fri, 26 Jan 2018 10:48:45 -0200

MAX_VM_CMD_PACKAGED_SIZE is a constant used in qemu_savevm_send_packaged
and loadvm_handle_cmd_packaged to determine whether a package is too
big to be sent or received. qemu_savevm_send_packaged is called inside
postcopy_start (migration/migration.c) to send the MigrationState
in a single blob to the destination, using the MIG_CMD_PACKAGED subcommand,
which will read it up using loadvm_handle_cmd_packaged. If the blob is
larger than MAX_VM_CMD_PACKAGED_SIZE, an error is thrown and the postcopy
migration is aborted. Both MAX_VM_CMD_PACKAGED_SIZE and MIG_CMD_PACKAGED
were introduced by commit 11cf1d984b ("MIG_CMD_PACKAGED: Send a packaged
chunk ..."). The constant has its original value of 1ul << 24 (16MB).

The current MAX_VM_CMD_PACKAGED_SIZE value is not enough to support postcopy
migration of bigger pseries guests. The blob size for a postcopy migration of
a pseries guest with the following setup:

qemu-system-ppc64 --nographic -vga none -machine pseries,accel=kvm -m 64G \
-smp 1,maxcpus=32 -device virtio-blk-pci,drive=rootdisk \
-drive file=f27.qcow2,if=none,cache=none,format=qcow2,id=rootdisk \
-netdev user,id=u1 -net nic,netdev=u1

goes around 12MB. Bumping the RAM to 128G makes the blob sizes goes to 20MB.
With 256G the blob goes to 37MB - more than twice the current maximum size.
At this moment the pseries machine can handle guests with up to 1TB of RAM,
making this postcopy blob goes to 128MB of size approximately.

One solution is to bump MAX_VM_CMD_PACKAGED_SIZE up to bigger values. A value
of 1ul << 27 would be enough for pseries guests up to 1TB of RAM, but there
are 2 problems with this approach:

- we'll keep supporting bigger and bigger guests as time goes by. This constant
would be bumped from time to time;

- if we're willing to bump the constant every time we need a bigger blob, why
have the constant in the first place? Considering that its current value
is 16MB, bumping it to 128MB already makes it 'unreasonably large' considering
the original design of MIG_CMD_PACKAGED.

A better long term solution is to determine whether the design of
MIG_CMD_PACKAGED can be changed to send partial blobs of smaller sizes or
even get rid of the size limitation.

Until then, this patch changes both qemu_savevm_send_packaged and
loadvm_handle_cmd_packaged to not bail out if the blob len is greater than
MAX_VM_CMD_PACKAGED_SIZE. To not fully ignore the occurrence (something can go
wrong and the MigrationState can inadvertently grow beyond expected), we also
change the traces of both functions to report both the current blob size and the
current recommended maximum. This way we allow big guests to execute postcopy
migration while retaining the information for debug purposes.

Signed-off-by: Daniel Henrique Barboza <address@hidden>
Reported-by: Balamuruhan S <address@hidden>
---
 migration/savevm.c     | 15 ++-------------
 migration/trace-events |  4 ++--
 2 files changed, 4 insertions(+), 15 deletions(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index b7908f62be..c7b9d69578 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -861,15 +861,9 @@ int qemu_savevm_send_packaged(QEMUFile *f, const uint8_t 
*buf, size_t len)
 {
     uint32_t tmp;
 
-    if (len > MAX_VM_CMD_PACKAGED_SIZE) {
-        error_report("%s: Unreasonably large packaged state: %zu",
-                     __func__, len);
-        return -1;
-    }
-
     tmp = cpu_to_be32(len);
 
-    trace_qemu_savevm_send_packaged();
+    trace_qemu_savevm_send_packaged(len, MAX_VM_CMD_PACKAGED_SIZE);
     qemu_savevm_command_send(f, MIG_CMD_PACKAGED, 4, (uint8_t *)&tmp);
 
     qemu_put_buffer(f, buf, len);
@@ -1718,12 +1712,7 @@ static int 
loadvm_handle_cmd_packaged(MigrationIncomingState *mis)
     QIOChannelBuffer *bioc;
 
     length = qemu_get_be32(mis->from_src_file);
-    trace_loadvm_handle_cmd_packaged(length);
-
-    if (length > MAX_VM_CMD_PACKAGED_SIZE) {
-        error_report("Unreasonably large packaged state: %zu", length);
-        return -1;
-    }
+    trace_loadvm_handle_cmd_packaged(length, MAX_VM_CMD_PACKAGED_SIZE);
 
     bioc = qio_channel_buffer_new(length);
     qio_channel_set_name(QIO_CHANNEL(bioc), "migration-loadvm-buffer");
diff --git a/migration/trace-events b/migration/trace-events
index 6f29fcc686..646963ffec 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -6,10 +6,10 @@ qemu_loadvm_state_section_command(int ret) "%d"
 qemu_loadvm_state_section_partend(uint32_t section_id) "%u"
 qemu_loadvm_state_post_main(int ret) "%d"
 qemu_loadvm_state_section_startfull(uint32_t section_id, const char *idstr, 
uint32_t instance_id, uint32_t version_id) "%u(%s) %u %u"
-qemu_savevm_send_packaged(void) ""
+qemu_savevm_send_packaged(size_t len, size_t max) "size=%zu, max 
recommended=%zu"
 loadvm_state_setup(void) ""
 loadvm_state_cleanup(void) ""
-loadvm_handle_cmd_packaged(unsigned int length) "%u"
+loadvm_handle_cmd_packaged(size_t len, size_t max) "size=%zu, max 
recommended=%zu"
 loadvm_handle_cmd_packaged_main(int ret) "%d"
 loadvm_handle_cmd_packaged_received(int ret) "%d"
 loadvm_postcopy_handle_advise(void) ""
-- 
2.14.3




reply via email to

[Prev in Thread] Current Thread [Next in Thread]