From bab01ed4e87a75ce2e779cf0a79b74ca97fd29ea Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Fri, 6 Oct 2017 22:58:44 +0200 Subject: [PATCH 1/9] migration: Make sure that we pass the right cache size Instead of passing silently round down the number of pages, make it an error that the cache size is not a power of 2. Signed-off-by: Juan Quintela Reviewed-by: Dr. David Alan Gilbert --- migration/page_cache.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/migration/page_cache.c b/migration/page_cache.c index 9a9d13d6a2..96268c3aea 100644 --- a/migration/page_cache.c +++ b/migration/page_cache.c @@ -58,6 +58,13 @@ PageCache *cache_init(int64_t new_size, size_t page_size, Error **errp) return NULL; } + /* round down to the nearest power of 2 */ + if (!is_power_of_2(num_pages)) { + error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "cache size", + "is not a power of two number of pages"); + return NULL; + } + /* We prefer not to abort if there is no memory */ cache = g_try_malloc(sizeof(*cache)); if (!cache) { @@ -65,11 +72,6 @@ PageCache *cache_init(int64_t new_size, size_t page_size, Error **errp) "Failed to allocate cache"); return NULL; } - /* round down to the nearest power of 2 */ - if (!is_power_of_2(num_pages)) { - num_pages = pow2floor(num_pages); - DPRINTF("rounding down to %" PRId64 "\n", num_pages); - } cache->page_size = page_size; cache->num_items = 0; cache->max_num_items = num_pages; From 2a313e5cf6ed90b932b0abe2b4f2055785397f93 Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Fri, 6 Oct 2017 23:00:12 +0200 Subject: [PATCH 2/9] migration: Don't play games with the requested cache size Now that we check that the value passed is a power of 2, we don't need to play games when comparing what is the size that is going to take the cache. Signed-off-by: Juan Quintela Reviewed-by: Dr. David Alan Gilbert --- migration/ram.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/migration/ram.c b/migration/ram.c index 7f6327f708..42f3b7cb28 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -136,12 +136,14 @@ int64_t xbzrle_cache_resize(int64_t new_size, Error **errp) return -1; } + if (new_size == migrate_xbzrle_cache_size()) { + /* nothing to do */ + return new_size; + } + XBZRLE_cache_lock(); if (XBZRLE.cache != NULL) { - if (pow2floor(new_size) == migrate_xbzrle_cache_size()) { - goto out_new_size; - } new_cache = cache_init(new_size, TARGET_PAGE_SIZE, errp); if (!new_cache) { ret = -1; @@ -152,8 +154,7 @@ int64_t xbzrle_cache_resize(int64_t new_size, Error **errp) XBZRLE.cache = new_cache; } -out_new_size: - ret = pow2floor(new_size); + ret = new_size; out: XBZRLE_cache_unlock(); return ret; From c9dede2d482676440cb6e826ebe87450965fe679 Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Fri, 6 Oct 2017 23:03:55 +0200 Subject: [PATCH 3/9] migration: No need to return the size of the cache After the previous commits, we make sure that the value passed is right, or we just drop an error. So now we return if there is one error or we have setup correctly the value passed. Signed-off-by: Juan Quintela Reviewed-by: Dr. David Alan Gilbert -- Improve error messasge Return 0 always for success --- migration/migration.c | 6 ++---- migration/ram.c | 10 ++++------ migration/ram.h | 2 +- 3 files changed, 7 insertions(+), 11 deletions(-) diff --git a/migration/migration.c b/migration/migration.c index 62761d5705..6bbd4715d3 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -1406,14 +1406,12 @@ void qmp_migrate_continue(MigrationStatus state, Error **errp) void qmp_migrate_set_cache_size(int64_t value, Error **errp) { MigrationState *s = migrate_get_current(); - int64_t new_size; - new_size = xbzrle_cache_resize(value, errp); - if (new_size < 0) { + if (xbzrle_cache_resize(value, errp) < 0) { return; } - s->xbzrle_cache_size = new_size; + s->xbzrle_cache_size = value; } int64_t qmp_query_migrate_cache_size(Error **errp) diff --git a/migration/ram.c b/migration/ram.c index 42f3b7cb28..997340c7c2 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -112,15 +112,15 @@ static void XBZRLE_cache_unlock(void) * migration may be using the cache and might finish during this call, * hence changes to the cache are protected by XBZRLE.lock(). * - * Returns the new_size or negative in case of error. + * Returns 0 for success or -1 for error * * @new_size: new cache size * @errp: set *errp if the check failed, with reason */ -int64_t xbzrle_cache_resize(int64_t new_size, Error **errp) +int xbzrle_cache_resize(int64_t new_size, Error **errp) { PageCache *new_cache; - int64_t ret; + int64_t ret = 0; /* Check for truncation */ if (new_size != (size_t)new_size) { @@ -138,7 +138,7 @@ int64_t xbzrle_cache_resize(int64_t new_size, Error **errp) if (new_size == migrate_xbzrle_cache_size()) { /* nothing to do */ - return new_size; + return 0; } XBZRLE_cache_lock(); @@ -153,8 +153,6 @@ int64_t xbzrle_cache_resize(int64_t new_size, Error **errp) cache_fini(XBZRLE.cache); XBZRLE.cache = new_cache; } - - ret = new_size; out: XBZRLE_cache_unlock(); return ret; diff --git a/migration/ram.h b/migration/ram.h index f9f7eef894..64d81e9f1d 100644 --- a/migration/ram.h +++ b/migration/ram.h @@ -35,7 +35,7 @@ extern MigrationStats ram_counters; extern XBZRLECacheStats xbzrle_counters; -int64_t xbzrle_cache_resize(int64_t new_size, Error **errp); +int xbzrle_cache_resize(int64_t new_size, Error **errp); uint64_t ram_bytes_remaining(void); uint64_t ram_bytes_total(void); From 73af8dd8d75a3e1f13e6c5d2a509fa56eb406519 Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Thu, 5 Oct 2017 21:30:10 +0200 Subject: [PATCH 4/9] migration: Make xbzrle_cache_size a migration parameter Right now it is a variable in MigrationState instead of a MigrationParameter. The change allows to set it as the rest of the Migration parameters, from the command line, with query_migration_paramters, set_migrate_parameters, etc. Signed-off-by: Juan Quintela Reviewed-by: Dr. David Alan Gilbert --- hmp.c | 14 ++++++++++++++ migration/migration.c | 43 ++++++++++++++++++++++++++++++++----------- migration/migration.h | 1 - migration/ram.c | 7 ------- qapi/migration.json | 26 +++++++++++++++++++++++--- 5 files changed, 69 insertions(+), 22 deletions(-) diff --git a/hmp.c b/hmp.c index 41fcce6f5a..a01be50daa 100644 --- a/hmp.c +++ b/hmp.c @@ -342,6 +342,9 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict) monitor_printf(mon, "%s: %" PRId64 "\n", MigrationParameter_str(MIGRATION_PARAMETER_X_MULTIFD_PAGE_COUNT), params->x_multifd_page_count); + monitor_printf(mon, "%s: %" PRId64 "\n", + MigrationParameter_str(MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE), + params->xbzrle_cache_size); } qapi_free_MigrationParameters(params); @@ -1578,6 +1581,7 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) Visitor *v = string_input_visitor_new(valuestr); MigrateSetParameters *p = g_new0(MigrateSetParameters, 1); uint64_t valuebw = 0; + uint64_t cache_size; Error *err = NULL; int val, ret; @@ -1653,6 +1657,16 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) p->has_x_multifd_page_count = true; visit_type_int(v, param, &p->x_multifd_page_count, &err); break; + case MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE: + p->has_xbzrle_cache_size = true; + visit_type_size(v, param, &cache_size, &err); + if (err || cache_size > INT64_MAX + || (size_t)cache_size != cache_size) { + error_setg(&err, "Invalid size %s", valuestr); + break; + } + p->xbzrle_cache_size = cache_size; + break; default: assert(0); } diff --git a/migration/migration.c b/migration/migration.c index 6bbd4715d3..4de3b551fe 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -71,7 +71,7 @@ #define DEFAULT_MIGRATE_CPU_THROTTLE_INCREMENT 10 /* Migration XBZRLE default cache size */ -#define DEFAULT_MIGRATE_CACHE_SIZE (64 * 1024 * 1024) +#define DEFAULT_MIGRATE_XBZRLE_CACHE_SIZE (64 * 1024 * 1024) /* The delay time (in ms) between two COLO checkpoints * Note: Please change this default value to 10000 when we support hybrid mode. @@ -515,6 +515,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp) params->x_multifd_channels = s->parameters.x_multifd_channels; params->has_x_multifd_page_count = true; params->x_multifd_page_count = s->parameters.x_multifd_page_count; + params->has_xbzrle_cache_size = true; + params->xbzrle_cache_size = s->parameters.xbzrle_cache_size; return params; } @@ -817,6 +819,16 @@ static bool migrate_params_check(MigrationParameters *params, Error **errp) return false; } + if (params->has_xbzrle_cache_size && + (params->xbzrle_cache_size < qemu_target_page_size() || + !is_power_of_2(params->xbzrle_cache_size))) { + error_setg(errp, QERR_INVALID_PARAMETER_VALUE, + "xbzrle_cache_size", + "is invalid, it should be bigger than target page size" + " and a power of two"); + return false; + } + return true; } @@ -878,9 +890,12 @@ static void migrate_params_test_apply(MigrateSetParameters *params, if (params->has_x_multifd_page_count) { dest->x_multifd_page_count = params->x_multifd_page_count; } + if (params->has_xbzrle_cache_size) { + dest->xbzrle_cache_size = params->xbzrle_cache_size; + } } -static void migrate_params_apply(MigrateSetParameters *params) +static void migrate_params_apply(MigrateSetParameters *params, Error **errp) { MigrationState *s = migrate_get_current(); @@ -946,6 +961,10 @@ static void migrate_params_apply(MigrateSetParameters *params) if (params->has_x_multifd_page_count) { s->parameters.x_multifd_page_count = params->x_multifd_page_count; } + if (params->has_xbzrle_cache_size) { + s->parameters.xbzrle_cache_size = params->xbzrle_cache_size; + xbzrle_cache_resize(params->xbzrle_cache_size, errp); + } } void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp) @@ -974,7 +993,7 @@ void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp) return; } - migrate_params_apply(params); + migrate_params_apply(params, errp); } @@ -1405,13 +1424,12 @@ void qmp_migrate_continue(MigrationStatus state, Error **errp) void qmp_migrate_set_cache_size(int64_t value, Error **errp) { - MigrationState *s = migrate_get_current(); + MigrateSetParameters p = { + .has_xbzrle_cache_size = true, + .xbzrle_cache_size = value, + }; - if (xbzrle_cache_resize(value, errp) < 0) { - return; - } - - s->xbzrle_cache_size = value; + qmp_migrate_set_parameters(&p, errp); } int64_t qmp_query_migrate_cache_size(Error **errp) @@ -1587,7 +1605,7 @@ int64_t migrate_xbzrle_cache_size(void) s = migrate_get_current(); - return s->xbzrle_cache_size; + return s->parameters.xbzrle_cache_size; } bool migrate_use_block(void) @@ -2405,6 +2423,9 @@ static Property migration_properties[] = { DEFINE_PROP_INT64("x-multifd-page-count", MigrationState, parameters.x_multifd_page_count, DEFAULT_MIGRATE_MULTIFD_PAGE_COUNT), + DEFINE_PROP_SIZE("xbzrle-cache-size", MigrationState, + parameters.xbzrle_cache_size, + DEFAULT_MIGRATE_XBZRLE_CACHE_SIZE), /* Migration capabilities */ DEFINE_PROP_MIG_CAP("x-xbzrle", MIGRATION_CAPABILITY_XBZRLE), @@ -2448,7 +2469,6 @@ static void migration_instance_init(Object *obj) MigrationParameters *params = &ms->parameters; ms->state = MIGRATION_STATUS_NONE; - ms->xbzrle_cache_size = DEFAULT_MIGRATE_CACHE_SIZE; ms->mbps = -1; qemu_sem_init(&ms->pause_sem, 0); qemu_mutex_init(&ms->error_mutex); @@ -2468,6 +2488,7 @@ static void migration_instance_init(Object *obj) params->has_block_incremental = true; params->has_x_multifd_channels = true; params->has_x_multifd_page_count = true; + params->has_xbzrle_cache_size = true; } /* diff --git a/migration/migration.h b/migration/migration.h index 8ccdd7a577..663415fe48 100644 --- a/migration/migration.h +++ b/migration/migration.h @@ -107,7 +107,6 @@ struct MigrationState int64_t downtime; int64_t expected_downtime; bool enabled_capabilities[MIGRATION_CAPABILITY__MAX]; - int64_t xbzrle_cache_size; int64_t setup_time; /* Flag set once the migration has been asked to enter postcopy */ diff --git a/migration/ram.c b/migration/ram.c index 997340c7c2..8620aa400a 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -129,13 +129,6 @@ int xbzrle_cache_resize(int64_t new_size, Error **errp) return -1; } - /* Cache should not be larger than guest ram size */ - if (new_size > ram_bytes_total()) { - error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "cache size", - "exceeds guest ram size"); - return -1; - } - if (new_size == migrate_xbzrle_cache_size()) { /* nothing to do */ return 0; diff --git a/qapi/migration.json b/qapi/migration.json index 6ae866e1aa..bbc4671ded 100644 --- a/qapi/migration.json +++ b/qapi/migration.json @@ -483,6 +483,11 @@ # @x-multifd-page-count: Number of pages sent together to a thread. # The default value is 16 (since 2.11) # +# @xbzrle-cache-size: cache size to be used by XBZRLE migration. It +# needs to be a multiple of the target page size +# and a power of 2 +# (Since 2.11) +# # Since: 2.4 ## { 'enum': 'MigrationParameter', @@ -490,7 +495,8 @@ 'cpu-throttle-initial', 'cpu-throttle-increment', 'tls-creds', 'tls-hostname', 'max-bandwidth', 'downtime-limit', 'x-checkpoint-delay', 'block-incremental', - 'x-multifd-channels', 'x-multifd-page-count' ] } + 'x-multifd-channels', 'x-multifd-page-count', + 'xbzrle-cache-size' ] } ## # @MigrateSetParameters: @@ -554,6 +560,10 @@ # @x-multifd-page-count: Number of pages sent together to a thread. # The default value is 16 (since 2.11) # +# @xbzrle-cache-size: cache size to be used by XBZRLE migration. It +# needs to be a multiple of the target page size +# and a power of 2 +# (Since 2.11) # Since: 2.4 ## # TODO either fuse back into MigrationParameters, or make @@ -571,7 +581,8 @@ '*x-checkpoint-delay': 'int', '*block-incremental': 'bool', '*x-multifd-channels': 'int', - '*x-multifd-page-count': 'int' } } + '*x-multifd-page-count': 'int', + '*xbzrle-cache-size': 'size' } } ## # @migrate-set-parameters: @@ -650,6 +661,10 @@ # @x-multifd-page-count: Number of pages sent together to a thread. # The default value is 16 (since 2.11) # +# @xbzrle-cache-size: cache size to be used by XBZRLE migration. It +# needs to be a multiple of the target page size +# and a power of 2 +# (Since 2.11) # Since: 2.4 ## { 'struct': 'MigrationParameters', @@ -665,7 +680,8 @@ '*x-checkpoint-delay': 'int', '*block-incremental': 'bool' , '*x-multifd-channels': 'int', - '*x-multifd-page-count': 'int' } } + '*x-multifd-page-count': 'int', + '*xbzrle-cache-size': 'size' } } ## # @query-migrate-parameters: @@ -947,6 +963,8 @@ # # Returns: nothing on success # +# Notes: This command is deprecated in favor of 'migrate-set-parameters' +# # Since: 1.2 # # Example: @@ -965,6 +983,8 @@ # # Returns: XBZRLE cache size in bytes # +# Notes: This command is deprecated in favor of 'query-migrate-parameters' +# # Since: 1.2 # # Example: From 2656bfd9731d430f2767f90906a3f1b1bfbf62d2 Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Wed, 11 Oct 2017 11:03:22 +0200 Subject: [PATCH 5/9] tests: rename postcopy-test to migration-test Instead of repeating the code, we are going to bo more tests on this file Signed-off-by: Juan Quintela Reviewed-by: Dr. David Alan Gilbert --- MAINTAINERS | 1 + tests/Makefile.include | 6 +++--- tests/{postcopy-test.c => migration-test.c} | 8 +++----- 3 files changed, 7 insertions(+), 8 deletions(-) rename tests/{postcopy-test.c => migration-test.c} (98%) diff --git a/MAINTAINERS b/MAINTAINERS index 2650063242..1fd7a6273c 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1563,6 +1563,7 @@ F: include/migration/ F: migration/ F: scripts/vmstate-static-checker.py F: tests/vmstate-static-checker-data/ +F: tests/migration-test.c F: docs/devel/migration.txt F: qapi/migration.json diff --git a/tests/Makefile.include b/tests/Makefile.include index 70dc711bca..434a2ce868 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -287,7 +287,7 @@ endif check-qtest-i386-$(CONFIG_SLIRP) += tests/test-netfilter$(EXESUF) check-qtest-i386-$(CONFIG_POSIX) += tests/test-filter-mirror$(EXESUF) check-qtest-i386-$(CONFIG_POSIX) += tests/test-filter-redirector$(EXESUF) -check-qtest-i386-y += tests/postcopy-test$(EXESUF) +check-qtest-i386-y += tests/migration-test$(EXESUF) check-qtest-i386-y += tests/test-x86-cpuid-compat$(EXESUF) check-qtest-i386-y += tests/numa-test$(EXESUF) check-qtest-x86_64-y += $(check-qtest-i386-y) @@ -315,7 +315,7 @@ check-qtest-ppc64-y += tests/boot-order-test$(EXESUF) check-qtest-ppc64-y += tests/prom-env-test$(EXESUF) check-qtest-ppc64-y += tests/pnv-xscom-test$(EXESUF) check-qtest-ppc64-y += tests/drive_del-test$(EXESUF) -check-qtest-ppc64-y += tests/postcopy-test$(EXESUF) +check-qtest-ppc64-y += tests/migration-test$(EXESUF) check-qtest-ppc64-y += tests/boot-serial-test$(EXESUF) check-qtest-ppc64-y += tests/rtas-test$(EXESUF) check-qtest-ppc64-$(CONFIG_SLIRP) += tests/pxe-test$(EXESUF) @@ -784,7 +784,7 @@ tests/usb-hcd-uhci-test$(EXESUF): tests/usb-hcd-uhci-test.o $(libqos-usb-obj-y) tests/usb-hcd-ehci-test$(EXESUF): tests/usb-hcd-ehci-test.o $(libqos-usb-obj-y) tests/usb-hcd-xhci-test$(EXESUF): tests/usb-hcd-xhci-test.o $(libqos-usb-obj-y) tests/pc-cpu-test$(EXESUF): tests/pc-cpu-test.o -tests/postcopy-test$(EXESUF): tests/postcopy-test.o +tests/migration-test$(EXESUF): tests/migration-test.o tests/vhost-user-test$(EXESUF): tests/vhost-user-test.o $(test-util-obj-y) \ $(qtest-obj-y) $(test-io-obj-y) $(libqos-virtio-obj-y) $(libqos-pc-obj-y) \ $(chardev-obj-y) diff --git a/tests/postcopy-test.c b/tests/migration-test.c similarity index 98% rename from tests/postcopy-test.c rename to tests/migration-test.c index 8142f2ab90..55c4aed719 100644 --- a/tests/postcopy-test.c +++ b/tests/migration-test.c @@ -1,5 +1,5 @@ /* - * QTest testcase for postcopy + * QTest testcase for migration * * Copyright (c) 2016 Red Hat, Inc. and/or its affiliates * based on the vhost-user-test.c that is: @@ -243,8 +243,6 @@ static QDict *return_or_event(QDict *response) /* * It's tricky to use qemu's migration event capability with qtest, * events suddenly appearing confuse the qmp()/hmp() responses. - * so wait for a couple of passes to have happened before - * going postcopy. */ static uint64_t get_migration_pass(void) @@ -504,7 +502,7 @@ static void test_migrate(void) int main(int argc, char **argv) { - char template[] = "/tmp/postcopy-test-XXXXXX"; + char template[] = "/tmp/migration-test-XXXXXX"; int ret; g_test_init(&argc, &argv, NULL); @@ -521,7 +519,7 @@ int main(int argc, char **argv) module_call_init(MODULE_INIT_QOM); - qtest_add_func("/postcopy", test_migrate); + qtest_add_func("/migration/postcopy/unix", test_migrate); ret = g_test_run(); From d62fbe603969c54f39eaed2876238c574cbed5cb Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Wed, 11 Oct 2017 11:46:00 +0200 Subject: [PATCH 6/9] tests: Refactor setting of parameters/capabilities So we can use them in future tests Signed-off-by: Juan Quintela Reviewed-by: Dr. David Alan Gilbert --- tests/migration-test.c | 101 ++++++++++++++++++++++++++--------------- 1 file changed, 64 insertions(+), 37 deletions(-) diff --git a/tests/migration-test.c b/tests/migration-test.c index 55c4aed719..19a1445076 100644 --- a/tests/migration-test.c +++ b/tests/migration-test.c @@ -351,12 +351,69 @@ static void cleanup(const char *filename) g_free(path); } +static void migrate_set_downtime(QTestState *who, const char *value) +{ + QDict *rsp; + gchar *cmd; + + cmd = g_strdup_printf("{ 'execute': 'migrate_set_downtime'," + "'arguments': { 'value': %s } }", value); + rsp = qtest_qmp(who, cmd); + g_free(cmd); + g_assert(qdict_haskey(rsp, "return")); + QDECREF(rsp); +} + +static void migrate_set_speed(QTestState *who, const char *value) +{ + QDict *rsp; + gchar *cmd; + + cmd = g_strdup_printf("{ 'execute': 'migrate_set_speed'," + "'arguments': { 'value': %s } }", value); + rsp = qtest_qmp(who, cmd); + g_free(cmd); + g_assert(qdict_haskey(rsp, "return")); + QDECREF(rsp); +} + +static void migrate_set_capability(QTestState *who, const char *capability, + const char *value) +{ + QDict *rsp; + gchar *cmd; + + cmd = g_strdup_printf("{ 'execute': 'migrate-set-capabilities'," + "'arguments': { " + "'capabilities': [ { " + "'capability': '%s', 'state': %s } ] } }", + capability, value); + rsp = qtest_qmp(who, cmd); + g_free(cmd); + g_assert(qdict_haskey(rsp, "return")); + QDECREF(rsp); +} + +static void migrate(QTestState *who, const char *uri) +{ + QDict *rsp; + gchar *cmd; + + cmd = g_strdup_printf("{ 'execute': 'migrate'," + "'arguments': { 'uri': '%s' } }", + uri); + rsp = qtest_qmp(who, cmd); + g_free(cmd); + g_assert(qdict_haskey(rsp, "return")); + QDECREF(rsp); +} + static void test_migrate(void) { char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); QTestState *global = global_qtest, *from, *to; unsigned char dest_byte_a, dest_byte_b, dest_byte_c, dest_byte_d; - gchar *cmd, *cmd_src, *cmd_dst; + gchar *cmd_src, *cmd_dst; QDict *rsp; char *bootpath = g_strdup_printf("%s/bootsect", tmpfs); @@ -405,52 +462,22 @@ static void test_migrate(void) to = qtest_init(cmd_dst); g_free(cmd_dst); - global_qtest = from; - rsp = qmp("{ 'execute': 'migrate-set-capabilities'," - "'arguments': { " - "'capabilities': [ {" - "'capability': 'postcopy-ram'," - "'state': true } ] } }"); - g_assert(qdict_haskey(rsp, "return")); - QDECREF(rsp); - - global_qtest = to; - rsp = qmp("{ 'execute': 'migrate-set-capabilities'," - "'arguments': { " - "'capabilities': [ {" - "'capability': 'postcopy-ram'," - "'state': true } ] } }"); - g_assert(qdict_haskey(rsp, "return")); - QDECREF(rsp); + migrate_set_capability(from, "postcopy-ram", "true"); + migrate_set_capability(to, "postcopy-ram", "true"); /* We want to pick a speed slow enough that the test completes * quickly, but that it doesn't complete precopy even on a slow * machine, so also set the downtime. */ - global_qtest = from; - rsp = qmp("{ 'execute': 'migrate_set_speed'," - "'arguments': { 'value': 100000000 } }"); - g_assert(qdict_haskey(rsp, "return")); - QDECREF(rsp); - - /* 1ms downtime - it should never finish precopy */ - rsp = qmp("{ 'execute': 'migrate_set_downtime'," - "'arguments': { 'value': 0.001 } }"); - g_assert(qdict_haskey(rsp, "return")); - QDECREF(rsp); - + migrate_set_speed(from, "100000000"); + migrate_set_downtime(from, "0.001"); /* Wait for the first serial output from the source */ wait_for_serial("src_serial"); - cmd = g_strdup_printf("{ 'execute': 'migrate'," - "'arguments': { 'uri': '%s' } }", - uri); - rsp = qmp(cmd); - g_free(cmd); - g_assert(qdict_haskey(rsp, "return")); - QDECREF(rsp); + migrate(from, uri); + global_qtest = from; wait_for_migration_pass(); rsp = return_or_event(qmp("{ 'execute': 'migrate-start-postcopy' }")); From 7195a87130b2d58b27a0d5b1641a26e48423348b Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Wed, 11 Oct 2017 11:57:04 +0200 Subject: [PATCH 7/9] tests: Factorize out migrate_test_start/end We fix global_test users left and right Signed-off-by: Juan Quintela Reviewed-by: Dr. David Alan Gilbert --- tests/migration-test.c | 86 +++++++++++++++++++++++------------------- 1 file changed, 48 insertions(+), 38 deletions(-) diff --git a/tests/migration-test.c b/tests/migration-test.c index 19a1445076..91fb0277d6 100644 --- a/tests/migration-test.c +++ b/tests/migration-test.c @@ -301,7 +301,7 @@ static void wait_for_migration_pass(void) } while (pass == initial_pass && !got_stop); } -static void check_guests_ram(void) +static void check_guests_ram(QTestState *who) { /* Our ASM test will have been incrementing one byte from each page from * 1MB to <100MB in order. @@ -316,13 +316,13 @@ static void check_guests_ram(void) bool hit_edge = false; bool bad = false; - qtest_memread(global_qtest, start_address, &first_byte, 1); + qtest_memread(who, start_address, &first_byte, 1); last_byte = first_byte; for (address = start_address + 4096; address < end_address; address += 4096) { uint8_t b; - qtest_memread(global_qtest, address, &b, 1); + qtest_memread(who, address, &b, 1); if (b != last_byte) { if (((b + 1) % 256) == last_byte && !hit_edge) { /* This is OK, the guest stopped at the point of @@ -408,14 +408,10 @@ static void migrate(QTestState *who, const char *uri) QDECREF(rsp); } -static void test_migrate(void) +static void test_migrate_start(QTestState **from, QTestState **to, + const char *uri) { - char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); - QTestState *global = global_qtest, *from, *to; - unsigned char dest_byte_a, dest_byte_b, dest_byte_c, dest_byte_d; gchar *cmd_src, *cmd_dst; - QDict *rsp; - char *bootpath = g_strdup_printf("%s/bootsect", tmpfs); const char *arch = qtest_get_arch(); @@ -456,11 +452,51 @@ static void test_migrate(void) g_free(bootpath); - from = qtest_start(cmd_src); + *from = qtest_start(cmd_src); g_free(cmd_src); - to = qtest_init(cmd_dst); + *to = qtest_init(cmd_dst); g_free(cmd_dst); +} + +static void test_migrate_end(QTestState *from, QTestState *to) +{ + unsigned char dest_byte_a, dest_byte_b, dest_byte_c, dest_byte_d; + + qtest_quit(from); + + qtest_memread(to, start_address, &dest_byte_a, 1); + + /* Destination still running, wait for a byte to change */ + do { + qtest_memread(to, start_address, &dest_byte_b, 1); + usleep(10 * 1000); + } while (dest_byte_a == dest_byte_b); + + qtest_qmp_discard_response(to, "{ 'execute' : 'stop'}"); + /* With it stopped, check nothing changes */ + qtest_memread(to, start_address, &dest_byte_c, 1); + sleep(1); + qtest_memread(to, start_address, &dest_byte_d, 1); + g_assert_cmpint(dest_byte_c, ==, dest_byte_d); + + check_guests_ram(to); + + qtest_quit(to); + + cleanup("bootsect"); + cleanup("migsocket"); + cleanup("src_serial"); + cleanup("dest_serial"); +} + +static void test_migrate(void) +{ + char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); + QTestState *global = global_qtest, *from, *to; + QDict *rsp; + + test_migrate_start(&from, &to, uri); migrate_set_capability(from, "postcopy-ram", "true"); migrate_set_capability(to, "postcopy-ram", "true"); @@ -495,36 +531,10 @@ static void test_migrate(void) global_qtest = from; wait_for_migration_complete(); - qtest_quit(from); - - global_qtest = to; - - qtest_memread(to, start_address, &dest_byte_a, 1); - - /* Destination still running, wait for a byte to change */ - do { - qtest_memread(to, start_address, &dest_byte_b, 1); - usleep(10 * 1000); - } while (dest_byte_a == dest_byte_b); - - qmp_discard_response("{ 'execute' : 'stop'}"); - /* With it stopped, check nothing changes */ - qtest_memread(to, start_address, &dest_byte_c, 1); - sleep(1); - qtest_memread(to, start_address, &dest_byte_d, 1); - g_assert_cmpint(dest_byte_c, ==, dest_byte_d); - - check_guests_ram(); - - qtest_quit(to); g_free(uri); - global_qtest = global; - cleanup("bootsect"); - cleanup("migsocket"); - cleanup("src_serial"); - cleanup("dest_serial"); + test_migrate_end(from, to); } int main(int argc, char **argv) From 863e27a8fc85c7d1fb20c5a549d3e45ad835702e Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Wed, 11 Oct 2017 12:13:45 +0200 Subject: [PATCH 8/9] tests: Don't abuse global_qtest As we have two guests running, just pass always who we want to send a message to. Once there, refactor return_or_event() into wait_command. Signed-off-by: Juan Quintela Reviewed-by: Dr. David Alan Gilbert --- tests/migration-test.c | 55 +++++++++++++++++++++--------------------- 1 file changed, 27 insertions(+), 28 deletions(-) diff --git a/tests/migration-test.c b/tests/migration-test.c index 91fb0277d6..c429a13403 100644 --- a/tests/migration-test.c +++ b/tests/migration-test.c @@ -223,20 +223,23 @@ static void wait_for_serial(const char *side) /* * Events can get in the way of responses we are actually waiting for. */ -static QDict *return_or_event(QDict *response) +static QDict *wait_command(QTestState *who, const char *command) { const char *event_string; - if (!qdict_haskey(response, "event")) { - return response; - } + QDict *response; - /* OK, it was an event */ - event_string = qdict_get_str(response, "event"); - if (!strcmp(event_string, "STOP")) { - got_stop = true; + response = qtest_qmp(who, command); + + while (qdict_haskey(response, "event")) { + /* OK, it was an event */ + event_string = qdict_get_str(response, "event"); + if (!strcmp(event_string, "STOP")) { + got_stop = true; + } + QDECREF(response); + response = qtest_qmp_receive(who); } - QDECREF(response); - return return_or_event(qtest_qmp_receive(global_qtest)); + return response; } @@ -245,12 +248,12 @@ static QDict *return_or_event(QDict *response) * events suddenly appearing confuse the qmp()/hmp() responses. */ -static uint64_t get_migration_pass(void) +static uint64_t get_migration_pass(QTestState *who) { QDict *rsp, *rsp_return, *rsp_ram; uint64_t result; - rsp = return_or_event(qmp("{ 'execute': 'query-migrate' }")); + rsp = wait_command(who, "{ 'execute': 'query-migrate' }"); rsp_return = qdict_get_qdict(rsp, "return"); if (!qdict_haskey(rsp_return, "ram")) { /* Still in setup */ @@ -263,7 +266,7 @@ static uint64_t get_migration_pass(void) return result; } -static void wait_for_migration_complete(void) +static void wait_for_migration_complete(QTestState *who) { QDict *rsp, *rsp_return; bool completed; @@ -271,7 +274,7 @@ static void wait_for_migration_complete(void) do { const char *status; - rsp = return_or_event(qmp("{ 'execute': 'query-migrate' }")); + rsp = wait_command(who, "{ 'execute': 'query-migrate' }"); rsp_return = qdict_get_qdict(rsp, "return"); status = qdict_get_str(rsp_return, "status"); completed = strcmp(status, "completed") == 0; @@ -281,14 +284,14 @@ static void wait_for_migration_complete(void) } while (!completed); } -static void wait_for_migration_pass(void) +static void wait_for_migration_pass(QTestState *who) { - uint64_t initial_pass = get_migration_pass(); + uint64_t initial_pass = get_migration_pass(who); uint64_t pass; /* Wait for the 1st sync */ do { - initial_pass = get_migration_pass(); + initial_pass = get_migration_pass(who); if (got_stop || initial_pass) { break; } @@ -297,7 +300,7 @@ static void wait_for_migration_pass(void) do { usleep(1000 * 100); - pass = get_migration_pass(); + pass = get_migration_pass(who); } while (pass == initial_pass && !got_stop); } @@ -493,7 +496,7 @@ static void test_migrate_end(QTestState *from, QTestState *to) static void test_migrate(void) { char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); - QTestState *global = global_qtest, *from, *to; + QTestState *from, *to; QDict *rsp; test_migrate_start(&from, &to, uri); @@ -513,26 +516,22 @@ static void test_migrate(void) migrate(from, uri); - global_qtest = from; - wait_for_migration_pass(); + wait_for_migration_pass(from); - rsp = return_or_event(qmp("{ 'execute': 'migrate-start-postcopy' }")); + rsp = wait_command(from, "{ 'execute': 'migrate-start-postcopy' }"); g_assert(qdict_haskey(rsp, "return")); QDECREF(rsp); if (!got_stop) { - qmp_eventwait("STOP"); + qtest_qmp_eventwait(from, "STOP"); } - global_qtest = to; - qmp_eventwait("RESUME"); + qtest_qmp_eventwait(to, "RESUME"); wait_for_serial("dest_serial"); - global_qtest = from; - wait_for_migration_complete(); + wait_for_migration_complete(from); g_free(uri); - global_qtest = global; test_migrate_end(from, to); } From 56b4a42a2e0ae74cee629abcb82993e79deeb356 Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Wed, 25 Oct 2017 17:13:37 +0200 Subject: [PATCH 9/9] tests: check that migration parameters are really assigned Signed-off-by: Juan Quintela Reviewed-by: Dr. David Alan Gilbert --- tests/migration-test.c | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/tests/migration-test.c b/tests/migration-test.c index c429a13403..be598d3257 100644 --- a/tests/migration-test.c +++ b/tests/migration-test.c @@ -354,17 +354,37 @@ static void cleanup(const char *filename) g_free(path); } -static void migrate_set_downtime(QTestState *who, const char *value) +static void migrate_check_parameter(QTestState *who, const char *parameter, + const char *value) +{ + QDict *rsp, *rsp_return; + const char *result; + + rsp = wait_command(who, "{ 'execute': 'query-migrate-parameters' }"); + rsp_return = qdict_get_qdict(rsp, "return"); + result = g_strdup_printf("%" PRId64, + qdict_get_try_int(rsp_return, parameter, -1)); + g_assert_cmpstr(result, ==, value); + QDECREF(rsp); +} + +static void migrate_set_downtime(QTestState *who, const double value) { QDict *rsp; gchar *cmd; + char *expected; + int64_t result_int; cmd = g_strdup_printf("{ 'execute': 'migrate_set_downtime'," - "'arguments': { 'value': %s } }", value); + "'arguments': { 'value': %g } }", value); rsp = qtest_qmp(who, cmd); g_free(cmd); g_assert(qdict_haskey(rsp, "return")); QDECREF(rsp); + result_int = value * 1000L; + expected = g_strdup_printf("%" PRId64, result_int); + migrate_check_parameter(who, "downtime-limit", expected); + g_free(expected); } static void migrate_set_speed(QTestState *who, const char *value) @@ -378,6 +398,7 @@ static void migrate_set_speed(QTestState *who, const char *value) g_free(cmd); g_assert(qdict_haskey(rsp, "return")); QDECREF(rsp); + migrate_check_parameter(who, "max-bandwidth", value); } static void migrate_set_capability(QTestState *who, const char *capability, @@ -509,7 +530,7 @@ static void test_migrate(void) * machine, so also set the downtime. */ migrate_set_speed(from, "100000000"); - migrate_set_downtime(from, "0.001"); + migrate_set_downtime(from, 0.001); /* Wait for the first serial output from the source */ wait_for_serial("src_serial");