qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v3 0/3] script for crash-testing -device
@ 2017-05-26 18:11 Eduardo Habkost
  2017-05-26 18:11 ` [Qemu-devel] [PATCH v3 1/3] qemu.py: Don't set _popen=None on error/shutdown Eduardo Habkost
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Eduardo Habkost @ 2017-05-26 18:11 UTC (permalink / raw)
  To: qemu-devel; +Cc: Thomas Huth, Marcel Apfelbaum, Markus Armbruster

Changes v2 -> v3:
* Renamed to scripts/device-crash-test (removed .py suffix)
* Changed license to GPLv2+
* whitelist updates:
  * New whitelist entries
  * Documented whitelist expectations more clearly
  * Use loglevel=INFO on unknown exitcode=1 cases
  * Ignore ide-cd errors on older QEMU versions
  * Refactor of whitelist lookup code to make it clearer
* Optimization: when in quick mode, check if machine is usable before
  testing it using -device
* Run in verbose mode by default
* Include exception traceback on the log output.
* Use lowercase on "skipped:" message for consistency
* Run on quick mode by default
* Added --strict option
* Don't crash if -r argument is too large
* Eliminate useless genAllCases() wrapper function
* Eliminate dead pickRandomCase() function
* Eliminate dead debugging code
* Include exception details on debug log if a machine fails to run
* Removed code that tries to detect obsolete entries
* Update commenst to mention user_creatable instead of the old
  cannot_instantiate_with_device_add_yet name
* Coding style updates to make pylint and pep8 happier

Changes v1 -> v2:
* Use a simpler method to query QEMU exit code in qemu.py
* Use only qemu.py module, instead of qtest.py
* New whitelist entries:
  * "could not find stage1 bootloader"
  * Segfaults when using devices: a15mpcore_priv, sb16, cs4231a, arm-gicv3
* Format "success" line using formatTestCase(), and using DEBUG
  loglevel
* Reword "test case:" line with "running test case:", for clarity
* Fix "pc-.*" whitelist to include "q35" too
* Add --devtype option to test only a specific device type
* Send all log messages to stdout instead of stderr
* Avoid printing "obsolete whitelist entry?" messages if we know
  we are not testing every single accel/machine/device
  combination
* --quick mode, to skip cases where failures are always expected,
  and to print a warning in case we don't get an expected failure
* Use qemu.QEMUMachine instead of qtest.QEMUQtestMachine, as we don't
  use any of the QEMUQtestMachine features
* Fix handling of multiple '-t' options
* Simplify code that generate random sample of test cases

This series adds scripts/device-crashtest, that can be used to
crash-test -device with multiple machine/accel/device
combinations.

The script found a few crashes on some machines/devices. A dump
of existing cases can be seen here:
  https://gist.github.com/ehabkost/503b0af0375f0d98d3e84017e8ca54eb

The script contains a whitelist that can also be useful as
documentation of existing ways -device can fail or crash.

Note that the script takes a few hours to run on the default mode
(testing all accel/machine/device combinations), but the "-r N"
option can be used to make it only test N random samples.

Example script output:

  $ ../scripts/device-crash-test.py -v --shuffle
  INFO: test case: machine=verdex binary=./aarch64-softmmu/qemu-system-aarch64 device=exynos4210-ehci-usb accel=tcg
  INFO: test case: machine=none binary=./aarch64-softmmu/qemu-system-aarch64 device=onenand accel=tcg
  INFO: test case: machine=pc-i440fx-2.2 binary=./x86_64-softmmu/qemu-system-x86_64 device=ide-cd accel=kvm
  INFO: success: ./x86_64-softmmu/qemu-system-x86_64 -S -machine pc-i440fx-2.2,accel=kvm -device ide-cd
  INFO: test case: machine=SPARCClassic binary=./sparc-softmmu/qemu-system-sparc device=memory accel=tcg
  qemu received signal 6: -S -machine SPARCClassic,accel=tcg -device memory
  ERROR: failed: machine=SPARCClassic binary=./sparc-softmmu/qemu-system-sparc device=memory accel=tcg
  ERROR: cmdline: ./sparc-softmmu/qemu-system-sparc -S -machine SPARCClassic,accel=tcg -device memory
  ERROR: log: qemu-system-sparc: /root/qemu-build/exec.c:1500: find_ram_offset: Assertion `size != 0' failed.
  ERROR: exit code: -6
  INFO: test case: machine=romulus-bmc binary=./arm-softmmu/qemu-system-arm device=ich9-usb-uhci6 accel=tcg
  INFO: test case: machine=ref405ep binary=./ppc-softmmu/qemu-system-ppc device=ivshmem-doorbell accel=tcg
  INFO: test case: machine=romulus-bmc binary=./aarch64-softmmu/qemu-system-aarch64 device=l2x0 accel=tcg
  INFO: test case: machine=pc-i440fx-1.7 binary=./x86_64-softmmu/qemu-system-x86_64 device=virtio-input-host-pci accel=tcg
  INFO: test case: machine=none binary=./ppc-softmmu/qemu-system-ppc device=virtio-tablet-pci accel=tcg
  INFO: test case: machine=terrier binary=./aarch64-softmmu/qemu-system-aarch64 device=sst25vf016b accel=tcg
  INFO: success: ./aarch64-softmmu/qemu-system-aarch64 -S -machine terrier,accel=tcg -device sst25vf016b
  INFO: test case: machine=none binary=./i386-softmmu/qemu-system-i386 device=intel-iommu accel=kvm
  qemu received signal 6: -S -machine none,accel=kvm -device intel-iommu
  ERROR: failed: machine=none binary=./i386-softmmu/qemu-system-i386 device=intel-iommu accel=kvm
  ERROR: cmdline: ./i386-softmmu/qemu-system-i386 -S -machine none,accel=kvm -device intel-iommu
  ERROR: log: /root/qemu-build/hw/i386/intel_iommu.c:2565:vtd_realize: Object 0x7fe117fabfb0 is not an instance of type generic-pc-machine
  ERROR: exit code: -6
  INFO: test case: machine=tosa binary=./aarch64-softmmu/qemu-system-aarch64 device=integrator_core accel=tcg
  INFO: test case: machine=isapc binary=./i386-softmmu/qemu-system-i386 device=i82550 accel=kvm
  INFO: test case: machine=xlnx-ep108 binary=./aarch64-softmmu/qemu-system-aarch64 device=digic accel=tcg
  qemu received signal 6: -S -machine xlnx-ep108,accel=tcg -device digic
  ERROR: failed: machine=xlnx-ep108 binary=./aarch64-softmmu/qemu-system-aarch64 device=digic accel=tcg
  ERROR: cmdline: ./aarch64-softmmu/qemu-system-aarch64 -S -machine xlnx-ep108,accel=tcg -device digic
  ERROR: log: audio: Could not init `oss' audio driver
  ERROR: log: Unexpected error in qemu_chr_fe_init() at /root/qemu-build/chardev/char.c:512:
  ERROR: log: qemu-system-aarch64: -device digic: Device 'serial0' is in use
  ERROR: exit code: -6
  INFO: test case: machine=raspi2 binary=./arm-softmmu/qemu-system-arm device=sd-card accel=tcg
  INFO: success: ./arm-softmmu/qemu-system-arm -S -machine raspi2,accel=tcg -device sd-card
  [...]

Eduardo Habkost (3):
  qemu.py: Don't set _popen=None on error/shutdown
  qemu.py: Add QEMUMachine.exitcode() method
  scripts: Test script to look for -device crashes

 scripts/qemu.py           |  16 +-
 scripts/device-crash-test | 624 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 635 insertions(+), 5 deletions(-)
 create mode 100755 scripts/device-crash-test

-- 
2.11.0.259.g40922b1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH v3 1/3] qemu.py: Don't set _popen=None on error/shutdown
  2017-05-26 18:11 [Qemu-devel] [PATCH v3 0/3] script for crash-testing -device Eduardo Habkost
@ 2017-05-26 18:11 ` Eduardo Habkost
  2017-05-29 16:44   ` Markus Armbruster
  2017-05-26 18:11 ` [Qemu-devel] [PATCH v3 2/3] qemu.py: Add QEMUMachine.exitcode() method Eduardo Habkost
  2017-05-26 18:12 ` [Qemu-devel] [PATCH v3 3/3] scripts: Test script to look for -device crashes Eduardo Habkost
  2 siblings, 1 reply; 8+ messages in thread
From: Eduardo Habkost @ 2017-05-26 18:11 UTC (permalink / raw)
  To: qemu-devel; +Cc: Thomas Huth, Marcel Apfelbaum, Markus Armbruster

Keep the Popen object around to we can query its exit code later.

To keep the existing 'self._popen is None' checks working, add a
is_running() method, that will check if the process is still running.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 scripts/qemu.py | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index 6d1b6230b7..16934f1e02 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -85,8 +85,11 @@ class QEMUMachine(object):
                 return
             raise
 
+    def is_running(self):
+        return self._popen and (self._popen.returncode is None)
+
     def get_pid(self):
-        if not self._popen:
+        if not self.is_running():
             return None
         return self._popen.pid
 
@@ -128,16 +131,16 @@ class QEMUMachine(object):
                                            stderr=subprocess.STDOUT, shell=False)
             self._post_launch()
         except:
-            if self._popen:
+            if self.is_running():
                 self._popen.kill()
+                self._popen.wait()
             self._load_io_log()
             self._post_shutdown()
-            self._popen = None
             raise
 
     def shutdown(self):
         '''Terminate the VM and clean up'''
-        if not self._popen is None:
+        if self.is_running():
             try:
                 self._qmp.cmd('quit')
                 self._qmp.close()
@@ -149,7 +152,6 @@ class QEMUMachine(object):
                 sys.stderr.write('qemu received signal %i: %s\n' % (-exitcode, ' '.join(self._args)))
             self._load_io_log()
             self._post_shutdown()
-            self._popen = None
 
     underscore_to_dash = string.maketrans('_', '-')
     def qmp(self, cmd, conv_keys=True, **args):
-- 
2.11.0.259.g40922b1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH v3 2/3] qemu.py: Add QEMUMachine.exitcode() method
  2017-05-26 18:11 [Qemu-devel] [PATCH v3 0/3] script for crash-testing -device Eduardo Habkost
  2017-05-26 18:11 ` [Qemu-devel] [PATCH v3 1/3] qemu.py: Don't set _popen=None on error/shutdown Eduardo Habkost
