Re: [Qemu-devel] [PATCH v12 1/6] qemu.py: better control of created files

[Eduardo Habkost]

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 
> ---
>  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 

>  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,
> 

[Qemu-devel] [PATCH v12 1/6] qemu.py: better control of created files

[Amador Pahim]

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 
---
 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])
 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