[Qemu-devel] [PATCH] iotests: fix race in 030

[Qemu-devel] [PATCH] iotests: fix race in 030

[John Snow]

the stop_test case tests that we can resume a block-stream
command after it has stopped/paused due to error. We cannot
always reliably query it before it finishes after resume, though,
so make this a conditional.

The important thing is that we are still testing that it has stopped,
and that it finishes successfully after we send a resume command.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/qemu-iotests/030 | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
index 952a524..fd0d186 100755
--- a/tests/qemu-iotests/030
+++ b/tests/qemu-iotests/030
@@ -245,6 +245,7 @@ class TestEIO(TestErrors):
         while not completed:
             for event in self.vm.get_qmp_events(wait=True):
                 if event['event'] == 'BLOCK_JOB_ERROR':
+                    error = True
                     self.assert_qmp(event, 'data/device', 'drive0')
                     self.assert_qmp(event, 'data/operation', 'read')
 
@@ -257,9 +258,11 @@ class TestEIO(TestErrors):
                     self.assert_qmp(result, 'return', {})
 
                     result = self.vm.qmp('query-block-jobs')
+                    if (result == {'return': []}):
+                        # Race; likely already finished. Check.
+                        continue
                     self.assert_qmp(result, 'return[0]/paused', False)
                     self.assert_qmp(result, 'return[0]/io-status', 'ok')
-                    error = True
                 elif event['event'] == 'BLOCK_JOB_COMPLETED':
                     self.assertTrue(error, 'job completed unexpectedly')
                     self.assert_qmp(event, 'data/type', 'stream')
-- 
2.4.3

Re: [Qemu-devel] [PATCH] iotests: fix race in 030

[Jeff Cody]

On Wed, Nov 11, 2015 at 03:27:36PM -0500, John Snow wrote:
> the stop_test case tests that we can resume a block-stream
> command after it has stopped/paused due to error. We cannot
> always reliably query it before it finishes after resume, though,
> so make this a conditional.
> 
> The important thing is that we are still testing that it has stopped,
> and that it finishes successfully after we send a resume command.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  tests/qemu-iotests/030 | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
> index 952a524..fd0d186 100755
> --- a/tests/qemu-iotests/030
> +++ b/tests/qemu-iotests/030
> @@ -245,6 +245,7 @@ class TestEIO(TestErrors):
>          while not completed:
>              for event in self.vm.get_qmp_events(wait=True):
>                  if event['event'] == 'BLOCK_JOB_ERROR':
> +                    error = True
>                      self.assert_qmp(event, 'data/device', 'drive0')
>                      self.assert_qmp(event, 'data/operation', 'read')
>  
> @@ -257,9 +258,11 @@ class TestEIO(TestErrors):
>                      self.assert_qmp(result, 'return', {})
>  
>                      result = self.vm.qmp('query-block-jobs')
> +                    if (result == {'return': []}):
> +                        # Race; likely already finished. Check.
> +                        continue
>                      self.assert_qmp(result, 'return[0]/paused', False)
>                      self.assert_qmp(result, 'return[0]/io-status', 'ok')
> -                    error = True
>                  elif event['event'] == 'BLOCK_JOB_COMPLETED':
>                      self.assertTrue(error, 'job completed unexpectedly')
>                      self.assert_qmp(event, 'data/type', 'stream')
> -- 
> 2.4.3
> 

Reviewed-by: Jeff Cody <jcody@redhat.com>

Re: [Qemu-devel] [Qemu-block] [PATCH] iotests: fix race in 030

[Alberto Garcia]

On Wed 11 Nov 2015 09:27:36 PM CET, John Snow <jsnow@redhat.com> wrote:
> the stop_test case tests that we can resume a block-stream
> command after it has stopped/paused due to error. We cannot
> always reliably query it before it finishes after resume, though,
> so make this a conditional.
>
> The important thing is that we are still testing that it has stopped,
> and that it finishes successfully after we send a resume command.
>
> Signed-off-by: John Snow <jsnow@redhat.com>

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto

Re: [Qemu-devel] [PATCH] iotests: fix race in 030

[Fam Zheng]

On Wed, 11/11 15:27, John Snow wrote:
> the stop_test case tests that we can resume a block-stream
> command after it has stopped/paused due to error. We cannot
> always reliably query it before it finishes after resume, though,
> so make this a conditional.
> 
> The important thing is that we are still testing that it has stopped,
> and that it finishes successfully after we send a resume command.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  tests/qemu-iotests/030 | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
> index 952a524..fd0d186 100755
> --- a/tests/qemu-iotests/030
> +++ b/tests/qemu-iotests/030
> @@ -245,6 +245,7 @@ class TestEIO(TestErrors):
>          while not completed:
>              for event in self.vm.get_qmp_events(wait=True):
>                  if event['event'] == 'BLOCK_JOB_ERROR':
> +                    error = True
>                      self.assert_qmp(event, 'data/device', 'drive0')
>                      self.assert_qmp(event, 'data/operation', 'read')
>  
> @@ -257,9 +258,11 @@ class TestEIO(TestErrors):
>                      self.assert_qmp(result, 'return', {})
>  
>                      result = self.vm.qmp('query-block-jobs')
> +                    if (result == {'return': []}):

Using parenthisis around if conditions is not native Python but "(scalar)"
equals to "scalar", so the statement is still correct.  Maybe this could be
fixed when applying, or even if it stays as is:

Reviewed-by: Fam Zheng <famz@redhat.com>

> +                        # Race; likely already finished. Check.
> +                        continue
>                      self.assert_qmp(result, 'return[0]/paused', False)
>                      self.assert_qmp(result, 'return[0]/io-status', 'ok')
> -                    error = True
>                  elif event['event'] == 'BLOCK_JOB_COMPLETED':
>                      self.assertTrue(error, 'job completed unexpectedly')
>                      self.assert_qmp(event, 'data/type', 'stream')
> -- 
> 2.4.3
> 
> 

Re: [Qemu-devel] [PATCH] iotests: fix race in 030

[Kevin Wolf]

Am 11.11.2015 um 21:27 hat John Snow geschrieben:
> the stop_test case tests that we can resume a block-stream
> command after it has stopped/paused due to error. We cannot
> always reliably query it before it finishes after resume, though,
> so make this a conditional.
> 
> The important thing is that we are still testing that it has stopped,
> and that it finishes successfully after we send a resume command.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>

Thanks, applied to the block branch.

Kevin