@ 2017-05-26 18:11 ` Eduardo Habkost
  2017-05-29 16:53   ` Markus Armbruster
  2017-05-26 18:12 ` [Qemu-devel] [PATCH v3 3/3] scripts: Test script to look for -device crashes Eduardo Habkost
  2 siblings, 1 reply; 8+ messages in thread
From: Eduardo Habkost @ 2017-05-26 18:11 UTC (permalink / raw)
  To: qemu-devel; +Cc: Thomas Huth, Marcel Apfelbaum, Markus Armbruster

Allow the exit code of QEMU to be queried by scripts.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 scripts/qemu.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index 16934f1e02..ebe1c4b919 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -88,6 +88,10 @@ class QEMUMachine(object):
     def is_running(self):
         return self._popen and (self._popen.returncode is None)
 
+    def exitcode(self):
+        if self._popen:
+            return self._popen.returncode
+
     def get_pid(self):
         if not self.is_running():
             return None
-- 
2.11.0.259.g40922b1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH v3 3/3] scripts: Test script to look for -device crashes
  2017-05-26 18:11 [Qemu-devel] [PATCH v3 0/3] script for crash-testing -device Eduardo Habkost
  2017-05-26 18:11 ` [Qemu-devel] [PATCH v3 1/3] qemu.py: Don't set _popen=None on error/shutdown Eduardo Habkost
  2017-05-26 18:11 ` [Qemu-devel] [PATCH v3 2/3] qemu.py: Add QEMUMachine.exitcode() method Eduardo Habkost
@ 2017-05-26 18:12 ` Eduardo Habkost
  2 siblings, 0 replies; 8+ messages in thread
From: Eduardo Habkost @ 2017-05-26 18:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: Thomas Huth, Marcel Apfelbaum, Markus Armbruster

Test code to check if we can crash QEMU using -device. It will
test all accel/machine/device combinations by default, which may
take a few hours (it's more than 90k test cases). There's a "-r"
option that makes it test a random sample of combinations.

The scripts contains a whitelist for: 1) known error messages
that make QEMU exit cleanly; 2) known QEMU crashes.

This is the behavior when the script finds a failure:

* Known clean (exitcode=1) errors generate DEBUG messages
  (hidden by default)
* Unknown clean (exitcode=1) errors will generate INFO messages
  (visible by default)
* Known crashes generate error messages, but are not fatal
  (unless --strict mode is used)
* Unknown crashes generate fatal error messages

