Use the switchover-hold flag rather than tuning bw+downtime to guide test
convergence.

This can achieve similar goal of previous patch "tests/qtest: massively
speed up migration-test" but without magic offset to write or monitoring.

The initial solution can reduce migration-test time from 8min to 1min40s,
this patch can further reduce it from 1m40s to 1m1s per my local test.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 tests/qtest/migration-test.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index b0c355bbd9..62bdd67fd9 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -433,16 +433,15 @@ static void migrate_set_parameter_bool(QTestState *who, const char *parameter,
 
 static void migrate_ensure_non_converge(QTestState *who)
 {
-    /* Can't converge with 1ms downtime + 3 mbs bandwidth limit */
-    migrate_set_parameter_int(who, "max-bandwidth", 3 * 1000 * 1000);
-    migrate_set_parameter_int(who, "downtime-limit", 1);
+    /* Hold off switchover for precopy only */
+    migrate_set_parameter_bool(who, "switchover-hold", true);
 }
 
 static void migrate_ensure_converge(QTestState *who)
 {
-    /* Should converge with 30s downtime + 1 gbs bandwidth limit */
-    migrate_set_parameter_int(who, "max-bandwidth", 1 * 1000 * 1000 * 1000);
-    migrate_set_parameter_int(who, "downtime-limit", 30 * 1000);
+    /* No limitation on bandwidth so converge faster */
+    migrate_set_parameter_int(who, "max-bandwidth", 0);
+    migrate_set_parameter_bool(who, "switchover-hold", false);
 }
 
 static void migrate_pause(QTestState *who)
@@ -492,6 +491,13 @@ static void migrate_postcopy_start(QTestState *from, QTestState *to)
     }
 
     qtest_qmp_eventwait(to, "RESUME");
