[libvirt] [RFC v4 00/13] qmp: query-device-slots command

Eduardo Habkost posted 13 patches 6 years, 8 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20170814215748.5158-1-ehabkost@redhat.com
Test FreeBSD passed
Test checkpatch failed
Test docker passed
Test s390x passed
qapi-schema.json              |  89 ++++++
include/hw/qdev-core.h        |   5 +
include/hw/qdev-slotinfo.h    |  85 ++++++
include/hw/usb.h              |   6 +-
include/qapi/util.h           |  39 +++
hw/audio/intel-hda.c          |   7 +
hw/block/fdc.c                |  15 +-
hw/char/virtio-serial-bus.c   |   1 +
hw/core/bus.c                 |  42 +++
hw/core/slotinfo.c            | 610 ++++++++++++++++++++++++++++++++++++++++++
hw/core/sysbus.c              |   8 +
hw/i2c/core.c                 |   7 +
hw/ide/qdev.c                 |  27 ++
hw/input/adb.c                |   7 +
hw/ipack/ipack.c              |   7 +
hw/isa/isa-bus.c              |   1 +
hw/misc/auxbus.c              |   1 +
hw/pci/pci.c                  | 120 ++++++++-
hw/ppc/spapr_vio.c            |   1 +
hw/s390x/css-bridge.c         |   2 +
hw/s390x/event-facility.c     |   1 +
hw/s390x/s390-pci-bus.c       |   7 +
hw/scsi/scsi-bus.c            |   1 +
hw/sd/core.c                  |   7 +
hw/ssi/ssi.c                  |   7 +
hw/usb/bus.c                  |  37 +++
hw/usb/dev-smartcard-reader.c |   7 +
hw/virtio/virtio-bus.c        |   1 +
qapi/qapi-util.c              |  66 +++++
qdev-monitor.c                | 109 ++++++++
tests/test-qapi-util.c        |  53 ++++
tests/test-slotinfo.c         | 398 +++++++++++++++++++++++++++
hw/core/Makefile.objs         |   2 +
tests/Makefile.include        |  14 +-
tests/qmp-machine-info.py     | 300 +++++++++++++++++++++
35 files changed, 2075 insertions(+), 15 deletions(-)
create mode 100644 include/hw/qdev-slotinfo.h
create mode 100644 hw/core/slotinfo.c
create mode 100644 tests/test-slotinfo.c
create mode 100755 tests/qmp-machine-info.py
[libvirt] [RFC v4 00/13] qmp: query-device-slots command
Posted by Eduardo Habkost 6 years, 8 months ago
Changelog
---------

Changes v3 -> v4:
* New compact representation of slot sets.
* New generic code to automatically merge similar slots
  into a single entry in the command output while keeping
  implementations of the method simpler.
* Example implementation of IDE and USB bus enumeration

Changes v2 -> v3:
* Implemented a "slot set" structure, where multiple slots can be
  reported by using integer ranges or lists for possible
  values for each property. Added a ValueSet struct, that
  can represent a set of values using either a simple list of
  values, or integer ranges. (Its JSON representation is very
  verbose, though. See comments below).
* Removed the *Properties structs, and replaced them with
  a simple list of SlotOption structs.
* DeviceSlotInfo is not an union anymore, removed the 'type'
  field only because there are no slot-type-specific fields in
  the current implementation, but we may add it back if necessary
* The implementation is very quick and dirty, the main purpose of
  this RFC is to evaluate the schema and returned data.

Changes v1 -> v2:
* Don't show sysbus unless has_dynamic_sysbus is set for the
  machine type
* Removed max-devices and devices properties
* Introduced "non-slot" slot type, to explicitly indicate
  we are returning info on a bus that doesn't implement slot
  enumeration yet.
* Return bus name instead of full QOM path on "bus" field
* PCI: Replaced "addr" property (string parsed by property
  setter) with "device-number" uint32 property
* PCI: return only one slot for PCIe ports

Summary
-------

This adds a new command to QMP: query-device-slots. It will allow
management software to query possible slots where devices can be
plugged.

This implementation of the command will return:

* Multiple PCI slots per bus, in the case of PCI buses;
* One slot for each entry from query-hotpluggable-cpus.
* One slot per bus for the other buses (that don't
  implement slot enumeration yet), with opts-complete=false

Representation of slot sets in JSON
-----------------------------------

Slot sets are represented by a list of option names and sets of
possible values for each of those options.  The command uses a
compact representation for the set of valid values for an option.
For example, the following set of 5 PCI functions:

      bus: pcie.0
      device-number: 31
      functions: 1,4,5,6,7

