Re: [Qemu-devel] [PATCH v5 4/6] qemu.py: cleanup launch()

[Cleber Rosa]

On 07/25/2017 01:10 PM, Amador Pahim wrote:
> launch() is currently taking care of a number of flows, each one if its
> own exception treatment, depending on the VM state and the files
> creation state.
> 
> This patch makes launch() more resilient, off-loading the core calls to
> the new _launch() and calling shutdown() if any exception is raised by
> _launch(), making sure VM will be terminated and cleaned up.
> 
> Signed-off-by: Amador Pahim <address@hidden>
> ---
>  scripts/qemu.py | 42 +++++++++++++++++++++++++-----------------
>  1 file changed, 25 insertions(+), 17 deletions(-)
> 
> diff --git a/scripts/qemu.py b/scripts/qemu.py
> index 56142ed59b..45a63e8e9d 100644
> --- a/scripts/qemu.py
> +++ b/scripts/qemu.py
> @@ -99,8 +99,11 @@ class QEMUMachine(object):
>          return self._popen.pid
>  
>      def _load_io_log(self):
> -        with open(self._qemu_log_path, "r") as fh:
> -            self._iolog = fh.read()
> +        try:
> +            with open(self._qemu_log_path, "r") as fh:
> +                self._iolog = fh.read()
> +        except IOError:
> +            pass
>  
>      def _base_args(self):
>          if isinstance(self._monitor_address, tuple):
> @@ -126,23 +129,28 @@ class QEMUMachine(object):
>          self._remove_if_exists(self._qemu_log_path)
>  
>      def launch(self):
> -        '''Launch the VM and establish a QMP connection'''
> -        devnull = open('/dev/null', 'rb')
> -        qemulog = open(self._qemu_log_path, 'wb')
> +        '''
> +        Try to launch the VM and make sure we cleanup on exception.
> +        '''
> +        if self.is_running():
> +            return
> +
>          try:
> -            self._pre_launch()
> -            args = self._wrapper + [self._binary] + self._base_args() + 
> self.args
> -            self._popen = subprocess.Popen(args, stdin=devnull, 
> stdout=qemulog,
> -                                           stderr=subprocess.STDOUT, 
> shell=False)
> -            self._post_launch()
> +            self._launch()
>          except:
> -            if self.is_running():
> -                self._popen.kill()
> -                self._popen.wait()
> -            self._load_io_log()
> -            self._post_shutdown()
> +            self.shutdown()
>              raise
>  
> +    def _launch(self):
> +        '''Launch the VM and establish a QMP connection.'''
> +        devnull = open('/dev/null', 'rb')

Nitpick: using "os.path.devnull" can improve portability.

- Cleber.

> +        qemulog = open(self._qemu_log_path, 'wb')
> +        self._pre_launch()
> +        args = self._wrapper + [self._binary] + self._base_args() + 
> self._args
> +        self._popen = subprocess.Popen(args, stdin=devnull, stdout=qemulog,
> +                                       stderr=subprocess.STDOUT, shell=False)
> +        self._post_launch()
> +
>      def shutdown(self):
>          '''Terminate the VM and clean up'''
>          if self.is_running():
> @@ -156,8 +164,8 @@ class QEMUMachine(object):
>              if exitcode < 0:
>                  sys.stderr.write('qemu received signal %i\n' % -exitcode)
>  
> -            self._load_io_log()
> -            self._post_shutdown()
> +        self._load_io_log()
> +        self._post_shutdown()
>  
>      underscore_to_dash = string.maketrans('_', '-')
>      def qmp(self, cmd, conv_keys=True, **args):
> 

-- 
Cleber Rosa
[ Sr Software Engineer - Virtualization Team - Red Hat ]
[ Avocado Test Framework - avocado-framework.github.io ]
[  7ABB 96EB 8B46 B94D 5E0F  E9BB 657E 8D33 A5F2 09F3  ]

signature.asc
Description: OpenPGP digital signature