[v2,6/6] tests/migration: Only run auto_converge in slow mode

Commit Message

Daniel P. Berrangé April 21, 2023, 5:14 p.m. UTC

From: Juan Quintela <[e-mail]>

Signed-off-by: Juan Quintela <[e-mail]>
---
 tests/qtest/migration-test.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

Comments

Zhang Chen April 23, 2023, 2:41 a.m. UTC | #1

> -----Original Message-----
> From: Daniel P. Berrangé <[e-mail]>
> Sent: Saturday, April 22, 2023 1:14 AM
> To: [e-mail]
> Cc: [e-mail]; Paolo Bonzini <[e-mail]>;
> Thomas Huth <[e-mail]>; John Snow <[e-mail]>; Li
> Zhijian <[e-mail]>; Juan Quintela <[e-mail]>;
> Stefan Hajnoczi <[e-mail]>; Zhang, Chen
> <[e-mail]>; Laurent Vivier <[e-mail]>
> Subject: [PATCH v2 6/6] tests/migration: Only run auto_converge in slow
> mode
> 

What kind of scenario will the qtest open this g_test_init() -m slow to trigger the slow mode?

Thanks
Chen

> From: Juan Quintela <[e-mail]>
> 
> Signed-off-by: Juan Quintela <[e-mail]>
> ---
>  tests/qtest/migration-test.c | 23 +++++++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c index
> 63bd8a1893..9ed178aa03 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -1877,6 +1877,21 @@ static void test_validate_uuid_dst_not_set(void)
>      do_test_validate_uuid(&args, false);  }
> 
> +/*
> + * The way auto_converge works, we need to do too many passes to
> + * run this test.  Auto_converge logic is only run once every
> + * three iterations, so:
> + *
> + * - 3 iterations without auto_converge enabled
> + * - 3 iterations with pct = 5
> + * - 3 iterations with pct = 30
> + * - 3 iterations with pct = 55
> + * - 3 iterations with pct = 80
> + * - 3 iterations with pct = 95 (max(95, 80 + 25))
> + *
> + * To make things even worse, we need to run the initial stage at
> + * 3MB/s so we enter autoconverge even when host is (over)loaded.
> + */
>  static void test_migrate_auto_converge(void)  {
>      g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); @@ -
> 2660,8 +2675,12 @@ int main(int argc, char **argv)
>                     test_validate_uuid_src_not_set);
>      qtest_add_func("/migration/validate_uuid_dst_not_set",
>                     test_validate_uuid_dst_not_set);
> -
> -    qtest_add_func("/migration/auto_converge",
> test_migrate_auto_converge);
> +    /*
> +     * See explanation why this test is slow on function definition
> +     */
> +    if (g_test_slow()) {
> +        qtest_add_func("/migration/auto_converge",
> test_migrate_auto_converge);
> +    }
>      qtest_add_func("/migration/multifd/tcp/plain/none",
>                     test_multifd_tcp_none);
>      /*
> --
> 2.40.0

Juan Quintela April 24, 2023, 5:58 a.m. UTC | #2

"Zhang, Chen" <[e-mail]> wrote:
>> -----Original Message-----
>> From: Daniel P. Berrangé <[e-mail]>
>> Sent: Saturday, April 22, 2023 1:14 AM
>> To: [e-mail]
>> Cc: [e-mail]; Paolo Bonzini <[e-mail]>;
>> Thomas Huth <[e-mail]>; John Snow <[e-mail]>; Li
>> Zhijian <[e-mail]>; Juan Quintela <[e-mail]>;
>> Stefan Hajnoczi <[e-mail]>; Zhang, Chen
>> <[e-mail]>; Laurent Vivier <[e-mail]>
>> Subject: [PATCH v2 6/6] tests/migration: Only run auto_converge in slow
>> mode
>> 
>
> What kind of scenario will the qtest open this g_test_init() -m slow to trigger the slow mode?

The only way that I know is:

export G_TEST_SLOW=1
make check (or whatever individual test that you want)

Later, Juan.

Thomas Huth April 24, 2023, 6:56 a.m. UTC | #3

On 24/04/2023 07.58, Juan Quintela wrote:
> "Zhang, Chen" <[e-mail]> wrote:
>>> -----Original Message-----
>>> From: Daniel P. Berrangé <[e-mail]>
>>> Sent: Saturday, April 22, 2023 1:14 AM
>>> To: [e-mail]
>>> Cc: [e-mail]; Paolo Bonzini <[e-mail]>;
>>> Thomas Huth <[e-mail]>; John Snow <[e-mail]>; Li
>>> Zhijian <[e-mail]>; Juan Quintela <[e-mail]>;
>>> Stefan Hajnoczi <[e-mail]>; Zhang, Chen
>>> <[e-mail]>; Laurent Vivier <[e-mail]>
>>> Subject: [PATCH v2 6/6] tests/migration: Only run auto_converge in slow
>>> mode
>>>
>>
>> What kind of scenario will the qtest open this g_test_init() -m slow to trigger the slow mode?
> 
> The only way that I know is:
> 
> export G_TEST_SLOW=1
> make check (or whatever individual test that you want)

Or even simpler:

  make check SPEED=slow

Or if you want to run the test manually:

  QTEST_QEMU_BINARY=./qemu-system-x86_64 \
  tests/qtest/migration-test -m slow

HTH,
  Thomas

Zhang Chen April 24, 2023, 8:05 a.m. UTC | #4

> -----Original Message-----
> From: Thomas Huth <[e-mail]>
> Sent: Monday, April 24, 2023 2:56 PM
> To: [e-mail]; Zhang, Chen <[e-mail]>
> Cc: Daniel P. Berrangé <[e-mail]>; [e-mail];
> [e-mail]; Paolo Bonzini <[e-mail]>; John
> Snow <[e-mail]>; Li Zhijian <[e-mail]>; Stefan
> Hajnoczi <[e-mail]>; Laurent Vivier <[e-mail]>
> Subject: Re: [PATCH v2 6/6] tests/migration: Only run auto_converge in slow
> mode
> 
> On 24/04/2023 07.58, Juan Quintela wrote:
> > "Zhang, Chen" <[e-mail]> wrote:
> >>> -----Original Message-----
> >>> From: Daniel P. Berrangé <[e-mail]>
> >>> Sent: Saturday, April 22, 2023 1:14 AM
> >>> To: [e-mail]
> >>> Cc: [e-mail]; Paolo Bonzini <[e-mail]>;
> >>> Thomas Huth <[e-mail]>; John Snow <[e-mail]>; Li
> >>> Zhijian <[e-mail]>; Juan Quintela
> >>> <[e-mail]>; Stefan Hajnoczi <[e-mail]>;
> Zhang,
> >>> Chen <[e-mail]>; Laurent Vivier <[e-mail]>
> >>> Subject: [PATCH v2 6/6] tests/migration: Only run auto_converge in
> >>> slow mode
> >>>
> >>
> >> What kind of scenario will the qtest open this g_test_init() -m slow to
> trigger the slow mode?
> >
> > The only way that I know is:
> >
> > export G_TEST_SLOW=1
> > make check (or whatever individual test that you want)
> 
> Or even simpler:
> 
>   make check SPEED=slow
> 
> Or if you want to run the test manually:
> 
>   QTEST_QEMU_BINARY=./qemu-system-x86_64 \
>   tests/qtest/migration-test -m slow
> 

Thanks for the explanation.

Thanks
Chen

> HTH,
>   Thomas

Zhang Chen April 24, 2023, 8:06 a.m. UTC | #5

> -----Original Message-----
> From: Daniel P. Berrangé <[e-mail]>
> Sent: Saturday, April 22, 2023 1:14 AM
> To: [e-mail]
> Cc: [e-mail]; Paolo Bonzini <[e-mail]>;
> Thomas Huth <[e-mail]>; John Snow <[e-mail]>; Li
> Zhijian <[e-mail]>; Juan Quintela <[e-mail]>;
> Stefan Hajnoczi <[e-mail]>; Zhang, Chen
> <[e-mail]>; Laurent Vivier <[e-mail]>
> Subject: [PATCH v2 6/6] tests/migration: Only run auto_converge in slow
> mode
> 
> From: Juan Quintela <[e-mail]>
> 
> Signed-off-by: Juan Quintela <[e-mail]>

LGTM.
Reviewed-by: Zhang Chen <[e-mail]>

> ---
>  tests/qtest/migration-test.c | 23 +++++++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)

Patch

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 63bd8a1893..9ed178aa03 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -1877,6 +1877,21 @@  static void test_validate_uuid_dst_not_set(void)
     do_test_validate_uuid(&args, false);
 }
 
+/*
+ * The way auto_converge works, we need to do too many passes to
+ * run this test.  Auto_converge logic is only run once every
+ * three iterations, so:
+ *
+ * - 3 iterations without auto_converge enabled
+ * - 3 iterations with pct = 5
+ * - 3 iterations with pct = 30
+ * - 3 iterations with pct = 55
+ * - 3 iterations with pct = 80
+ * - 3 iterations with pct = 95 (max(95, 80 + 25))
+ *
+ * To make things even worse, we need to run the initial stage at
+ * 3MB/s so we enter autoconverge even when host is (over)loaded.
+ */
 static void test_migrate_auto_converge(void)
 {
     g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
@@ -2660,8 +2675,12 @@  int main(int argc, char **argv)
                    test_validate_uuid_src_not_set);
     qtest_add_func("/migration/validate_uuid_dst_not_set",
                    test_validate_uuid_dst_not_set);
-
-    qtest_add_func("/migration/auto_converge", test_migrate_auto_converge);
+    /*
+     * See explanation why this test is slow on function definition
+     */
+    if (g_test_slow()) {
+        qtest_add_func("/migration/auto_converge", test_migrate_auto_converge);
+    }
     qtest_add_func("/migration/multifd/tcp/plain/none",
                    test_multifd_tcp_none);
     /*