Having an updated whitelist of known clean errors is useful to make the
script less verbose and run faster when in --quick mode, but the
whitelist doesn't need to be always up to date.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v2 -> v3:
* Renamed to scripts/device-crash-test (removed .py suffix)
* Changed license to GPLv2+
* whitelist updates:
  * New whitelist entries
  * Documented whitelist expectations more clearly
  * Use loglevel=INFO on unknown exitcode=1 cases
  * Ignore ide-cd errors on older QEMU versions
  * Refactor of whitelist lookup code to make it clearer
* Optimization: when in quick mode, check if machine is usable before
  testing it using -device
* Run in verbose mode by default
* Include exception traceback on the log output.
* Use lowercase on "skipped:" message for consistency
* Run on quick mode by default
* Added --strict option
* Don't crash if -r argument is too large
* Eliminate useless genAllCases() wrapper function
* Eliminate dead pickRandomCase() function
* Eliminate dead debugging code
* Include exception details on debug log if a machine fails to run
* Removed code that tries to detect obsolete entries
* Update commenst to mention user_creatable instead of the old
  cannot_instantiate_with_device_add_yet name
* Coding style updates to make pylint and pep8 happier

Changes v1 -> v2:
* New whitelist entries:
  * "could not find stage1 bootloader"
  * Segfaults when using devices: a15mpcore_priv, sb16, cs4231a, arm-gicv3
* Format "success" line using formatTestCase(), and using DEBUg
  loglevel
* Reword "test case:" line with "running test case:", for clarity
* Fix "pc-.*" whitelist to include "q35" too
* Add --devtype option to test only a specific device type
* Send all log messages to stdout instead of stderr
* Avoid printing "obsolete whitelist entry?" messages if we know
  we are not testing every single accel/machine/device
  combination
* --quick mode, to skip cases where failures are always expected,
  and to print a warning in case we don't get an expected failure
* Use qemu.QEMUMachine instead of qtest.QEMUQtestMachine, as we don't
  use any of the QEMUQtestMachine features
* Fix handling of multiple '-t' options
* Simplify code that generate random sample of test cases
---
 scripts/device-crash-test | 624 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 624 insertions(+)
 create mode 100755 scripts/device-crash-test

