All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [RFC 0/6] device-crash-test: device_add test mode
@ 2017-09-26 23:07 Eduardo Habkost
  2017-09-26 23:07 ` [Qemu-devel] [RFC 1/6] device-crash-test: Make whitelist code a bit more flexible Eduardo Habkost
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Eduardo Habkost @ 2017-09-26 23:07 UTC (permalink / raw)
  To: qemu-devel; +Cc: Thomas Huth, Dr. David Alan Gilbert, Cleber Rosa

This series implements device_add testing on device-crash-test.
The new test mode will be enabled by default in addition to the
old test cases, but if only device_add testing is wanted, it can
be specified using the '-t' command-line option, e.g.:

  $ ./scripts/device-crash-test -t method=device_add

In comparison to the "-device" test cases, a full test run runs
reasonably quickly (17 seconds on my machine for testing all
hotpluggable devices on all 72 machine-types in
qemu-system-x86_64).  I believe it is a good candidate to be
eventually included on "make check".

TODO:
* Fix the QEMU crash found using '-t plug_all=1'
  (see patch 5/5 for details)
* Test non-x86 architectures
* Compare results with Thomas' HMP unit test
  ("tests: Add a device_add/del HMP test" patch").

Based-on: 20170926220319.12889-1-ehabkost@redhat.com
(Subject: [PATCH 0/4] qmp: query-device-type command)

Eduardo Habkost (6):
  device-crash-test: Make whitelist code a bit more flexible
  device-crash-test: Log detailed info on success too
  device-crash-test: Allow checkOneCase() to report multiple results
  device-crash-test: Exit immediately on fatal failures on quick mode
  device-crash-test: Basic device_add support
  device-crash-test: Multi-device device_add test

 scripts/device-crash-test | 132 +++++++++++++++++++++++++++++++++-------------
 1 file changed, 94 insertions(+), 38 deletions(-)

-- 
2.13.5

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

* [Qemu-devel] [RFC 1/6] device-crash-test: Make whitelist code a bit more flexible
  2017-09-26 23:07 [Qemu-devel] [RFC 0/6] device-crash-test: device_add test mode Eduardo Habkost
@ 2017-09-26 23:07 ` Eduardo Habkost
  2017-09-26 23:07 ` [Qemu-devel] [RFC 2/6] device-crash-test: Log detailed info on success too Eduardo Habkost
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Eduardo Habkost @ 2017-09-26 23:07 UTC (permalink / raw)
  To: qemu-devel; +Cc: Thomas Huth, Dr. David Alan Gilbert, Cleber Rosa

Allow test results without the 'log' and 'exitcode' keys.  The
new device_add code won't report exitcode/log because multiple
devices will be tested on a single QEMU run.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 scripts/device-crash-test | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/scripts/device-crash-test b/scripts/device-crash-test
index ddc8f72eca..598720ffa9 100755
--- a/scripts/device-crash-test
+++ b/scripts/device-crash-test
@@ -284,9 +284,10 @@ def whitelistResultMatch(wl, r):
     """
     assert whitelistTestCaseMatch(wl, r['testcase'])
     return ((wl.get('exitcode', 1) is None or
-             r['exitcode'] == wl.get('exitcode', 1)) and
+             r.get('exitcode') == wl.get('exitcode', 1)) and
             ('log' not in wl or
-             re.search(wl['log'], r['log'], re.MULTILINE)))
+             ('log' in r and
+              re.search(wl['log'], r['log'], re.MULTILINE))))
 
 
 def checkResultWhitelist(r):
-- 
2.13.5

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

* [Qemu-devel] [RFC 2/6] device-crash-test: Log detailed info on success too
  2017-09-26 23:07 [Qemu-devel] [RFC 0/6] device-crash-test: device_add test mode Eduardo Habkost
  2017-09-26 23:07 ` [Qemu-devel] [RFC 1/6] device-crash-test: Make whitelist code a bit more flexible Eduardo Habkost
