On Mon, Jan 22, 2018 at 09:50:28PM +0100, 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(). > > Signed-off-by: Amador Pahim <apa...@redhat.com> > --- > scripts/qemu.py | 39 +++++++++++++++++++++++++++++---------- > 1 file changed, 29 insertions(+), 10 deletions(-) > > diff --git a/scripts/qemu.py b/scripts/qemu.py > index 9bfdf6d37d..453a67250a 100644 > --- a/scripts/qemu.py > +++ b/scripts/qemu.py > @@ -18,6 +18,8 @@ import os > import sys > import subprocess > import qmp.qmp > +import shutil > +import tempfile > > > LOG = logging.getLogger(__name__) > @@ -73,10 +75,11 @@ 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._vm_monitor = None > + self._qemu_log_path = None > + self._qemu_log_file = None > self._popen = None > self._binary = binary > self._args = list(args) # Force copy args in case we modify them > @@ -86,6 +89,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() > @@ -168,36 +173,50 @@ class QEMUMachine(object): > self._monitor_address[0], > self._monitor_address[1])
If _monitor_address now needs to be translated to _vm_monitor, I'd like to remove all usage of _monitor_address outside _pre_launch, to avoid confusion between the two attributes. This can be done in a follow-up patch. Reviewed-by: Eduardo Habkost <ehabk...@redhat.com> > else: > - moncdev = 'socket,id=mon,path=%s' % self._monitor_address > + moncdev = 'socket,id=mon,path=%s' % self._vm_monitor > return ['-chardev', moncdev, > '-mon', 'chardev=mon,mode=control', > '-display', 'none', '-vga', 'none'] > > def _pre_launch(self): > - self._qmp = qmp.qmp.QEMUMonitorProtocol(self._monitor_address, > + self._temp_dir = tempfile.mkdtemp(dir=self._test_dir) > + if self._monitor_address is not None: > + self._vm_monitor = self._monitor_address > + else: > + self._vm_monitor = 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_file = open(self._qemu_log_path, 'wb') > + > + self._qmp = qmp.qmp.QEMUMonitorProtocol(self._vm_monitor, > server=True) > > def _post_launch(self): > 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_file is not None: > + self._qemu_log_file.close() > + self._qemu_log_file = 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_file, > stderr=subprocess.STDOUT, > shell=False) > self._post_launch() > -- > 2.14.3 > > -- Eduardo
