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 <apahim@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
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()
--
2.13.6
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 <apahim@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
On Tue, Nov 14, 2017 at 1:08 AM, Murilo Opsfelder Araújo <muriloo@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 <apahim@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 >
© 2016 - 2025 Red Hat, Inc.