@ 2017-09-26 23:07 ` Eduardo Habkost
  2017-09-26 23:07 ` [Qemu-devel] [RFC 3/6] device-crash-test: Allow checkOneCase() to report multiple results Eduardo Habkost
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Eduardo Habkost @ 2017-09-26 23:07 UTC (permalink / raw)
  To: qemu-devel; +Cc: Thomas Huth, Dr. David Alan Gilbert, Cleber Rosa

Instead of making checkOneCase() return None on success, return
detailed info even if no problems were found.  This will make
handling of device_add errors simpler, and improve debug output.

As logFailure() won't handle only failures, rename it to
logResult() and make it a bit more flexible about missing fields.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 scripts/device-crash-test | 61 ++++++++++++++++++++++-------------------------
 1 file changed, 29 insertions(+), 32 deletions(-)

diff --git a/scripts/device-crash-test b/scripts/device-crash-test
index 598720ffa9..782d7fd6c2 100755
--- a/scripts/device-crash-test
+++ b/scripts/device-crash-test
@@ -234,6 +234,9 @@ ERROR_WHITELIST = [
     {'exitcode':-11, 'device':'arm-gicv3', 'loglevel':logging.ERROR, 'expected':True},
     {'exitcode':-11, 'machine':'isapc', 'device':'.*-iommu', 'loglevel':logging.ERROR, 'expected':True},
 
+    # 0 exit code means test succeeded, so we just log it as logging.DEBUG:
+    {'exitcode':0},
+
     # everything else (including SIGABRT and SIGSEGV) will be a fatal error:
     {'exitcode':None, 'fatal':True, 'loglevel':logging.FATAL},
 ]
@@ -381,8 +384,7 @@ def getBinaryInfo(args, binary):
 def checkOneCase(args, testcase):
     """Check one specific case
 
-    Returns a dictionary containing failure information on error,
-    or None on success
+    Returns a dictionary containing results of test case.
     """
     binary = testcase['binary']
     accel = testcase['accel']
@@ -397,26 +399,21 @@ def checkOneCase(args, testcase):
     dbg("will launch QEMU: %s", cmdline)
     vm = QEMUMachine(binary=binary, args=args)
 
-    exc_traceback = None
+    r = {'testcase':testcase,
+         'cmdline':cmdline}
+
     try:
         vm.launch()
     except KeyboardInterrupt:
         raise
     except:
-        exc_traceback = traceback.format_exc()
+        r['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 {'exc_traceback':exc_traceback,
-                'exitcode':ec,
-                'log':log,
-                'testcase':testcase,
-                'cmdline':cmdline}
 
+    vm.shutdown()
+    r['exitcode'] = vm.exitcode()
+    r['log'] = vm.get_log()
+    return r
 
 def binariesToTest(args, testcase):
     if args.qemu:
@@ -490,17 +487,18 @@ def casesToTest(args, testcase):
     return cases
 
 
-def logFailure(f, level):
+def logResult(f, level):
     t = f['testcase']
-    logger.log(level, "failed: %s", formatTestCase(t))
+    logger.log(level, "result: %s", formatTestCase(t))
     logger.log(level, "cmdline: %s", f['cmdline'])
-    for l in f['log'].strip().split('\n'):
+    for l in f.get('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, "exit code: %r", f.get('exitcode'))
+    if 'exc_traceback' in f:
         logger.log(level, "exception:")
         for l in f['exc_traceback'].split('\n'):
             logger.log(level, "  %s", l.rstrip('\n'))
+    dbg('raw result dictionary: %r', f)
 
 
 def main():
@@ -579,18 +577,17 @@ def main():
         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))
+        i, wl = checkResultWhitelist(f)
+        dbg("testcase: %r, whitelist match: %r", t, wl)
+        wl_stats.setdefault(i, []).append(f)
+        level = wl.get('loglevel', logging.DEBUG)
+        logResult(f, level)
+
+        if wl.get('fatal') or (args.strict and level >= logging.WARN):
+            fatal_failures.append(f)
+
+        if expected_match and expected_match[0] != i:
+            logger.warn("Didn't fail as expected: %s", formatTestCase(t))
 
     logger.info("Total: %d test cases", total)
     if skipped:
-- 
2.13.5

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

