| Message ID | 20200710050649.32434-13-jsnow@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | python/machine.py: refactor shutdown | expand |
On 7/10/20 7:06 AM, John Snow wrote: > Machine.wait() does not appear to be used except in the acceptance tests, > and an infinite timeout by default in a test suite is not the most helpful. > > Change it to 3 seconds, like the default shutdown timeout. > > Signed-off-by: John Snow <jsnow@redhat.com> > --- > python/qemu/machine.py | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/python/qemu/machine.py b/python/qemu/machine.py > index 02d66e3cff..d08a8e4a6e 100644 > --- a/python/qemu/machine.py > +++ b/python/qemu/machine.py > @@ -472,12 +472,12 @@ def kill(self): > """ > self.shutdown(hard=True) > > - def wait(self, timeout: Optional[int] = None) -> None: > + def wait(self, timeout: Optional[int] = 3) -> None: > """ > Wait for the VM to power off and perform post-shutdown cleanup. > > :param timeout: Optional timeout in seconds. > - Default None, an infinite wait. > + Default 3 seconds, A value of None is an infinite wait. > """ > self.shutdown(has_quit=True, timeout=timeout) > > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
On Fri, Jul 10, 2020 at 01:06:49AM -0400, John Snow wrote: > Machine.wait() does not appear to be used except in the acceptance tests, > and an infinite timeout by default in a test suite is not the most helpful. > > Change it to 3 seconds, like the default shutdown timeout. > > Signed-off-by: John Snow <jsnow@redhat.com> > --- > python/qemu/machine.py | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > Well, for the acceptance tests, there's usually a test wide timeout, but this is indeed a good idea! Reviewed-by: Cleber Rosa <crosa@redhat.com> Tested-by: Cleber Rosa <crosa@redhat.com>
On 7/14/20 12:20 AM, Cleber Rosa wrote: > On Fri, Jul 10, 2020 at 01:06:49AM -0400, John Snow wrote: >> Machine.wait() does not appear to be used except in the acceptance tests, >> and an infinite timeout by default in a test suite is not the most helpful. >> >> Change it to 3 seconds, like the default shutdown timeout. >> >> Signed-off-by: John Snow <jsnow@redhat.com> >> --- >> python/qemu/machine.py | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> > > Well, for the acceptance tests, there's usually a test wide timeout, > but this is indeed a good idea! > > Reviewed-by: Cleber Rosa <crosa@redhat.com> > Tested-by: Cleber Rosa <crosa@redhat.com> > Yes, there's a bigger timeout for acceptance tests, but iotests doesn't have the same just yet. In general, it helps for most of the python library methods to time out by default to prevent hangs in the various test suites. So, anticipating that iotest callers will probably want to use wait() sooner or later, I just went ahead and made the change primarily for consistency again. --js
diff --git a/python/qemu/machine.py b/python/qemu/machine.py index 02d66e3cff..d08a8e4a6e 100644 --- a/python/qemu/machine.py +++ b/python/qemu/machine.py @@ -472,12 +472,12 @@ def kill(self): """ self.shutdown(hard=True) - def wait(self, timeout: Optional[int] = None) -> None: + def wait(self, timeout: Optional[int] = 3) -> None: """ Wait for the VM to power off and perform post-shutdown cleanup. :param timeout: Optional timeout in seconds. - Default None, an infinite wait. + Default 3 seconds, A value of None is an infinite wait. """ self.shutdown(has_quit=True, timeout=timeout)
Machine.wait() does not appear to be used except in the acceptance tests, and an infinite timeout by default in a test suite is not the most helpful. Change it to 3 seconds, like the default shutdown timeout. Signed-off-by: John Snow <jsnow@redhat.com> --- python/qemu/machine.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)