[Qemu-devel] [PATCH v11 2/8] qemu.py: better control of created files

Amador Pahim posted 8 patches 8 years, 2 months ago
There is a newer version of this series
[Qemu-devel] [PATCH v11 2/8] qemu.py: better control of created files
Posted by Amador Pahim 8 years, 2 months ago
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 | 32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index 65d9ad688c..d5b1cde044 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_file = 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()
@@ -173,6 +177,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_file = open(self._qemu_log_path, 'wb')
+
         self._qmp = qmp.qmp.QEMUMonitorProtocol(self._monitor_address,
                                                 server=True)
 
@@ -180,23 +191,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_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.13.6


Re: [Qemu-devel] [PATCH v11 2/8] qemu.py: better control of created files
Posted by Eduardo Habkost 8 years ago
Hi Amador,

First of all, sorry for reviewing this so late.

On Tue, Nov 14, 2017 at 11:22:40AM +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 <apahim@redhat.com>
> ---
>  scripts/qemu.py | 32 ++++++++++++++++++++++++--------
>  1 file changed, 24 insertions(+), 8 deletions(-)
> 
> diff --git a/scripts/qemu.py b/scripts/qemu.py
> index 65d9ad688c..d5b1cde044 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_file = 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()
> @@ -173,6 +177,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")

Won't this break QEMUMachine(..., monitor_addres='/some/unix/path')?

What about checking if self._monitor_address is None instead?

The rest looks good to me.

> +        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._monitor_address,
>                                                  server=True)
>  
> @@ -180,23 +191,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_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.13.6
> 
> 

-- 
Eduardo