* [Qemu-devel] [RFC 3/6] device-crash-test: Allow checkOneCase() to report multiple results
  2017-09-26 23:07 [Qemu-devel] [RFC 0/6] device-crash-test: device_add test mode Eduardo Habkost
  2017-09-26 23:07 ` [Qemu-devel] [RFC 1/6] device-crash-test: Make whitelist code a bit more flexible Eduardo Habkost
  2017-09-26 23:07 ` [Qemu-devel] [RFC 2/6] device-crash-test: Log detailed info on success too Eduardo Habkost
@ 2017-09-26 23:07 ` Eduardo Habkost
  2017-09-26 23:07 ` [Qemu-devel] [RFC 4/6] device-crash-test: Exit immediately on fatal failures on quick mode Eduardo Habkost
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Eduardo Habkost @ 2017-09-26 23:07 UTC (permalink / raw)
  To: qemu-devel; +Cc: Thomas Huth, Dr. David Alan Gilbert, Cleber Rosa

This will allow the test code to be improved to test multiple
devices in a single run.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 scripts/device-crash-test | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/scripts/device-crash-test b/scripts/device-crash-test
index 782d7fd6c2..0bd599d395 100755
--- a/scripts/device-crash-test
+++ b/scripts/device-crash-test
@@ -384,7 +384,8 @@ def getBinaryInfo(args, binary):
 def checkOneCase(args, testcase):
     """Check one specific case
 
-    Returns a dictionary containing results of test case.
+    Generates a list of dictionaries containing results of testing.  If
+    multiple devices are being tested, multiple entries can be returned.
     """
     binary = testcase['binary']
     accel = testcase['accel']
@@ -413,7 +414,7 @@ def checkOneCase(args, testcase):
     vm.shutdown()
     r['exitcode'] = vm.exitcode()
     r['log'] = vm.get_log()
-    return r
+    yield r
 
 def binariesToTest(args, testcase):
     if args.qemu:
@@ -572,22 +573,20 @@ def main():
         if args.dry_run:
             continue
 
-        try:
-            f = checkOneCase(args, t)
-        except KeyboardInterrupt:
-            break
 
-        i, wl = checkResultWhitelist(f)
-        dbg("testcase: %r, whitelist match: %r", t, wl)
-        wl_stats.setdefault(i, []).append(f)
-        level = wl.get('loglevel', logging.DEBUG)
-        logResult(f, level)
+        for r in checkOneCase(args, t):
+            i, wl = checkResultWhitelist(r)
+            dbg("testcase: %r, whitelist match: %r", t, wl)
+            wl_stats.setdefault(i, []).append(r)
+            level = wl.get('loglevel', logging.DEBUG)
+            logResult(r, level)
+
+            if wl.get('fatal') or (args.strict and level >= logging.WARN):
+                fatal_failures.append(r)
 
-        if wl.get('fatal') or (args.strict and level >= logging.WARN):
-            fatal_failures.append(f)
+            if expected_match and expected_match[0] != i:
+                logger.warn("Didn't fail as expected: %s", formatTestCase(t))
 
-        if expected_match and expected_match[0] != i:
-            logger.warn("Didn't fail as expected: %s", formatTestCase(t))
 
     logger.info("Total: %d test cases", total)
     if skipped:
-- 
2.13.5

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

* [Qemu-devel] [RFC 4/6] device-crash-test: Exit immediately on fatal failures on quick mode
  2017-09-26 23:07 [Qemu-devel] [RFC 0/6] device-crash-test: device_add test mode Eduardo Habkost
                   ` (2 preceding siblings ...)
  2017-09-26 23:07 ` [Qemu-devel] [RFC 3/6] device-crash-test: Allow checkOneCase() to report multiple results Eduardo Habkost
