| Message ID | 20220722145654.81103-2-peterx@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | migration-test: Allow test to run without uffd | expand |
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
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.
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); }
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(-)