On 11/13/2017 07:39 PM, Amador Pahim wrote: > To launch a VM, we need to create basically two files: the monitor > socket (if it's a UNIX socket) and the qemu log file. > > For the qemu log file, we currently just open the path, which will > create the file if it does not exist or overwrite the file if it does > exist. > > For the monitor socket, if it already exists, we are currently removing > it, even if it's not created by us. > > This patch moves to pre_launch() the responsibility to create a > temporary directory to host the files so we can remove the whole > directory on post_shutdown().
s/pre_launch()/_pre_launch()/ s/post_shutdown()/_post_shutdown()/ > Signed-off-by: Amador Pahim <apa...@redhat.com> > --- > scripts/qemu.py | 42 ++++++++++++++++++++++++------------------ > 1 file changed, 24 insertions(+), 18 deletions(-) > > diff --git a/scripts/qemu.py b/scripts/qemu.py > index 65d9ad688c..58f00bdd64 100644 > --- a/scripts/qemu.py > +++ b/scripts/qemu.py > @@ -17,6 +17,8 @@ import logging > import os > import subprocess > import qmp.qmp > +import shutil > +import tempfile > > > LOG = logging.getLogger(__name__) > @@ -72,10 +74,10 @@ class QEMUMachine(object): > wrapper = [] > if name is None: > name = "qemu-%d" % os.getpid() > - if monitor_address is None: > - monitor_address = os.path.join(test_dir, name + "-monitor.sock") > + self._name = name > self._monitor_address = monitor_address > - self._qemu_log_path = os.path.join(test_dir, name + ".log") > + self._qemu_log_path = None > + self._qemu_log_fd = None Is our intent to hold an integer file descriptor in self._qemu_log_fd? Python's built-in open() returns a file object, which is what we're using here. Do you think it shall be named, for example, _qemu_log_file to avoid confusion with integer file descriptors, usually denoted by "fd"? > self._popen = None > self._binary = binary > self._args = list(args) # Force copy args in case we modify them > @@ -85,6 +87,8 @@ class QEMUMachine(object): > self._socket_scm_helper = socket_scm_helper > self._qmp = None > self._qemu_full_args = None > + self._test_dir = test_dir > + self._temp_dir = None > > # just in case logging wasn't configured by the main script: > logging.basicConfig() > @@ -134,16 +138,6 @@ class QEMUMachine(object): > > return proc.returncode > > - @staticmethod > - def _remove_if_exists(path): > - '''Remove file object at path if it exists''' > - try: > - os.remove(path) > - except OSError as exception: > - if exception.errno == errno.ENOENT: > - return > - raise > - > def is_running(self): > return self._popen is not None and self._popen.returncode is None > > @@ -173,6 +167,13 @@ class QEMUMachine(object): > '-display', 'none', '-vga', 'none'] > > def _pre_launch(self): > + self._temp_dir = tempfile.mkdtemp(dir=self._test_dir) > + if not isinstance(self._monitor_address, tuple): > + self._monitor_address = os.path.join(self._temp_dir, > + self._name + > "-monitor.sock") > + self._qemu_log_path = os.path.join(self._temp_dir, self._name + > ".log") > + self._qemu_log_fd = open(self._qemu_log_path, 'wb') > + > self._qmp = qmp.qmp.QEMUMonitorProtocol(self._monitor_address, > server=True) > > @@ -180,23 +181,28 @@ class QEMUMachine(object): > self._qmp.accept() > > def _post_shutdown(self): > - if not isinstance(self._monitor_address, tuple): > - self._remove_if_exists(self._monitor_address) > - self._remove_if_exists(self._qemu_log_path) > + if self._qemu_log_fd is not None: > + self._qemu_log_fd.close() > + self._qemu_log_fd = None > + > + self._qemu_log_path = None > + > + if self._temp_dir is not None: > + shutil.rmtree(self._temp_dir) > + self._temp_dir = None > > def launch(self): > '''Launch the VM and establish a QMP connection''' > self._iolog = None > self._qemu_full_args = None > devnull = open(os.path.devnull, 'rb') > - qemulog = open(self._qemu_log_path, 'wb') > try: > self._pre_launch() > self._qemu_full_args = (self._wrapper + [self._binary] + > self._base_args() + self._args) > self._popen = subprocess.Popen(self._qemu_full_args, > stdin=devnull, > - stdout=qemulog, > + stdout=self._qemu_log_fd, > stderr=subprocess.STDOUT, > shell=False) > self._post_launch() > -- Murilo