@ 2017-09-26 23:07 ` Eduardo Habkost
  2017-09-26 23:07 ` [Qemu-devel] [RFC 5/6] device-crash-test: Basic device_add support Eduardo Habkost
  2017-09-26 23:07 ` [Qemu-devel] [RFC 6/6] device-crash-test: Multi-device device_add test Eduardo Habkost
  5 siblings, 0 replies; 9+ messages in thread
From: Eduardo Habkost @ 2017-09-26 23:07 UTC (permalink / raw)
  To: qemu-devel; +Cc: Thomas Huth, Dr. David Alan Gilbert, Cleber Rosa

If we're running on quick mode, we won't wait for the full test
run, and will exit immediately if a fatal failure is found.

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

diff --git a/scripts/device-crash-test b/scripts/device-crash-test
index 0bd599d395..4c23ffa449 100755
--- a/scripts/device-crash-test
+++ b/scripts/device-crash-test
@@ -583,10 +583,14 @@ def main():
 
             if wl.get('fatal') or (args.strict and level >= logging.WARN):
                 fatal_failures.append(r)
+                if args.quick:
+                    break
 
             if expected_match and expected_match[0] != i:
                 logger.warn("Didn't fail as expected: %s", formatTestCase(t))
 
+        if fatal_failures and args.quick:
+            break
 
     logger.info("Total: %d test cases", total)
     if skipped:
-- 
2.13.5

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

* [Qemu-devel] [RFC 5/6] device-crash-test: Basic device_add support
  2017-09-26 23:07 [Qemu-devel] [RFC 0/6] device-crash-test: device_add test mode Eduardo Habkost
                   ` (3 preceding siblings ...)
  2017-09-26 23:07 ` [Qemu-devel] [RFC 4/6] device-crash-test: Exit immediately on fatal failures on quick mode Eduardo Habkost
@ 2017-09-26 23:07 ` Eduardo Habkost
  2017-09-26 23:07 ` [Qemu-devel] [RFC 6/6] device-crash-test: Multi-device device_add test Eduardo Habkost
  5 siblings, 0 replies; 9+ messages in thread
From: Eduardo Habkost @ 2017-09-26 23:07 UTC (permalink / raw)
  To: qemu-devel; +Cc: Thomas Huth, Dr. David Alan Gilbert, Cleber Rosa

Add a 'method' testcase argument that will test the device using
device_add instead of -device.  A new device_add_error whitelist
key is now supported, to catch device_add errors.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 scripts/device-crash-test | 38 +++++++++++++++++++++++++++++++++-----
 1 file changed, 33 insertions(+), 5 deletions(-)

diff --git a/scripts/device-crash-test b/scripts/device-crash-test
index 4c23ffa449..1245e214a0 100755
--- a/scripts/device-crash-test
+++ b/scripts/device-crash-test
@@ -35,7 +35,7 @@ import argparse
 from itertools import chain
 
 sys.path.append(os.path.join(os.path.dirname(__file__), '..', 'scripts'))
-from qemu import QEMUMachine
+from qemu import QEMUMachine, MonitorResponseError
 
 logger = logging.getLogger('device-crash-test')
 dbg = logger.debug
@@ -203,6 +203,11 @@ ERROR_WHITELIST = [
     {'log':r"core 0 already populated"},
     {'log':r"could not find stage1 bootloader"},
 
+    {'exitcode':None, 'device_add_error':r"^Bus '[\w.-]+' does not support hotplugging$"},
+    {'exitcode':None, 'device_add_error':r"^No '[\w-]+' bus found for device '[\w-]+'$"},
+    # There are too many ways device_add can fail, ignore all of them as long as QEMU doesn't crash:
+    {'exitcode':None, 'device_add_error':r".+"},
+
     # other exitcode=1 failures not listed above will just generate INFO messages:
     {'exitcode':1, 'loglevel':logging.INFO},
 
@@ -290,7 +295,10 @@ def whitelistResultMatch(wl, r):
              r.get('exitcode') == wl.get('exitcode', 1)) and
             ('log' not in wl or
              ('log' in r and
-              re.search(wl['log'], r['log'], re.MULTILINE))))
+              re.search(wl['log'], r['log'], re.MULTILINE))) and
+            ('device_add_error' not in wl or
+             ('device_add_error' in r and
+              re.search(wl['device_add_error'], r['device_add_error']))))
 
 
 def checkResultWhitelist(r):
@@ -339,6 +347,7 @@ class QemuBinaryInfo(object):
             self.alldevs = set(qomListTypeNames(vm, implements=devtype, abstract=False))
             self.dev_info = dict((t, vm.command('query-device-type', typename=t)) for t in self.alldevs)
             self.user_devs = set(d for d in self.alldevs if self.dev_info[d]['user-creatable'])
+            self.hotpluggable_devs = set(d for d in self.alldevs if self.dev_info[d]['hotpluggable'])
             self.machines = list(m['name'] for m in vm.command('query-machines'))
             self.kvm_available = vm.command('query-kvm')['enabled']
         finally:
@@ -391,11 +400,14 @@ def checkOneCase(args, testcase):
     accel = testcase['accel']
     machine = testcase['machine']
     device = testcase['device']
+    method = testcase['method']
+    assert method in ['-device', 'device_add']
 
     dbg("will test: %r", testcase)
 
-    args = ['-S', '-machine', '%s,accel=%s' % (machine, accel),
-            '-device', qemuOptsEscape(device)]
+    args = ['-S', '-machine', '%s,accel=%s' % (machine, accel)]
+    if method == '-device':
+        args.extend(['-device', qemuOptsEscape(device)])
     cmdline = ' '.join([binary] + args)
     dbg("will launch QEMU: %s", cmdline)
     vm = QEMUMachine(binary=binary, args=args)
@@ -405,6 +417,12 @@ def checkOneCase(args, testcase):
 
     try:
         vm.launch()
+        if method == 'device_add':
+            dbg('running device_add %s', device)
+            try:
+                vm.command('device_add', driver=device)
+            except MonitorResponseError, e:
+                r['device_add_error'] = e.reply['error']['desc']
     except KeyboardInterrupt:
         raise
     except:
@@ -434,14 +452,22 @@ def machinesToTest(args, testcase):
     return getBinaryInfo(args, testcase['binary']).machines
 
 
+def plugMethods(args, testcase):
+    return ['-device', 'device_add']
+
+
 def devicesToTest(args, testcase):
-    return getBinaryInfo(args, testcase['binary']).user_devs
+    if testcase['method'] == 'device_add':
+        return getBinaryInfo(args, testcase['binary']).hotpluggable_devs
+    else:
+        return getBinaryInfo(args, testcase['binary']).user_devs
 
 
 TESTCASE_VARIABLES = [
     ('binary', binariesToTest),
     ('accel', accelsToTest),
     ('machine', machinesToTest),
+    ('method', plugMethods),
     ('device', devicesToTest),
 ]
 
@@ -494,6 +520,8 @@ def logResult(f, level):
     logger.log(level, "cmdline: %s", f['cmdline'])
     for l in f.get('log', '').strip().split('\n'):
         logger.log(level, "log: %s", l)
+    if 'device_add_error' in f:
+        logger.log(level, "device_add error: %s", f['device_add_error'])
     logger.log(level, "exit code: %r", f.get('exitcode'))
     if 'exc_traceback' in f:
         logger.log(level, "exception:")
-- 
2.13.5

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

* [Qemu-devel] [RFC 6/6] device-crash-test: Multi-device device_add test
  2017-09-26 23:07 [Qemu-devel] [RFC 0/6] device-crash-test: device_add test mode Eduardo Habkost
                   ` (4 preceding siblings ...)
  2017-09-26 23:07 ` [Qemu-devel] [RFC 5/6] device-crash-test: Basic device_add support Eduardo Habkost
@ 2017-09-26 23:07 ` Eduardo Habkost
  2017-09-27 11:17   ` Thomas Huth
  5 siblings, 1 reply; 9+ messages in thread
From: Eduardo Habkost @ 2017-09-26 23:07 UTC (permalink / raw)
  To: qemu-devel; +Cc: Thomas Huth, Dr. David Alan Gilbert, Cleber Rosa

When running device_add tests, test all devices in a single run
instead of restarting QEMU every time.

There's a plug_all testcase argument that can be used to make the
test code plug all devices at once.  I'm adding this mode because
there's a crash that was detected while testing the script
without any device_del command, but the crash goes away if we
device_del every device immediately:

  $ ../scripts/device-crash-test ./x86_64-softmmu/qemu-system-x86_64 --quick -t method=device_add  --strict -t plug_all=1
  INFO: running test case: binary=./x86_64-softmmu/qemu-system-x86_64 plug_all=1 accel=kvm machine=pc-0.12 device=* method=device_add
  WARNING: qemu received signal -6: ./x86_64-softmmu/qemu-system-x86_64 -chardev socket,id=mon,path=/var/tmp/qemu-23578-monitor.sock -mon chardev=mon,mode=control -display none -vga none -S -machine pc-0.12,accel=kvm
  ERROR: result: binary=./x86_64-softmmu/qemu-system-x86_64 plug_all=1 accel=kvm machine=pc-0.12 device=* method=device_add
  ERROR: cmdline: ./x86_64-softmmu/qemu-system-x86_64 -S -machine pc-0.12,accel=kvm
  ERROR: log: audio: Could not init `oss' audio driver
  ERROR: log: qemu-system-x86_64: .../qemu/migration/savevm.c:721: vmstate_register_with_alias_id: Assertion `!se->compat || se->instance_id == 0' failed.
  ERROR: last device_add device: usb-net
  ERROR: exit code: -6
  ERROR: exception:
  ERROR:   Traceback (most recent call last):
  ERROR:     File "../scripts/device-crash-test", line 437, in checkOneCase
  ERROR:       vm.command('device_add', driver=device, id=devid)
  ERROR:     File ".../qemu/scripts/qemu.py", line 269, in command
  ERROR:       raise qmp.qmp.QMPError("Monitor is closed")
  ERROR:   QMPError: Monitor is closed
  ERROR:
  INFO: Total: 1 test cases
  ERROR: Fatal failure: binary=./x86_64-softmmu/qemu-system-x86_64 plug_all=1 accel=kvm machine=pc-0.12 device=* method=device_add
  ERROR: Fatal failures on some machine/device combinations

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 scripts/device-crash-test | 41 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 34 insertions(+), 7 deletions(-)

diff --git a/scripts/device-crash-test b/scripts/device-crash-test
index 1245e214a0..43110fed15 100755
--- a/scripts/device-crash-test
+++ b/scripts/device-crash-test
@@ -404,10 +404,19 @@ def checkOneCase(args, testcase):
     assert method in ['-device', 'device_add']
 
     dbg("will test: %r", testcase)
+    if device == '*':
+        bi = getBinaryInfo(args, binary)
+        if method == '-device':
+            devices = list(bi.user_devs)
+        else:
+            devices = list(bi.hotpluggable_devs)
+    else:
+        devices = [device]
 
     args = ['-S', '-machine', '%s,accel=%s' % (machine, accel)]
     if method == '-device':
-        args.extend(['-device', qemuOptsEscape(device)])
+        for device in devices:
+            args.extend(['-device', qemuOptsEscape(device)])
     cmdline = ' '.join([binary] + args)
     dbg("will launch QEMU: %s", cmdline)
     vm = QEMUMachine(binary=binary, args=args)
@@ -418,11 +427,27 @@ def checkOneCase(args, testcase):
     try:
         vm.launch()
         if method == 'device_add':
-            dbg('running device_add %s', device)
-            try:
-                vm.command('device_add', driver=device)
-            except MonitorResponseError, e:
-                r['device_add_error'] = e.reply['error']['desc']
+            plug_all = int(testcase.get('plug_all', 0))
+            for device in devices:
+                r['device_add_device'] = device
+                dbg('running device_add %s', device)
+                devid = 'test-%s' % (device)
+                try:
+                    vm.command('device_add', driver=device, id=devid)
+                    if not plug_all:
+                        vm.command('device_del', id=devid)
+                except MonitorResponseError, e:
+                    r2 = r.copy()
+                    r2['device_add_error'] = e.reply['error']['desc']
+                    yield r2
+
+            if plug_all:
+                for device in devices:
+                    devid = 'test-%s' % (device)
+                    vm.command('device_del', id=devid)
+
+    except GeneratorExit:
+        raise
     except KeyboardInterrupt:
         raise
     except:
@@ -458,7 +483,7 @@ def plugMethods(args, testcase):
 
 def devicesToTest(args, testcase):
     if testcase['method'] == 'device_add':
-        return getBinaryInfo(args, testcase['binary']).hotpluggable_devs
+        return '*' # will test all devices in a single checkOneCase() call
     else:
         return getBinaryInfo(args, testcase['binary']).user_devs
 
@@ -520,6 +545,8 @@ def logResult(f, level):
     logger.log(level, "cmdline: %s", f['cmdline'])
     for l in f.get('log', '').strip().split('\n'):
         logger.log(level, "log: %s", l)
+    if 'device_add_device' in f:
+        logger.log(level, 'last device_add device: %s', f['device_add_device'])
     if 'device_add_error' in f:
         logger.log(level, "device_add error: %s", f['device_add_error'])
     logger.log(level, "exit code: %r", f.get('exitcode'))
-- 
2.13.5

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

* Re: [Qemu-devel] [RFC 6/6] device-crash-test: Multi-device device_add test
  2017-09-26 23:07 ` [Qemu-devel] [RFC 6/6] device-crash-test: Multi-device device_add test Eduardo Habkost