+
+    /*
+     * Now allow precopy switchover (which will allow completion).  This
+     * needs to be done after migrate-start-postcopy to make sure we switch
+     * to postcopy first.
+     */
+    migrate_ensure_converge(from);
 }
 
 typedef struct {
@@ -1164,6 +1170,8 @@ static int migrate_postcopy_prepare(QTestState **from_ptr,
     }
 
     migrate_ensure_non_converge(from);
+    /* Still use unlimited precopy speed to finish 1st iteration fast */
+    migrate_set_parameter_int(from, "max-bandwidth", 0);
 
     /* Wait for the first serial output from the source */
     wait_for_serial("src_serial");
-- 
2.40.1

On Thu, Jun 01, 2023 at 09:16:26PM -0400, Peter Xu wrote:
> Use the switchover-hold flag rather than tuning bw+downtime to guide test
> convergence.
> 
> This can achieve similar goal of previous patch "tests/qtest: massively
> speed up migration-test" but without magic offset to write or monitoring.
> 
> The initial solution can reduce migration-test time from 8min to 1min40s,
> this patch can further reduce it from 1m40s to 1m1s per my local test.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  tests/qtest/migration-test.c | 20 ++++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index b0c355bbd9..62bdd67fd9 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -433,16 +433,15 @@ static void migrate_set_parameter_bool(QTestState *who, const char *parameter,
>  
>  static void migrate_ensure_non_converge(QTestState *who)
>  {
> -    /* Can't converge with 1ms downtime + 3 mbs bandwidth limit */
> -    migrate_set_parameter_int(who, "max-bandwidth", 3 * 1000 * 1000);
> -    migrate_set_parameter_int(who, "downtime-limit", 1);
> +    /* Hold off switchover for precopy only */
> +    migrate_set_parameter_bool(who, "switchover-hold", true);
>  }
>  
>  static void migrate_ensure_converge(QTestState *who)
>  {
> -    /* Should converge with 30s downtime + 1 gbs bandwidth limit */
> -    migrate_set_parameter_int(who, "max-bandwidth", 1 * 1000 * 1000 * 1000);
> -    migrate_set_parameter_int(who, "downtime-limit", 30 * 1000);
> +    /* No limitation on bandwidth so converge faster */
> +    migrate_set_parameter_int(who, "max-bandwidth", 0);

You're already setting max-bandwith==0 in migrate_postcopy_prepare.

If you also set it in test_precopy_common, so we don't need to
set it here, and we'll avoid the initial phase running with
bandwidth=32mbs.

> +    migrate_set_parameter_bool(who, "switchover-hold", false);
>  }
>  
>  static void migrate_pause(QTestState *who)
> @@ -492,6 +491,13 @@ static void migrate_postcopy_start(QTestState *from, QTestState *to)
>      }
>  
>      qtest_qmp_eventwait(to, "RESUME");
> +
> +    /*
> +     * Now allow precopy switchover (which will allow completion).  This
> +     * needs to be done after migrate-start-postcopy to make sure we switch
> +     * to postcopy first.
> +     */
> +    migrate_ensure_converge(from);
>  }
>  
>  typedef struct {
> @@ -1164,6 +1170,8 @@ static int migrate_postcopy_prepare(QTestState **from_ptr,
>      }
>  
>      migrate_ensure_non_converge(from);
> +    /* Still use unlimited precopy speed to finish 1st iteration fast */
> +    migrate_set_parameter_int(from, "max-bandwidth", 0);
>  
>      /* Wait for the first serial output from the source */
>      wait_for_serial("src_serial");
> -- 
> 2.40.1
> 

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 :|

On Fri, Jun 02, 2023 at 12:03:05PM +0100, Daniel P. Berrangé wrote:
> On Thu, Jun 01, 2023 at 09:16:26PM -0400, Peter Xu wrote:
> > Use the switchover-hold flag rather than tuning bw+downtime to guide test
> > convergence.
> > 
> > This can achieve similar goal of previous patch "tests/qtest: massively
> > speed up migration-test" but without magic offset to write or monitoring.
> > 
> > The initial solution can reduce migration-test time from 8min to 1min40s,
> > this patch can further reduce it from 1m40s to 1m1s per my local test.
> > 
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >  tests/qtest/migration-test.c | 20 ++++++++++++++------
> >  1 file changed, 14 insertions(+), 6 deletions(-)
> > 
> > diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> > index b0c355bbd9..62bdd67fd9 100644
> > --- a/tests/qtest/migration-test.c
> > +++ b/tests/qtest/migration-test.c
> > @@ -433,16 +433,15 @@ static void migrate_set_parameter_bool(QTestState *who, const char *parameter,
> >  
> >  static void migrate_ensure_non_converge(QTestState *who)
> >  {
> > -    /* Can't converge with 1ms downtime + 3 mbs bandwidth limit */
> > -    migrate_set_parameter_int(who, "max-bandwidth", 3 * 1000 * 1000);
> > -    migrate_set_parameter_int(who, "downtime-limit", 1);
> > +    /* Hold off switchover for precopy only */
> > +    migrate_set_parameter_bool(who, "switchover-hold", true);
> >  }
> >  
> >  static void migrate_ensure_converge(QTestState *who)
> >  {
> > -    /* Should converge with 30s downtime + 1 gbs bandwidth limit */
> > -    migrate_set_parameter_int(who, "max-bandwidth", 1 * 1000 * 1000 * 1000);
> > -    migrate_set_parameter_int(who, "downtime-limit", 30 * 1000);
> > +    /* No limitation on bandwidth so converge faster */
> > +    migrate_set_parameter_int(who, "max-bandwidth", 0);
> 
> You're already setting max-bandwith==0 in migrate_postcopy_prepare.
> 
> If you also set it in test_precopy_common, so we don't need to
> set it here, and we'll avoid the initial phase running with
> bandwidth=32mbs.

There are more tests than test_precopy_common() that leverages this, so I
used to kept it there to still share some code:

test_migrate_auto_converge[2021] migrate_ensure_converge(from);
test_multifd_tcp_cancel[2349]  migrate_ensure_converge(from);

But I think you're right - moving the bw=0 setup out should be cleaner.
I'll see whether I should just move it into all the specific tests.

Thanks,

-- 
Peter Xu