would be represented in the JSON data as:

  {"available":false,"count":5,
   "device-types":["pci-device"],"hotpluggable":false,
   "opts":[
      {"option":"function","values":[1,[4,7]]},
      {"option":"device-number","values":31},
      {"option":"bus","values":"pcie.0"}],
   "opts-complete":true}

I planned to use QAPI alternates to model/document that in the
schema, but it would require implementing a few missing features
in QAPI alternate support.

TODO
----

* Differentiation between legacy-pci-device and pcie-device
* Implement enumeration for other buses
* Document the slotinfo.c functions
* Optimize the slot/option merging algorithm

Example output
--------------

Using the following QEMU command-line:

  $ qemu-system-x86_64 -machine q35,accel=kvm \
    -smp 16,maxcpus=32,threads=2,cores=2

query-device-slots will return the following entries:

  {"available":true,"count":224,
   "device-types":["pci-device"],"hotpluggable":false,
   "opts":[
      {"option":"function","values":[[0,7]]},
      {"option":"device-number","values":[[3,30]]},
      {"option":"bus","values":"pcie.0"}],
   "opts-complete":true}
  {"available":true,"count":1,
   "device-types":["ide-device"],"hotpluggable":false,
   "opts":[
      {"option":"unit","values":0},
      {"option":"bus","values":"ide.2"}],
   "opts-complete":true}
  {"available":true,"count":10,
   "device-types":["ide-device"],"hotpluggable":false,
   "opts":[
      {"option":"unit","values":[[0,1]]},
      {"option":"bus","values":["ide.4","ide.3","ide.5","ide.0","ide.1"]}],
   "opts-complete":true}
  {"available":true,
   "device-types":["isa-device"],"hotpluggable":false,
   "opts":[
      {"option":"bus","values":"isa.0"}],
   "opts-complete":false}
  {"available":true,"count":16,
   "device-types":["qemu64-x86_64-cpu"],"hotpluggable":true,
   "opts":[
      {"option":"socket-id","values":[[4,7]]},
      {"option":"thread-id","values":[[0,1]]},
      {"option":"core-id","values":[[0,1]]}],
   "opts-complete":true}
  {"available":false,"count":1,"device":"/machine/unattached/device[16]",
   "device-types":["qemu64-x86_64-cpu"],"hotpluggable":true,
   "opts":[
      {"option":"socket-id","values":3},
      {"option":"thread-id","values":1},
      {"option":"core-id","values":1}],
   "opts-complete":true}
  {"available":false,"count":1,"device":"/machine/unattached/device[15]",
   "device-types":["qemu64-x86_64-cpu"],"hotpluggable":true,
   "opts":[
      {"option":"socket-id","values":3},
      {"option":"thread-id","values":0},
      {"option":"core-id","values":1}],
   "opts-complete":true}
  {"available":false,"count":1,"device":"/machine/unattached/device[14]",
   "device-types":["qemu64-x86_64-cpu"],"hotpluggable":true,
   "opts":[
      {"option":"socket-id","values":3},
      {"option":"thread-id","values":1},
      {"option":"core-id","values":0}],
   "opts-complete":true}
  {"available":false,"count":1,"device":"/machine/unattached/device[13]",
   "device-types":["qemu64-x86_64-cpu"],"hotpluggable":true,
   "opts":[
      {"option":"socket-id","values":3},
      {"option":"thread-id","values":0},
      {"option":"core-id","values":0}],
   "opts-complete":true}
  {"available":false,"count":1,"device":"/machine/unattached/device[12]",
   "device-types":["qemu64-x86_64-cpu"],"hotpluggable":true,
   "opts":[
      {"option":"socket-id","values":2},
      {"option":"thread-id","values":1},
      {"option":"core-id","values":1}],
   "opts-complete":true}
  {"available":false,"count":1,"device":"/machine/unattached/device[11]",
   "device-types":["qemu64-x86_64-cpu"],"hotpluggable":true,
   "opts":[
      {"option":"socket-id","values":2},
      {"option":"thread-id","values":0},
      {"option":"core-id","values":1}],
   "opts-complete":true}
  {"available":false,"count":1,"device":"/machine/unattached/device[10]",
   "device-types":["qemu64-x86_64-cpu"],"hotpluggable":true,
   "opts":[
      {"option":"socket-id","values":2},
      {"option":"thread-id","values":1},
      {"option":"core-id","values":0}],
   "opts-complete":true}
  {"available":false,"count":1,"device":"/machine/unattached/device[9]",
   "device-types":["qemu64-x86_64-cpu"],"hotpluggable":true,
   "opts":[
      {"option":"socket-id","values":2},
      {"option":"thread-id","values":0},
      {"option":"core-id","values":0}],
   "opts-complete":true}
  {"available":false,"count":1,"device":"/machine/unattached/device[8]",
   "device-types":["qemu64-x86_64-cpu"],"hotpluggable":true,
   "opts":[
      {"option":"socket-id","values":1},
      {"option":"thread-id","values":1},
      {"option":"core-id","values":1}],
   "opts-complete":true}
  {"available":false,"count":1,"device":"/machine/unattached/device[7]",
   "device-types":["qemu64-x86_64-cpu"],"hotpluggable":true,
   "opts":[
      {"option":"socket-id","values":1},
      {"option":"thread-id","values":0},
      {"option":"core-id","values":1}],
   "opts-complete":true}
  {"available":false,"count":1,"device":"/machine/unattached/device[6]",
   "device-types":["qemu64-x86_64-cpu"],"hotpluggable":true,
   "opts":[
      {"option":"socket-id","values":1},
      {"option":"thread-id","values":1},
      {"option":"core-id","values":0}],
   "opts-complete":true}
  {"available":false,"count":1,"device":"/machine/unattached/device[5]",
   "device-types":["qemu64-x86_64-cpu"],"hotpluggable":true,
   "opts":[
      {"option":"socket-id","values":1},
      {"option":"thread-id","values":0},
      {"option":"core-id","values":0}],
   "opts-complete":true}
  {"available":false,"count":1,"device":"/machine/unattached/device[4]",
   "device-types":["qemu64-x86_64-cpu"],"hotpluggable":true,
   "opts":[
      {"option":"socket-id","values":0},
      {"option":"thread-id","values":1},
      {"option":"core-id","values":1}],
   "opts-complete":true}
  {"available":false,"count":1,"device":"/machine/unattached/device[3]",
   "device-types":["qemu64-x86_64-cpu"],"hotpluggable":true,
   "opts":[
      {"option":"socket-id","values":0},
      {"option":"thread-id","values":0},
      {"option":"core-id","values":1}],
   "opts-complete":true}
  {"available":false,"count":1,"device":"/machine/unattached/device[2]",
   "device-types":["qemu64-x86_64-cpu"],"hotpluggable":true,
   "opts":[
      {"option":"socket-id","values":0},
      {"option":"thread-id","values":1},
      {"option":"core-id","values":0}],
   "opts-complete":true}
  {"available":false,"count":1,"device":"/machine/unattached/device[0]",
   "device-types":["qemu64-x86_64-cpu"],"hotpluggable":true,
   "opts":[
      {"option":"socket-id","values":0},
      {"option":"thread-id","values":0},
      {"option":"core-id","values":0}],
   "opts-complete":true}
  {"available":false,"count":1,"device":"/machine/q35/mch",
   "device-types":["pci-device"],"hotpluggable":false,
   "opts":[
      {"option":"function","values":0},
      {"option":"device-number","values":0},
      {"option":"bus","values":"pcie.0"}],
   "opts-complete":true}
  {"available":false,"count":21,
   "device-types":["pci-device"],"hotpluggable":false,
   "opts":[
      {"option":"function","values":[[1,7]]},
      {"option":"device-number","values":[[0,2]]},
      {"option":"bus","values":"pcie.0"}],
   "opts-complete":true}
  {"available":false,"count":1,"device":"/machine/unattached/device[44]",
   "device-types":["pci-device"],"hotpluggable":false,
   "opts":[
      {"option":"function","values":0},
      {"option":"device-number","values":1},
      {"option":"bus","values":"pcie.0"}],
   "opts-complete":true}
  {"available":false,"count":1,"device":"/machine/unattached/device[45]",
   "device-types":["pci-device"],"hotpluggable":false,
   "opts":[
      {"option":"function","values":0},
      {"option":"device-number","values":2},
      {"option":"bus","values":"pcie.0"}],
   "opts-complete":true}
  {"available":false,"count":1,"device":"/machine/unattached/device[18]",
   "device-types":["pci-device"],"hotpluggable":false,
   "opts":[
      {"option":"function","values":0},
      {"option":"device-number","values":31},
      {"option":"bus","values":"pcie.0"}],
   "opts-complete":true}
  {"available":false,"count":5,
   "device-types":["pci-device"],"hotpluggable":false,
   "opts":[
      {"option":"function","values":[1,[4,7]]},
      {"option":"device-number","values":31},
      {"option":"bus","values":"pcie.0"}],
   "opts-complete":true}
  {"available":false,"count":1,"device":"/machine/unattached/device[33]",
   "device-types":["pci-device"],"hotpluggable":false,
   "opts":[
      {"option":"function","values":2},
      {"option":"device-number","values":31},
      {"option":"bus","values":"pcie.0"}],
   "opts-complete":true}
  {"available":false,"count":1,"device":"/machine/unattached/device[35]",
   "device-types":["pci-device"],"hotpluggable":false,
   "opts":[
      {"option":"function","values":3},
      {"option":"device-number","values":31},
      {"option":"bus","values":"pcie.0"}],
   "opts-complete":true}
  {"available":false,"count":1,"device":"/machine/unattached/device[34]",
   "device-types":["ide-device"],"hotpluggable":false,
   "opts":[
      {"option":"unit","values":1},
      {"option":"bus","values":"ide.2"}],
   "opts-complete":true}
  {"available":false,"device":"/machine/unattached/device[32]",
   "device-types":["isa-device"],"hotpluggable":false,
   "opts":[
      {"option":"bus","values":"isa.0"}],
   "opts-complete":false}
  {"available":false,"device":"/machine/unattached/device[31]",
   "device-types":["isa-device"],"hotpluggable":false,
   "opts":[
      {"option":"bus","values":"isa.0"}],
   "opts-complete":false}
  {"available":false,"device":"/machine/unattached/device[30]",
   "device-types":["isa-device"],"hotpluggable":false,
   "opts":[
      {"option":"bus","values":"isa.0"}],
   "opts-complete":false}
  {"available":false,"device":"/machine/unattached/device[29]",
   "device-types":["isa-device"],"hotpluggable":false,
   "opts":[
      {"option":"bus","values":"isa.0"}],
   "opts-complete":false}
  {"available":false,"device":"/machine/unattached/device[28]",
   "device-types":["isa-device"],"hotpluggable":false,
   "opts":[
      {"option":"bus","values":"isa.0"}],
   "opts-complete":false}
  {"available":false,"device":"/machine/unattached/device[27]",
   "device-types":["isa-device"],"hotpluggable":false,
   "opts":[
      {"option":"bus","values":"isa.0"}],
   "opts-complete":false}
  {"available":false,"device":"/machine/unattached/device[26]",
   "device-types":["isa-device"],"hotpluggable":false,
   "opts":[
      {"option":"bus","values":"isa.0"}],
   "opts-complete":false}
  {"available":false,"device":"/machine/unattached/device[25]",
   "device-types":["isa-device"],"hotpluggable":false,
   "opts":[
      {"option":"bus","values":"isa.0"}],
   "opts-complete":false}
  {"available":false,"device":"/machine/unattached/device[24]",
   "device-types":["isa-device"],"hotpluggable":false,
   "opts":[
      {"option":"bus","values":"isa.0"}],
   "opts-complete":false}
  {"available":false,"device":"/machine/unattached/device[23]",
   "device-types":["isa-device"],"hotpluggable":false,
   "opts":[
      {"option":"bus","values":"isa.0"}],
   "opts-complete":false}
  {"available":false,"device":"/machine/unattached/device[22]",
   "device-types":["isa-device"],"hotpluggable":false,
   "opts":[
      {"option":"bus","values":"isa.0"}],
   "opts-complete":false}
  {"available":false,"device":"/machine/unattached/device[20]",
   "device-types":["isa-device"],"hotpluggable":false,
   "opts":[
      {"option":"bus","values":"isa.0"}],
   "opts-complete":false}
  {"available":false,"device":"/machine/unattached/device[19]",
   "device-types":["isa-device"],"hotpluggable":false,
   "opts":[
      {"option":"bus","values":"isa.0"}],
   "opts-complete":false}

