From fe39a4d44004408b8c847151191f1948a4aa6570 Mon Sep 17 00:00:00 2001 From: zhanghailiang Date: Wed, 2 Nov 2016 15:42:09 +0800 Subject: migration: fix missing assignment for has_x_checkpoint_delay We forgot to assign true to params->has_x_checkpoint_delay parameter in qmp_query_migrate_parameters. Without this, qmp command 'query-migrate-parameters' doesn't show the default value for x-checkpoint-delay option. This also fixes the fact that HMP was relying on unspecified behavior by reading x_checkpoint_delay without checking has_x_checkpoint_delay. Signed-off-by: zhanghailiang Reviewed-by: Eric Blake Reviewed-by: Juan Quintela Signed-off-by: Juan Quintela --- hmp.c | 1 + migration/migration.c | 1 + 2 files changed, 2 insertions(+) diff --git a/hmp.c b/hmp.c index b5e3f54534..02103dfb75 100644 --- a/hmp.c +++ b/hmp.c @@ -318,6 +318,7 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict) monitor_printf(mon, " %s: %" PRId64 " milliseconds", MigrationParameter_lookup[MIGRATION_PARAMETER_DOWNTIME_LIMIT], params->downtime_limit); + assert(params->has_x_checkpoint_delay); monitor_printf(mon, " %s: %" PRId64, MigrationParameter_lookup[MIGRATION_PARAMETER_X_CHECKPOINT_DELAY], params->x_checkpoint_delay); diff --git a/migration/migration.c b/migration/migration.c index e331f28382..f498ab84f2 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -593,6 +593,7 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp) params->max_bandwidth = s->parameters.max_bandwidth; params->has_downtime_limit = true; params->downtime_limit = s->parameters.downtime_limit; + params->has_x_checkpoint_delay = true; params->x_checkpoint_delay = s->parameters.x_checkpoint_delay; return params; -- cgit v1.2.1 From 6d57b4c0008ef4b51b149cfd1eb3b058be623e5b Mon Sep 17 00:00:00 2001 From: Halil Pasic Date: Fri, 21 Oct 2016 16:37:38 +0200 Subject: tests/test-vmstate.c: add save_buffer util func Let us de-duplicate some code by introducing an utility function for saving a chunk of bytes (used when testing load based on wire). Signed-off-by: Halil Pasic Reviewed-by: Guenther Hutzl Reviewed-by: Juan Quintela Signed-off-by: Juan Quintela --- tests/test-vmstate.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/tests/test-vmstate.c b/tests/test-vmstate.c index d8da26f974..d513dc6870 100644 --- a/tests/test-vmstate.c +++ b/tests/test-vmstate.c @@ -83,6 +83,13 @@ static void save_vmstate(const VMStateDescription *desc, void *obj) qemu_fclose(f); } +static void save_buffer(const uint8_t *buf, size_t buf_size) +{ + QEMUFile *fsave = open_test_file(true); + qemu_put_buffer(fsave, buf, buf_size); + qemu_fclose(fsave); +} + static void compare_vmstate(uint8_t *wire, size_t size) { QEMUFile *f = open_test_file(false); @@ -309,15 +316,13 @@ static const VMStateDescription vmstate_versioned = { static void test_load_v1(void) { - QEMUFile *fsave = open_test_file(true); uint8_t buf[] = { 0, 0, 0, 10, /* a */ 0, 0, 0, 30, /* c */ 0, 0, 0, 0, 0, 0, 0, 40, /* d */ QEMU_VM_EOF, /* just to ensure we won't get EOF reported prematurely */ }; - qemu_put_buffer(fsave, buf, sizeof(buf)); - qemu_fclose(fsave); + save_buffer(buf, sizeof(buf)); QEMUFile *loading = open_test_file(false); TestStruct obj = { .b = 200, .e = 500, .f = 600 }; @@ -334,7 +339,6 @@ static void test_load_v1(void) static void test_load_v2(void) { - QEMUFile *fsave = open_test_file(true); uint8_t buf[] = { 0, 0, 0, 10, /* a */ 0, 0, 0, 20, /* b */ @@ -344,8 +348,7 @@ static void test_load_v2(void) 0, 0, 0, 0, 0, 0, 0, 60, /* f */ QEMU_VM_EOF, /* just to ensure we won't get EOF reported prematurely */ }; - qemu_put_buffer(fsave, buf, sizeof(buf)); - qemu_fclose(fsave); + save_buffer(buf, sizeof(buf)); QEMUFile *loading = open_test_file(false); TestStruct obj; @@ -423,7 +426,6 @@ static void test_save_skip(void) static void test_load_noskip(void) { - QEMUFile *fsave = open_test_file(true); uint8_t buf[] = { 0, 0, 0, 10, /* a */ 0, 0, 0, 20, /* b */ @@ -433,8 +435,7 @@ static void test_load_noskip(void) 0, 0, 0, 0, 0, 0, 0, 60, /* f */ QEMU_VM_EOF, /* just to ensure we won't get EOF reported prematurely */ }; - qemu_put_buffer(fsave, buf, sizeof(buf)); - qemu_fclose(fsave); + save_buffer(buf, sizeof(buf)); QEMUFile *loading = open_test_file(false); TestStruct obj = { .skip_c_e = false }; @@ -451,7 +452,6 @@ static void test_load_noskip(void) static void test_load_skip(void) { - QEMUFile *fsave = open_test_file(true); uint8_t buf[] = { 0, 0, 0, 10, /* a */ 0, 0, 0, 20, /* b */ @@ -459,8 +459,7 @@ static void test_load_skip(void) 0, 0, 0, 0, 0, 0, 0, 60, /* f */ QEMU_VM_EOF, /* just to ensure we won't get EOF reported prematurely */ }; - qemu_put_buffer(fsave, buf, sizeof(buf)); - qemu_fclose(fsave); + save_buffer(buf, sizeof(buf)); QEMUFile *loading = open_test_file(false); TestStruct obj = { .skip_c_e = true, .c = 300, .e = 500 }; -- cgit v1.2.1 From 8cc49f0302ff23353034b6f9cbf4e5d778239a3d Mon Sep 17 00:00:00 2001 From: Halil Pasic Date: Fri, 21 Oct 2016 16:37:39 +0200 Subject: tests/test-vmstate.c: add array of pointer to struct Increase test coverage by adding tests for the macro VMSTATE_ARRAY_OF_POINTER_TO_STRUCT. Signed-off-by: Halil Pasic Reviewed-by: Guenther Hutzl Reviewed-by: Juan Quintela Signed-off-by: Juan Quintela --- tests/test-vmstate.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/tests/test-vmstate.c b/tests/test-vmstate.c index d513dc6870..d2f529b831 100644 --- a/tests/test-vmstate.c +++ b/tests/test-vmstate.c @@ -474,6 +474,76 @@ static void test_load_skip(void) qemu_fclose(loading); } + +typedef struct { + int32_t i; +} TestStructTriv; + +const VMStateDescription vmsd_tst = { + .name = "test/tst", + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_INT32(i, TestStructTriv), + VMSTATE_END_OF_LIST() + } +}; + +#define AR_SIZE 4 + +typedef struct { + TestStructTriv *ar[AR_SIZE]; +} TestArrayOfPtrToStuct; + +const VMStateDescription vmsd_arps = { + .name = "test/arps", + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_ARRAY_OF_POINTER_TO_STRUCT(ar, TestArrayOfPtrToStuct, + AR_SIZE, 0, vmsd_tst, TestStructTriv), + VMSTATE_END_OF_LIST() + } +}; +static void test_arr_ptr_str_no0_save(void) +{ + TestStructTriv ar[AR_SIZE] = {{.i = 0}, {.i = 1}, {.i = 2}, {.i = 3} }; + TestArrayOfPtrToStuct sample = {.ar = {&ar[0], &ar[1], &ar[2], &ar[3]} }; + uint8_t wire_sample[] = { + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x01, + 0x00, 0x00, 0x00, 0x02, + 0x00, 0x00, 0x00, 0x03, + QEMU_VM_EOF + }; + + save_vmstate(&vmsd_arps, &sample); + compare_vmstate(wire_sample, sizeof(wire_sample)); +} + +static void test_arr_ptr_str_no0_load(void) +{ + TestStructTriv ar_gt[AR_SIZE] = {{.i = 0}, {.i = 1}, {.i = 2}, {.i = 3} }; + TestStructTriv ar[AR_SIZE] = {}; + TestArrayOfPtrToStuct obj = {.ar = {&ar[0], &ar[1], &ar[2], &ar[3]} }; + int idx; + uint8_t wire_sample[] = { + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x01, + 0x00, 0x00, 0x00, 0x02, + 0x00, 0x00, 0x00, 0x03, + QEMU_VM_EOF + }; + + save_buffer(wire_sample, sizeof(wire_sample)); + SUCCESS(load_vmstate_one(&vmsd_arps, &obj, 1, + wire_sample, sizeof(wire_sample))); + for (idx = 0; idx < AR_SIZE; ++idx) { + /* compare the target array ar with the ground truth array ar_gt */ + g_assert_cmpint(ar_gt[idx].i, ==, ar[idx].i); + } +} + int main(int argc, char **argv) { temp_fd = mkstemp(temp_file); @@ -488,6 +558,10 @@ int main(int argc, char **argv) g_test_add_func("/vmstate/field_exists/load/skip", test_load_skip); g_test_add_func("/vmstate/field_exists/save/noskip", test_save_noskip); g_test_add_func("/vmstate/field_exists/save/skip", test_save_skip); + g_test_add_func("/vmstate/array/ptr/str/no0/save", + test_arr_ptr_str_no0_save); + g_test_add_func("/vmstate/array/ptr/str/no0/load", + test_arr_ptr_str_no0_load); g_test_run(); close(temp_fd); -- cgit v1.2.1 From 5c90308f07335451a08c030dc40a9eed4698152b Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Fri, 4 Nov 2016 14:10:17 +0100 Subject: migration: Fix return code of ram_save_iterate() qemu_savevm_state_iterate() expects the iterators to return 1 when they are done, and 0 if there is still something left to do. However, ram_save_iterate() does not obey this rule and returns the number of saved pages instead. This causes a fatal hang with ppc64 guests when you run QEMU like this (also works with TCG): qemu-img create -f qcow2 /tmp/test.qcow2 1M qemu-system-ppc64 -nographic -nodefaults -m 256 \ -hda /tmp/test.qcow2 -serial mon:stdio ... then switch to the monitor by pressing CTRL-a c and try to save a snapshot with "savevm test1" for example. After the first iteration, ram_save_iterate() always returns 0 here, so that qemu_savevm_state_iterate() hangs in an endless loop and you can only "kill -9" the QEMU process. Fix it by using proper return values in ram_save_iterate(). Signed-off-by: Thomas Huth Reviewed-by: Juan Quintela Signed-off-by: Juan Quintela --- migration/ram.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/migration/ram.c b/migration/ram.c index fb9252d722..a1c8089010 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -1987,7 +1987,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque) int ret; int i; int64_t t0; - int pages_sent = 0; + int done = 0; rcu_read_lock(); if (ram_list.version != last_version) { @@ -2007,9 +2007,9 @@ static int ram_save_iterate(QEMUFile *f, void *opaque) pages = ram_find_and_save_block(f, false, &bytes_transferred); /* no more pages to sent */ if (pages == 0) { + done = 1; break; } - pages_sent += pages; acct_info.iterations++; /* we want to check in the 1st loop, just in case it was the 1st time @@ -2044,7 +2044,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque) return ret; } - return pages_sent; + return done; } /* Called with iothread lock */ -- cgit v1.2.1