@ 2017-09-27 11:17   ` Thomas Huth
  2017-09-27 16:47     ` Dr. David Alan Gilbert
  0 siblings, 1 reply; 9+ messages in thread
From: Thomas Huth @ 2017-09-27 11:17 UTC (permalink / raw)
  To: Eduardo Habkost, qemu-devel; +Cc: Dr. David Alan Gilbert, Cleber Rosa

On 27.09.2017 01:07, Eduardo Habkost wrote:
> When running device_add tests, test all devices in a single run
> instead of restarting QEMU every time.
> 
> There's a plug_all testcase argument that can be used to make the
> test code plug all devices at once.  I'm adding this mode because
> there's a crash that was detected while testing the script
> without any device_del command, but the crash goes away if we
> device_del every device immediately:
> 
>   $ ../scripts/device-crash-test ./x86_64-softmmu/qemu-system-x86_64 --quick -t method=device_add  --strict -t plug_all=1
>   INFO: running test case: binary=./x86_64-softmmu/qemu-system-x86_64 plug_all=1 accel=kvm machine=pc-0.12 device=* method=device_add
>   WARNING: qemu received signal -6: ./x86_64-softmmu/qemu-system-x86_64 -chardev socket,id=mon,path=/var/tmp/qemu-23578-monitor.sock -mon chardev=mon,mode=control -display none -vga none -S -machine pc-0.12,accel=kvm
>   ERROR: result: binary=./x86_64-softmmu/qemu-system-x86_64 plug_all=1 accel=kvm machine=pc-0.12 device=* method=device_add
>   ERROR: cmdline: ./x86_64-softmmu/qemu-system-x86_64 -S -machine pc-0.12,accel=kvm
>   ERROR: log: audio: Could not init `oss' audio driver
>   ERROR: log: qemu-system-x86_64: .../qemu/migration/savevm.c:721: vmstate_register_with_alias_id: Assertion `!se->compat || se->instance_id == 0' failed.
>   ERROR: last device_add device: usb-net