Cc: Markus Armbruster <armbru@redhat.com>
Cc: Marcel Apfelbaum <marcel@redhat.com>
Cc: libvir-list@redhat.com
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Laine Stump <laine@redhat.com>

Eduardo Habkost (13):
  qmp: Define query-device-slots command
  qapi: qobject_compare() helper
  qdev: Add BusClass::device_type field
  qdev: Slot info helpers
  query-device-slots: Collapse similar entries
  qdev core: generic enumerate_slots implementation
  qdev: Enumerate CPU slots on query-device-slots
  ide: enumerate_slots implementation
  pci: pci_bus_has_pcie_upstream_port() function
  pci: device-number & function properties
  pci: enumerate_slots implementation
  usb: enumerate_slots implementation
  tests: Experimental query-device-slots test code

 qapi-schema.json              |  89 ++++++
 include/hw/qdev-core.h        |   5 +
 include/hw/qdev-slotinfo.h    |  85 ++++++
 include/hw/usb.h              |   6 +-
 include/qapi/util.h           |  39 +++
 hw/audio/intel-hda.c          |   7 +
 hw/block/fdc.c                |  15 +-
 hw/char/virtio-serial-bus.c   |   1 +
 hw/core/bus.c                 |  42 +++
 hw/core/slotinfo.c            | 610 ++++++++++++++++++++++++++++++++++++++++++
 hw/core/sysbus.c              |   8 +
 hw/i2c/core.c                 |   7 +
 hw/ide/qdev.c                 |  27 ++
 hw/input/adb.c                |   7 +
 hw/ipack/ipack.c              |   7 +
 hw/isa/isa-bus.c              |   1 +
 hw/misc/auxbus.c              |   1 +
 hw/pci/pci.c                  | 120 ++++++++-
 hw/ppc/spapr_vio.c            |   1 +
 hw/s390x/css-bridge.c         |   2 +
 hw/s390x/event-facility.c     |   1 +
 hw/s390x/s390-pci-bus.c       |   7 +
 hw/scsi/scsi-bus.c            |   1 +
 hw/sd/core.c                  |   7 +
 hw/ssi/ssi.c                  |   7 +
 hw/usb/bus.c                  |  37 +++
 hw/usb/dev-smartcard-reader.c |   7 +
 hw/virtio/virtio-bus.c        |   1 +
 qapi/qapi-util.c              |  66 +++++
 qdev-monitor.c                | 109 ++++++++
 tests/test-qapi-util.c        |  53 ++++
 tests/test-slotinfo.c         | 398 +++++++++++++++++++++++++++
 hw/core/Makefile.objs         |   2 +
 tests/Makefile.include        |  14 +-
 tests/qmp-machine-info.py     | 300 +++++++++++++++++++++
 35 files changed, 2075 insertions(+), 15 deletions(-)
 create mode 100644 include/hw/qdev-slotinfo.h
 create mode 100644 hw/core/slotinfo.c
 create mode 100644 tests/test-slotinfo.c
 create mode 100755 tests/qmp-machine-info.py

