Re: [PATCH v4 2/4] iotests.py: Add wait_for_runstate()

2020-08-20 Thread Max Reitz 
On 20.08.20 16:34, Vladimir Sementsov-Ogievskiy wrote:
> 20.08.2020 17:23, Dr. David Alan Gilbert wrote:
>> * Eric Blake (ebl...@redhat.com) wrote:
>>> On 8/18/20 8:32 AM, Max Reitz wrote:
 Signed-off-by: Max Reitz 
 ---
    tests/qemu-iotests/iotests.py | 4 
    1 file changed, 4 insertions(+)
>>>
>>> Reviewed-by: Eric Blake 
>>>

 diff --git a/tests/qemu-iotests/iotests.py
 b/tests/qemu-iotests/iotests.py
 index 717b5b652c..ee93cf22db 100644
 --- a/tests/qemu-iotests/iotests.py
 +++ b/tests/qemu-iotests/iotests.py
 @@ -833,6 +833,10 @@ class VM(qtest.QEMUQtestMachine):
   'Found node %s under %s (but expected %s)' % \
   (node['name'], path, expected_node)
 +    def wait_for_runstate(self, runstate: str) -> None:
 +    while self.qmp('query-status')['return']['status'] !=
 runstate:
 +    time.sleep(0.2)
>>>
>>> This looks like it could inf-loop if we have a bug where the status
>>> never
>>> changes as expected; but I guess CI bots have timeouts at higher
>>> layers that
>>> would detect if such a bug sneaks in.
>>
>> Although it might be useful to make sure when such a timeout lands, you
>> know which state you thought you were waiting for.
>>
>> Dave
>>
> 
> Timeout class is defined in iotests.py, so we can simply insert
> something like
> 
>  ... , timeout=60) -> None:
>   with Timeout(timeout, f"Timeout waiting for '{runstate}' runstate"):
>  ...

Actually, I’m not even using this function here in v4 anymore, so this
patch might as well be dropped.

Max



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v4 2/4] iotests.py: Add wait_for_runstate()

2020-08-20 Thread Vladimir Sementsov-Ogievskiy 

20.08.2020 17:23, Dr. David Alan Gilbert wrote:

* Eric Blake (ebl...@redhat.com) wrote:

On 8/18/20 8:32 AM, Max Reitz wrote:

Signed-off-by: Max Reitz 
---
   tests/qemu-iotests/iotests.py | 4 
   1 file changed, 4 insertions(+)


Reviewed-by: Eric Blake 



diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 717b5b652c..ee93cf22db 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -833,6 +833,10 @@ class VM(qtest.QEMUQtestMachine):
  'Found node %s under %s (but expected %s)' % \
  (node['name'], path, expected_node)
+def wait_for_runstate(self, runstate: str) -> None:
+while self.qmp('query-status')['return']['status'] != runstate:
+time.sleep(0.2)


This looks like it could inf-loop if we have a bug where the status never
changes as expected; but I guess CI bots have timeouts at higher layers that
would detect if such a bug sneaks in.


Although it might be useful to make sure when such a timeout lands, you
know which state you thought you were waiting for.

Dave



Timeout class is defined in iotests.py, so we can simply insert something like

 ... , timeout=60) -> None:
  with Timeout(timeout, f"Timeout waiting for '{runstate}' runstate"):
 ...


--
Best regards,
Vladimir

Re: [PATCH v4 2/4] iotests.py: Add wait_for_runstate()

2020-08-20 Thread Dr. David Alan Gilbert 
* Eric Blake (ebl...@redhat.com) wrote:
> On 8/18/20 8:32 AM, Max Reitz wrote:
> > Signed-off-by: Max Reitz 
> > ---
> >   tests/qemu-iotests/iotests.py | 4 
> >   1 file changed, 4 insertions(+)
> 
> Reviewed-by: Eric Blake 
> 
> > 
> > diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> > index 717b5b652c..ee93cf22db 100644
> > --- a/tests/qemu-iotests/iotests.py
> > +++ b/tests/qemu-iotests/iotests.py
> > @@ -833,6 +833,10 @@ class VM(qtest.QEMUQtestMachine):
> >  'Found node %s under %s (but expected %s)' % \
> >  (node['name'], path, expected_node)
> > +def wait_for_runstate(self, runstate: str) -> None:
> > +while self.qmp('query-status')['return']['status'] != runstate:
> > +time.sleep(0.2)
> 
> This looks like it could inf-loop if we have a bug where the status never
> changes as expected; but I guess CI bots have timeouts at higher layers that
> would detect if such a bug sneaks in.

Although it might be useful to make sure when such a timeout lands, you
know which state you thought you were waiting for.

Dave

> > +
> >   index_re = re.compile(r'([^\[]+)\[([^\]]+)\]')
> >   class QMPTestCase(unittest.TestCase):
> > 
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.   +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
-- 
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [PATCH v4 2/4] iotests.py: Add wait_for_runstate()

2020-08-20 Thread Vladimir Sementsov-Ogievskiy 

18.08.2020 16:32, Max Reitz wrote:

Signed-off-by: Max Reitz


Reviewed-by: Vladimir Sementsov-Ogievskiy 

--
Best regards,
Vladimir

Re: [PATCH v4 2/4] iotests.py: Add wait_for_runstate()

2020-08-19 Thread Eric Blake 

On 8/18/20 8:32 AM, Max Reitz wrote:

Signed-off-by: Max Reitz 
---
  tests/qemu-iotests/iotests.py | 4 
  1 file changed, 4 insertions(+)


Reviewed-by: Eric Blake 



diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 717b5b652c..ee93cf22db 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -833,6 +833,10 @@ class VM(qtest.QEMUQtestMachine):
 'Found node %s under %s (but expected %s)' % \
 (node['name'], path, expected_node)
  
+def wait_for_runstate(self, runstate: str) -> None:

+while self.qmp('query-status')['return']['status'] != runstate:
+time.sleep(0.2)


This looks like it could inf-loop if we have a bug where the status 
never changes as expected; but I guess CI bots have timeouts at higher 
layers that would detect if such a bug sneaks in.



+
  index_re = re.compile(r'([^\[]+)\[([^\]]+)\]')
  
  class QMPTestCase(unittest.TestCase):




--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org