FWIW, I've seen similar crashes while working on my HMP device_add
tester, when I skipped the device_del and plugged-in the devices a
couple of times instead. I think this happens because some devices are
doing a vmstate_register() in their instance_init() or realize()
function, instead of using dc->vmsd as they should. However, with the
git master branch as of today (31bc1d8481af414cfa2857f905e), I am
currently not able anymore to reproduce the problem with the HMP
device_add tester, so one of the recent "set user_creatable = false"
patches might have fixed the issue for me already...

 Thomas

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

* Re: [Qemu-devel] [RFC 6/6] device-crash-test: Multi-device device_add test
  2017-09-27 11:17   ` Thomas Huth
@ 2017-09-27 16:47     ` Dr. David Alan Gilbert
  0 siblings, 0 replies; 9+ messages in thread
From: Dr. David Alan Gilbert @ 2017-09-27 16:47 UTC (permalink / raw)
  To: Thomas Huth; +Cc: Eduardo Habkost, qemu-devel, Cleber Rosa

* Thomas Huth (thuth@redhat.com) wrote:
> On 27.09.2017 01:07, Eduardo Habkost wrote:
> > When running device_add tests, test all devices in a single run
> > instead of restarting QEMU every time.
> > 
> > There's a plug_all testcase argument that can be used to make the
> > test code plug all devices at once.  I'm adding this mode because
> > there's a crash that was detected while testing the script
> > without any device_del command, but the crash goes away if we
> > device_del every device immediately:
> > 
> >   $ ../scripts/device-crash-test ./x86_64-softmmu/qemu-system-x86_64 --quick -t method=device_add  --strict -t plug_all=1
> >   INFO: running test case: binary=./x86_64-softmmu/qemu-system-x86_64 plug_all=1 accel=kvm machine=pc-0.12 device=* method=device_add
> >   WARNING: qemu received signal -6: ./x86_64-softmmu/qemu-system-x86_64 -chardev socket,id=mon,path=/var/tmp/qemu-23578-monitor.sock -mon chardev=mon,mode=control -display none -vga none -S -machine pc-0.12,accel=kvm
> >   ERROR: result: binary=./x86_64-softmmu/qemu-system-x86_64 plug_all=1 accel=kvm machine=pc-0.12 device=* method=device_add
> >   ERROR: cmdline: ./x86_64-softmmu/qemu-system-x86_64 -S -machine pc-0.12,accel=kvm
> >   ERROR: log: audio: Could not init `oss' audio driver
> >   ERROR: log: qemu-system-x86_64: .../qemu/migration/savevm.c:721: vmstate_register_with_alias_id: Assertion `!se->compat || se->instance_id == 0' failed.
> >   ERROR: last device_add device: usb-net
> 
> FWIW, I've seen similar crashes while working on my HMP device_add
> tester, when I skipped the device_del and plugged-in the devices a
> couple of times instead. I think this happens because some devices are
> doing a vmstate_register() in their instance_init() or realize()
> function, instead of using dc->vmsd as they should. However, with the
> git master branch as of today (31bc1d8481af414cfa2857f905e), I am
> currently not able anymore to reproduce the problem with the HMP
> device_add tester, so one of the recent "set user_creatable = false"
> patches might have fixed the issue for me already...