-- 
2.9.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Qemu-devel] [libvirt] [RFC v4 00/13] qmp: query-device-slots command
Posted by no-reply@patchew.org 6 years, 8 months ago
Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: 20170814215748.5158-1-ehabkost@redhat.com
Subject: [libvirt] [RFC v4 00/13] qmp: query-device-slots command
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
74ba25a5a6 tests: Experimental query-device-slots test code
7f1ba7578a usb: enumerate_slots implementation
d4d30fcce1 pci: enumerate_slots implementation
c1efb02a42 pci: device-number & function properties
d846237b2b pci: pci_bus_has_pcie_upstream_port() function
303d9fc784 ide: enumerate_slots implementation
4141d83d87 qdev: Enumerate CPU slots on query-device-slots
d6b8967936 qdev core: generic enumerate_slots implementation
1ef51c3a5d query-device-slots: Collapse similar entries
0d84442287 qdev: Slot info helpers
3cfd385bcb qdev: Add BusClass::device_type field
6c98a4872f qapi: qobject_compare() helper
f695dbd533 qmp: Define query-device-slots command

=== OUTPUT BEGIN ===
Checking PATCH 1/13: qmp: Define query-device-slots command...
Checking PATCH 2/13: qapi: qobject_compare() helper...
ERROR: line over 90 characters
#140: FILE: qapi/qapi-util.c:140:
+        return strcmp(qstring_get_str(qobject_to_qstring(a)), qstring_get_str(qobject_to_qstring(b)));

