[Qemu-devel] [PULL 0/9] Migration pull request

[Qemu-devel] [PULL 0/9] Migration pull request

[Juan Quintela]

Hi

This includes the patches reviewed form:
- migrate_set_cache_size series (quintela)
- migrate tests (quintela)

Please apply.

Later, Juan.

The following changes since commit a93ece47fd9edbd4558db24300056c9a57d3bcd4:

  Merge remote-tracking branch 'remotes/sstabellini/tags/xen-20171026-tag' into staging (2017-10-27 14:43:02 +0100)

are available in the git repository at:

  git://github.com/juanquintela/qemu.git tags/migration/20171029

for you to fetch changes up to 56b4a42a2e0ae74cee629abcb82993e79deeb356:

  tests: check that migration parameters are really assigned (2017-10-29 14:06:16 +0100)

----------------------------------------------------------------
migration/next for 20171029

----------------------------------------------------------------
Juan Quintela (9):
      migration: Make sure that we pass the right cache size
      migration: Don't play games with the requested cache size
      migration: No need to return the size of the cache
      migration: Make xbzrle_cache_size a migration parameter
      tests: rename postcopy-test to migration-test
      tests: Refactor setting of parameters/capabilities
      tests: Factorize out migrate_test_start/end
      tests: Don't abuse global_qtest
      tests: check that migration parameters are really assigned

 MAINTAINERS                                 |   1 +
 hmp.c                                       |  14 ++
 migration/migration.c                       |  45 +++--
 migration/migration.h                       |   1 -
 migration/page_cache.c                      |  12 +-
 migration/ram.c                             |  20 +--
 migration/ram.h                             |   2 +-
 qapi/migration.json                         |  26 ++-
 tests/Makefile.include                      |   6 +-
 tests/{postcopy-test.c => migration-test.c} | 259 +++++++++++++++++-----------
 10 files changed, 244 insertions(+), 142 deletions(-)
 rename tests/{postcopy-test.c => migration-test.c} (78%)

[Qemu-devel] [PULL 1/9] migration: Make sure that we pass the right cache size

[Juan Quintela]

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 <quintela@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 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;
-- 
2.13.6

[Qemu-devel] [PULL 2/9] migration: Don't play games with the requested cache size

[Juan Quintela]

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 <quintela@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 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;
-- 
2.13.6

[Qemu-devel] [PULL 3/9] migration: No need to return the size of the cache

[Juan Quintela]

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 <quintela@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

--

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);
 
-- 
2.13.6

[Qemu-devel] [PULL 4/9] migration: Make xbzrle_cache_size a migration parameter

[Juan Quintela]

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 <quintela@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 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:
-- 
2.13.6

[Qemu-devel] [PULL 5/9] tests: rename postcopy-test to migration-test

[Juan Quintela]

Instead of repeating the code, we are going to bo more tests on this file

Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 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();
 
-- 
2.13.6

[Qemu-devel] [PULL 6/9] tests: Refactor setting of parameters/capabilities

[Juan Quintela]

So we can use them in future tests

Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 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' }"));
-- 
2.13.6

[Qemu-devel] [PULL 7/9] tests: Factorize out migrate_test_start/end

[Juan Quintela]

We fix global_test users left and right

Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 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)
-- 
2.13.6

[Qemu-devel] [PULL 8/9] tests: Don't abuse global_qtest

[Juan Quintela]

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 <quintela@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 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;
+
+    response = qtest_qmp(who, command);
 
-    /* OK, it was an event */
-    event_string = qdict_get_str(response, "event");
-    if (!strcmp(event_string, "STOP")) {
-        got_stop = true;
+    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);
 }
-- 
2.13.6

[Qemu-devel] [PULL 9/9] tests: check that migration parameters are really assigned

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 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");
-- 
2.13.6

Re: [Qemu-devel] [PULL 0/9] Migration pull request

[Peter Maydell]

On 29 October 2017 at 13:09, Juan Quintela <quintela@redhat.com> wrote:
> Hi
>
> This includes the patches reviewed form:
> - migrate_set_cache_size series (quintela)
> - migrate tests (quintela)
>
> Please apply.
>
> Later, Juan.
>
> The following changes since commit a93ece47fd9edbd4558db24300056c9a57d3bcd4:
>
>   Merge remote-tracking branch 'remotes/sstabellini/tags/xen-20171026-tag' into staging (2017-10-27 14:43:02 +0100)
>
> are available in the git repository at:
>
>   git://github.com/juanquintela/qemu.git tags/migration/20171029
>
> for you to fetch changes up to 56b4a42a2e0ae74cee629abcb82993e79deeb356:
>
>   tests: check that migration parameters are really assigned (2017-10-29 14:06:16 +0100)
>
> ----------------------------------------------------------------
> migration/next for 20171029
>
> ----------------------------------------------------------------

Applied, thanks.

-- PMM