[PATCH v3 0/2] migration-test: Allow test to run without uffd

[PATCH v3 0/2] migration-test: Allow test to run without uffd

[Peter Xu]

v2:
- Fix warning in patch 1 [Thomas]
- Collected R-b for Daniel

Compare to v1, this added a new patch as reported by Thomas to (hopefully)
allow auto-converge test to pass on some MacOS testbeds.

Please review, thanks.

Peter Xu (2):
  migration-test: Use migrate_ensure_converge() for auto-converge
  migration-test: Allow test to run without uffd

 tests/qtest/migration-test.c | 67 +++++++++++++++---------------------
 1 file changed, 27 insertions(+), 40 deletions(-)

-- 
2.32.0

[PATCH v3 1/2] migration-test: Use migrate_ensure_converge() for auto-converge

[Peter Xu]

Thomas reported that auto-converge test will timeout on MacOS CI gatings.
Use the migrate_ensure_converge() helper too in the auto-converge as when
Daniel reworked the other test cases.

Since both max_bandwidth / downtime_limit will not be used for converge
calculations, make it simple by removing the remaining check, then we can
completely remove both variables altogether, since migrate_ensure_converge
is used the remaining check won't make much sense anyway.

Suggested-by: Daniel P. Berrange <berrange@redhat.com>
Reported-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 tests/qtest/migration-test.c | 19 ++-----------------
 1 file changed, 2 insertions(+), 17 deletions(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 71595a74fd..c1e002087d 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -1768,7 +1768,7 @@ static void test_migrate_auto_converge(void)
     g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
     MigrateStart args = {};
     QTestState *from, *to;
-    int64_t remaining, percentage;
+    int64_t percentage;
 
     /*
      * We want the test to be stable and as fast as possible.
@@ -1776,14 +1776,6 @@ static void test_migrate_auto_converge(void)
      * so we need to decrease a bandwidth.
      */
     const int64_t init_pct = 5, inc_pct = 50, max_pct = 95;
-    const int64_t max_bandwidth = 400000000; /* ~400Mb/s */
-    const int64_t downtime_limit = 250; /* 250ms */
-    /*
-     * We migrate through unix-socket (> 500Mb/s).
-     * Thus, expected migration speed ~= bandwidth limit (< 500Mb/s).
-     * So, we can predict expected_threshold
-     */
-    const int64_t expected_threshold = max_bandwidth * downtime_limit / 1000;
 
     if (test_migrate_start(&from, &to, uri, &args)) {
         return;
@@ -1818,8 +1810,7 @@ static void test_migrate_auto_converge(void)
     /* The first percentage of throttling should be equal to init_pct */
     g_assert_cmpint(percentage, ==, init_pct);
     /* Now, when we tested that throttling works, let it converge */
-    migrate_set_parameter_int(from, "downtime-limit", downtime_limit);
-    migrate_set_parameter_int(from, "max-bandwidth", max_bandwidth);
+    migrate_ensure_converge(from);
 
     /*
      * Wait for pre-switchover status to check last throttle percentage
@@ -1830,11 +1821,6 @@ static void test_migrate_auto_converge(void)
     /* The final percentage of throttling shouldn't be greater than max_pct */
     percentage = read_migrate_property_int(from, "cpu-throttle-percentage");
     g_assert_cmpint(percentage, <=, max_pct);
-
-    remaining = read_ram_property_int(from, "remaining");
-    g_assert_cmpint(remaining, <,
-                    (expected_threshold + expected_threshold / 100));
-
     migrate_continue(from, "pre-switchover");
 
     qtest_qmp_eventwait(to, "RESUME");
@@ -1842,7 +1828,6 @@ static void test_migrate_auto_converge(void)
     wait_for_serial("dest_serial");
     wait_for_migration_complete(from);
 
-
     test_migrate_end(from, to, true);
 }
 
-- 
2.32.0

[PATCH v3 2/2] migration-test: Allow test to run without uffd

[Peter Xu]

We used to stop running all tests if uffd is not detected.  However
logically that's only needed for postcopy not the rest of tests.

Keep running the rest when still possible.

Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 tests/qtest/migration-test.c | 48 +++++++++++++++++++-----------------
 1 file changed, 25 insertions(+), 23 deletions(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index c1e002087d..10ab7a708c 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -2424,14 +2424,11 @@ int main(int argc, char **argv)
 {
     char template[] = "/tmp/migration-test-XXXXXX";
     const bool has_kvm = qtest_has_accel("kvm");
+    const bool has_uffd = ufd_version_check();
     int ret;
 
     g_test_init(&argc, &argv, NULL);
 
-    if (!ufd_version_check()) {
-        return g_test_run();
-    }
-
     /*
      * On ppc64, the test only works with kvm-hv, but not with kvm-pr and TCG
      * is touchy due to race conditions on dirty bits (especially on PPC for
@@ -2460,13 +2457,15 @@ int main(int argc, char **argv)
 
     module_call_init(MODULE_INIT_QOM);
 
-    qtest_add_func("/migration/postcopy/unix", test_postcopy);
-    qtest_add_func("/migration/postcopy/plain", test_postcopy);
-    qtest_add_func("/migration/postcopy/recovery/plain",
-                   test_postcopy_recovery);
-    qtest_add_func("/migration/postcopy/preempt/plain", test_postcopy_preempt);
-    qtest_add_func("/migration/postcopy/preempt/recovery/plain",
-                    test_postcopy_preempt_recovery);
+    if (has_uffd) {
+        qtest_add_func("/migration/postcopy/unix", test_postcopy);
+        qtest_add_func("/migration/postcopy/plain", test_postcopy);
+        qtest_add_func("/migration/postcopy/recovery/plain",
+                       test_postcopy_recovery);
+        qtest_add_func("/migration/postcopy/preempt/plain", test_postcopy_preempt);
+        qtest_add_func("/migration/postcopy/preempt/recovery/plain",
+                       test_postcopy_preempt_recovery);
+    }
 
     qtest_add_func("/migration/bad_dest", test_baddest);
     qtest_add_func("/migration/precopy/unix/plain", test_precopy_unix_plain);
@@ -2474,18 +2473,21 @@ int main(int argc, char **argv)
 #ifdef CONFIG_GNUTLS
     qtest_add_func("/migration/precopy/unix/tls/psk",
                    test_precopy_unix_tls_psk);
-    /*
-     * NOTE: psk test is enough for postcopy, as other types of TLS
-     * channels are tested under precopy.  Here what we want to test is the
-     * general postcopy path that has TLS channel enabled.
-     */
-    qtest_add_func("/migration/postcopy/tls/psk", test_postcopy_tls_psk);
-    qtest_add_func("/migration/postcopy/recovery/tls/psk",
-                   test_postcopy_recovery_tls_psk);
-    qtest_add_func("/migration/postcopy/preempt/tls/psk",
-                   test_postcopy_preempt_tls_psk);
-    qtest_add_func("/migration/postcopy/preempt/recovery/tls/psk",
-                   test_postcopy_preempt_all);
+
+    if (has_uffd) {
+        /*
+         * NOTE: psk test is enough for postcopy, as other types of TLS
+         * channels are tested under precopy.  Here what we want to test is the
+         * general postcopy path that has TLS channel enabled.
+         */
+        qtest_add_func("/migration/postcopy/tls/psk", test_postcopy_tls_psk);
+        qtest_add_func("/migration/postcopy/recovery/tls/psk",
+                       test_postcopy_recovery_tls_psk);
+        qtest_add_func("/migration/postcopy/preempt/tls/psk",
+                       test_postcopy_preempt_tls_psk);
+        qtest_add_func("/migration/postcopy/preempt/recovery/tls/psk",
+                       test_postcopy_preempt_all);
+    }
 #ifdef CONFIG_TASN1
     qtest_add_func("/migration/precopy/unix/tls/x509/default-host",
                    test_precopy_unix_tls_x509_default_host);
-- 
2.32.0

Re: [PATCH v3 0/2] migration-test: Allow test to run without uffd

[Thomas Huth]

On 28/07/2022 15.35, Peter Xu wrote:
> v2:
> - Fix warning in patch 1 [Thomas]
> - Collected R-b for Daniel
> 
> Compare to v1, this added a new patch as reported by Thomas to (hopefully)
> allow auto-converge test to pass on some MacOS testbeds.
> 
> Please review, thanks.
> 
> Peter Xu (2):
>    migration-test: Use migrate_ensure_converge() for auto-converge
>    migration-test: Allow test to run without uffd
> 
>   tests/qtest/migration-test.c | 67 +++++++++++++++---------------------
>   1 file changed, 27 insertions(+), 40 deletions(-)

Seems to work now:

https://api.cirrus-ci.com/v1/task/4760264934424576/logs/build.log

Citing:

" 2/59 qemu:qtest+qtest-x86_64 / qtest-x86_64/migration-test         OK 
         218.87s   33 subtests passed"

Tested-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v3 0/2] migration-test: Allow test to run without uffd

[Peter Xu]

On Thu, Jul 28, 2022 at 04:44:49PM +0200, Thomas Huth wrote:
> On 28/07/2022 15.35, Peter Xu wrote:
> > v2:
> > - Fix warning in patch 1 [Thomas]
> > - Collected R-b for Daniel
> > 
> > Compare to v1, this added a new patch as reported by Thomas to (hopefully)
> > allow auto-converge test to pass on some MacOS testbeds.
> > 
> > Please review, thanks.
> > 
> > Peter Xu (2):
> >    migration-test: Use migrate_ensure_converge() for auto-converge
> >    migration-test: Allow test to run without uffd
> > 
> >   tests/qtest/migration-test.c | 67 +++++++++++++++---------------------
> >   1 file changed, 27 insertions(+), 40 deletions(-)
> 
> Seems to work now:
> 
> https://api.cirrus-ci.com/v1/task/4760264934424576/logs/build.log
> 
> Citing:
> 
> " 2/59 qemu:qtest+qtest-x86_64 / qtest-x86_64/migration-test         OK
> 218.87s   33 subtests passed"
> 
> Tested-by: Thomas Huth <thuth@redhat.com>

Thanks!

-- 
Peter Xu

Re: [PATCH v3 0/2] migration-test: Allow test to run without uffd

[Thomas Huth]

On 28/07/2022 15.35, Peter Xu wrote:
> v2:
> - Fix warning in patch 1 [Thomas]
> - Collected R-b for Daniel
> 
> Compare to v1, this added a new patch as reported by Thomas to (hopefully)
> allow auto-converge test to pass on some MacOS testbeds.
> 
> Please review, thanks.
> 
> Peter Xu (2):
>    migration-test: Use migrate_ensure_converge() for auto-converge
>    migration-test: Allow test to run without uffd
> 
>   tests/qtest/migration-test.c | 67 +++++++++++++++---------------------
>   1 file changed, 27 insertions(+), 40 deletions(-)

Thanks, I've queued this to my testing-next branch now:

  https://gitlab.com/thuth/qemu/-/commits/testing-next/

  Thomas