ERROR: line over 90 characters
#142: FILE: qapi/qapi-util.c:142:
+        return (int)qbool_get_bool(qobject_to_qbool(a)) - (int)qbool_get_bool(qobject_to_qbool(b));

total: 2 errors, 0 warnings, 193 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 3/13: qdev: Add BusClass::device_type field...
Checking PATCH 4/13: qdev: Slot info helpers...
ERROR: do not use C99 // comments
#54: FILE: hw/core/slotinfo.c:13:
+//#define DEBUG_QOBJECTS

ERROR: __func__ should be used instead of gcc specific __FUNCTION__
#59: FILE: hw/core/slotinfo.c:18:
+#define DBG(args...) do { _DBG("%s: ", __FUNCTION__); \

ERROR: line over 90 characters
#559: FILE: hw/core/slotinfo.c:518:
+static DeviceSlotInfoList **slot_list_try_combine_slot(DeviceSlotInfoList **l, DeviceSlotInfo *slot)

ERROR: code indent should never use tabs
#709: FILE: include/hw/qdev-slotinfo.h:52:
+^I                       const char **opt_name);$

WARNING: line over 80 characters
#734: FILE: include/hw/qdev-slotinfo.h:77:
+static inline SlotOption *slot_find_opt(DeviceSlotInfo *slot, const char *option)

