* [PATCH 0/2] tests/acceptance: Use QEMUMachine on tests that expect failure @ 2019-11-12 13:57 Wainer dos Santos Moschetta 2019-11-12 13:58 ` [PATCH 1/2] python/qemu: Add set_qmp_monitor() to QEMUMachine Wainer dos Santos Moschetta ` (2 more replies) 0 siblings, 3 replies; 7+ messages in thread From: Wainer dos Santos Moschetta @ 2019-11-12 13:57 UTC (permalink / raw) To: qemu-devel; +Cc: philmd, ehabkost, crosa The linux_initrd and empty_cpu_model tests assert that QEMU exit with failure on certain scenarios. Currently they are not able to use QEMUMachine object due to the QMP monitor connection which is tentatively established always. Instead they handle the QEMU binary directy, but ideally they should use QEMUMachine in order to: a) Take advantage of error handling and logging in QEMUMachine. b) Follow the pattern in other acceptance tests. Notes worth it: - Patch 01 first appeared in [1]. Here I propose the same implementation but the code was rebased. - empty_cpu_model used to check both stdout and stderr of the process. The QEMUMachine doesn't provide an interface to access the underneath process object, instead the process' output is available through get_log(). This method in turn returns the content of stdout (stderr is redirected to stdout). Therefore, I adapted the assertion statement to check the '-cpu option cannot be empty' message shows in the output. Git: - Tree: https://github.com/wainersm/qemu - Branch: tests_without_qmp CI: - Travis (PASS): https://travis-ci.org/wainersm/qemu/builds/610499693 References: [1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg627498.html Wainer dos Santos Moschetta (2): python/qemu: Add set_qmp_monitor() to QEMUMachine tests/acceptance: Makes linux_initrd and empty_cpu_model use QEMUMachine python/qemu/machine.py | 68 +++++++++++++++++++---------- tests/acceptance/empty_cpu_model.py | 13 +++--- tests/acceptance/linux_initrd.py | 13 +++--- 3 files changed, 59 insertions(+), 35 deletions(-) -- 2.18.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] python/qemu: Add set_qmp_monitor() to QEMUMachine 2019-11-12 13:57 [PATCH 0/2] tests/acceptance: Use QEMUMachine on tests that expect failure Wainer dos Santos Moschetta @ 2019-11-12 13:58 ` Wainer dos Santos Moschetta 2019-12-10 5:17 ` Cleber Rosa 2019-11-12 13:58 ` [PATCH 2/2] tests/acceptance: Makes linux_initrd and empty_cpu_model use QEMUMachine Wainer dos Santos Moschetta 2019-12-09 17:44 ` [PATCH 0/2] tests/acceptance: Use QEMUMachine on tests that expect failure Wainer dos Santos Moschetta 2 siblings, 1 reply; 7+ messages in thread From: Wainer dos Santos Moschetta @ 2019-11-12 13:58 UTC (permalink / raw) To: qemu-devel; +Cc: philmd, ehabkost, crosa The QEMUMachine VM has a monitor setup on which an QMP connection is always attempted on _post_launch() (executed by launch()). In case the QEMU process immediatly exits then the qmp.accept() (used to establish the connection) stalls until it reaches timeout and consequently an exception raises. That behavior is undesirable when, for instance, it needs to gather information from the QEMU binary ($ qemu -cpu list) or a test which launches the VM expecting its failure. This patch adds the set_qmp_monitor() method to QEMUMachine that allows turn off the creation of the monitor machinery on VM launch. Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com> --- python/qemu/machine.py | 68 ++++++++++++++++++++++++++++-------------- 1 file changed, 45 insertions(+), 23 deletions(-) diff --git a/python/qemu/machine.py b/python/qemu/machine.py index a4631d6934..04ee86e1ba 100644 --- a/python/qemu/machine.py +++ b/python/qemu/machine.py @@ -104,6 +104,7 @@ class QEMUMachine(object): self._events = [] self._iolog = None self._socket_scm_helper = socket_scm_helper + self._qmp_set = True # Enable QMP monitor by default. self._qmp = None self._qemu_full_args = None self._test_dir = test_dir @@ -228,15 +229,16 @@ class QEMUMachine(object): self._iolog = iolog.read() def _base_args(self): - if isinstance(self._monitor_address, tuple): - moncdev = "socket,id=mon,host=%s,port=%s" % ( + args = ['-display', 'none', '-vga', 'none'] + if self._qmp_set: + if isinstance(self._monitor_address, tuple): + moncdev = "socket,id=mon,host=%s,port=%s" % ( self._monitor_address[0], self._monitor_address[1]) - else: - moncdev = 'socket,id=mon,path=%s' % self._vm_monitor - args = ['-chardev', moncdev, - '-mon', 'chardev=mon,mode=control', - '-display', 'none', '-vga', 'none'] + else: + moncdev = 'socket,id=mon,path=%s' % self._vm_monitor + args.extend(['-chardev', moncdev, '-mon', + 'chardev=mon,mode=control']) if self._machine is not None: args.extend(['-machine', self._machine]) if self._console_set: @@ -255,20 +257,21 @@ class QEMUMachine(object): def _pre_launch(self): 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._sock_dir, - self._name + "-monitor.sock") - self._remove_files.append(self._vm_monitor) 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.QEMUMonitorProtocol(self._vm_monitor, - server=True) + if self._qmp_set: + if self._monitor_address is not None: + self._vm_monitor = self._monitor_address + else: + self._vm_monitor = os.path.join(self._sock_dir, + self._name + "-monitor.sock") + self._remove_files.append(self._vm_monitor) + self._qmp = qmp.QEMUMonitorProtocol(self._vm_monitor, server=True) def _post_launch(self): - self._qmp.accept() + if self._qmp: + self._qmp.accept() def _post_shutdown(self): if self._qemu_log_file is not None: @@ -330,7 +333,8 @@ class QEMUMachine(object): Wait for the VM to power off """ self._popen.wait() - self._qmp.close() + if self._qmp: + self._qmp.close() self._load_io_log() self._post_shutdown() @@ -346,12 +350,13 @@ class QEMUMachine(object): self._console_socket = None if self.is_running(): - try: - if not has_quit: - self._qmp.cmd('quit') - self._qmp.close() - except: - self._popen.kill() + if self._qmp: + try: + if not has_quit: + self._qmp.cmd('quit') + self._qmp.close() + except: + self._popen.kill() self._popen.wait() self._load_io_log() @@ -368,6 +373,23 @@ class QEMUMachine(object): self._launched = False + def set_qmp_monitor(self, disabled=False, monitor_address=None): + """ + Set the QMP monitor. + + @param disabled: if True, qmp monitor options will be removed from the + base arguments of the resulting QEMU command line. + @param monitor_address: address for the QMP monitor. + @note: call this function before launch(). + """ + if disabled: + self._qmp_set = False + self._qmp = None + else: + self._qmp_set = True + if monitor_address: + self._monitor_address = monitor_address + def qmp(self, cmd, conv_keys=True, **args): """ Invoke a QMP command and return the response dict -- 2.18.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] python/qemu: Add set_qmp_monitor() to QEMUMachine 2019-11-12 13:58 ` [PATCH 1/2] python/qemu: Add set_qmp_monitor() to QEMUMachine Wainer dos Santos Moschetta @ 2019-12-10 5:17 ` Cleber Rosa 2019-12-11 14:55 ` Wainer dos Santos Moschetta 0 siblings, 1 reply; 7+ messages in thread From: Cleber Rosa @ 2019-12-10 5:17 UTC (permalink / raw) To: Wainer dos Santos Moschetta; +Cc: philmd, qemu-devel, ehabkost [-- Attachment #1: Type: text/plain, Size: 6349 bytes --] On Tue, Nov 12, 2019 at 08:58:00AM -0500, Wainer dos Santos Moschetta wrote: > The QEMUMachine VM has a monitor setup on which an QMP > connection is always attempted on _post_launch() (executed > by launch()). In case the QEMU process immediatly exits > then the qmp.accept() (used to establish the connection) stalls > until it reaches timeout and consequently an exception raises. > > That behavior is undesirable when, for instance, it needs to > gather information from the QEMU binary ($ qemu -cpu list) or a > test which launches the VM expecting its failure. > > This patch adds the set_qmp_monitor() method to QEMUMachine that > allows turn off the creation of the monitor machinery on VM launch. > > Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com> > --- > python/qemu/machine.py | 68 ++++++++++++++++++++++++++++-------------- > 1 file changed, 45 insertions(+), 23 deletions(-) > > diff --git a/python/qemu/machine.py b/python/qemu/machine.py > index a4631d6934..04ee86e1ba 100644 > --- a/python/qemu/machine.py > +++ b/python/qemu/machine.py > @@ -104,6 +104,7 @@ class QEMUMachine(object): > self._events = [] > self._iolog = None > self._socket_scm_helper = socket_scm_helper > + self._qmp_set = True # Enable QMP monitor by default. > self._qmp = None > self._qemu_full_args = None > self._test_dir = test_dir > @@ -228,15 +229,16 @@ class QEMUMachine(object): > self._iolog = iolog.read() > > def _base_args(self): > - if isinstance(self._monitor_address, tuple): > - moncdev = "socket,id=mon,host=%s,port=%s" % ( > + args = ['-display', 'none', '-vga', 'none'] > + if self._qmp_set: > + if isinstance(self._monitor_address, tuple): > + moncdev = "socket,id=mon,host=%s,port=%s" % ( > self._monitor_address[0], > self._monitor_address[1]) > - else: > - moncdev = 'socket,id=mon,path=%s' % self._vm_monitor > - args = ['-chardev', moncdev, > - '-mon', 'chardev=mon,mode=control', > - '-display', 'none', '-vga', 'none'] > + else: > + moncdev = 'socket,id=mon,path=%s' % self._vm_monitor > + args.extend(['-chardev', moncdev, '-mon', > + 'chardev=mon,mode=control']) > if self._machine is not None: > args.extend(['-machine', self._machine]) > if self._console_set: > @@ -255,20 +257,21 @@ class QEMUMachine(object): > > def _pre_launch(self): > 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._sock_dir, > - self._name + "-monitor.sock") > - self._remove_files.append(self._vm_monitor) > 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.QEMUMonitorProtocol(self._vm_monitor, > - server=True) > + if self._qmp_set: > + if self._monitor_address is not None: > + self._vm_monitor = self._monitor_address > + else: > + self._vm_monitor = os.path.join(self._sock_dir, > + self._name + "-monitor.sock") > + self._remove_files.append(self._vm_monitor) > + self._qmp = qmp.QEMUMonitorProtocol(self._vm_monitor, server=True) > > def _post_launch(self): > - self._qmp.accept() > + if self._qmp: > + self._qmp.accept() > > def _post_shutdown(self): > if self._qemu_log_file is not None: > @@ -330,7 +333,8 @@ class QEMUMachine(object): > Wait for the VM to power off > """ > self._popen.wait() > - self._qmp.close() > + if self._qmp: > + self._qmp.close() > self._load_io_log() > self._post_shutdown() > > @@ -346,12 +350,13 @@ class QEMUMachine(object): > self._console_socket = None > > if self.is_running(): > - try: > - if not has_quit: > - self._qmp.cmd('quit') > - self._qmp.close() > - except: > - self._popen.kill() > + if self._qmp: > + try: > + if not has_quit: > + self._qmp.cmd('quit') > + self._qmp.close() > + except: > + self._popen.kill() > self._popen.wait() > > self._load_io_log() > @@ -368,6 +373,23 @@ class QEMUMachine(object): > > self._launched = False > > + def set_qmp_monitor(self, disabled=False, monitor_address=None): > + """ > + Set the QMP monitor. > + > + @param disabled: if True, qmp monitor options will be removed from the > + base arguments of the resulting QEMU command line. I personally tend avoid double negatives as long as I'm aware of them. Wouldn't "enabled=True" be more straightforward? Just my personal preference though. > + @param monitor_address: address for the QMP monitor. Do you have a use case for passing the monitor address here too, in addition to also being available as a parameter to __init__()? In the next patch, I don't see it being used (or here for that matter). > + @note: call this function before launch(). > + """ > + if disabled: > + self._qmp_set = False > + self._qmp = None > + else: > + self._qmp_set = True > + if monitor_address: > + self._monitor_address = monitor_address > + > def qmp(self, cmd, conv_keys=True, **args): > """ > Invoke a QMP command and return the response dict > -- > 2.18.1 > Other than those comments, it LGTM. - Cleber. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] python/qemu: Add set_qmp_monitor() to QEMUMachine 2019-12-10 5:17 ` Cleber Rosa @ 2019-12-11 14:55 ` Wainer dos Santos Moschetta 0 siblings, 0 replies; 7+ messages in thread From: Wainer dos Santos Moschetta @ 2019-12-11 14:55 UTC (permalink / raw) To: Cleber Rosa; +Cc: philmd, qemu-devel, ehabkost On 12/10/19 3:17 AM, Cleber Rosa wrote: > On Tue, Nov 12, 2019 at 08:58:00AM -0500, Wainer dos Santos Moschetta wrote: >> The QEMUMachine VM has a monitor setup on which an QMP >> connection is always attempted on _post_launch() (executed >> by launch()). In case the QEMU process immediatly exits >> then the qmp.accept() (used to establish the connection) stalls >> until it reaches timeout and consequently an exception raises. >> >> That behavior is undesirable when, for instance, it needs to >> gather information from the QEMU binary ($ qemu -cpu list) or a >> test which launches the VM expecting its failure. >> >> This patch adds the set_qmp_monitor() method to QEMUMachine that >> allows turn off the creation of the monitor machinery on VM launch. >> >> Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com> >> --- >> python/qemu/machine.py | 68 ++++++++++++++++++++++++++++-------------- >> 1 file changed, 45 insertions(+), 23 deletions(-) >> >> diff --git a/python/qemu/machine.py b/python/qemu/machine.py >> index a4631d6934..04ee86e1ba 100644 >> --- a/python/qemu/machine.py >> +++ b/python/qemu/machine.py >> @@ -104,6 +104,7 @@ class QEMUMachine(object): >> self._events = [] >> self._iolog = None >> self._socket_scm_helper = socket_scm_helper >> + self._qmp_set = True # Enable QMP monitor by default. >> self._qmp = None >> self._qemu_full_args = None >> self._test_dir = test_dir >> @@ -228,15 +229,16 @@ class QEMUMachine(object): >> self._iolog = iolog.read() >> >> def _base_args(self): >> - if isinstance(self._monitor_address, tuple): >> - moncdev = "socket,id=mon,host=%s,port=%s" % ( >> + args = ['-display', 'none', '-vga', 'none'] >> + if self._qmp_set: >> + if isinstance(self._monitor_address, tuple): >> + moncdev = "socket,id=mon,host=%s,port=%s" % ( >> self._monitor_address[0], >> self._monitor_address[1]) >> - else: >> - moncdev = 'socket,id=mon,path=%s' % self._vm_monitor >> - args = ['-chardev', moncdev, >> - '-mon', 'chardev=mon,mode=control', >> - '-display', 'none', '-vga', 'none'] >> + else: >> + moncdev = 'socket,id=mon,path=%s' % self._vm_monitor >> + args.extend(['-chardev', moncdev, '-mon', >> + 'chardev=mon,mode=control']) >> if self._machine is not None: >> args.extend(['-machine', self._machine]) >> if self._console_set: >> @@ -255,20 +257,21 @@ class QEMUMachine(object): >> >> def _pre_launch(self): >> 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._sock_dir, >> - self._name + "-monitor.sock") >> - self._remove_files.append(self._vm_monitor) >> 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.QEMUMonitorProtocol(self._vm_monitor, >> - server=True) >> + if self._qmp_set: >> + if self._monitor_address is not None: >> + self._vm_monitor = self._monitor_address >> + else: >> + self._vm_monitor = os.path.join(self._sock_dir, >> + self._name + "-monitor.sock") >> + self._remove_files.append(self._vm_monitor) >> + self._qmp = qmp.QEMUMonitorProtocol(self._vm_monitor, server=True) >> >> def _post_launch(self): >> - self._qmp.accept() >> + if self._qmp: >> + self._qmp.accept() >> >> def _post_shutdown(self): >> if self._qemu_log_file is not None: >> @@ -330,7 +333,8 @@ class QEMUMachine(object): >> Wait for the VM to power off >> """ >> self._popen.wait() >> - self._qmp.close() >> + if self._qmp: >> + self._qmp.close() >> self._load_io_log() >> self._post_shutdown() >> >> @@ -346,12 +350,13 @@ class QEMUMachine(object): >> self._console_socket = None >> >> if self.is_running(): >> - try: >> - if not has_quit: >> - self._qmp.cmd('quit') >> - self._qmp.close() >> - except: >> - self._popen.kill() >> + if self._qmp: >> + try: >> + if not has_quit: >> + self._qmp.cmd('quit') >> + self._qmp.close() >> + except: >> + self._popen.kill() >> self._popen.wait() >> >> self._load_io_log() >> @@ -368,6 +373,23 @@ class QEMUMachine(object): >> >> self._launched = False >> >> + def set_qmp_monitor(self, disabled=False, monitor_address=None): >> + """ >> + Set the QMP monitor. >> + >> + @param disabled: if True, qmp monitor options will be removed from the >> + base arguments of the resulting QEMU command line. > I personally tend avoid double negatives as long as I'm aware of them. > Wouldn't "enabled=True" be more straightforward? Just my personal > preference though. I don't have a strong opinion about double negatives. So I'm fine with use 'enabled' instead. Besides make you happier. ;) > >> + @param monitor_address: address for the QMP monitor. > Do you have a use case for passing the monitor address here too, in > addition to also being available as a parameter to __init__()? In the > next patch, I don't see it being used (or here for that matter). I thought it would be useful in case an acceptance test needs to set the monitor address. And because avocado_qemu transparently creates the QEMUMachine instance, the test doesn't have access to the constructor. But no, I don't have a concrete case. I'm just being overcareful, so I don't mind to remove that parameter. Good catch. Thanks! - Wainer > >> + @note: call this function before launch(). >> + """ >> + if disabled: >> + self._qmp_set = False >> + self._qmp = None >> + else: >> + self._qmp_set = True >> + if monitor_address: >> + self._monitor_address = monitor_address >> + >> def qmp(self, cmd, conv_keys=True, **args): >> """ >> Invoke a QMP command and return the response dict >> -- >> 2.18.1 >> > Other than those comments, it LGTM. > > - Cleber. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] tests/acceptance: Makes linux_initrd and empty_cpu_model use QEMUMachine 2019-11-12 13:57 [PATCH 0/2] tests/acceptance: Use QEMUMachine on tests that expect failure Wainer dos Santos Moschetta 2019-11-12 13:58 ` [PATCH 1/2] python/qemu: Add set_qmp_monitor() to QEMUMachine Wainer dos Santos Moschetta @ 2019-11-12 13:58 ` Wainer dos Santos Moschetta 2019-12-10 5:24 ` Cleber Rosa 2019-12-09 17:44 ` [PATCH 0/2] tests/acceptance: Use QEMUMachine on tests that expect failure Wainer dos Santos Moschetta 2 siblings, 1 reply; 7+ messages in thread From: Wainer dos Santos Moschetta @ 2019-11-12 13:58 UTC (permalink / raw) To: qemu-devel; +Cc: philmd, ehabkost, crosa On linux_initrd and empty_cpu_model tests the same effect of calling QEMU through run() to inspect the terminated process is achieved with a sequence of set_qmp_monitor() / launch() / wait() commands on an QEMUMachine object. This patch changes those tests to use QEMUMachine instead, so they follow the same pattern to launch QEMU found on other acceptance tests. Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com> --- tests/acceptance/empty_cpu_model.py | 13 +++++++------ tests/acceptance/linux_initrd.py | 13 +++++++------ 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/tests/acceptance/empty_cpu_model.py b/tests/acceptance/empty_cpu_model.py index 3f4f663582..8c20a4ef4a 100644 --- a/tests/acceptance/empty_cpu_model.py +++ b/tests/acceptance/empty_cpu_model.py @@ -7,13 +7,14 @@ # # This work is licensed under the terms of the GNU GPL, version 2 or # later. See the COPYING file in the top-level directory. -import subprocess from avocado_qemu import Test class EmptyCPUModel(Test): def test(self): - cmd = [self.qemu_bin, '-S', '-display', 'none', '-machine', 'none', '-cpu', ''] - r = subprocess.run(cmd, stderr=subprocess.PIPE, stdout=subprocess.PIPE) - self.assertEquals(r.returncode, 1, "QEMU exit code should be 1") - self.assertEquals(r.stdout, b'', "QEMU stdout should be empty") - self.assertNotEquals(r.stderr, b'', "QEMU stderr shouldn't be empty") + self.vm.add_args('-S', '-display', 'none', '-machine', 'none', + '-cpu', '') + self.vm.set_qmp_monitor(disabled=True) + self.vm.launch() + self.vm.wait() + self.assertEquals(self.vm.exitcode(), 1, "QEMU exit code should be 1") + self.assertRegex(self.vm.get_log(), r'-cpu option cannot be empty') diff --git a/tests/acceptance/linux_initrd.py b/tests/acceptance/linux_initrd.py index c61d9826a4..158ec4d46c 100644 --- a/tests/acceptance/linux_initrd.py +++ b/tests/acceptance/linux_initrd.py @@ -10,7 +10,6 @@ import logging import tempfile -from avocado.utils.process import run from avocado_qemu import Test @@ -41,13 +40,15 @@ class LinuxInitrd(Test): initrd.seek(max_size) initrd.write(b'\0') initrd.flush() - cmd = "%s -kernel %s -initrd %s -m 4096" % ( - self.qemu_bin, kernel_path, initrd.name) - res = run(cmd, ignore_status=True) - self.assertEqual(res.exit_status, 1) + self.vm.add_args('-kernel', kernel_path, '-initrd', initrd.name, + '-m', '4096') + self.vm.set_qmp_monitor(disabled=True) + self.vm.launch() + self.vm.wait() + self.assertEqual(self.vm.exitcode(), 1) expected_msg = r'.*initrd is too large.*max: \d+, need %s.*' % ( max_size + 1) - self.assertRegex(res.stderr_text, expected_msg) + self.assertRegex(self.vm.get_log(), expected_msg) def test_with_2gib_file_should_work_with_linux_v4_16(self): """ -- 2.18.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] tests/acceptance: Makes linux_initrd and empty_cpu_model use QEMUMachine 2019-11-12 13:58 ` [PATCH 2/2] tests/acceptance: Makes linux_initrd and empty_cpu_model use QEMUMachine Wainer dos Santos Moschetta @ 2019-12-10 5:24 ` Cleber Rosa 0 siblings, 0 replies; 7+ messages in thread From: Cleber Rosa @ 2019-12-10 5:24 UTC (permalink / raw) To: Wainer dos Santos Moschetta; +Cc: philmd, qemu-devel, ehabkost [-- Attachment #1: Type: text/plain, Size: 3243 bytes --] On Tue, Nov 12, 2019 at 08:58:01AM -0500, Wainer dos Santos Moschetta wrote: > On linux_initrd and empty_cpu_model tests the same effect of > calling QEMU through run() to inspect the terminated process is > achieved with a sequence of set_qmp_monitor() / launch() / wait() > commands on an QEMUMachine object. This patch changes those > tests to use QEMUMachine instead, so they follow the same pattern > to launch QEMU found on other acceptance tests. > > Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com> > --- > tests/acceptance/empty_cpu_model.py | 13 +++++++------ > tests/acceptance/linux_initrd.py | 13 +++++++------ > 2 files changed, 14 insertions(+), 12 deletions(-) > > diff --git a/tests/acceptance/empty_cpu_model.py b/tests/acceptance/empty_cpu_model.py > index 3f4f663582..8c20a4ef4a 100644 > --- a/tests/acceptance/empty_cpu_model.py > +++ b/tests/acceptance/empty_cpu_model.py > @@ -7,13 +7,14 @@ > # > # This work is licensed under the terms of the GNU GPL, version 2 or > # later. See the COPYING file in the top-level directory. > -import subprocess > from avocado_qemu import Test > > class EmptyCPUModel(Test): > def test(self): > - cmd = [self.qemu_bin, '-S', '-display', 'none', '-machine', 'none', '-cpu', ''] > - r = subprocess.run(cmd, stderr=subprocess.PIPE, stdout=subprocess.PIPE) > - self.assertEquals(r.returncode, 1, "QEMU exit code should be 1") > - self.assertEquals(r.stdout, b'', "QEMU stdout should be empty") > - self.assertNotEquals(r.stderr, b'', "QEMU stderr shouldn't be empty") > + self.vm.add_args('-S', '-display', 'none', '-machine', 'none', > + '-cpu', '') > + self.vm.set_qmp_monitor(disabled=True) > + self.vm.launch() > + self.vm.wait() > + self.assertEquals(self.vm.exitcode(), 1, "QEMU exit code should be 1") > + self.assertRegex(self.vm.get_log(), r'-cpu option cannot be empty') > diff --git a/tests/acceptance/linux_initrd.py b/tests/acceptance/linux_initrd.py > index c61d9826a4..158ec4d46c 100644 > --- a/tests/acceptance/linux_initrd.py > +++ b/tests/acceptance/linux_initrd.py > @@ -10,7 +10,6 @@ > > import logging > import tempfile > -from avocado.utils.process import run > > from avocado_qemu import Test > > @@ -41,13 +40,15 @@ class LinuxInitrd(Test): > initrd.seek(max_size) > initrd.write(b'\0') > initrd.flush() > - cmd = "%s -kernel %s -initrd %s -m 4096" % ( > - self.qemu_bin, kernel_path, initrd.name) > - res = run(cmd, ignore_status=True) > - self.assertEqual(res.exit_status, 1) > + self.vm.add_args('-kernel', kernel_path, '-initrd', initrd.name, > + '-m', '4096') > + self.vm.set_qmp_monitor(disabled=True) Like I've said on the previous patch, I'd personally go with `enabled=True` as the default method signature, so this would become `enabled=False`, or simply: self.vm.set_qmp_monitor(False) But either way, Reviewed-by: Cleber Rosa <crosa@redhat.com> Tested-by: Cleber Rosa <crosa@redhat.com> [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] tests/acceptance: Use QEMUMachine on tests that expect failure 2019-11-12 13:57 [PATCH 0/2] tests/acceptance: Use QEMUMachine on tests that expect failure Wainer dos Santos Moschetta 2019-11-12 13:58 ` [PATCH 1/2] python/qemu: Add set_qmp_monitor() to QEMUMachine Wainer dos Santos Moschetta 2019-11-12 13:58 ` [PATCH 2/2] tests/acceptance: Makes linux_initrd and empty_cpu_model use QEMUMachine Wainer dos Santos Moschetta @ 2019-12-09 17:44 ` Wainer dos Santos Moschetta 2 siblings, 0 replies; 7+ messages in thread From: Wainer dos Santos Moschetta @ 2019-12-09 17:44 UTC (permalink / raw) To: qemu-devel; +Cc: philmd, ehabkost, crosa Ping. Any brave soul to review it? :) - Wainer On 11/12/19 11:57 AM, Wainer dos Santos Moschetta wrote: > The linux_initrd and empty_cpu_model tests assert that QEMU exit > with failure on certain scenarios. Currently they are not able > to use QEMUMachine object due to the QMP monitor connection which > is tentatively established always. Instead they handle the QEMU binary > directy, but ideally they should use QEMUMachine in order to: > a) Take advantage of error handling and logging in QEMUMachine. > b) Follow the pattern in other acceptance tests. > > Notes worth it: > - Patch 01 first appeared in [1]. Here I propose the same > implementation but the code was rebased. > - empty_cpu_model used to check both stdout and stderr of the process. The > QEMUMachine doesn't provide an interface to access the underneath > process object, instead the process' output is available through > get_log(). This method in turn returns the content of stdout (stderr is > redirected to stdout). Therefore, I adapted the assertion statement > to check the '-cpu option cannot be empty' message shows in the output. > > Git: > - Tree: https://github.com/wainersm/qemu > - Branch: tests_without_qmp > > CI: > - Travis (PASS): https://travis-ci.org/wainersm/qemu/builds/610499693 > > References: > [1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg627498.html > > Wainer dos Santos Moschetta (2): > python/qemu: Add set_qmp_monitor() to QEMUMachine > tests/acceptance: Makes linux_initrd and empty_cpu_model use > QEMUMachine > > python/qemu/machine.py | 68 +++++++++++++++++++---------- > tests/acceptance/empty_cpu_model.py | 13 +++--- > tests/acceptance/linux_initrd.py | 13 +++--- > 3 files changed, 59 insertions(+), 35 deletions(-) > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-12-11 14:56 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-11-12 13:57 [PATCH 0/2] tests/acceptance: Use QEMUMachine on tests that expect failure Wainer dos Santos Moschetta 2019-11-12 13:58 ` [PATCH 1/2] python/qemu: Add set_qmp_monitor() to QEMUMachine Wainer dos Santos Moschetta 2019-12-10 5:17 ` Cleber Rosa 2019-12-11 14:55 ` Wainer dos Santos Moschetta 2019-11-12 13:58 ` [PATCH 2/2] tests/acceptance: Makes linux_initrd and empty_cpu_model use QEMUMachine Wainer dos Santos Moschetta 2019-12-10 5:24 ` Cleber Rosa 2019-12-09 17:44 ` [PATCH 0/2] tests/acceptance: Use QEMUMachine on tests that expect failure Wainer dos Santos Moschetta
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).