I think I've seen this problem when two things end up trying to register
with the same migration name;  I'll be honest I can't remember the
details, but I think the choice is either between having a full bus
name, e.g.

    pci-xx:yy.z:aa:bb:.c:...... then similar for the USB chain

or
    mydevice  + number

where number is the instance=id.  So to hit this I think
it's typically a case of messing up that long path and it no
longer being unique.
Printing the path in vmstate_register_with_alias_id after the
pstrcat might help.
I did add some sanity checking in there for a case I'd
previously hit where the name just got truncated, but I don't
think you should be able to hit that any more.

Dave

>  Thomas
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

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

end of thread, other threads:[~2017-09-27 16:47 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-26 23:07 [Qemu-devel] [RFC 0/6] device-crash-test: device_add test mode Eduardo Habkost
2017-09-26 23:07 ` [Qemu-devel] [RFC 1/6] device-crash-test: Make whitelist code a bit more flexible Eduardo Habkost
2017-09-26 23:07 ` [Qemu-devel] [RFC 2/6] device-crash-test: Log detailed info on success too Eduardo Habkost
2017-09-26 23:07 ` [Qemu-devel] [RFC 3/6] device-crash-test: Allow checkOneCase() to report multiple results Eduardo Habkost
2017-09-26 23:07 ` [Qemu-devel] [RFC 4/6] device-crash-test: Exit immediately on fatal failures on quick mode Eduardo Habkost
2017-09-26 23:07 ` [Qemu-devel] [RFC 5/6] device-crash-test: Basic device_add support Eduardo Habkost
2017-09-26 23:07 ` [Qemu-devel] [RFC 6/6] device-crash-test: Multi-device device_add test Eduardo Habkost
2017-09-27 11:17   ` Thomas Huth
2017-09-27 16:47     ` Dr. David Alan Gilbert

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.