ERROR: code indent should never use tabs
#736: FILE: include/hw/qdev-slotinfo.h:79:
+^Ireturn slot_options_find_opt(slot->opts, option);$

ERROR: space prohibited after that open parenthesis '('
#848: FILE: tests/test-slotinfo.c:54:
+    g_assert_true( json_valuelist_contains(TEST_RANGE, "-100"));

ERROR: space prohibited after that open parenthesis '('
#849: FILE: tests/test-slotinfo.c:55:
+    g_assert_true( json_valuelist_contains(TEST_RANGE,  "-99"));

ERROR: space prohibited after that open parenthesis '('
#852: FILE: tests/test-slotinfo.c:58:
+    g_assert_true( json_valuelist_contains(TEST_RANGE,  "-51"));

ERROR: space prohibited after that open parenthesis '('
#853: FILE: tests/test-slotinfo.c:59:
+    g_assert_true( json_valuelist_contains(TEST_RANGE,  "-50"));

ERROR: space prohibited after that open parenthesis '('
#854: FILE: tests/test-slotinfo.c:60:
+    g_assert_true( json_valuelist_contains(TEST_RANGE,  "-49"));

ERROR: space prohibited after that open parenthesis '('
#857: FILE: tests/test-slotinfo.c:63:
+    g_assert_true( json_valuelist_contains(TEST_RANGE,   "-1"));

ERROR: space prohibited after that open parenthesis '('
#858: FILE: tests/test-slotinfo.c:64:
+    g_assert_true( json_valuelist_contains(TEST_RANGE,    "0"));

ERROR: space prohibited after that open parenthesis '('
#859: FILE: tests/test-slotinfo.c:65:
+    g_assert_true( json_valuelist_contains(TEST_RANGE,    "1"));

ERROR: space prohibited after that open parenthesis '('
#860: FILE: tests/test-slotinfo.c:66:
+    g_assert_true( json_valuelist_contains(TEST_RANGE,    "2"));

ERROR: space prohibited after that open parenthesis '('
#863: FILE: tests/test-slotinfo.c:69:
+    g_assert_true( json_valuelist_contains(TEST_RANGE,    "4"));

ERROR: space prohibited after that open parenthesis '('
#864: FILE: tests/test-slotinfo.c:70:
+    g_assert_true( json_valuelist_contains(TEST_RANGE,    "5"));

ERROR: space prohibited after that open parenthesis '('
#865: FILE: tests/test-slotinfo.c:71:
+    g_assert_true( json_valuelist_contains(TEST_RANGE,    "6"));

ERROR: space prohibited after that open parenthesis '('
#868: FILE: tests/test-slotinfo.c:74:
+    g_assert_true( json_valuelist_contains(TEST_RANGE,    "9"));

ERROR: space prohibited after that open parenthesis '('
#869: FILE: tests/test-slotinfo.c:75:
+    g_assert_true( json_valuelist_contains(TEST_RANGE,   "10"));

ERROR: space prohibited after that open parenthesis '('
#874: FILE: tests/test-slotinfo.c:80:
+    g_assert_true( json_valuelist_contains(TEST_RANGE,   "14"));

ERROR: space prohibited after that open parenthesis '('
#879: FILE: tests/test-slotinfo.c:85:
+    g_assert_true( json_valuelist_contains(TEST_RANGE,   "18"));

ERROR: space prohibited after that open parenthesis '('
#880: FILE: tests/test-slotinfo.c:86:
+    g_assert_true( json_valuelist_contains(TEST_RANGE,   "19"));

ERROR: space prohibited after that open parenthesis '('
#881: FILE: tests/test-slotinfo.c:87:
+    g_assert_true( json_valuelist_contains(TEST_RANGE,   "20"));

ERROR: space prohibited after that open parenthesis '('
#886: FILE: tests/test-slotinfo.c:92:
+    g_assert_true( json_valuelist_contains(TEST_RANGE,   "51"));

ERROR: space prohibited after that open parenthesis '('
#892: FILE: tests/test-slotinfo.c:98:
+    g_assert_true( json_valuelist_contains(TEST_RANGE,   "\"aaaa2\""));

