tests: improve reliability of migration test

[PATCH 2/5] tests: wait for migration completion before looking for STOP event

Posted by Daniel P. Berrangé 3 years, 7 months ago

When moving into the convergance phase, the precopy tests will first
look for a STOP event and once found will look for migration completion
status. If the test VM is not converging, the test suite will be waiting
for the STOP event forever. If we wait for the migration completion
status first, then we will trigger the previously added timeout and
prevent the test hanging forever.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/qtest/migration-test.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index d33e8060f9..ac9e303b1f 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -1232,6 +1232,10 @@ static void test_precopy_common(MigrateCommon *args)
 
         migrate_set_parameter_int(from, "downtime-limit", CONVERGE_DOWNTIME);
 
+        /* We do this first, as it has a timeout to stop us
+         * hanging forever if migration didn't converge */
+        wait_for_migration_complete(from);
+
         if (!got_stop) {
             qtest_qmp_eventwait(from, "STOP");
         }
@@ -1239,7 +1243,6 @@ static void test_precopy_common(MigrateCommon *args)
         qtest_qmp_eventwait(to, "RESUME");
 
         wait_for_serial("dest_serial");
-        wait_for_migration_complete(from);
     }
 
     if (args->finish_hook) {
-- 
2.36.1

Re: [PATCH 2/5] tests: wait for migration completion before looking for STOP event

Posted by Dr. David Alan Gilbert 3 years, 7 months ago

* Daniel P. Berrangé (berrange@redhat.com) wrote:
> When moving into the convergance phase, the precopy tests will first
> look for a STOP event and once found will look for migration completion
> status. If the test VM is not converging, the test suite will be waiting
> for the STOP event forever. If we wait for the migration completion
> status first, then we will trigger the previously added timeout and
> prevent the test hanging forever.

Yeh OK, I guess we might end up with a similar time limit being added to
qtest_qmp_eventwait.


Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  tests/qtest/migration-test.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index d33e8060f9..ac9e303b1f 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -1232,6 +1232,10 @@ static void test_precopy_common(MigrateCommon *args)
>  
>          migrate_set_parameter_int(from, "downtime-limit", CONVERGE_DOWNTIME);
>  
> +        /* We do this first, as it has a timeout to stop us
> +         * hanging forever if migration didn't converge */
> +        wait_for_migration_complete(from);
> +
>          if (!got_stop) {
>              qtest_qmp_eventwait(from, "STOP");
>          }
> @@ -1239,7 +1243,6 @@ static void test_precopy_common(MigrateCommon *args)
>          qtest_qmp_eventwait(to, "RESUME");
>  
>          wait_for_serial("dest_serial");
> -        wait_for_migration_complete(from);
>      }
>  
>      if (args->finish_hook) {
> -- 
> 2.36.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH 2/5] tests: wait for migration completion before looking for STOP event

Posted by Daniel P. Berrangé 3 years, 7 months ago

On Tue, Jun 28, 2022 at 03:08:29PM +0100, Dr. David Alan Gilbert wrote:
> * Daniel P. Berrangé (berrange@redhat.com) wrote:
> > When moving into the convergance phase, the precopy tests will first
> > look for a STOP event and once found will look for migration completion
> > status. If the test VM is not converging, the test suite will be waiting
> > for the STOP event forever. If we wait for the migration completion
> > status first, then we will trigger the previously added timeout and
> > prevent the test hanging forever.
> 
> Yeh OK, I guess we might end up with a similar time limit being added to
> qtest_qmp_eventwait.

Yeah, my first inclination was to modify that method, but it was
not so straightforward, since the wait is done by sitting in recv()
on a blocking socket and I didn't fancy getting into refactoring
that side of things.

> 
> 
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  tests/qtest/migration-test.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> > index d33e8060f9..ac9e303b1f 100644
> > --- a/tests/qtest/migration-test.c
> > +++ b/tests/qtest/migration-test.c
> > @@ -1232,6 +1232,10 @@ static void test_precopy_common(MigrateCommon *args)
> >  
> >          migrate_set_parameter_int(from, "downtime-limit", CONVERGE_DOWNTIME);
> >  
> > +        /* We do this first, as it has a timeout to stop us
> > +         * hanging forever if migration didn't converge */
> > +        wait_for_migration_complete(from);
> > +
> >          if (!got_stop) {
> >              qtest_qmp_eventwait(from, "STOP");
> >          }
> > @@ -1239,7 +1243,6 @@ static void test_precopy_common(MigrateCommon *args)
> >          qtest_qmp_eventwait(to, "RESUME");
> >  
> >          wait_for_serial("dest_serial");
> > -        wait_for_migration_complete(from);
> >      }
> >  
> >      if (args->finish_hook) {
> > -- 
> > 2.36.1
> > 
> -- 
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> 

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

Re: [PATCH 2/5] tests: wait for migration completion before looking for STOP event

Posted by Laurent Vivier 3 years, 7 months ago

On 28/06/2022 12:54, Daniel P. Berrangé wrote:
> When moving into the convergance phase, the precopy tests will first
> look for a STOP event and once found will look for migration completion
> status. If the test VM is not converging, the test suite will be waiting
> for the STOP event forever. If we wait for the migration completion
> status first, then we will trigger the previously added timeout and
> prevent the test hanging forever.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   tests/qtest/migration-test.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index d33e8060f9..ac9e303b1f 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -1232,6 +1232,10 @@ static void test_precopy_common(MigrateCommon *args)
>   
>           migrate_set_parameter_int(from, "downtime-limit", CONVERGE_DOWNTIME);
>   
> +        /* We do this first, as it has a timeout to stop us
> +         * hanging forever if migration didn't converge */
> +        wait_for_migration_complete(from);
> +
>           if (!got_stop) {
>               qtest_qmp_eventwait(from, "STOP");
>           }
> @@ -1239,7 +1243,6 @@ static void test_precopy_common(MigrateCommon *args)
>           qtest_qmp_eventwait(to, "RESUME");
>   
>           wait_for_serial("dest_serial");
> -        wait_for_migration_complete(from);
>       }
>   
>       if (args->finish_hook) {

Reviewed-by: Laurent Vivier <laurent@vivier.eu>