diff --git a/scripts/device-crash-test b/scripts/device-crash-test
new file mode 100755
index 0000000000..3b089099ce
--- /dev/null
+++ b/scripts/device-crash-test
@@ -0,0 +1,624 @@
+#!/usr/bin/env python2.7
+#
+#  Copyright (c) 2017 Red Hat Inc
+#
+# Author:
+#  Eduardo Habkost <ehabkost@redhat.com>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with this program; if not, write to the Free Software Foundation, Inc.,
+# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+"""
+Run QEMU with all combinations of -machine and -device types,
+check for crashes and unexpected errors.
+"""
+
+import sys
+import os
+import glob
+import logging
+import traceback
+import re
+import random
+import argparse
+from itertools import chain
+
+sys.path.append(os.path.join(os.path.dirname(__file__), '..', 'scripts'))
+from qemu import QEMUMachine
+
+logger = logging.getLogger('device-crash-test')
+dbg = logger.debug
+
+
+# Purposes of the following whitelist:
+# * Avoiding verbose log messages when we find known non-fatal
+#   (exitcode=1) errors
+# * Avoiding fatal errors when we find known crashes
+# * Skipping machines/devices that are known not to work out of
+#   the box, when running in --quick mode
+#
+# Keeping the whitelist updated is desirable, but not required,
+# because unexpected cases where QEMU exits with exitcode=1 will
+# just trigger a INFO message.
+
+# Valid whitelist entry keys:
+# * accel: regexp, full match only
+# * machine: regexp, full match only
+# * device: regexp, full match only
+# * log: regexp, partial match allowed
+# * exitcode: if not present, defaults to 1. If None, matches any exitcode
+# * warn: if True, matching failures will be logged as warnings
+# * expected: if True, QEMU is expected to always fail every time
+#   when testing the corresponding test case
+# * loglevel: log level of log output when there's a match.
+ERROR_WHITELIST = [
+    # Machines that won't work out of the box:
+    #             MACHINE                         | ERROR MESSAGE
+    dict(machine='niagara', expected=True),       # Unable to load a firmware for -M niagara
+    dict(machine='boston', expected=True),        # Please provide either a -kernel or -bios argument
+    dict(machine='leon3_generic', expected=True), # Can't read bios image (null)
+
+    # devices that don't work out of the box because they require extra options to "-device DEV":
+    #            DEVICE                                    | ERROR MESSAGE
+    dict(device='.*-(i386|x86_64)-cpu', expected=True),    # CPU socket-id is not set
+    dict(device='ARM,bitband-memory', expected=True),      # source-memory property not set
+    dict(device='arm.cortex-a9-global-timer', expected=True), # a9_gtimer_realize: num-cpu must be between 1 and 4
+    dict(device='arm_mptimer', expected=True),             # num-cpu must be between 1 and 4
+    dict(device='armv7m', expected=True),                  # memory property was not set
+    dict(device='aspeed.scu', expected=True),              # Unknown silicon revision: 0x0
+    dict(device='aspeed.sdmc', expected=True),             # Unknown silicon revision: 0x0
+    dict(device='bcm2835-dma', expected=True),             # bcm2835_dma_realize: required dma-mr link not found: Property '.dma-mr' not found
+    dict(device='bcm2835-fb', expected=True),              # bcm2835_fb_realize: required vcram-base property not set
+    dict(device='bcm2835-mbox', expected=True),            # bcm2835_mbox_realize: required mbox-mr link not found: Property '.mbox-mr' not found
+    dict(device='bcm2835-peripherals', expected=True),     # bcm2835_peripherals_realize: required ram link not found: Property '.ram' not found
+    dict(device='bcm2835-property', expected=True),        # bcm2835_property_realize: required fb link not found: Property '.fb' not found
+    dict(device='bcm2835_gpio', expected=True),            # bcm2835_gpio_realize: required sdhci link not found: Property '.sdbus-sdhci' not found
+    dict(device='bcm2836', expected=True),                 # bcm2836_realize: required ram link not found: Property '.ram' not found
+    dict(device='cfi.pflash01', expected=True),            # attribute "sector-length" not specified or zero.
+    dict(device='cfi.pflash02', expected=True),            # attribute "sector-length" not specified or zero.
+    dict(device='icp', expected=True),                     # icp_realize: required link 'xics' not found: Property '.xics' not found
+    dict(device='ics', expected=True),                     # ics_base_realize: required link 'xics' not found: Property '.xics' not found
+    # "-device ide-cd" does work on more recent QEMU versions, so it doesn't have expected=True
+    dict(device='ide-cd'),                                 # No drive specified
+    dict(device='ide-drive', expected=True),               # No drive specified
+    dict(device='ide-hd', expected=True),                  # No drive specified
+    dict(device='ipmi-bmc-extern', expected=True),         # IPMI external bmc requires chardev attribute
+    dict(device='isa-debugcon', expected=True),            # Can't create serial device, empty char device
+    dict(device='isa-ipmi-bt', expected=True),             # IPMI device requires a bmc attribute to be set
+    dict(device='isa-ipmi-kcs', expected=True),            # IPMI device requires a bmc attribute to be set
+    dict(device='isa-parallel', expected=True),            # Can't create serial device, empty char device
+    dict(device='isa-serial', expected=True),              # Can't create serial device, empty char device
+    dict(device='ivshmem', expected=True),                 # You must specify either 'shm' or 'chardev'
+    dict(device='ivshmem-doorbell', expected=True),        # You must specify a 'chardev'
+    dict(device='ivshmem-plain', expected=True),           # You must specify a 'memdev'
+    dict(device='kvm-pci-assign', expected=True),          # no host device specified
+    dict(device='loader', expected=True),                  # please include valid arguments
+    dict(device='nand', expected=True),                    # Unsupported NAND block size 0x1
+    dict(device='nvdimm', expected=True),                  # 'memdev' property is not set
+    dict(device='nvme', expected=True),                    # Device initialization failed
+    dict(device='pc-dimm', expected=True),                 # 'memdev' property is not set
+    dict(device='pci-bridge', expected=True),              # Bridge chassis not specified. Each bridge is required to be assigned a unique chassis id > 0.
+    dict(device='pci-bridge-seat', expected=True),         # Bridge chassis not specified. Each bridge is required to be assigned a unique chassis id > 0.
+    dict(device='pci-serial', expected=True),              # Can't create serial device, empty char device
+    dict(device='pci-serial-2x', expected=True),           # Can't create serial device, empty char device
+    dict(device='pci-serial-4x', expected=True),           # Can't create serial device, empty char device
+    dict(device='pxa2xx-dma', expected=True),              # channels value invalid
+    dict(device='pxb', expected=True),                     # Bridge chassis not specified. Each bridge is required to be assigned a unique chassis id > 0.
+    dict(device='scsi-block', expected=True),              # drive property not set
+    dict(device='scsi-disk', expected=True),               # drive property not set
+    dict(device='scsi-generic', expected=True),            # drive property not set
+    dict(device='scsi-hd', expected=True),                 # drive property not set
+    dict(device='spapr-pci-host-bridge', expected=True),   # BUID not specified for PHB
+    dict(device='spapr-pci-vfio-host-bridge', expected=True), # BUID not specified for PHB
+    dict(device='spapr-rng', expected=True),               # spapr-rng needs an RNG backend!
+    dict(device='spapr-vty', expected=True),               # chardev property not set
+    dict(device='tpm-tis', expected=True),                 # tpm_tis: backend driver with id (null) could not be found
+    dict(device='unimplemented-device', expected=True),    # property 'size' not specified or zero
+    dict(device='usb-braille', expected=True),             # Property chardev is required
+    dict(device='usb-mtp', expected=True),                 # x-root property must be configured
+    dict(device='usb-redir', expected=True),               # Parameter 'chardev' is missing
+    dict(device='usb-serial', expected=True),              # Property chardev is required
+    dict(device='usb-storage', expected=True),             # drive property not set
+    dict(device='vfio-amd-xgbe', expected=True),           # -device vfio-amd-xgbe: vfio error: wrong host device name
+    dict(device='vfio-calxeda-xgmac', expected=True),      # -device vfio-calxeda-xgmac: vfio error: wrong host device name
+    dict(device='vfio-pci', expected=True),                # No provided host device
+    dict(device='vfio-pci-igd-lpc-bridge', expected=True), # VFIO dummy ISA/LPC bridge must have address 1f.0
+    dict(device='vhost-scsi.*', expected=True),            # vhost-scsi: missing wwpn
+    dict(device='vhost-vsock-device', expected=True),      # guest-cid property must be greater than 2
+    dict(device='vhost-vsock-pci', expected=True),         # guest-cid property must be greater than 2
+    dict(device='virtio-9p-ccw', expected=True),           # 9pfs device couldn't find fsdev with the id = NULL
+    dict(device='virtio-9p-device', expected=True),        # 9pfs device couldn't find fsdev with the id = NULL
+    dict(device='virtio-9p-pci', expected=True),           # 9pfs device couldn't find fsdev with the id = NULL
+    dict(device='virtio-blk-ccw', expected=True),          # drive property not set
+    dict(device='virtio-blk-device', expected=True),       # drive property not set
+    dict(device='virtio-blk-device', expected=True),       # drive property not set
+    dict(device='virtio-blk-pci', expected=True),          # drive property not set
+    dict(device='virtio-crypto-ccw', expected=True),       # 'cryptodev' parameter expects a valid object
+    dict(device='virtio-crypto-device', expected=True),    # 'cryptodev' parameter expects a valid object
+    dict(device='virtio-crypto-pci', expected=True),       # 'cryptodev' parameter expects a valid object
+    dict(device='virtio-input-host-device', expected=True), # evdev property is required
+    dict(device='virtio-input-host-pci', expected=True),   # evdev property is required
+    dict(device='xen-pvdevice', expected=True),            # Device ID invalid, it must always be supplied
+    dict(device='vhost-vsock-ccw', expected=True),         # guest-cid property must be greater than 2
+    dict(device='ALTR.timer', expected=True),              # "clock-frequency" property must be provided
+    dict(device='zpci', expected=True),                    # target must be defined
+    dict(device='pnv-(occ|icp|lpc)', expected=True),       # required link 'xics' not found: Property '.xics' not found
+    dict(device='powernv-cpu-.*', expected=True),          # pnv_core_realize: required link 'xics' not found: Property '.xics' not found
+
+    # ioapic devices are already created by pc and will fail:
+    dict(machine='q35|pc.*', device='kvm-ioapic', expected=True), # Only 1 ioapics allowed
+    dict(machine='q35|pc.*', device='ioapic', expected=True),     # Only 1 ioapics allowed
+
+    # KVM-specific devices shouldn't be tried without accel=kvm:
+    dict(accel='(?!kvm).*', device='kvmclock', expected=True),
+    dict(accel='(?!kvm).*', device='kvm-pci-assign', expected=True),
+
+    # xen-specific machines and devices:
+    dict(accel='(?!xen).*', machine='xen.*', expected=True),
+    dict(accel='(?!xen).*', device='xen-.*', expected=True),
+
+    # this fails on some machine-types, but not all, so they don't have expected=True:
+    dict(device='vmgenid'), # vmgenid requires DMA write support in fw_cfg, which this machine type does not provide
+
+    # Silence INFO messages for errors that are common on multiple
+    # devices/machines:
+    dict(log=r"No '[\w-]+' bus found for device '[\w-]+'"),
+    dict(log=r"images* must be given with the 'pflash' parameter"),
+    dict(log=r"(Guest|ROM|Flash|Kernel) image must be specified"),
+    dict(log=r"[cC]ould not load [\w ]+ (BIOS|bios) '[\w-]+\.bin'"),
+    dict(log=r"Couldn't find rom image '[\w-]+\.bin'"),
+    dict(log=r"speed mismatch trying to attach usb device"),
+    dict(log=r"Can't create a second ISA bus"),
+    dict(log=r"duplicate fw_cfg file name"),
+    # sysbus-related error messages: most machines reject most dynamic sysbus devices:
+    dict(log=r"Option '-device [\w.,-]+' cannot be handled by this machine"),
+    dict(log=r"Device [\w.,-]+ is not supported by this machine yet"),
+    dict(log=r"Device [\w.,-]+ can not be dynamically instantiated"),
+    dict(log=r"Platform Bus: Can not fit MMIO region of size "),
+    # other more specific errors we will ignore:
+    dict(device='allwinner-a10', log="Unsupported NIC model:"),
+    dict(device='.*-spapr-cpu-core', log=r"CPU core type should be"),
+    dict(log=r"MSI(-X)? is not supported by interrupt controller"),
+    dict(log=r"pxb-pcie? devices cannot reside on a PCIe? bus"),
+    dict(log=r"Ignoring smp_cpus value"),
+    dict(log=r"sd_init failed: Drive 'sd0' is already in use because it has been automatically connected to another device"),
+    dict(log=r"This CPU requires a smaller page size than the system is using"),
+    dict(log=r"MSI-X support is mandatory in the S390 architecture"),
+    dict(log=r"rom check and register reset failed"),
+    dict(log=r"Unable to initialize GIC, CPUState for CPU#0 not valid"),
+    dict(log=r"Multiple VT220 operator consoles are not supported"),
+    dict(log=r"core 0 already populated"),
+    dict(log=r"could not find stage1 bootloader"),
+
+    # other exitcode=1 failures not listed above will just generate INFO messages:
+    dict(exitcode=1, loglevel=logging.INFO),
+
+    # KNOWN CRASHES:
+    # Known crashes will generate error messages, but won't be fatal.
+    # Those entries must be removed once we fix the crashes.
+    dict(exitcode=-6, log=r"Device 'serial0' is in use", loglevel=logging.ERROR),
+    dict(exitcode=-6, log=r"spapr_rtas_register: Assertion .*rtas_table\[token\]\.name.* failed", loglevel=logging.ERROR),
+    dict(exitcode=-6, log=r"qemu_net_client_setup: Assertion `!peer->peer' failed", loglevel=logging.ERROR),
+    dict(exitcode=-6, log=r'RAMBlock "[\w.-]+" already registered', loglevel=logging.ERROR),
+    dict(exitcode=-6, log=r"find_ram_offset: Assertion `size != 0' failed.", loglevel=logging.ERROR),
+    dict(exitcode=-6, log=r"puv3_load_kernel: Assertion `kernel_filename != NULL' failed", loglevel=logging.ERROR),
+    dict(exitcode=-6, log=r"add_cpreg_to_hashtable: code should not be reached", loglevel=logging.ERROR),
+    dict(exitcode=-6, log=r"qemu_alloc_display: Assertion `surface->image != NULL' failed", loglevel=logging.ERROR),
+    dict(exitcode=-6, log=r"Unexpected error in error_set_from_qdev_prop_error", loglevel=logging.ERROR),
+    dict(exitcode=-6, log=r"Object .* is not an instance of type spapr-machine", loglevel=logging.ERROR),
+    dict(exitcode=-6, log=r"Object .* is not an instance of type generic-pc-machine", loglevel=logging.ERROR),
+    dict(exitcode=-6, log=r"Object .* is not an instance of type e500-ccsr", loglevel=logging.ERROR),
+    dict(exitcode=-6, log=r"vmstate_register_with_alias_id: Assertion `!se->compat || se->instance_id == 0' failed", loglevel=logging.ERROR),
+    dict(exitcode=-11, device='stm32f205-soc', loglevel=logging.ERROR, expected=True),
+    dict(exitcode=-11, device='xlnx,zynqmp', loglevel=logging.ERROR, expected=True),
+    dict(exitcode=-11, device='mips-cps', loglevel=logging.ERROR, expected=True),
+    dict(exitcode=-11, device='gus', loglevel=logging.ERROR, expected=True),
+    dict(exitcode=-11, device='a9mpcore_priv', loglevel=logging.ERROR, expected=True),
+    dict(exitcode=-11, device='a15mpcore_priv', loglevel=logging.ERROR, expected=True),
+    dict(exitcode=-11, device='isa-serial', loglevel=logging.ERROR, expected=True),
+    dict(exitcode=-11, device='sb16', loglevel=logging.ERROR, expected=True),
+    dict(exitcode=-11, device='cs4231a', loglevel=logging.ERROR, expected=True),
+    dict(exitcode=-11, device='arm-gicv3', loglevel=logging.ERROR, expected=True),
+    dict(exitcode=-11, machine='isapc', device='.*-iommu', loglevel=logging.ERROR, expected=True),
+
+    # everything else (including SIGABRT and SIGSEGV) will be a fatal error:
+    dict(exitcode=None, fatal=True, loglevel=logging.FATAL),
+]
+
+
+def whitelistTestCaseMatch(wl, t):
+    """Check if a test case specification can match a whitelist entry
+
+    This only checks if a whitelist entry is a candidate match
+    for a given test case, it won't check if the test case
+    results/output match the entry.  See whitelistResultMatch().
+    """
+    return (('machine' not in wl or
+             'machine' not in t or
+             re.match(wl['machine'] + '$', t['machine'])) and
+            ('accel' not in wl or
+             'accel' not in t or
+             re.match(wl['accel'] + '$', t['accel'])) and
+            ('device' not in wl or
+             'device' not in t or
+             re.match(wl['device'] + '$', t['device'])))
+
+
+def whitelistCandidates(t):
+    """Generate the list of candidates that can match a test case"""
+    for i, wl in enumerate(ERROR_WHITELIST):
+        if whitelistTestCaseMatch(wl, t):
+            yield (i, wl)
+
+
+def findExpectedResult(t):
+    """Check if there's an expected=True whitelist entry for a test case
+
+    Returns (i, wl) tuple, where i is the index in
+    ERROR_WHITELIST and wl is the whitelist entry itself.
+    """
+    for i, wl in whitelistCandidates(t):
+        if wl.get('expected'):
+            return (i, wl)
+
+
+def whitelistResultMatch(wl, r):
+    """Check if test case results/output match a whitelist entry
+
+    It is valid to call this function only if
+    whitelistTestCaseMatch() is True for the entry (e.g. on
+    entries returned by whitelistCandidates())
+    """
+    assert whitelistTestCaseMatch(wl, r['testcase'])
+    return ((wl.get('exitcode', 1) is None or
+             r['exitcode'] == wl.get('exitcode', 1)) and
+            ('log' not in wl or
+             re.search(wl['log'], r['log'], re.MULTILINE)))
+
+
+def checkResultWhitelist(r):
+    """Look up whitelist entry for a given test case result
+
+    Returns (i, wl) tuple, where i is the index in
+    ERROR_WHITELIST and wl is the whitelist entry itself.
+    """
+    for i, wl in whitelistCandidates(r['testcase']):
+        if whitelistResultMatch(wl, r):
+            return i, wl
+
+    raise Exception("this should never happen")
+
+
+def qemuOptsEscape(s):
+    """Escape option value QemuOpts"""
+    return s.replace(",", ",,")
+
+
+def formatTestCase(t):
+    """Format test case info as "key=value key=value" for prettier logging output"""
+    return ' '.join('%s=%s' % (k, v) for k, v in t.items())
+
+
+def qomListTypeNames(vm, **kwargs):
+    """Run qom-list-types QMP command, return type names"""
+    types = vm.command('qom-list-types', **kwargs)
+    return [t['name'] for t in types]
+
+
+def infoQDM(vm):
+    """Parse 'info qdm' output"""
+    args = {'command-line': 'info qdm'}
+    devhelp = vm.command('human-monitor-command', **args)
+    for l in devhelp.split('\n'):
+        l = l.strip()
+        if l == '' or l.endswith(':'):
+            continue
+        d = {'name': re.search(r'name "([^"]+)"', l).group(1),
+             'no-user': (re.search(', no-user', l) is not None)}
+        yield d
+
+
+class QemuBinaryInfo(object):
+    def __init__(self, binary, devtype):
+        if devtype is None:
+            devtype = 'device'
+
+        self.binary = binary
+        self._machine_info = {}
+
+        dbg("devtype: %r", devtype)
+        args = ['-S', '-machine', 'none,accel=kvm:tcg']
+        dbg("querying info for QEMU binary: %s", binary)
+        vm = QEMUMachine(binary=binary, args=args)
+        vm.launch()
+        try:
+            self.alldevs = set(qomListTypeNames(vm, implements=devtype, abstract=False))
+            # there's no way to query DeviceClass::user_creatable using QMP,
+            # so use 'info qdm':
+            self.no_user_devs = set([d['name'] for d in infoQDM(vm, ) if d['no-user']])
+            self.machines = list(m['name'] for m in vm.command('query-machines'))
+            self.user_devs = self.alldevs.difference(self.no_user_devs)
+            self.kvm_available = vm.command('query-kvm')['enabled']
+        finally:
+            vm.shutdown()
+
+    def machineInfo(self, machine):
+        """Query for information on a specific machine-type
+
+        Results are cached internally, in case the same machine-
+        type is queried multiple times.
+        """
+        if machine in self._machine_info:
+            return self._machine_info[machine]
+
+        mi = {}
+        args = ['-S', '-machine', '%s' % (machine)]
+        dbg("querying machine info for binary=%s machine=%s", self.binary, machine)
+        vm = QEMUMachine(binary=self.binary, args=args)
+        try:
+            vm.launch()
+            mi['runnable'] = True
+        except KeyboardInterrupt:
+            raise
+        except:
+            dbg("exception trying to run binary=%s machine=%s", self.binary, machine, exc_info=sys.exc_info())
+            dbg("log: %r", vm.get_log())
+            mi['runnable'] = False
+
+        vm.shutdown()
+        self._machine_info[machine] = mi
+        return mi
+
+
+BINARY_INFO = {}
+
+
+def getBinaryInfo(args, binary):
+    if binary not in BINARY_INFO:
+        BINARY_INFO[binary] = QemuBinaryInfo(binary, args.devtype)
+    return BINARY_INFO[binary]
+
+
+def checkOneCase(args, testcase):
+    """Check one specific case
+
+    Returns a dictionary containing failure information on error,
+    or None on success
+    """
+    binary = testcase['binary']
+    accel = testcase['accel']
+    machine = testcase['machine']
+    device = testcase['device']
+
+    dbg("will test: %r", testcase)
+
+    args = ['-S', '-machine', '%s,accel=%s' % (machine, accel),
+            '-device', qemuOptsEscape(device)]
+    cmdline = ' '.join([binary] + args)
+    dbg("will launch QEMU: %s", cmdline)
+    vm = QEMUMachine(binary=binary, args=args)
+
+    exc_traceback = None
+    try:
+        vm.launch()
+    except KeyboardInterrupt:
+        raise
+    except:
+        exc_traceback = traceback.format_exc()
+        dbg("Exception while running test case")
+    finally:
+        vm.shutdown()
+        ec = vm.exitcode()
+        log = vm.get_log()
+
+    if exc_traceback is not None or ec != 0:
+        return dict(exc_traceback=exc_traceback,
+                    exitcode=ec,
+                    log=log,
+                    testcase=testcase,
+                    cmdline=cmdline)
+
+
+def binariesToTest(args, testcase):
+    if args.qemu:
+        r = args.qemu
+    else:
+        r = glob.glob('./*-softmmu/qemu-system-*')
+    return r
+
+
+def accelsToTest(args, testcase):
+    if getBinaryInfo(args, testcase['binary']).kvm_available:
+        yield 'kvm'
+    yield 'tcg'
+
+
+def machinesToTest(args, testcase):
+    return getBinaryInfo(args, testcase['binary']).machines
+
+
+def devicesToTest(args, testcase):
+    return getBinaryInfo(args, testcase['binary']).user_devs
+
+
+TESTCASE_VARIABLES = [
+    ('binary', binariesToTest),
+    ('accel', accelsToTest),
+    ('machine', machinesToTest),
+    ('device', devicesToTest),
+]
+
+
+def genCases1(args, testcases, var, fn):
+    """Generate new testcases for one variable
+
+    If an existing item already has a variable set, don't
+    generate new items and just return it directly. This
+    allows the "-t" command-line option to be used to choose
+    a specific test case.
+    """
+    for testcase in testcases:
+        if var in testcase:
+            yield testcase.copy()
+        else:
+            for i in fn(args, testcase):
+                t = testcase.copy()
+                t[var] = i
+                yield t
+
+
+def genCases(args, testcase):
+    """Generate test cases for all variables
+    """
+    cases = [testcase.copy()]
+    for var, fn in TESTCASE_VARIABLES:
+        dbg("var: %r, fn: %r", var, fn)
+        cases = genCases1(args, cases, var, fn)
+    return cases
+
+
+def casesToTest(args, testcase):
+    cases = genCases(args, testcase)
+    if args.random:
+        cases = list(cases)
+        cases = random.sample(cases, min(args.random, len(cases)))
+    if args.debug:
+        cases = list(cases)
+        dbg("%d test cases to test", len(cases))
+    if args.shuffle:
+        cases = list(cases)
+        random.shuffle(cases)
+    return cases
+
+
+def logFailure(f, level):
+    t = f['testcase']
+    logger.log(level, "failed: %s", formatTestCase(t))
+    logger.log(level, "cmdline: %s", f['cmdline'])
+    for l in f['log'].strip().split('\n'):
+        logger.log(level, "log: %s", l)
+    logger.log(level, "exit code: %r", f['exitcode'])
+    if f['exc_traceback']:
+        logger.log(level, "exception:")
+        for l in f['exc_traceback'].split('\n'):
+            logger.log(level, "  %s", l.rstrip('\n'))
+
+
+def main():
+    parser = argparse.ArgumentParser(description="QEMU -device crash test")
+    parser.add_argument('-t', metavar='KEY=VALUE', nargs='*',
+                        help="Limit test cases to KEY=VALUE",
+                        action='append', dest='testcases', default=[])
+    parser.add_argument('-d', '--debug', action='store_true',
+                        help='debug output')
+    parser.add_argument('-v', '--verbose', action='store_true', default=True,
+                        help='verbose output')
+    parser.add_argument('-q', '--quiet', dest='verbose', action='store_false',
+                        help='non-verbose output')
+    parser.add_argument('-r', '--random', type=int, metavar='COUNT',
+                        help='run a random sample of COUNT test cases',
+                        default=0)
+    parser.add_argument('--shuffle', action='store_true',
+                        help='Run test cases in random order')
+    parser.add_argument('--dry-run', action='store_true',
+                        help="Don't run any tests, just generate list")
+    parser.add_argument('-D', '--devtype', metavar='TYPE',
+                        help="Test only device types that implement TYPE")
+    parser.add_argument('-Q', '--quick', action='store_true', default=True,
+                        help="Quick mode: skip test cases that are expected to fail")
+    parser.add_argument('-F', '--full', action='store_false', dest='quick',
+                        help="Full mode: test cases that are expected to fail")
+    parser.add_argument('--strict', action='store_true', dest='strict',
+                        help="Treat all warnings as fatal")
+    parser.add_argument('qemu', nargs='*', metavar='QEMU',
+                        help='QEMU binary to run')
+    args = parser.parse_args()
+
+    if args.debug:
+        lvl = logging.DEBUG
+    elif args.verbose:
+        lvl = logging.INFO
+    else:
+        lvl = logging.WARN
+    logging.basicConfig(stream=sys.stdout, level=lvl, format='%(levelname)s: %(message)s')
+
+    fatal_failures = []
+    wl_stats = {}
+    skipped = 0
+    total = 0
+
+    tc = {}
+    dbg("testcases: %r", args.testcases)
+    if args.testcases:
+        for t in chain(*args.testcases):
+            for kv in t.split():
+                k, v = kv.split('=', 1)
+                tc[k] = v
+
+    if len(binariesToTest(args, tc)) == 0:
+        print >>sys.stderr, "No QEMU binary found"
+        parser.print_usage(sys.stderr)
+        return 1
+
+    for t in casesToTest(args, tc):
+        logger.info("running test case: %s", formatTestCase(t))
+        total += 1
+
+        expected_match = findExpectedResult(t)
+        if (args.quick and
+                (expected_match or
+                 not getBinaryInfo(args, t['binary']).machineInfo(t['machine'])['runnable'])):
+            dbg("skipped: %s", formatTestCase(t))
+            skipped += 1
+            continue
+
+        if args.dry_run:
+            continue
+
+        try:
+            f = checkOneCase(args, t)
+        except KeyboardInterrupt:
+            break
+
+        if f:
+            i, wl = checkResultWhitelist(f)
+            dbg("testcase: %r, whitelist match: %r", t, wl)
+            wl_stats.setdefault(i, []).append(f)
+            level = wl.get('loglevel', logging.DEBUG)
+            logFailure(f, level)
+            if wl.get('fatal') or (args.strict and level >= logging.WARN):
+                fatal_failures.append(f)
+        else:
+            dbg("success: %s", formatTestCase(t))
+            if expected_match:
+                logger.warn("Didn't fail as expected: %s", formatTestCase(t))
+
+    logger.info("Total: %d test cases", total)
+    if skipped:
+        logger.info("Skipped %d test cases", skipped)
+
+    if args.debug:
+        stats = sorted([(len(wl_stats.get(i, [])), wl) for i, wl in enumerate(ERROR_WHITELIST)])
+        for count, wl in stats:
+            dbg("whitelist entry stats: %d: %r", count, wl)
+
+    if fatal_failures:
+        for f in fatal_failures:
+            t = f['testcase']
+            logger.error("Fatal failure: %s", formatTestCase(t))
+        logger.error("Fatal failures on some machine/device combinations")
+        return 1
+
+if __name__ == '__main__':
+    sys.exit(main())
-- 
2.11.0.259.g40922b1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH v3 1/3] qemu.py: Don't set _popen=None on error/shutdown
  2017-05-26 18:11 ` [Qemu-devel] [PATCH v3 1/3] qemu.py: Don't set _popen=None on error/shutdown Eduardo Habkost
@ 2017-05-29 16:44   ` Markus Armbruster
  0 siblings, 0 replies; 8+ messages in thread
From: Markus Armbruster @ 2017-05-29 16:44 UTC (permalink / raw)
  To: Eduardo Habkost; +Cc: qemu-devel, Marcel Apfelbaum, Thomas Huth

Eduardo Habkost <ehabkost@redhat.com> writes:

> Keep the Popen object around to we can query its exit code later.
>
> To keep the existing 'self._popen is None' checks working, add a
> is_running() method, that will check if the process is still running.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Reviewed-by: Markus Armbruster <armbru@redhat.com>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH v3 2/3] qemu.py: Add QEMUMachine.exitcode() method
  2017-05-26 18:11 ` [Qemu-devel] [PATCH v3 2/3] qemu.py: Add QEMUMachine.exitcode() method Eduardo Habkost
@ 2017-05-29 16:53   ` Markus Armbruster
  2017-05-29 17:04     ` Eduardo Habkost
  0 siblings, 1 reply; 8+ messages in thread
From: Markus Armbruster @ 2017-05-29 16:53 UTC (permalink / raw)
  To: Eduardo Habkost; +Cc: qemu-devel, Marcel Apfelbaum, Thomas Huth

Eduardo Habkost <ehabkost@redhat.com> writes:

> Allow the exit code of QEMU to be queried by scripts.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  scripts/qemu.py | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/scripts/qemu.py b/scripts/qemu.py
> index 16934f1e02..ebe1c4b919 100644
> --- a/scripts/qemu.py
> +++ b/scripts/qemu.py
> @@ -88,6 +88,10 @@ class QEMUMachine(object):
>      def is_running(self):
>          return self._popen and (self._popen.returncode is None)
>  
> +    def exitcode(self):
> +        if self._popen:
> +            return self._popen.returncode
> +

Falling off the function's end returns None.  Do we really want to rely
on that?

For what it's worth, I checked the Python Language Reference, found it
less than clear, so I tried it out, too.

>      def get_pid(self):
>          if not self.is_running():
>              return None

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH v3 2/3] qemu.py: Add QEMUMachine.exitcode() method
  2017-05-29 16:53   ` Markus Armbruster
@ 2017-05-29 17:04     ` Eduardo Habkost
  2017-05-30  5:25       ` Markus Armbruster
  0 siblings, 1 reply; 8+ messages in thread
From: Eduardo Habkost @ 2017-05-29 17:04 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: qemu-devel, Marcel Apfelbaum, Thomas Huth

On Mon, May 29, 2017 at 06:53:47PM +0200, Markus Armbruster wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
> 
> > Allow the exit code of QEMU to be queried by scripts.
> >
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> >  scripts/qemu.py | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/scripts/qemu.py b/scripts/qemu.py
> > index 16934f1e02..ebe1c4b919 100644
> > --- a/scripts/qemu.py
> > +++ b/scripts/qemu.py
> > @@ -88,6 +88,10 @@ class QEMUMachine(object):
> >      def is_running(self):
> >          return self._popen and (self._popen.returncode is None)
> >  
> > +    def exitcode(self):
> > +        if self._popen:
> > +            return self._popen.returncode
> > +
> 
> Falling off the function's end returns None.  Do we really want to rely
> on that?
> 
> For what it's worth, I checked the Python Language Reference, found it
> less than clear, so I tried it out, too.

I agree that the intent may not be clear when looking at the
code.  I can squash this in:

diff --git a/scripts/qemu.py b/scripts/qemu.py
index ebe1c4b919..bf00eddab8 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -89,8 +89,9 @@ class QEMUMachine(object):
         return self._popen and (self._popen.returncode is None)
 
     def exitcode(self):
-        if self._popen:
-            return self._popen.returncode
+        if not self._popen:
+            return None
+        return self._popen.returncode
 
     def get_pid(self):
         if not self.is_running():

-- 
Eduardo

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH v3 2/3] qemu.py: Add QEMUMachine.exitcode() method
  2017-05-29 17:04     ` Eduardo Habkost
@ 2017-05-30  5:25       ` Markus Armbruster
  0 siblings, 0 replies; 8+ messages in thread
From: Markus Armbruster @ 2017-05-30  5:25 UTC (permalink / raw)
  To: Eduardo Habkost; +Cc: Marcel Apfelbaum, Thomas Huth, qemu-devel

Eduardo Habkost <ehabkost@redhat.com> writes:

> On Mon, May 29, 2017 at 06:53:47PM +0200, Markus Armbruster wrote:
>> Eduardo Habkost <ehabkost@redhat.com> writes:
>> 
>> > Allow the exit code of QEMU to be queried by scripts.
>> >
>> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>> > ---
>> >  scripts/qemu.py | 4 ++++
>> >  1 file changed, 4 insertions(+)
>> >
>> > diff --git a/scripts/qemu.py b/scripts/qemu.py
>> > index 16934f1e02..ebe1c4b919 100644
>> > --- a/scripts/qemu.py
>> > +++ b/scripts/qemu.py
>> > @@ -88,6 +88,10 @@ class QEMUMachine(object):
>> >      def is_running(self):
>> >          return self._popen and (self._popen.returncode is None)
>> >  
>> > +    def exitcode(self):
>> > +        if self._popen:
>> > +            return self._popen.returncode
>> > +
>> 
>> Falling off the function's end returns None.  Do we really want to rely
>> on that?
>> 
>> For what it's worth, I checked the Python Language Reference, found it
>> less than clear, so I tried it out, too.
>
> I agree that the intent may not be clear when looking at the
> code.  I can squash this in:
>
> diff --git a/scripts/qemu.py b/scripts/qemu.py
> index ebe1c4b919..bf00eddab8 100644
> --- a/scripts/qemu.py
> +++ b/scripts/qemu.py
> @@ -89,8 +89,9 @@ class QEMUMachine(object):
>          return self._popen and (self._popen.returncode is None)
>  
>      def exitcode(self):
> -        if self._popen:
> -            return self._popen.returncode
> +        if not self._popen:
> +            return None
> +        return self._popen.returncode
>  
>      def get_pid(self):
>          if not self.is_running():

Works for me.  The equivalent

    return self._popen and self._popen.returncode

would also work.  Nicely terse.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-05-30  5:25 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-05-26 18:11 [Qemu-devel] [PATCH v3 0/3] script for crash-testing -device Eduardo Habkost
2017-05-26 18:11 ` [Qemu-devel] [PATCH v3 1/3] qemu.py: Don't set _popen=None on error/shutdown Eduardo Habkost
2017-05-29 16:44   ` Markus Armbruster
2017-05-26 18:11 ` [Qemu-devel] [PATCH v3 2/3] qemu.py: Add QEMUMachine.exitcode() method Eduardo Habkost
2017-05-29 16:53   ` Markus Armbruster
2017-05-29 17:04     ` Eduardo Habkost
2017-05-30  5:25       ` Markus Armbruster
2017-05-26 18:12 ` [Qemu-devel] [PATCH v3 3/3] scripts: Test script to look for -device crashes Eduardo Habkost

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