1. Patchew
  2. QEMU
  3. [PATCH v2 0/2] migration-test: Allow test to run without uffd
  4. View patch

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

Peter Xu posted 2 patches 3 years, 6 months ago
Maintainers: Juan Quintela <quintela@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Thomas Huth <thuth@redhat.com>, Laurent Vivier <lvivier@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>
There is a newer version of this series
[PATCH v2 1/2] migration-test: Use migrate_ensure_converge() for auto-converge
Posted by Peter Xu 3 years, 6 months ago 
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>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 tests/qtest/migration-test.c | 17 +----------------
 1 file changed, 1 insertion(+), 16 deletions(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 71595a74fd..dd50aa600c 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -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
Re: [PATCH v2 1/2] migration-test: Use migrate_ensure_converge() for auto-converge
Posted by Thomas Huth 3 years, 6 months ago 
On 22/07/2022 16.56, Peter Xu wrote:
> 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>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>   tests/qtest/migration-test.c | 17 +----------------
>   1 file changed, 1 insertion(+), 16 deletions(-)
> 
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index 71595a74fd..dd50aa600c 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -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));
> -

I'm getting this on FreeBSD:

../tests/qtest/migration-test.c:1771:13: error: unused variable 'remaining' 
[-Werror,-Wunused-variable]
     int64_t remaining, percentage;
             ^
1 error generated.

Did you try this with -Werror ?

  Thomas
Re: [PATCH v2 1/2] migration-test: Use migrate_ensure_converge() for auto-converge
Posted by Peter Xu 3 years, 6 months ago 
On Thu, Jul 28, 2022 at 03:04:27PM +0200, Thomas Huth wrote:
> On 22/07/2022 16.56, Peter Xu wrote:
> > 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>
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >   tests/qtest/migration-test.c | 17 +----------------
> >   1 file changed, 1 insertion(+), 16 deletions(-)
> > 
> > diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> > index 71595a74fd..dd50aa600c 100644
> > --- a/tests/qtest/migration-test.c
> > +++ b/tests/qtest/migration-test.c
> > @@ -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));
> > -
> 
> I'm getting this on FreeBSD:
> 
> ../tests/qtest/migration-test.c:1771:13: error: unused variable 'remaining'
> [-Werror,-Wunused-variable]
>     int64_t remaining, percentage;
>             ^
> 1 error generated.
> 
> Did you try this with -Werror ?

Thanks for catching that, I'll start to do so and repost.

-- 
Peter Xu
Re: [PATCH v2 1/2] migration-test: Use migrate_ensure_converge() for auto-converge
Posted by Daniel P. Berrangé 3 years, 6 months ago 
On Fri, Jul 22, 2022 at 10:56:53AM -0400, Peter Xu wrote:
> 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>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  tests/qtest/migration-test.c | 17 +----------------
>  1 file changed, 1 insertion(+), 16 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.