From 74af2e59d26712aa673832ec03ec6eac53066c03 Mon Sep 17 00:00:00 2001 From: Amador Pahim Date: Tue, 14 Nov 2017 11:22:39 +0100 Subject: qemu.py: remove unused import Removing 'import sys' as it's not used anywhere. Signed-off-by: Amador Pahim Message-Id: <20171114102246.22221-2-apahim@redhat.com> Signed-off-by: Eduardo Habkost --- scripts/qemu.py | 1 - 1 file changed, 1 deletion(-) (limited to 'scripts/qemu.py') diff --git a/scripts/qemu.py b/scripts/qemu.py index 9bfdf6d..65d9ad6 100644 --- a/scripts/qemu.py +++ b/scripts/qemu.py @@ -15,7 +15,6 @@ import errno import logging import os -import sys import subprocess import qmp.qmp -- cgit v1.1 From af99fa9fe22ba1c6139d1fbe6a66f3b01d575cab Mon Sep 17 00:00:00 2001 From: Amador Pahim Date: Mon, 22 Jan 2018 21:50:28 +0100 Subject: qemu.py: better control of created files 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 Message-Id: <20180122205033.24893-2-apahim@redhat.com> Reviewed-by: Eduardo Habkost Signed-off-by: Eduardo Habkost --- scripts/qemu.py | 39 +++++++++++++++++++++++++++++---------- 1 file changed, 29 insertions(+), 10 deletions(-) (limited to 'scripts/qemu.py') diff --git a/scripts/qemu.py b/scripts/qemu.py index 65d9ad6..8d53920 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,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 @@ -85,6 +88,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() @@ -167,36 +172,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() -- cgit v1.1 From d301bccf7323ff3069e46f600aa7b31dbbc2f2f7 Mon Sep 17 00:00:00 2001 From: Amador Pahim Date: Mon, 22 Jan 2018 21:50:29 +0100 Subject: qemu.py: refactor launch() This is just a refactor to separate the exception handler from the actual launch procedure, improving the readability and making future maintenances in this piece of code easier. Reviewed-by: Fam Zheng Reviewed-by: Eduardo Habkost Signed-off-by: Amador Pahim Message-Id: <20180122205033.24893-3-apahim@redhat.com> Signed-off-by: Eduardo Habkost --- scripts/qemu.py | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) (limited to 'scripts/qemu.py') diff --git a/scripts/qemu.py b/scripts/qemu.py index 8d53920..0333d1e 100644 --- a/scripts/qemu.py +++ b/scripts/qemu.py @@ -205,20 +205,14 @@ class QEMUMachine(object): self._temp_dir = None def launch(self): - '''Launch the VM and establish a QMP connection''' + """ + Launch the VM and make sure we cleanup and expose the + command line/output in case of exception + """ self._iolog = None self._qemu_full_args = None - devnull = open(os.path.devnull, 'rb') 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=self._qemu_log_file, - stderr=subprocess.STDOUT, - shell=False) - self._post_launch() + self._launch() except: if self.is_running(): self._popen.kill() @@ -233,6 +227,19 @@ class QEMUMachine(object): LOG.debug('Output: %r', self._iolog) raise + def _launch(self): + '''Launch the VM and establish a QMP connection''' + devnull = open(os.path.devnull, 'rb') + 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=self._qemu_log_file, + stderr=subprocess.STDOUT, + shell=False) + self._post_launch() + def wait(self): '''Wait for the VM to power off''' self._popen.wait() -- cgit v1.1 From 04a963b4953be6c7f1899cfe0a0a11d03292c18b Mon Sep 17 00:00:00 2001 From: Amador Pahim Date: Mon, 22 Jan 2018 21:50:30 +0100 Subject: qemu.py: always cleanup on shutdown() Currently we only cleanup on shutdown() if the VM is running. To make sure we will always cleanup, this patch makes the self._load_io_log() and the self._post_shutdown() to always be called on shutdown(), regardless the VM running state. Reviewed-by: Fam Zheng Reviewed-by: Eduardo Habkost Signed-off-by: Amador Pahim Message-Id: <20180122205033.24893-4-apahim@redhat.com> Signed-off-by: Eduardo Habkost --- scripts/qemu.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'scripts/qemu.py') diff --git a/scripts/qemu.py b/scripts/qemu.py index 0333d1e..52cf09e 100644 --- a/scripts/qemu.py +++ b/scripts/qemu.py @@ -163,8 +163,9 @@ class QEMUMachine(object): return self._popen.pid def _load_io_log(self): - with open(self._qemu_log_path, "r") as iolog: - self._iolog = iolog.read() + if self._qemu_log_path is not None: + with open(self._qemu_log_path, "r") as iolog: + self._iolog = iolog.read() def _base_args(self): if isinstance(self._monitor_address, tuple): @@ -257,8 +258,8 @@ class QEMUMachine(object): self._popen.kill() self._popen.wait() - self._load_io_log() - self._post_shutdown() + self._load_io_log() + self._post_shutdown() exitcode = self.exitcode() if exitcode is not None and exitcode < 0: -- cgit v1.1 From 17589cae908222d572953d4c85f72aa833e87d58 Mon Sep 17 00:00:00 2001 From: Amador Pahim Date: Mon, 22 Jan 2018 21:50:31 +0100 Subject: qemu.py: use poll() instead of 'returncode' The 'returncode' Popen attribute is not guaranteed to be updated. It actually depends on a call to either poll(), wait() or communicate(). On the other hand, poll() will: "Check if child process has terminated. Set and return returncode attribute." Let's use the poll() to check whether the process is running and to get the updated process exit code, when the process is finished. Reviewed-by: Fam Zheng eviewed-by: Eduardo Habkost Signed-off-by: Amador Pahim Message-Id: <20180122205033.24893-5-apahim@redhat.com> Signed-off-by: Eduardo Habkost --- scripts/qemu.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'scripts/qemu.py') diff --git a/scripts/qemu.py b/scripts/qemu.py index 52cf09e..dcb4f0f 100644 --- a/scripts/qemu.py +++ b/scripts/qemu.py @@ -150,12 +150,12 @@ class QEMUMachine(object): raise def is_running(self): - return self._popen is not None and self._popen.returncode is None + return self._popen is not None and self._popen.poll() is None def exitcode(self): if self._popen is None: return None - return self._popen.returncode + return self._popen.poll() def get_pid(self): if not self.is_running(): -- cgit v1.1 From c58b535f8396f7ab71dc7074713c99ec042da95b Mon Sep 17 00:00:00 2001 From: Amador Pahim Date: Mon, 22 Jan 2018 21:50:32 +0100 Subject: qemu.py: cleanup redundant calls in launch() Now that shutdown() is guaranteed to always execute self._load_io_log() and self._post_shutdown(), their calls in 'except' became redundant and we can safely replace it by a call to shutdown(). Reviewed-by: Fam Zheng Reviewed-by: Eduardo Habkost Signed-off-by: Amador Pahim Message-Id: <20180122205033.24893-6-apahim@redhat.com> Signed-off-by: Eduardo Habkost --- scripts/qemu.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) (limited to 'scripts/qemu.py') diff --git a/scripts/qemu.py b/scripts/qemu.py index dcb4f0f..09db624 100644 --- a/scripts/qemu.py +++ b/scripts/qemu.py @@ -215,11 +215,7 @@ class QEMUMachine(object): try: self._launch() except: - if self.is_running(): - self._popen.kill() - self._popen.wait() - self._load_io_log() - self._post_shutdown() + self.shutdown() LOG.debug('Error launching VM') if self._qemu_full_args: -- cgit v1.1 From 156dc7b1740d9f71baf4aef277d7f812f1a784ba Mon Sep 17 00:00:00 2001 From: Amador Pahim Date: Mon, 22 Jan 2018 21:50:33 +0100 Subject: qemu.py: don't launch again before shutdown() If a VM is launched, files are created and a cleanup is required before a new launch. This cleanup is executed by shutdown(), so shutdown() must be called even if the VM is manually terminated (i.e. using kill). This patch creates a control to make sure launch() will not be executed again if shutdown() is not called after the previous launch(). Signed-off-by: Amador Pahim Message-Id: <20180122205033.24893-7-apahim@redhat.com> Signed-off-by: Eduardo Habkost --- scripts/qemu.py | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'scripts/qemu.py') diff --git a/scripts/qemu.py b/scripts/qemu.py index 09db624..305a946 100644 --- a/scripts/qemu.py +++ b/scripts/qemu.py @@ -90,6 +90,7 @@ class QEMUMachine(object): self._qemu_full_args = None self._test_dir = test_dir self._temp_dir = None + self._launched = False # just in case logging wasn't configured by the main script: logging.basicConfig() @@ -210,10 +211,15 @@ class QEMUMachine(object): Launch the VM and make sure we cleanup and expose the command line/output in case of exception """ + + if self._launched: + raise QEMUMachineError('VM already launched') + self._iolog = None self._qemu_full_args = None try: self._launch() + self._launched = True except: self.shutdown() @@ -266,6 +272,8 @@ class QEMUMachine(object): command = '' LOG.warn(msg, exitcode, command) + self._launched = False + def qmp(self, cmd, conv_keys=True, **args): '''Invoke a QMP command and return the response dict''' qmp_args = dict() -- cgit v1.1