[PATCH] tests/acceptance/migration.py: Wait for both sides

Dr. David Alan Gilbert (git) posted 1 patch 3 years, 11 months ago
Test docker-mingw@fedora passed
Test checkpatch passed
Test asan passed
Test docker-quick@centos7 passed
Test FreeBSD passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200528112404.121972-1-dgilbert@redhat.com
tests/acceptance/migration.py | 4 ++++
1 file changed, 4 insertions(+)
[PATCH] tests/acceptance/migration.py: Wait for both sides
Posted by Dr. David Alan Gilbert (git) 3 years, 11 months ago
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

When the source finishes migration the destination will still be
receiving the data sent by the source, so it might not have quite
finished yet, so won't quite have reached 'completed'.
This lead to occasional asserts in the next few checks.

After the source has finished, check the destination as well.
(We can't just switch to checking the destination, because it doesn't
give a status until it has started receiving the migration).

Reported-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Tested-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/acceptance/migration.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/acceptance/migration.py b/tests/acceptance/migration.py
index 0365289cda..792639cb69 100644
--- a/tests/acceptance/migration.py
+++ b/tests/acceptance/migration.py
@@ -35,6 +35,10 @@ class Migration(Test):
                       timeout=self.timeout,
                       step=0.1,
                       args=(src_vm,))
+        wait.wait_for(self.migration_finished,
+                      timeout=self.timeout,
+                      step=0.1,
+                      args=(dst_vm,))
         self.assertEqual(src_vm.command('query-migrate')['status'], 'completed')
         self.assertEqual(dst_vm.command('query-migrate')['status'], 'completed')
         self.assertEqual(dst_vm.command('query-status')['status'], 'running')
-- 
2.26.2


Re: [PATCH] tests/acceptance/migration.py: Wait for both sides
Posted by Philippe Mathieu-Daudé 3 years, 11 months ago
On 5/28/20 1:24 PM, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> When the source finishes migration the destination will still be
> receiving the data sent by the source, so it might not have quite
> finished yet, so won't quite have reached 'completed'.
> This lead to occasional asserts in the next few checks.
> 
> After the source has finished, check the destination as well.
> (We can't just switch to checking the destination, because it doesn't
> give a status until it has started receiving the migration).
> 

Fixes: a7abb53765 ?

> Reported-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Tested-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  tests/acceptance/migration.py | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tests/acceptance/migration.py b/tests/acceptance/migration.py
> index 0365289cda..792639cb69 100644
> --- a/tests/acceptance/migration.py
> +++ b/tests/acceptance/migration.py
> @@ -35,6 +35,10 @@ class Migration(Test):
>                        timeout=self.timeout,
>                        step=0.1,
>                        args=(src_vm,))
> +        wait.wait_for(self.migration_finished,
> +                      timeout=self.timeout,

I'm not sure the Test.timeout is well used (it represents the maximum
total time the framework will wait this test can take). Anyway this is
incorrectly used before your patch, so I wouldn't bother...

> +                      step=0.1,
> +                      args=(dst_vm,))

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

>          self.assertEqual(src_vm.command('query-migrate')['status'], 'completed')
>          self.assertEqual(dst_vm.command('query-migrate')['status'], 'completed')
>          self.assertEqual(dst_vm.command('query-status')['status'], 'running')
> 


Re: [PATCH] tests/acceptance/migration.py: Wait for both sides
Posted by Philippe Mathieu-Daudé 3 years, 11 months ago
On 5/28/20 1:24 PM, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> When the source finishes migration the destination will still be
> receiving the data sent by the source, so it might not have quite
> finished yet, so won't quite have reached 'completed'.
> This lead to occasional asserts in the next few checks.
> 
> After the source has finished, check the destination as well.
> (We can't just switch to checking the destination, because it doesn't
> give a status until it has started receiving the migration).
> 
> Reported-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Tested-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  tests/acceptance/migration.py | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tests/acceptance/migration.py b/tests/acceptance/migration.py
> index 0365289cda..792639cb69 100644
> --- a/tests/acceptance/migration.py
> +++ b/tests/acceptance/migration.py
> @@ -35,6 +35,10 @@ class Migration(Test):
>                        timeout=self.timeout,
>                        step=0.1,
>                        args=(src_vm,))
> +        wait.wait_for(self.migration_finished,
> +                      timeout=self.timeout,
> +                      step=0.1,
> +                      args=(dst_vm,))
>          self.assertEqual(src_vm.command('query-migrate')['status'], 'completed')
>          self.assertEqual(dst_vm.command('query-migrate')['status'], 'completed')
>          self.assertEqual(dst_vm.command('query-status')['status'], 'running')
> 

Thanks, applied to my python-next tree:
https://gitlab.com/philmd/qemu/commits/python-next