tests/qtest/migration-test: Only wait for serial output where migration succeeds

[PATCH] tests/qtest/migration-test: Only wait for serial output where migration succeeds

Posted by Thomas Huth 1 year, 8 months ago

Waiting for the serial output can take a couple of seconds - and since
we're doing a lot of migration tests, this time easily sums up to
multiple minutes. But if a test is supposed to fail, it does not make
much sense to wait for the source to be in the right state first, so
we can skip the waiting here. This way we can speed up all tests where
the migration is supposed to fail. In the gitlab-CI gprov-gcov test,
each of the migration-tests now run two minutes faster!

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/qtest/migration-test.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 520a5f917c..7be321b62d 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -1307,7 +1307,9 @@ static void test_precopy_common(MigrateCommon *args)
     }
 
     /* Wait for the first serial output from the source */
-    wait_for_serial("src_serial");
+    if (args->result == MIG_TEST_SUCCEED) {
+        wait_for_serial("src_serial");
+    }
 
     if (!args->connect_uri) {
         g_autofree char *local_connect_uri =
-- 
2.31.1

Re: [PATCH] tests/qtest/migration-test: Only wait for serial output where migration succeeds

Posted by Daniel P. Berrangé 1 year, 8 months ago

On Thu, Aug 18, 2022 at 06:21:44PM +0200, Thomas Huth wrote:
> Waiting for the serial output can take a couple of seconds - and since
> we're doing a lot of migration tests, this time easily sums up to
> multiple minutes. But if a test is supposed to fail, it does not make
> much sense to wait for the source to be in the right state first, so
> we can skip the waiting here. This way we can speed up all tests where
> the migration is supposed to fail. In the gitlab-CI gprov-gcov test,
> each of the migration-tests now run two minutes faster!
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  tests/qtest/migration-test.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index 520a5f917c..7be321b62d 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -1307,7 +1307,9 @@ static void test_precopy_common(MigrateCommon *args)
>      }
>  
>      /* Wait for the first serial output from the source */
> -    wait_for_serial("src_serial");
> +    if (args->result == MIG_TEST_SUCCEED) {
> +        wait_for_serial("src_serial");
> +    }
>  
>      if (!args->connect_uri) {
>          g_autofree char *local_connect_uri =

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

Re: [PATCH] tests/qtest/migration-test: Only wait for serial output where migration succeeds

Posted by Dr. David Alan Gilbert 1 year, 8 months ago

* Thomas Huth (thuth@redhat.com) wrote:
> Waiting for the serial output can take a couple of seconds - and since
> we're doing a lot of migration tests, this time easily sums up to
> multiple minutes. But if a test is supposed to fail, it does not make
> much sense to wait for the source to be in the right state first, so
> we can skip the waiting here. This way we can speed up all tests where
> the migration is supposed to fail. In the gitlab-CI gprov-gcov test,
> each of the migration-tests now run two minutes faster!
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  tests/qtest/migration-test.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index 520a5f917c..7be321b62d 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -1307,7 +1307,9 @@ static void test_precopy_common(MigrateCommon *args)
>      }
>  
>      /* Wait for the first serial output from the source */
> -    wait_for_serial("src_serial");
> +    if (args->result == MIG_TEST_SUCCEED) {
> +        wait_for_serial("src_serial");
> +    }

I think this is OK, albeit only because all of the current fail-tests
are ones where the connection fails; we're not relying on the behaviour
of the emulator at all.  I wonder if it's worth going further and
running the source qemu's with -S (which may or not fail in other ways).

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

>  
>      if (!args->connect_uri) {
>          g_autofree char *local_connect_uri =
> -- 
> 2.31.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH] tests/qtest/migration-test: Only wait for serial output where migration succeeds

Posted by Daniel P. Berrangé 1 year, 8 months ago

On Thu, Aug 18, 2022 at 06:07:18PM +0100, Dr. David Alan Gilbert wrote:
> * Thomas Huth (thuth@redhat.com) wrote:
> > Waiting for the serial output can take a couple of seconds - and since
> > we're doing a lot of migration tests, this time easily sums up to
> > multiple minutes. But if a test is supposed to fail, it does not make
> > much sense to wait for the source to be in the right state first, so
> > we can skip the waiting here. This way we can speed up all tests where
> > the migration is supposed to fail. In the gitlab-CI gprov-gcov test,
> > each of the migration-tests now run two minutes faster!
> > 
> > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > ---
> >  tests/qtest/migration-test.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> > index 520a5f917c..7be321b62d 100644
> > --- a/tests/qtest/migration-test.c
> > +++ b/tests/qtest/migration-test.c
> > @@ -1307,7 +1307,9 @@ static void test_precopy_common(MigrateCommon *args)
> >      }
> >  
> >      /* Wait for the first serial output from the source */
> > -    wait_for_serial("src_serial");
> > +    if (args->result == MIG_TEST_SUCCEED) {
> > +        wait_for_serial("src_serial");
> > +    }
> 
> I think this is OK, albeit only because all of the current fail-tests
> are ones where the connection fails; we're not relying on the behaviour
> of the emulator at all.  I wonder if it's worth going further and
> running the source qemu's with -S (which may or not fail in other ways).

Yeah, if we need to cope with yet another failure scenario later
we can add more to the MIG_TEST enum. For now, also adding -S seems
like a reasonable idea to me.

> 
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> 
> >  
> >      if (!args->connect_uri) {
> >          g_autofree char *local_connect_uri =
> > -- 
> > 2.31.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 :|