ERROR: space prohibited after that open parenthesis '('
#899: FILE: tests/test-slotinfo.c:105:
+    g_assert_true( json_valuelist_contains(TEST_RANGE,   "\"jyz\""));

ERROR: space prohibited after that open parenthesis '('
#900: FILE: tests/test-slotinfo.c:106:
+    g_assert_true( json_valuelist_contains(TEST_RANGE,   "\"jyz2\""));

ERROR: space prohibited after that open parenthesis '('
#901: FILE: tests/test-slotinfo.c:107:
+    g_assert_true( json_valuelist_contains(TEST_RANGE,   "\"jyz3\""));

ERROR: space prohibited after that open parenthesis '('
#906: FILE: tests/test-slotinfo.c:112:
+    g_assert_true( json_valuelist_contains(TEST_RANGE,   "\"kkk\""));

ERROR: space prohibited after that open parenthesis '('
#912: FILE: tests/test-slotinfo.c:118:
+    g_assert_true( json_valuelist_contains(TEST_RANGE,   "[30, 31]"));

ERROR: do not use C99 // comments
#1003: FILE: tests/test-slotinfo.c:209:
+    //TODO: make this work:

total: 31 errors, 1 warnings, 1142 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 5/13: query-device-slots: Collapse similar entries...
Checking PATCH 6/13: qdev core: generic enumerate_slots implementation...
Checking PATCH 7/13: qdev: Enumerate CPU slots on query-device-slots...
Checking PATCH 8/13: ide: enumerate_slots implementation...
Checking PATCH 9/13: pci: pci_bus_has_pcie_upstream_port() function...
ERROR: space required before the open parenthesis '('
#35: FILE: hw/pci/pci.c:2612:
+    if(pci_bus_has_pcie_upstream_port(pci_dev->bus)) {

total: 1 errors, 0 warnings, 19 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 10/13: pci: device-number & function properties...
Checking PATCH 11/13: pci: enumerate_slots implementation...
ERROR: space required before the open parenthesis '('
#166: FILE: hw/pci/pci.c:167:
+    for(devnr = PCI_SLOT(pb->devfn_min); devnr < devnrs; devnr++) {

WARNING: line over 80 characters
#171: FILE: hw/pci/pci.c:172:
+            /*TODO: add info about accepting only bridges on extra PCI root buses */

total: 1 errors, 1 warnings, 68 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 12/13: usb: enumerate_slots implementation...
ERROR: line over 90 characters
#54: FILE: hw/usb/bus.c:29:
+static void usb_bus_enumerate_slot_list(DeviceSlotInfoList **r, USBBus *bus, USBPortList *l)

total: 1 errors, 0 warnings, 72 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 13/13: tests: Experimental query-device-slots test code...
WARNING: line over 80 characters
#152: FILE: tests/qmp-machine-info.py:132:
+        types = self.vm.command('qom-list-types', implements=implements, **kwargs)

WARNING: line over 80 characters
#160: FILE: tests/qmp-machine-info.py:140:
+        no_user_devs = set([d['name'] for d in infoQDM(self.vm, ) if d['no-user']])

WARNING: line over 80 characters
#170: FILE: tests/qmp-machine-info.py:150:
+        """Check if the bus identified by the slot matches the information returned

ERROR: line over 90 characters
#181: FILE: tests/qmp-machine-info.py:161:
+        ## but the bus _name_ (accepted by qbus_find()) does not necessarily matches the bus _path_

WARNING: line over 80 characters
#207: FILE: tests/qmp-machine-info.py:187:
+            for st in self.vm.command('qom-list-types', implements=t, abstract=False):

WARNING: line over 80 characters
#208: FILE: tests/qmp-machine-info.py:188:
+                dprops = self.vm.command('device-list-properties', typename=st['name'])

ERROR: line over 90 characters
#215: FILE: tests/qmp-machine-info.py:195:
+            if slot.has_key('max-devices') and len(slot['devices']) >= slot['max-devices']:

ERROR: line over 90 characters
#240: FILE: tests/qmp-machine-info.py:220:
+                self.assertTrue(any(self.typeImplements(dtype, t) for t in slot['device-types']))

ERROR: line over 90 characters
#243: FILE: tests/qmp-machine-info.py:223:
+                self.assertTrue(len(self.getUserCreatableSubtypes(dt)) > 0, "There's no user-creatable subtype of %s" % (dt))

WARNING: line over 80 characters
#248: FILE: tests/qmp-machine-info.py:228:
+                all_counts = [len(ValuesIterator(p['values'])) for p in slot['opts']]

WARNING: line over 80 characters
#266: FILE: tests/qmp-machine-info.py:246:
+        """Dynamically add a testMachine_<arch>_<name>_<machine> method to the class"""

WARNING: line over 80 characters
#273: FILE: tests/qmp-machine-info.py:253:
+        method_name = 'test_%s_%s_%s' % (method_name, machine['arch'], machine_name)

WARNING: line over 80 characters
#286: FILE: tests/qmp-machine-info.py:266:
+        vm = qtest.QEMUQtestMachine(binary=binary, args=['-S', '-machine', 'none'])

WARNING: line over 80 characters
#305: FILE: tests/qmp-machine-info.py:285:
+        method_names = unittest.loader.getTestCaseNames(klass, prefix='machineTest')

total: 4 errors, 10 warnings, 300 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
Re: [Qemu-devel] [RFC v4 00/13] qmp: query-device-slots command
Posted by Eric Blake 6 years, 8 months ago
On 08/14/2017 04:57 PM, Eduardo Habkost wrote:
> Changelog
> ---------
> 
> Changes v3 -> v4:
> * New compact representation of slot sets.
> * New generic code to automatically merge similar slots
>   into a single entry in the command output while keeping
>   implementations of the method simpler.
> * Example implementation of IDE and USB bus enumeration

> 
> Slot sets are represented by a list of option names and sets of
> possible values for each of those options.  The command uses a
> compact representation for the set of valid values for an option.
> For example, the following set of 5 PCI functions:
> 
>       bus: pcie.0
>       device-number: 31
>       functions: 1,4,5,6,7
> 
> would be represented in the JSON data as:
> 
>   {"available":false,"count":5,
>    "device-types":["pci-device"],"hotpluggable":false,
>    "opts":[
>       {"option":"function","values":[1,[4,7]]},

A list (and not just a single-type list, but a list that mixes scalar
and sublist),

>       {"option":"device-number","values":31},

vs. a scalar.  Why not a one-element array?

>       {"option":"bus","values":"pcie.0"}],
>    "opts-complete":true}
> 
> I planned to use QAPI alternates to model/document that in the
> schema, but it would require implementing a few missing features
> in QAPI alternate support.

Yeah, I can see how existing QAPI alternates do not yet support arrays,
which becomes important to your representation.  Do you need help
getting the QAPI generator improved to support a particular feature that
you found to be lacking?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [RFC v4 00/13] qmp: query-device-slots command
Posted by Eduardo Habkost 6 years, 8 months ago
On Tue, Aug 15, 2017 at 01:57:50PM -0500, Eric Blake wrote:
> On 08/14/2017 04:57 PM, Eduardo Habkost wrote:
> > Changelog
> > ---------
> > 
> > Changes v3 -> v4:
> > * New compact representation of slot sets.
> > * New generic code to automatically merge similar slots
> >   into a single entry in the command output while keeping
> >   implementations of the method simpler.
> > * Example implementation of IDE and USB bus enumeration
> 
> > 
> > Slot sets are represented by a list of option names and sets of
> > possible values for each of those options.  The command uses a
> > compact representation for the set of valid values for an option.
> > For example, the following set of 5 PCI functions:
> > 
> >       bus: pcie.0
> >       device-number: 31
> >       functions: 1,4,5,6,7
> > 
> > would be represented in the JSON data as:
> > 
> >   {"available":false,"count":5,
> >    "device-types":["pci-device"],"hotpluggable":false,
> >    "opts":[
> >       {"option":"function","values":[1,[4,7]]},
> 
> A list (and not just a single-type list, but a list that mixes scalar
> and sublist),
> 
> >       {"option":"device-number","values":31},
> 
> vs. a scalar.  Why not a one-element array?

It was just to keep the representation as compact as possible, in
the common case of single-value sets.  Probably we can drop that
feature as it saves only 2 bytes in the JSON representation.

> 
> >       {"option":"bus","values":"pcie.0"}],
> >    "opts-complete":true}
> > 
> > I planned to use QAPI alternates to model/document that in the
> > schema, but it would require implementing a few missing features
> > in QAPI alternate support.
> 
> Yeah, I can see how existing QAPI alternates do not yet support arrays,
> which becomes important to your representation.  Do you need help
> getting the QAPI generator improved to support a particular feature that
> you found to be lacking?

I think the lack of support for lists on alternates was the main
obstacle.

Probably we would also need to remove the restriction against
alternates with ambiguous string representations, to allow a
list/number/string/bool alternate to be defined.

Being able to set constraints on the number of elements of a list
would be nice to have, but not required.

-- 
Eduardo