On Tue, Nov 14, 2017 at 1:08 AM, Murilo Opsfelder Araújo <muri...@linux.vnet.ibm.com> wrote: > 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"?
Sure. > >> 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 >
