* [Qemu-devel] [RFC v4 00/13] qmp: query-device-slots command
@ 2017-08-14 21:57 Eduardo Habkost
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 01/13] qmp: Define " Eduardo Habkost
` (14 more replies)
0 siblings, 15 replies; 26+ messages in thread
From: Eduardo Habkost @ 2017-08-14 21:57 UTC (permalink / raw)
To: Eric Blake, qemu-devel, Markus Armbruster, Michael S. Tsirkin,
Marcel Apfelbaum, Laine Stump
Cc: libvir-list, Igor Mammedov
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
^ permalink raw reply [flat|nested] 26+ messages in thread
* [Qemu-devel] [RFC v4 01/13] qmp: Define query-device-slots command
2017-08-14 21:57 [Qemu-devel] [RFC v4 00/13] qmp: query-device-slots command Eduardo Habkost
@ 2017-08-14 21:57 ` Eduardo Habkost
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 02/13] qapi: qobject_compare() helper Eduardo Habkost
` (13 subsequent siblings)
14 siblings, 0 replies; 26+ messages in thread
From: Eduardo Habkost @ 2017-08-14 21:57 UTC (permalink / raw)
To: Eric Blake, qemu-devel, Markus Armbruster, Michael S. Tsirkin,
Marcel Apfelbaum, Laine Stump
Cc: libvir-list, Igor Mammedov
This adds a new command to QMP: query-device-slots. It will allow
management software to query possible slots where devices can be
plugged.
Slot sets are represented by a list of option names and sets of
possible values for each of those options. We use a compact
format for set of valid values, to keep JSON response size
reasonable. The format is documented at the SlotOption struct.
In the future, we may use QAPI alternates to define and document
the exact data format more strictly.
Actual implementations of BusClass::enumerate_slots will be added
by other patches.
Cc: Marcel Apfelbaum <marcel@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: libvir-list@redhat.com,
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Laine Stump <laine@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v3 -> v4:
* Use a more compact format for the "slot sets"
* Renamed DeviceSlotInfo.props to DeviceSlotInfo.opts
* Replaced DeviceSlotInfo.incomplete with
DeviceSlotInfo.opts-complete
* Now DeviceSlotInfo.available will be true even if hotplug is
unavailable, to indicate the slot would be available using
-device.
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
---
qapi-schema.json | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++
include/hw/qdev-core.h | 2 ++
qdev-monitor.c | 38 +++++++++++++++++++++
3 files changed, 129 insertions(+)
diff --git a/qapi-schema.json b/qapi-schema.json
index 802ea53..6a9329e 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4325,6 +4325,95 @@
##
{ 'command': 'closefd', 'data': {'fdname': 'str'} }
+
+##
+# @SlotOption:
+#
+# A option to be used when plugging a device to a slot.
+#
+# @option: Option name.
+#
+# @values: Set of valid option values.
+#
+# @values follow a compact format to represent a set of values:
+# - If @values is a string, bool, or number, it representes a
+# single-element set containing that value. In other words,
+# @values is the only valid value for the option.
+# - If @values is a list, each element E on the list represents
+# one or more possible values for the option:
+# - If E is a string, bool, or number, it indicates that E
+# is a valid value for the option.
+# - If E is a one-element list [V], it means V is a valid value
+# for the option.
+# - If E is a two-element list [A, B] where A and B are
+# numbers, it means any number X, A <= X <= B is a valid
+# value for the option.
+#
+# TODO: @values could be represented by nested 'alternate' types,
+# for more consistent documentation and automatic
+# validation/parsing, but the following limitations on
+# alternates prevent that today:
+# - alternate branches can't be lists
+# - alternate string members conflict with scalar values
+# - alternate types should have 2 or more branches
+##
+{ 'struct': 'SlotOption',
+ 'data': { 'option': 'str', 'values': 'any' } }
+
+##
+# @DeviceSlotInfo:
+#
+# Information on a set of slots where devices can be plugged.
+#
+# @device-types: List of device types accepted by the slots.
+# Any device plugged to the slot should implement
+# one of the accepted device types.
+#
+# @device: QOM path of device plugged to the slot, if any.
+#
+# @available: If false, the slot is not available for plugging any device.
+# This value can change at runtime if condition changes.
+#
+# @hotpluggable: If true, the slot accepts hotplugged devices. If false,
+# device_add won't work on the slot even if @available=true.
+#
+# @count: Number of slots represented by this slot set.
+#
+# @opts-complete: If true, all options required to plug devices
+# to slots in this set are present in @opts. If
+# false, slot information is incomplete in this
+# QEMU version and additional options may be
+# required to plug devices on those slots.
+#
+# @opts: Information on the arguments that should be provided to
+# @device_add if plugging a device to this slot.
+#
+# Incomplete Slot Sets
+# --------------------
+#
+# Slot sets with @opts-complete=false represent a bus that
+# doesn't support slot enumeration yet. Slots of this type should
+# be replaced by more detailed slot sets in future QEMU versions.
+#
+# If @count is omitted, the entry may or may not represent more
+# than one slots. Future QEMU versions should include additional
+# code to set @count on those entries.
+##
+{ 'struct': 'DeviceSlotInfo',
+ 'data': { 'device-types': [ 'str' ], '*device': 'str',
+ 'available': 'bool', 'hotpluggable': 'bool',
+ '*count': 'int', 'opts-complete': 'bool',
+ 'opts': [ 'SlotOption' ] } }
+
+##
+# @query-device-slots:
+#
+# Return the list of possible slots for plugging devices using
+# @device_add.
+##
+{ 'command': 'query-device-slots',
+ 'returns': [ 'DeviceSlotInfo' ] }
+
##
# @MachineInfo:
#
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index ae31728..0111350 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -191,6 +191,8 @@ struct BusClass {
void (*reset)(BusState *bus);
BusRealize realize;
BusUnrealize unrealize;
+ /*TODO: write doc */
+ DeviceSlotInfoList *(*enumerate_slots)(BusState *bus);
/* maximum devices allowed on the bus, 0: no limit. */
int max_dev;
diff --git a/qdev-monitor.c b/qdev-monitor.c
index 8fd6df9..785f4af 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -30,6 +30,8 @@
#include "qemu/help_option.h"
#include "sysemu/block-backend.h"
#include "migration/misc.h"
+#include "qapi/qobject-output-visitor.h"
+#include "hw/boards.h"
/*
* Aliases were a bad idea from the start. Let's keep them
@@ -638,6 +640,42 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)
return dev;
}
+typedef struct SlotListState {
+ DeviceSlotInfoList *result;
+ DeviceSlotInfoList **next;
+} SlotListState;
+
+static void append_slots(SlotListState *s, DeviceSlotInfoList *l)
+{
+ *s->next = l;
+ for (; l; l = l->next) {
+ s->next = &l->next;
+ }
+}
+
+static int enumerate_bus(Object *obj, void *opaque)
+{
+ SlotListState *s = opaque;
+
+ if (object_dynamic_cast(obj, TYPE_BUS)) {
+ BusState *bus = BUS(obj);
+ BusClass *bc = BUS_GET_CLASS(bus);
+
+ if (bc->enumerate_slots) {
+ append_slots(s, bc->enumerate_slots(bus));
+ }
+ }
+ return 0;
+}
+
+DeviceSlotInfoList *qmp_query_device_slots(Error **errp)
+{
+ SlotListState s = { .next = &s.result };
+
+ object_child_foreach_recursive(qdev_get_machine(), enumerate_bus, &s);
+ return s.result;
+}
+
#define qdev_printf(fmt, ...) monitor_printf(mon, "%*s" fmt, indent, "", ## __VA_ARGS__)
static void qbus_print(Monitor *mon, BusState *bus, int indent);
--
2.9.4
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [RFC v4 02/13] qapi: qobject_compare() helper
2017-08-14 21:57 [Qemu-devel] [RFC v4 00/13] qmp: query-device-slots command Eduardo Habkost
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 01/13] qmp: Define " Eduardo Habkost
@ 2017-08-14 21:57 ` Eduardo Habkost
2017-08-15 16:16 ` Eric Blake
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 03/13] qdev: Add BusClass::device_type field Eduardo Habkost
` (12 subsequent siblings)
14 siblings, 1 reply; 26+ messages in thread
From: Eduardo Habkost @ 2017-08-14 21:57 UTC (permalink / raw)
To: Eric Blake, qemu-devel, Markus Armbruster, Michael S. Tsirkin,
Marcel Apfelbaum, Laine Stump
The helper function will be useful when writing support code to
deal with device slot information.
TODO: documentation is incomplete and unclear, needs to be
improved.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
include/qapi/util.h | 39 +++++++++++++++++++++++++++++
qapi/qapi-util.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++
tests/test-qapi-util.c | 53 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 158 insertions(+)
diff --git a/include/qapi/util.h b/include/qapi/util.h
index 7436ed8..d4562c4 100644
--- a/include/qapi/util.h
+++ b/include/qapi/util.h
@@ -11,9 +11,48 @@
#ifndef QAPI_UTIL_H
#define QAPI_UTIL_H
+#include "qapi/qmp/qobject.h"
+#include "qapi-types.h"
+
int qapi_enum_parse(const char * const lookup[], const char *buf,
int max, int def, Error **errp);
int parse_qapi_name(const char *name, bool complete);
+/**
+ * qobject_compare:
+ *
+ * Compare the value of @a and @b.
+ *
+ * If @a and @b have the same type and the same value (see list
+ * of supported types below), return 0.
+ *
+ * If @a and @b are both strings, return strcmp(a, b).
+ *
+ * If @a and @b are numbers, return a negative value if a < b,
+ * and a positive value if a > b.
+ *
+ * Otherwise (if @a and @b are not the same, have different types,
+ * are of an unsupported type, or are different), return a non-zero value.
+ *
+ * Note that this function doesn't support some types, and may
+ * return false if the types are unsupported, or if the types don't
+ * match exactly.
+ *
+ * Supported types:
+ * - QTYPE_QNULL
+ * - QTYPE_QSTRING
+ * - QTYPE_QBOOL
+ * - QTYPE_QNUM (integers only)
+ * - QTYPE_QLIST
+ *
+ * Unsupported (always return false):
+ * - QTYPE_QNUM (non-integer values)
+ * - QTYPE_QDICT
+ *
+ * TODO: rewrite documentation to be clearer.
+ * TODO: support non-integer QTYPE_NUM values and QTYPE_QDICT.
+ */
+int qobject_compare(QObject *a, QObject *b);
+
#endif
diff --git a/qapi/qapi-util.c b/qapi/qapi-util.c
index 46eda7d..67c5e82 100644
--- a/qapi/qapi-util.c
+++ b/qapi/qapi-util.c
@@ -13,6 +13,9 @@
#include "qemu/osdep.h"
#include "qapi/error.h"
#include "qemu-common.h"
+#include "qapi/qmp/qstring.h"
+#include "qapi/qmp/qnum.h"
+#include "qapi/qmp/qbool.h"
#include "qapi/util.h"
int qapi_enum_parse(const char * const lookup[], const char *buf,
@@ -80,3 +83,66 @@ int parse_qapi_name(const char *str, bool complete)
}
return p - str;
}
+
+static int qnum_compare(QNum *a, QNum *b)
+{
+ int64_t ia, ib;
+ bool va = qnum_get_try_int(a, &ia);
+ bool vb = qnum_get_try_int(b, &ib);
+
+ if (va && vb) {
+ return (ia < ib) ? -1 : (ia > ib) ? 1 : 0;
+ }
+
+ /*TODO: uint, double */
+ return -1;
+}
+
+static int qlist_compare(QList *a, QList *b)
+{
+ const QListEntry *ea, *eb;
+
+ for (ea = qlist_first(a), eb = qlist_first(b);
+ ea && eb;
+ ea = qlist_next(ea), eb = qlist_next(eb)) {
+ QObject *va = qlist_entry_obj(ea);
+ QObject *vb = qlist_entry_obj(eb);
+ int c = qobject_compare(va, vb);
+ if (c) {
+ return c;
+ }
+ }
+
+ if (eb) {
+ return -1;
+ } else if (ea) {
+ return 1;
+ } else {
+ return 0;
+ }
+}
+
+int qobject_compare(QObject *a, QObject *b)
+{
+ QType ta = qobject_type(a);
+ QType tb = qobject_type(b);
+
+ if (ta != tb) {
+ return -1;
+ }
+
+ switch (ta) {
+ case QTYPE_QNULL:
+ return true;
+ case QTYPE_QNUM:
+ return qnum_compare(qobject_to_qnum(a), qobject_to_qnum(b));
+ case QTYPE_QSTRING:
+ return strcmp(qstring_get_str(qobject_to_qstring(a)), qstring_get_str(qobject_to_qstring(b)));
+ case QTYPE_QBOOL:
+ return (int)qbool_get_bool(qobject_to_qbool(a)) - (int)qbool_get_bool(qobject_to_qbool(b));
+ case QTYPE_QLIST:
+ return qlist_compare(qobject_to_qlist(a), qobject_to_qlist(b));
+ default:
+ return -1;
+ }
+}
diff --git a/tests/test-qapi-util.c b/tests/test-qapi-util.c
index e869757..c0cf46c 100644
--- a/tests/test-qapi-util.c
+++ b/tests/test-qapi-util.c
@@ -13,6 +13,10 @@
#include "qemu/osdep.h"
#include "qapi/error.h"
#include "qapi/util.h"
+#include "qapi/qmp/qnum.h"
+#include "qapi/qmp/qstring.h"
+#include "qapi/qmp/qlist.h"
+#include "qapi/qmp/qjson.h"
#include "test-qapi-types.h"
static void test_qapi_enum_parse(void)
@@ -75,11 +79,60 @@ static void test_parse_qapi_name(void)
g_assert(ret == -1);
}
+static void test_qobject_compare(void)
+{
+ QString *a1 = qstring_from_str("abc");
+ QString *a2 = qstring_from_str("abc");
+ QString *b = qstring_from_str("bcd");
+ QNum *i1 = qnum_from_int(100);
+ QNum *i2 = qnum_from_int(100);
+ QNum *j = qnum_from_int(200);
+ QList *l1 = qlist_new();
+ QList *l2 = qlist_new();
+ QList *m = qlist_new();
+
+ qlist_append_int(l1, 100);
+ qlist_append_int(l1, 200);
+ qlist_append_int(l2, 100);
+ qlist_append_int(l2, 200);
+
+ qlist_append_int(m, 100);
+ qlist_append_int(m, 300);
+
+ g_assert_cmpint(qobject_compare(QOBJECT(a1), QOBJECT(a2)), ==, 0);
+ g_assert_cmpint(qobject_compare(QOBJECT(i1), QOBJECT(i2)), ==, 0);
+ g_assert_cmpint(qobject_compare(QOBJECT(l1), QOBJECT(l2)), ==, 0);
+
+ g_assert_cmpint(qobject_compare(QOBJECT(a1), QOBJECT(b)), <, 0);
+ g_assert_cmpint(qobject_compare(QOBJECT(b), QOBJECT(a1)), >, 0);
+
+ g_assert_cmpint(qobject_compare(QOBJECT(i1), QOBJECT(j)), <, 0);
+ g_assert_cmpint(qobject_compare(QOBJECT(j), QOBJECT(i1)), >, 0);
+
+ g_assert_cmpint(qobject_compare(QOBJECT(l1), QOBJECT(m)), <, 0);
+ g_assert_cmpint(qobject_compare(QOBJECT(m), QOBJECT(l1)), >, 0);
+
+ g_assert_cmpint(qobject_compare(QOBJECT(a1), QOBJECT(i1)), !=, 0);
+ g_assert_cmpint(qobject_compare(QOBJECT(a1), QOBJECT(l1)), !=, 0);
+ g_assert_cmpint(qobject_compare(QOBJECT(l1), QOBJECT(i1)), !=, 0);
+
+ QDECREF(a1);
+ QDECREF(a2);
+ QDECREF(b);
+ QDECREF(i1);
+ QDECREF(i2);
+ QDECREF(j);
+ QDECREF(l1);
+ QDECREF(l2);
+ QDECREF(m);
+}
+
int main(int argc, char *argv[])
{
g_test_init(&argc, &argv, NULL);
g_test_add_func("/qapi/util/qapi_enum_parse", test_qapi_enum_parse);
g_test_add_func("/qapi/util/parse_qapi_name", test_parse_qapi_name);
+ g_test_add_func("/qapi/util/qobject_compare", test_qobject_compare);
g_test_run();
return 0;
}
--
2.9.4
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [RFC v4 03/13] qdev: Add BusClass::device_type field
2017-08-14 21:57 [Qemu-devel] [RFC v4 00/13] qmp: query-device-slots command Eduardo Habkost
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 01/13] qmp: Define " Eduardo Habkost
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 02/13] qapi: qobject_compare() helper Eduardo Habkost
@ 2017-08-14 21:57 ` Eduardo Habkost
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 04/13] qdev: Slot info helpers Eduardo Habkost
` (11 subsequent siblings)
14 siblings, 0 replies; 26+ messages in thread
From: Eduardo Habkost @ 2017-08-14 21:57 UTC (permalink / raw)
To: Eric Blake, qemu-devel, Markus Armbruster, Michael S. Tsirkin,
Marcel Apfelbaum, Laine Stump
The field will be useful when building device slot information
for buses.
TODO: Confirm if all bus types were handled by the patch.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
include/hw/qdev-core.h | 3 +++
hw/audio/intel-hda.c | 7 +++++++
hw/block/fdc.c | 15 +++++++++++----
hw/char/virtio-serial-bus.c | 1 +
hw/core/sysbus.c | 1 +
hw/i2c/core.c | 7 +++++++
hw/ide/qdev.c | 1 +
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 | 1 +
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 | 1 +
hw/usb/dev-smartcard-reader.c | 7 +++++++
hw/virtio/virtio-bus.c | 1 +
22 files changed, 83 insertions(+), 4 deletions(-)
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 0111350..8aa1455 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -198,6 +198,9 @@ struct BusClass {
int max_dev;
/* number of automatically allocated bus ids (e.g. ide.0) */
int automatic_ids;
+
+ /* Device type accepted by bus */
+ const char *device_type;
};
typedef struct BusChild {
diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
index 06acc98..13b6904 100644
--- a/hw/audio/intel-hda.c
+++ b/hw/audio/intel-hda.c
@@ -36,9 +36,16 @@ static Property hda_props[] = {
DEFINE_PROP_END_OF_LIST()
};
+static void hda_codec_bus_class_init(ObjectClass *oc, void *opaque)
+{
+ BusClass *bc = BUS_CLASS(oc);
+ bc->device_type = TYPE_HDA_CODEC_DEVICE;
+}
+
static const TypeInfo hda_codec_bus_info = {
.name = TYPE_HDA_BUS,
.parent = TYPE_BUS,
+ .class_init = hda_codec_bus_class_init,
.instance_size = sizeof(HDACodecBus),
};
diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 4011290..976500f 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -60,6 +60,10 @@
#define TYPE_FLOPPY_BUS "floppy-bus"
#define FLOPPY_BUS(obj) OBJECT_CHECK(FloppyBus, (obj), TYPE_FLOPPY_BUS)
+#define TYPE_FLOPPY_DRIVE "floppy"
+#define FLOPPY_DRIVE(obj) \
+ OBJECT_CHECK(FloppyDrive, (obj), TYPE_FLOPPY_DRIVE)
+
typedef struct FDCtrl FDCtrl;
typedef struct FDrive FDrive;
static FDrive *get_drv(FDCtrl *fdctrl, int unit);
@@ -69,9 +73,16 @@ typedef struct FloppyBus {
FDCtrl *fdc;
} FloppyBus;
+static void floppy_bus_class_init(ObjectClass *oc, void *opaque)
+{
+ BusClass *bc = BUS_CLASS(oc);
+ bc->device_type = TYPE_FLOPPY_DRIVE;
+}
+
static const TypeInfo floppy_bus_info = {
.name = TYPE_FLOPPY_BUS,
.parent = TYPE_BUS,
+ .class_init = floppy_bus_class_init,
.instance_size = sizeof(FloppyBus),
};
@@ -497,10 +508,6 @@ static const BlockDevOps fd_block_ops = {
};
-#define TYPE_FLOPPY_DRIVE "floppy"
-#define FLOPPY_DRIVE(obj) \
- OBJECT_CHECK(FloppyDrive, (obj), TYPE_FLOPPY_DRIVE)
-
typedef struct FloppyDrive {
DeviceState qdev;
uint32_t unit;
diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index f5bc173..28342af 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -837,6 +837,7 @@ static void virtser_bus_class_init(ObjectClass *klass, void *data)
{
BusClass *k = BUS_CLASS(klass);
k->print_dev = virtser_bus_dev_print;
+ k->device_type = TYPE_VIRTIO_SERIAL_PORT;
}
static const TypeInfo virtser_bus_info = {
diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
index 5d0887f..9259931 100644
--- a/hw/core/sysbus.c
+++ b/hw/core/sysbus.c
@@ -76,6 +76,7 @@ static void system_bus_class_init(ObjectClass *klass, void *data)
k->print_dev = sysbus_dev_print;
k->get_fw_dev_path = sysbus_get_fw_dev_path;
+ k->device_type = TYPE_SYS_BUS_DEVICE;
}
static const TypeInfo system_bus_info = {
diff --git a/hw/i2c/core.c b/hw/i2c/core.c
index 2c1234c..fb5aaad 100644
--- a/hw/i2c/core.c
+++ b/hw/i2c/core.c
@@ -35,9 +35,16 @@ static Property i2c_props[] = {
#define TYPE_I2C_BUS "i2c-bus"
#define I2C_BUS(obj) OBJECT_CHECK(I2CBus, (obj), TYPE_I2C_BUS)
+static void i2c_bus_class_init(ObjectClass *oc, void *opaque)
+{
+ BusClass *bc = BUS_CLASS(oc);
+ bc->device_type = TYPE_I2C_SLAVE;
+}
+
static const TypeInfo i2c_bus_info = {
.name = TYPE_I2C_BUS,
.parent = TYPE_BUS,
+ .class_init = i2c_bus_class_init,
.instance_size = sizeof(I2CBus),
};
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index cc2f5bd..f17da1f 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -44,6 +44,7 @@ static void ide_bus_class_init(ObjectClass *klass, void *data)
k->get_fw_dev_path = idebus_get_fw_dev_path;
k->unrealize = idebus_unrealize;
+ k->device_type = TYPE_IDE_DEVICE;
}
static void idebus_unrealize(BusState *bus, Error **errp)
diff --git a/hw/input/adb.c b/hw/input/adb.c
index fcca3a8..2fc292c 100644
--- a/hw/input/adb.c
+++ b/hw/input/adb.c
@@ -121,9 +121,16 @@ int adb_poll(ADBBusState *s, uint8_t *obuf, uint16_t poll_mask)
return olen;
}
+static void adb_bus_class_init(ObjectClass *oc, void *opaque)
+{
+ BusClass *bc = BUS_CLASS(oc);
+ bc->device_type = TYPE_ADB_DEVICE;
+}
+
static const TypeInfo adb_bus_type_info = {
.name = TYPE_ADB_BUS,
.parent = TYPE_BUS,
+ .class_init = adb_bus_class_init,
.instance_size = sizeof(ADBBusState),
};
diff --git a/hw/ipack/ipack.c b/hw/ipack/ipack.c
index 6021e6d..f22c504 100644
--- a/hw/ipack/ipack.c
+++ b/hw/ipack/ipack.c
@@ -106,9 +106,16 @@ static const TypeInfo ipack_device_info = {
.abstract = true,
};
+static void ipack_bus_class_init(ObjectClass *oc, void *opaque)
+{
+ BusClass *bc = BUS_CLASS(oc);
+ bc->device_type = TYPE_IPACK_DEVICE;
+}
+
static const TypeInfo ipack_bus_info = {
.name = TYPE_IPACK_BUS,
.parent = TYPE_BUS,
+ .class_init = ipack_bus_class_init,
.instance_size = sizeof(IPackBus),
};
diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
index 348e0ea..45349c6 100644
--- a/hw/isa/isa-bus.c
+++ b/hw/isa/isa-bus.c
@@ -36,6 +36,7 @@ static void isa_bus_class_init(ObjectClass *klass, void *data)
k->print_dev = isabus_dev_print;
k->get_fw_dev_path = isabus_get_fw_dev_path;
+ k->device_type = TYPE_ISA_DEVICE;
}
static const TypeInfo isa_dma_info = {
diff --git a/hw/misc/auxbus.c b/hw/misc/auxbus.c
index 8a90ddd..7f4c87f 100644
--- a/hw/misc/auxbus.c
+++ b/hw/misc/auxbus.c
@@ -57,6 +57,7 @@ static void aux_bus_class_init(ObjectClass *klass, void *data)
* in monitor.
*/
k->print_dev = aux_slave_dev_print;
+ k->device_type = TYPE_AUX_SLAVE;
}
AUXBus *aux_init_bus(DeviceState *parent, const char *name)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 258fbe5..31b4d69 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -154,6 +154,7 @@ static void pci_bus_class_init(ObjectClass *klass, void *data)
k->realize = pci_bus_realize;
k->unrealize = pci_bus_unrealize;
k->reset = pcibus_reset;
+ k->device_type = TYPE_PCI_DEVICE;
pbc->is_root = pcibus_is_root;
pbc->bus_num = pcibus_num;
diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
index ea3bc8b..d07dcaa 100644
--- a/hw/ppc/spapr_vio.c
+++ b/hw/ppc/spapr_vio.c
@@ -61,6 +61,7 @@ static void spapr_vio_bus_class_init(ObjectClass *klass, void *data)
k->get_dev_path = spapr_vio_get_dev_name;
k->get_fw_dev_path = spapr_vio_get_dev_name;
+ k->device_type = TYPE_VIO_SPAPR_DEVICE;
}
static const TypeInfo spapr_vio_bus_info = {
diff --git a/hw/s390x/css-bridge.c b/hw/s390x/css-bridge.c
index c4a9735..c442c23 100644
--- a/hw/s390x/css-bridge.c
+++ b/hw/s390x/css-bridge.c
@@ -17,6 +17,7 @@
#include "hw/s390x/css.h"
#include "ccw-device.h"
#include "hw/s390x/css-bridge.h"
+#include "hw/s390x/virtio-ccw.h"
#include "cpu.h"
/*
@@ -82,6 +83,7 @@ static void virtual_css_bus_class_init(ObjectClass *klass, void *data)
k->reset = virtual_css_bus_reset;
k->get_dev_path = virtual_css_bus_get_dev_path;
+ k->device_type = TYPE_VIRTIO_CCW_DEVICE;
}
static const TypeInfo virtual_css_bus_info = {
diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
index 34b2faf..f2562ce 100644
--- a/hw/s390x/event-facility.c
+++ b/hw/s390x/event-facility.c
@@ -307,6 +307,7 @@ static void sclp_events_bus_class_init(ObjectClass *klass, void *data)
BusClass *bc = BUS_CLASS(klass);
bc->realize = sclp_events_bus_realize;
+ bc->device_type = TYPE_SCLP_EVENT;
}
static const TypeInfo sclp_events_bus_info = {
diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index 61cfd21..0d71d7e 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -888,10 +888,17 @@ static const TypeInfo s390_pcihost_info = {
}
};
+static void s390_pcibus_class_init(ObjectClass *oc, void *opaque)
+{
+ BusClass *bc = BUS_CLASS(oc);
+ bc->device_type = TYPE_S390_PCI_DEVICE;
+}
+
static const TypeInfo s390_pcibus_info = {
.name = TYPE_S390_PCI_BUS,
.parent = TYPE_BUS,
.instance_size = sizeof(S390PCIBus),
+ .class_init = s390_pcibus_class_init,
};
static uint16_t s390_pci_generate_uid(S390pciState *s)
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index e364410..181428f7 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -31,6 +31,7 @@ static void scsi_bus_class_init(ObjectClass *klass, void *data)
k->get_dev_path = scsibus_get_dev_path;
k->get_fw_dev_path = scsibus_get_fw_dev_path;
+ k->device_type = TYPE_SCSI_DEVICE;
hc->unplug = qdev_simple_device_unplug_cb;
}
diff --git a/hw/sd/core.c b/hw/sd/core.c
index 295dc44..5bfe404 100644
--- a/hw/sd/core.c
+++ b/hw/sd/core.c
@@ -158,11 +158,18 @@ void sdbus_reparent_card(SDBus *from, SDBus *to)
sdbus_set_readonly(to, readonly);
}
+static void sd_bus_class_init(ObjectClass *oc, void *opaque)
+{
+ BusClass *bc = BUS_CLASS(oc);
+ bc->device_type = TYPE_SD_CARD;
+}
+
static const TypeInfo sd_bus_info = {
.name = TYPE_SD_BUS,
.parent = TYPE_BUS,
.instance_size = sizeof(SDBus),
.class_size = sizeof(SDBusClass),
+ .class_init = sd_bus_class_init,
};
static void sd_bus_register_types(void)
diff --git a/hw/ssi/ssi.c b/hw/ssi/ssi.c
index 7eaaf56..c48920d 100644
--- a/hw/ssi/ssi.c
+++ b/hw/ssi/ssi.c
@@ -22,10 +22,17 @@ struct SSIBus {
#define TYPE_SSI_BUS "SSI"
#define SSI_BUS(obj) OBJECT_CHECK(SSIBus, (obj), TYPE_SSI_BUS)
+static void ssi_bus_class_init(ObjectClass *oc, void *opaque)
+{
+ BusClass *bc = BUS_CLASS(oc);
+ bc->device_type = TYPE_SSI_SLAVE;
+}
+
static const TypeInfo ssi_bus_info = {
.name = TYPE_SSI_BUS,
.parent = TYPE_BUS,
.instance_size = sizeof(SSIBus),
+ .class_init = ssi_bus_class_init,
};
static void ssi_cs_default(void *opaque, int n, int level)
diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index d910f84..42f85a7 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -33,6 +33,7 @@ static void usb_bus_class_init(ObjectClass *klass, void *data)
k->print_dev = usb_bus_dev_print;
k->get_dev_path = usb_get_dev_path;
k->get_fw_dev_path = usb_get_fw_dev_path;
+ k->device_type = TYPE_USB_DEVICE;
hc->unplug = qdev_simple_device_unplug_cb;
}
diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
index bef1f03..01aa804 100644
--- a/hw/usb/dev-smartcard-reader.c
+++ b/hw/usb/dev-smartcard-reader.c
@@ -1195,10 +1195,17 @@ static Property ccid_props[] = {
#define TYPE_CCID_BUS "ccid-bus"
#define CCID_BUS(obj) OBJECT_CHECK(CCIDBus, (obj), TYPE_CCID_BUS)
+static void ccid_bus_class_init(ObjectClass *oc, void *opaque)
+{
+ BusClass *bc = BUS_CLASS(oc);
+ bc->device_type = TYPE_CCID_CARD;
+}
+
static const TypeInfo ccid_bus_info = {
.name = TYPE_CCID_BUS,
.parent = TYPE_BUS,
.instance_size = sizeof(CCIDBus),
+ .class_init = ccid_bus_class_init,
};
void ccid_card_send_apdu_to_guest(CCIDCardState *card,
diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
index 3042232..76ce427 100644
--- a/hw/virtio/virtio-bus.c
+++ b/hw/virtio/virtio-bus.c
@@ -316,6 +316,7 @@ static void virtio_bus_class_init(ObjectClass *klass, void *data)
BusClass *bus_class = BUS_CLASS(klass);
bus_class->get_dev_path = virtio_bus_get_dev_path;
bus_class->get_fw_dev_path = virtio_bus_get_fw_dev_path;
+ bus_class->device_type = TYPE_VIRTIO_DEVICE;
}
static const TypeInfo virtio_bus_info = {
--
2.9.4
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [RFC v4 04/13] qdev: Slot info helpers
2017-08-14 21:57 [Qemu-devel] [RFC v4 00/13] qmp: query-device-slots command Eduardo Habkost
` (2 preceding siblings ...)
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 03/13] qdev: Add BusClass::device_type field Eduardo Habkost
@ 2017-08-14 21:57 ` Eduardo Habkost
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 05/13] query-device-slots: Collapse similar entries Eduardo Habkost
` (10 subsequent siblings)
14 siblings, 0 replies; 26+ messages in thread
From: Eduardo Habkost @ 2017-08-14 21:57 UTC (permalink / raw)
To: Eric Blake, qemu-devel, Markus Armbruster, Michael S. Tsirkin,
Marcel Apfelbaum, Laine Stump
Helper functions to deal with slot info data structures, and
automatically merge similar device slots
There's room for improvement on the slot merging algorithm, but
this version should be enough to demonstrate how bus
implementations can benefit from it.
TODO: write API documentation, make object ownership/reference
rules clear, clean up code.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
include/hw/qdev-slotinfo.h | 85 +++++++
hw/core/slotinfo.c | 610 +++++++++++++++++++++++++++++++++++++++++++++
tests/test-slotinfo.c | 398 +++++++++++++++++++++++++++++
hw/core/Makefile.objs | 2 +
tests/Makefile.include | 14 +-
5 files changed, 1103 insertions(+), 6 deletions(-)
create mode 100644 include/hw/qdev-slotinfo.h
create mode 100644 hw/core/slotinfo.c
create mode 100644 tests/test-slotinfo.c
diff --git a/include/hw/qdev-slotinfo.h b/include/hw/qdev-slotinfo.h
new file mode 100644
index 0000000..6d8fb54
--- /dev/null
+++ b/include/hw/qdev-slotinfo.h
@@ -0,0 +1,85 @@
+#ifndef QDEV_SLOTINFO_H
+#define QDEV_SLOTINFO_H
+
+#include "qapi/qmp/qobject.h"
+#include "qapi/qmp/qstring.h"
+
+/**
+ * valuelist_contains:
+ *
+ * Returns true if the value list represented by @values
+ * contains @v.
+ *
+ * @values follows the format documented at SlotOption.values
+ * in the QAPI schema.
+ */
+bool valuelist_contains(QObject *values, QObject *v);
+
+/**
+ * valuelist_extend:
+ *
+ * Extend a value list with elements from another value list.
+ *
+ * Ownership of 'new' is transfered to the function.
+ */
+void valuelist_extend(QObject **valuelist, QObject *new);
+
+/*
+ * TODO: Use more efficient data structurs (instead of
+ * DeviceSlotInfoList and SlotOptionList) when building the list
+ * and combining items.
+ */
+
+/**
+ * slot_options_can_be_combined:
+ *
+ * Check if two SlotOptionLists can be combined in one.
+ *
+ * Two slot option lists can be combined if all options have exactly
+ * the same value except (at most) one.
+ *
+ * Returns true if the option lists can be combined.
+ *
+ * If return value is true, *@opt_name is set to the only
+ * mismatching option name. If all options match, *@opt_name is
+ * set to NULL.
+ */
+bool slot_options_can_be_combined(SlotOptionList *a, SlotOptionList *b,
+ const char **opt_name);
+
+/*TODO: doc */
+bool slots_can_be_combined(DeviceSlotInfo *a, DeviceSlotInfo *b,
+ const char **opt_name);
+
+/*TODO: doc */
+void slots_combine(DeviceSlotInfo *a, DeviceSlotInfo *b, const char *opt_name);
+
+/*TODO: doc */
+bool slots_try_combine(DeviceSlotInfo *a, DeviceSlotInfo *b);
+
+/*TODO: doc */
+void slot_list_add_slot(DeviceSlotInfoList **l, DeviceSlotInfo *slot);
+
+/*TODO: doc */
+DeviceSlotInfoList *slot_list_collapse(DeviceSlotInfoList *l);
+
+/*TODO: doc */
+void slot_add_opt(DeviceSlotInfo *slot, const char *option, QObject *values);
+
+#define slot_add_opt_str(slot, option, s) \
+ slot_add_opt(slot, option, QOBJECT(qstring_from_str(s)));
+
+#define slot_add_opt_int(slot, option, i) \
+ slot_add_opt(slot, option, QOBJECT(qnum_from_int(i)));
+
+SlotOption *slot_options_find_opt(SlotOptionList *opts, const char *option);
+
+static inline SlotOption *slot_find_opt(DeviceSlotInfo *slot, const char *option)
+{
+ return slot_options_find_opt(slot->opts, option);
+}
+
+/*TODO: doc */
+DeviceSlotInfo *make_slot(BusState *bus);
+
+#endif /* QDEV_SLOTINFO_H */
diff --git a/hw/core/slotinfo.c b/hw/core/slotinfo.c
new file mode 100644
index 0000000..c03bda2
--- /dev/null
+++ b/hw/core/slotinfo.c
@@ -0,0 +1,610 @@
+#include "qemu/osdep.h"
+#include "hw/qdev-core.h"
+#include "hw/qdev-slotinfo.h"
+#include "qapi/qmp/qlist.h"
+#include "qapi/qmp/qstring.h"
+#include "qapi/qmp/qnum.h"
+#include "qapi/qmp/qbool.h"
+#include "qapi/qmp/qjson.h"
+#include "qapi/util.h"
+#include "qapi/qobject-output-visitor.h"
+#include "qapi-visit.h"
+
+//#define DEBUG_QOBJECTS
+
+#ifdef DEBUG_QOBJECTS
+#define _DBG(args...) fprintf(stderr, args);
+
+#define DBG(args...) do { _DBG("%s: ", __FUNCTION__); \
+ _DBG(args); \
+ } while (0)
+#define QDBG(fmt, obj, args...) do { \
+ QString *js = qobject_to_json(obj); \
+ DBG(fmt ## args); \
+ _DBG(": %s\n", qstring_get_str(js)); \
+ QDECREF(js); \
+ } while (0)
+#else
+#define DBG(...) do { } while (0)
+#define QDBG(...) do { } while (0)
+#endif
+
+/* Ensure a value list is normalized to a list of values
+ *
+ * This does NOT normalize individual elements of the list to be
+ * in the [A] or [A, B] format.
+ *
+ * Returns a new reference to the normalized value.
+ */
+static QList *valuelist_normalize(QObject *values)
+{
+ if (qobject_type(values) == QTYPE_QLIST) {
+ qobject_incref(values);
+ return qobject_to_qlist(values);
+ } else {
+ QList *l = qlist_new();
+
+ qobject_incref(values);
+ qlist_append_obj(l, values);
+ return l;
+ }
+}
+
+/* Simplify value list, if possible
+ *
+ * Onwership of @values is transfered to the function, and a
+ * new object is returned.
+ */
+static QObject *valuelist_simplify(QList *values)
+{
+ if (qlist_size(values) == 1) {
+ QObject *o = qlist_entry_obj(qlist_first(values));
+ QType t = qobject_type(o);
+ if (t == QTYPE_QNULL ||
+ t == QTYPE_QNUM ||
+ t == QTYPE_QSTRING ||
+ t == QTYPE_QBOOL) {
+ qobject_incref(o);
+ QDECREF(values);
+ return o;
+ }
+ }
+
+ return QOBJECT(values);
+}
+
+/* Check if a given single-element value can be represented as a [A, B] range */
+static bool value_can_be_range(QObject *v)
+{
+ QType t = qobject_type(v);
+ return (t == QTYPE_QNUM || t == QTYPE_QSTRING);
+}
+
+/*
+ * Represent a single value (if max is NULL), or a [min, max] range
+ * if @max is not NULL
+ */
+typedef struct ValueRange {
+ QObject *min, *max;
+} ValueRange;
+
+/*
+ * Validate a value list element and put result on ValueRange
+ *
+ * NOTE: this won't increase ref count of @vr->min and @vr->max
+ */
+static bool valuelist_element_get_range(QObject *elm, ValueRange *vr)
+{
+ QObject *min = NULL, *max = NULL;
+
+ if (qobject_type(elm) == QTYPE_QLIST) {
+ QList *l = qobject_to_qlist(elm);
+ if (qlist_size(l) < 1 || qlist_size(l) > 2) {
+ return false;
+ }
+ min = qlist_entry_obj(qlist_first(l));
+ if (qlist_size(l) == 2) {
+ max = qlist_entry_obj(qlist_next(qlist_first(l)));
+ }
+ } else {
+ min = elm;
+ }
+
+ assert(min);
+ /* Invalid range: */
+ if (max && (!value_can_be_range(min) || !value_can_be_range(max) ||
+ qobject_type(min) != qobject_type(max))) {
+ return false;
+ }
+
+ /* If the value can be a range, make it a range */
+ if (!max && value_can_be_range(min)) {
+ max = min;
+ }
+
+ vr->min = min;
+ vr->max = max;
+ return true;
+}
+
+/* Check if @v is inside the @vr range */
+static bool range_contains(ValueRange *vr, QObject *v)
+{
+ assert(vr->min);
+ if (vr->max) { /* range */
+ return qobject_type(vr->min) == qobject_type(v) &&
+ qobject_compare(vr->max, v) >= 0 &&
+ qobject_compare(v, vr->min) >= 0;
+ } else { /* single element */
+ return qobject_compare(vr->min, v) == 0;
+ }
+}
+
+/* Check if @a contains @b */
+static bool range_contains_range(ValueRange *a, ValueRange *b)
+{
+ bool r = range_contains(a, b->min);
+ if (b->max) {
+ r &= range_contains(a, b->max);
+ }
+ return r;
+}
+
+/* Check if the intersection of @a and @b is not empty */
+static bool range_overlaps_range(ValueRange *a, ValueRange *b)
+{
+ return range_contains(a, b->min) ||
+ (b->max && range_contains(a, b->max)) ||
+ range_contains_range(b, a);
+}
+
+/*
+ * Check if a given entry of a value list contains the range @minv-@maxv
+ * If @maxv is NULL, only check if the entry contains @minv
+ */
+static bool valuelist_entry_contains(QObject *ev, ValueRange *vr)
+{
+ ValueRange er;
+
+ if (!valuelist_element_get_range(ev, &er)) {
+ return false;
+ }
+ return range_contains_range(&er, vr);
+}
+
+/*
+ * Check if a given entry of a value list contains the range @minv-@maxv
+ * If @maxv is NULL, only check if the entry contains @minv
+ */
+static bool valuelist_entry_overlaps(QObject *ev, ValueRange *vr)
+{
+ ValueRange er;
+
+ if (!valuelist_element_get_range(ev, &er)) {
+ return false;
+ }
+
+ return range_overlaps_range(&er, vr);
+}
+
+/*
+ * Find the entry in the value list that contains the range @minv-@maxv
+ * If @maxv is NULL, check if the value list contains @minv
+ *
+ * Returns the list entry that contains the range, or NULL if
+ * not found.
+ */
+static QListEntry *nvaluelist_find_range_match(QList *l, ValueRange *vr)
+{
+ QListEntry *e;
+
+ QLIST_FOREACH_ENTRY(l, e) {
+ QObject *ev = qlist_entry_obj(e);
+ if (valuelist_entry_contains(ev, vr)) {
+ return e;
+ }
+ }
+
+ return NULL;
+}
+
+/*
+ * Find the entry in the value list that contains @v
+ *
+ * Returns the list entry that contains the range, or NULL if
+ * not found.
+ */
+static QListEntry *nvaluelist_find_value_match(QList *l, QObject *v)
+{
+ ValueRange vr = { .min = v };
+ return nvaluelist_find_range_match(l, &vr);
+}
+
+/*
+ * Try to sum i to number, if it's an integer.
+ * Otherwise, just return a new reference to @v
+ */
+static QObject *qnum_try_int_add(QObject *v, int i)
+{
+ QNum *qn;
+ uint64_t u64;
+ int64_t i64;
+
+ if (qobject_type(v) != QTYPE_QNUM) {
+ qobject_incref(v);
+ return v;
+ }
+
+ /*TODO: we should be able to convert uint to int and vice-versa. e.g.:
+ * - qnum_try_int_add(qnum_from_int(INT64_MAX), 1)
+ * - qnum_try_int_add(qnum_from_uint(UINT64_MIN), -1)
+ */
+ qn = qobject_to_qnum(v);
+ if (qnum_get_try_int(qn, &i64)) {
+ if ((i < 0) && INT64_MIN - i >= i64) {
+ i64 = INT64_MIN;
+ } else if ((i > 0) && INT64_MAX - i <= i64) {
+ i64 = INT64_MAX;
+ } else {
+ i64 = i64 + i;
+ }
+ return QOBJECT(qnum_from_int(i64));
+ } else if (qnum_get_try_uint(qn, &u64)) {
+ if ((i < 0) && -i >= u64) {
+ u64 = 0;
+ } else if ((i > 0) && UINT64_MAX - i <= u64) {
+ u64 = UINT64_MAX;
+ } else {
+ u64 = u64 + i;
+ }
+ return QOBJECT(qnum_from_uint(u64));
+ } else {
+ qobject_incref(v);
+ return v;
+ }
+}
+
+/*
+ * Look for any entry that overlaps or touches @vr
+ * If @skip is not NULL, @skip is not considered as a match.
+ */
+static QListEntry *nvaluelist_find_overlap(QList *l, ValueRange *vr,
+ QListEntry *skip)
+{
+ QListEntry *r = NULL;
+ QListEntry *e;
+ ValueRange key;
+
+ key.min = qnum_try_int_add(vr->min, -1);
+ key.max = qnum_try_int_add(vr->max, 1);
+
+ QLIST_FOREACH_ENTRY(l, e) {
+ QObject *ev = qlist_entry_obj(e);
+ if (e == skip) {
+ continue;
+ }
+ if (valuelist_entry_overlaps(ev, &key)) {
+ r = e;
+ break;
+ }
+ }
+
+ qobject_decref(key.min);
+ qobject_decref(key.max);
+ return r;
+}
+
+bool valuelist_contains(QObject *values, QObject *v)
+{
+ QList *l = valuelist_normalize(values);
+ bool r = !!nvaluelist_find_value_match(l, v);
+
+ QDECREF(l);
+ return r;
+}
+
+static QListEntry *valuelist_try_overlap(QList *l, ValueRange *vr,
+ QListEntry *skip)
+{
+ ValueRange ovr;
+ QList *newrange;
+ QListEntry *ov = nvaluelist_find_overlap(l, vr, skip);
+
+ if (!ov) {
+ return NULL;
+ }
+
+ valuelist_element_get_range(ov->value, &ovr);
+ if (qobject_compare(ovr.min, vr->min) > 0) {
+ ovr.min = vr->min;
+ }
+ if (qobject_compare(vr->max, ovr.max) > 0) {
+ ovr.max = vr->max;
+ }
+
+ newrange = qlist_new();
+ qobject_incref(ovr.min);
+ qlist_append_obj(newrange, ovr.min);
+ qobject_incref(ovr.max);
+ qlist_append_obj(newrange, ovr.max);
+
+ /*FIXME: this is a hack */
+ qobject_decref(ov->value);
+ ov->value = QOBJECT(newrange);
+ return ov;
+}
+
+/* Ownership of @e is passed to the function */
+static QListEntry *valuelist_try_merge(QList *l, QListEntry *e)
+{
+ ValueRange vr;
+ QListEntry *ov;
+
+ if (!valuelist_element_get_range(e->value, &vr)) {
+ return NULL;
+ }
+
+ ov = valuelist_try_overlap(l, &vr, e);
+ assert(ov != e);
+ if (ov) {
+ /*TODO: this is a hack */
+ QTAILQ_REMOVE(&l->head, e, next);
+ qobject_decref(e->value);
+ g_free(e);
+ }
+ return ov;
+}
+
+/* Ownership of @elm is NOT given to the function: only the reference
+ * count is increased if necessary.
+ */
+static void valuelist_append_element(QList *l, QObject *elm)
+{
+ ValueRange vr;
+
+ if (valuelist_element_get_range(elm, &vr)) {
+ QListEntry *ov;
+
+ if (nvaluelist_find_range_match(l, &vr)) {
+ return;
+ }
+
+ ov = valuelist_try_overlap(l, &vr, NULL);
+ /* If we find an overlapping entry, keep trying to merge it with
+ * other elements.
+ */
+ if (ov) {
+ while (ov) {
+ ov = valuelist_try_merge(l, ov);
+ }
+ return;
+ }
+ }
+
+ /* No overlap found, just append element to the list */
+ qobject_incref(elm);
+ qlist_append_obj(l, elm);
+}
+
+void valuelist_extend(QObject **valuelist, QObject *new)
+{
+ QObject *old = *valuelist;
+ QList *l = valuelist_normalize(old);
+ QList *newl = valuelist_normalize(new);
+ QListEntry *e;
+
+ QLIST_FOREACH_ENTRY(newl, e) {
+ QObject *elm = qlist_entry_obj(e);
+ valuelist_append_element(l, elm);
+ }
+ QDECREF(newl);
+
+ *valuelist = valuelist_simplify(l);
+ qobject_decref(old);
+}
+
+SlotOption *slot_options_find_opt(SlotOptionList *opts, const char *option)
+{
+ for (; opts; opts = opts->next) {
+ if (!strcmp(opts->value->option, option)) {
+ return opts->value;
+ }
+ }
+ return NULL;
+}
+
+bool slot_options_can_be_combined(SlotOptionList *a, SlotOptionList *b,
+ const char **opt_name)
+{
+ SlotOptionList *ol;
+ const char *mismatch = NULL;
+
+ /* Check if all options in @b will be handled when we loop through @a */
+ for (ol = b; ol; ol = ol->next) {
+ if (!slot_options_find_opt(a, ol->value->option)) {
+ return false;
+ }
+ }
+
+ for (ol = a; ol; ol = ol->next) {
+ SlotOption *ao = ol->value;
+ SlotOption *bo = slot_options_find_opt(b, ao->option);
+
+ if (!bo) {
+ return false;
+ }
+
+ if (qobject_compare(bo->values, ao->values)) {
+ if (mismatch && strcmp(mismatch, ao->option)) {
+ return false;
+ }
+
+ mismatch = ao->option;
+ }
+ }
+
+ if (opt_name) {
+ *opt_name = mismatch;
+ }
+ return true;
+}
+
+static int compare_strList(strList *a, strList *b)
+{
+ for (; a && b; a = a->next, b = b->next) {
+ int c = strcmp(a->value, b->value);
+ if (c) {
+ return c;
+ }
+ }
+
+ if (b) {
+ return -1;
+ } else if (a) {
+ return 1;
+ } else {
+ return 0;
+ }
+
+}
+
+bool slots_can_be_combined(DeviceSlotInfo *a, DeviceSlotInfo *b,
+ const char **opt_name)
+{
+ if (a->available != b->available ||
+ a->hotpluggable != b->hotpluggable ||
+ a->has_count != b->has_count ||
+ a->opts_complete != b->opts_complete ||
+ a->has_device || b->has_device ||
+ compare_strList(a->device_types, b->device_types)) {
+ return false;
+ }
+
+ return slot_options_can_be_combined(a->opts, b->opts, opt_name);
+}
+
+void slots_combine(DeviceSlotInfo *a, DeviceSlotInfo *b, const char *opt_name)
+{
+ assert(slots_can_be_combined(a, b, NULL));
+ if (a->has_count) {
+ a->count += b->count;
+ }
+ if (opt_name) {
+ SlotOption *aopt = slot_options_find_opt(a->opts, opt_name);
+ SlotOption *bopt = slot_options_find_opt(b->opts, opt_name);
+
+ valuelist_extend(&aopt->values, bopt->values);
+ }
+}
+
+bool slots_try_combine(DeviceSlotInfo *a, DeviceSlotInfo *b)
+{
+ const char *opt = NULL;
+ assert(a != b);
+
+ if (slots_can_be_combined(a, b, &opt)) {
+ slots_combine(a, b, opt);
+ return true;
+ }
+
+ return false;
+}
+
+/* Try to combine @slot with an entry in @l
+ *
+ * Will return a pointer to the 'next' pointer in the previous entry,
+ * to allow callers to remove the entry from the list if necessary.
+ */
+static DeviceSlotInfoList **slot_list_try_combine_slot(DeviceSlotInfoList **l, DeviceSlotInfo *slot)
+{
+ DeviceSlotInfoList **pprev;
+
+ for (pprev = l; *pprev; pprev = &(*pprev)->next) {
+ DeviceSlotInfo *i = (*pprev)->value;
+ if (slots_try_combine(i, slot)) {
+ return pprev;
+ }
+ }
+
+ return NULL;
+}
+
+DeviceSlotInfoList *slot_list_collapse(DeviceSlotInfoList *l)
+{
+ DeviceSlotInfoList *newlist = NULL;
+ DeviceSlotInfoList *queue = l;
+
+ while (queue) {
+ DeviceSlotInfoList **pprev;
+ DeviceSlotInfoList *next = queue->next;
+
+ pprev = slot_list_try_combine_slot(&newlist, queue->value);
+ if (pprev) {
+ DeviceSlotInfoList *removed = *pprev;
+ /* Remove modified item from @newlist@ */
+ *pprev = (*pprev)->next;
+ /* Dequeue item from @queue */
+ queue->next = NULL;
+ qapi_free_DeviceSlotInfoList(queue);
+ /* Queue modified item on @queue */
+ removed->next = next;
+ queue = removed;
+ } else {
+ /* Not combined, just insert into newlist */
+ queue->next = newlist;
+ newlist = queue;
+ queue = next;
+ }
+ }
+
+ return newlist;
+}
+
+/* Ownership of @slot is given to the function */
+void slot_list_add_slot(DeviceSlotInfoList **l, DeviceSlotInfo *slot)
+{
+ DeviceSlotInfoList *li;
+
+ /* Avoid adding new entry if it can be combined */
+ if (slot_list_try_combine_slot(l, slot)) {
+ qapi_free_DeviceSlotInfo(slot);
+ return;
+ }
+
+ li = g_new0(DeviceSlotInfoList, 1);
+ li->value = slot;
+ li->next = *l;
+ *l = li;
+}
+
+void slot_add_opt(DeviceSlotInfo *slot, const char *option, QObject *values)
+{
+ SlotOptionList *l = g_new0(SlotOptionList, 1);
+
+ l->value = g_new0(SlotOption, 1);
+ l->value->option = g_strdup(option);
+ l->value->values = values;
+ l->next = slot->opts;
+ slot->opts = l;
+}
+
+/*TODO: move it to common code */
+static inline bool qbus_is_full(BusState *bus)
+{
+ BusClass *bus_class = BUS_GET_CLASS(bus);
+ return bus_class->max_dev && bus->max_index >= bus_class->max_dev;
+}
+
+DeviceSlotInfo *make_slot(BusState *bus)
+{
+ DeviceSlotInfo *s = g_new0(DeviceSlotInfo, 1);
+
+ s->device_types = g_new0(strList, 1);
+ s->device_types->value = g_strdup(BUS_GET_CLASS(bus)->device_type);
+ s->hotpluggable = qbus_is_hotpluggable(bus);
+ s->available = !qbus_is_full(bus);
+
+ slot_add_opt_str(s, "bus", bus->name);
+
+ return s;
+}
diff --git a/tests/test-slotinfo.c b/tests/test-slotinfo.c
new file mode 100644
index 0000000..b169712
--- /dev/null
+++ b/tests/test-slotinfo.c
@@ -0,0 +1,398 @@
+/*
+ * Unit tests for QAPI utility functions
+ *
+ * Copyright (C) 2017 Red Hat Inc.
+ *
+ * Authors:
+ * Eduardo Habkost <ehabkost@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qapi/util.h"
+#include "qapi/qmp/qnum.h"
+#include "qapi/qmp/qstring.h"
+#include "qapi/qmp/qlist.h"
+#include "qapi/qmp/qjson.h"
+#include "hw/qdev-slotinfo.h"
+
+#define JS(json) qobject_from_json((json), &error_abort)
+
+static bool json_valuelist_contains(const char *jvalues, const char *jvalue)
+{
+ QObject *values = JS(jvalues);
+ QObject *value = JS(jvalue);
+ bool r = valuelist_contains(values, value);
+
+ qobject_decref(values);
+ qobject_decref(value);
+ return r;
+}
+
+static void test_valuelist_contains(void)
+{
+ g_assert_true(json_valuelist_contains("100", "100"));
+ g_assert_false(json_valuelist_contains("100", "200"));
+
+ g_assert_false(json_valuelist_contains("[]", "100"));
+ g_assert_true(json_valuelist_contains("[100, 200, 300]", "200"));
+ g_assert_false(json_valuelist_contains("[100, 200, 300]", "150"));
+
+ g_assert_true(json_valuelist_contains("\"abc\"", "\"abc\""));
+ g_assert_false(json_valuelist_contains("\"abc\"", "\"xyz\""));
+ g_assert_true(json_valuelist_contains("[\"abc\"]", "\"abc\""));
+ g_assert_false(json_valuelist_contains("[\"abc\", \"cde\"]", "\"xyz\""));
+
+#define TEST_RANGE "[[1,10], [18,20], [\"aaaa2\", \"jyz3\"], [-100, 5]," \
+ "\"kkk\", 14, -50, [51], [[30, 31]] ]"
+
+ /* [-100, 5] */
+ g_assert_false(json_valuelist_contains(TEST_RANGE, "-101"));
+ g_assert_true( json_valuelist_contains(TEST_RANGE, "-100"));
+ g_assert_true( json_valuelist_contains(TEST_RANGE, "-99"));
+
+ /* -50 */
+ g_assert_true( json_valuelist_contains(TEST_RANGE, "-51"));
+ g_assert_true( json_valuelist_contains(TEST_RANGE, "-50"));
+ g_assert_true( json_valuelist_contains(TEST_RANGE, "-49"));
+
+ /* [-100, 5], [1, 10] */
+ g_assert_true( json_valuelist_contains(TEST_RANGE, "-1"));
+ g_assert_true( json_valuelist_contains(TEST_RANGE, "0"));
+ g_assert_true( json_valuelist_contains(TEST_RANGE, "1"));
+ g_assert_true( json_valuelist_contains(TEST_RANGE, "2"));
+
+ /* [-100, 5] */
+ g_assert_true( json_valuelist_contains(TEST_RANGE, "4"));
+ g_assert_true( json_valuelist_contains(TEST_RANGE, "5"));
+ g_assert_true( json_valuelist_contains(TEST_RANGE, "6"));
+
+ /* [1, 10] */
+ g_assert_true( json_valuelist_contains(TEST_RANGE, "9"));
+ g_assert_true( json_valuelist_contains(TEST_RANGE, "10"));
+ g_assert_false(json_valuelist_contains(TEST_RANGE, "11"));
+
+ /* 14 */
+ g_assert_false(json_valuelist_contains(TEST_RANGE, "13"));
+ g_assert_true( json_valuelist_contains(TEST_RANGE, "14"));
+ g_assert_false(json_valuelist_contains(TEST_RANGE, "15"));
+
+ /* [18, 20] */
+ g_assert_false(json_valuelist_contains(TEST_RANGE, "17"));
+ g_assert_true( json_valuelist_contains(TEST_RANGE, "18"));
+ g_assert_true( json_valuelist_contains(TEST_RANGE, "19"));
+ g_assert_true( json_valuelist_contains(TEST_RANGE, "20"));
+ g_assert_false(json_valuelist_contains(TEST_RANGE, "21"));
+
+ /* [51] */
+ g_assert_false(json_valuelist_contains(TEST_RANGE, "50"));
+ g_assert_true( json_valuelist_contains(TEST_RANGE, "51"));
+ g_assert_false(json_valuelist_contains(TEST_RANGE, "52"));
+
+ /* [ "aaa2" , "jyz3" ] */
+ g_assert_false(json_valuelist_contains(TEST_RANGE, "\"aaaa\""));
+ g_assert_false(json_valuelist_contains(TEST_RANGE, "\"aaaa1\""));
+ g_assert_true( json_valuelist_contains(TEST_RANGE, "\"aaaa2\""));
+ g_assert_true(json_valuelist_contains(TEST_RANGE, "\"aaaa3\""));
+
+ /* [ "aaa2" , "jyz3" ] */
+ g_assert_true(json_valuelist_contains(TEST_RANGE, "\"bcde\""));
+
+ /* [ "aaa2" , "jyz3" ] */
+ g_assert_true( json_valuelist_contains(TEST_RANGE, "\"jyz\""));
+ g_assert_true( json_valuelist_contains(TEST_RANGE, "\"jyz2\""));
+ g_assert_true( json_valuelist_contains(TEST_RANGE, "\"jyz3\""));
+ g_assert_false(json_valuelist_contains(TEST_RANGE, "\"jyz4\""));
+
+ /* "kkk" */
+ g_assert_false(json_valuelist_contains(TEST_RANGE, "\"kk\""));
+ g_assert_true( json_valuelist_contains(TEST_RANGE, "\"kkk\""));
+ g_assert_false(json_valuelist_contains(TEST_RANGE, "\"kkkk\""));
+
+ /* [[30, 31]] */
+ g_assert_false(json_valuelist_contains(TEST_RANGE, "30"));
+ g_assert_false(json_valuelist_contains(TEST_RANGE, "[30]"));
+ g_assert_true( json_valuelist_contains(TEST_RANGE, "[30, 31]"));
+
+ /* empty set doesn't contain an empty list: */
+ g_assert_false(json_valuelist_contains("[]", "[]"));
+
+ /* [] is an invalid element on a value list: */
+ g_assert_false(json_valuelist_contains("[[]]", "[]"));
+
+ /* [[]] indicates [] is a valid value */
+ g_assert_true(json_valuelist_contains("[[[]]]", "[]"));
+}
+
+#define assert_valuelist_extend(before, extend, after) \
+ do { \
+ QObject *set = JS(before); \
+ QObject *expected = JS(after); \
+ \
+ valuelist_extend(&set, JS(extend));; \
+ g_assert_cmpint(qobject_compare(set, expected), ==, 0); \
+ \
+ qobject_decref(set); \
+ qobject_decref(expected); \
+ } while (0)
+
+static void test_valuelist_extend(void)
+{
+ assert_valuelist_extend("[]",
+ "1",
+ "1");
+
+ assert_valuelist_extend("1",
+ "1",
+ "1");
+
+ assert_valuelist_extend("1",
+ "3",
+ "[1, 3]");
+
+ assert_valuelist_extend("[1, 3]",
+ "6",
+ "[1, 3, 6]");
+
+ /*TODO: implement and test range merging */
+
+ /* single-element becomes range: */
+ assert_valuelist_extend("[1, 3, 6]",
+ "4",
+ "[1, [3, 4], 6]");
+ assert_valuelist_extend("[1, 4, 6]",
+ "3",
+ "[1, [3, 4], 6]");
+
+
+ /* single-element merges two elements: */
+ assert_valuelist_extend("[1, 3, 6]",
+ "2",
+ "[[1, 3], 6]");
+
+ /* [] -> empty set */
+ assert_valuelist_extend("[1, 3, 6]",
+ "[]",
+ "[1, 3, 6]");
+
+ /* [3, 100] -> two elements: 3 and 100 (not a range) */
+ assert_valuelist_extend("[[1, 4], 6]",
+ "[3, 100]",
+ "[[1, 4], 6, 100]");
+
+ /* tests for appending new ranges: */
+
+ /* add two ranges: 7-30, 40-50 */
+ assert_valuelist_extend("[[1, 4], 6, 100]",
+ "[[7, 30], [40, 50]]",
+ "[[1, 4], [6, 30], 100, [40, 50]]");
+
+ /* multiple ways of appending to a range: */
+ assert_valuelist_extend("[[1, 4], [6, 30], [40, 50], [53, 60]]",
+ "30",
+ "[[1, 4], [6, 30], [40, 50], [53, 60]]");
+ assert_valuelist_extend("[[1, 4], [6, 30], [40, 50], [53, 60]]",
+ "31",
+ "[[1, 4], [6, 31], [40, 50], [53, 60]]");
+ assert_valuelist_extend("[[1, 4], [6, 30], [40, 50], [53, 60]]",
+ "[[25, 35]]",
+ "[[1, 4], [6, 35], [40, 50], [53, 60]]");
+ assert_valuelist_extend("[[1, 4], [6, 30], [40, 50], [53, 60]]",
+ "[[30, 35]]",
+ "[[1, 4], [6, 35], [40, 50], [53, 60]]");
+ assert_valuelist_extend("[[1, 4], [6, 30], [40, 50], [53, 60]]",
+ "[[31, 35]]",
+ "[[1, 4], [6, 35], [40, 50], [53, 60]]");
+ //TODO: make this work:
+ assert_valuelist_extend("[[1, 4], [6, 30], [40, 50], [53, 60]]",
+ "[[38, 51]]",
+ "[[1, 4], [6, 30], [38, 51], [53, 60]]");
+ assert_valuelist_extend("[[1, 4], [6, 30], [40, 50], [53, 60]]",
+ "[[38, 52]]",
+ "[[1, 4], [6, 30], [38, 60]]");
+ /* off-by-one check: */
+ assert_valuelist_extend("[[1, 4], [6, 30], [40, 50], [53, 60]]",
+ "51",
+ "[[1, 4], [6, 30], [40, 51], [53, 60]]");
+ /* _not_ appending to a range: */
+ assert_valuelist_extend("[[1, 4], [6, 30], [40, 50], [53, 60]]",
+ "32",
+ "[[1, 4], [6, 30], [40, 50], [53, 60], 32]");
+ assert_valuelist_extend("[[1, 4], [6, 30], [40, 50], [53, 60]]",
+ "[[32, 35]]",
+ "[[1, 4], [6, 30], [40, 50], [53, 60], [32, 35]]");
+
+ /* multiple ways of prepending to a range: */
+ assert_valuelist_extend("[[1, 4], [6, 30], [40, 50], [53, 60]]",
+ "40",
+ "[[1, 4], [6, 30], [40, 50], [53, 60]]");
+ assert_valuelist_extend("[[1, 4], [6, 30], [40, 50], [53, 60]]",
+ "39",
+ "[[1, 4], [6, 30], [39, 50], [53, 60]]");
+ assert_valuelist_extend("[[1, 4], [6, 30], [40, 50], [53, 60]]",
+ "[[35, 45]]",
+ "[[1, 4], [6, 30], [35, 50], [53, 60]]");
+ assert_valuelist_extend("[[1, 4], [6, 30], [40, 50], [53, 60]]",
+ "[[35, 40]]",
+ "[[1, 4], [6, 30], [35, 50], [53, 60]]");
+ assert_valuelist_extend("[[1, 4], [6, 30], [40, 50], [53, 60]]",
+ "[[35, 39]]",
+ "[[1, 4], [6, 30], [35, 50], [53, 60]]");
+ /* off-by-one check: */
+ assert_valuelist_extend("[[1, 4], [6, 30], [33, 50], [53, 60]]",
+ "32",
+ "[[1, 4], [6, 30], [32, 50], [53, 60]]");
+ /* _not_ prepending to a range: */
+ assert_valuelist_extend("[[1, 4], [6, 30], [40, 50], [53, 60]]",
+ "38",
+ "[[1, 4], [6, 30], [40, 50], [53, 60], 38]");
+ assert_valuelist_extend("[[1, 4], [6, 30], [40, 50], [53, 60]]",
+ "[[35, 38]]",
+ "[[1, 4], [6, 30], [40, 50], [53, 60], [35, 38]]");
+
+ /* multiple ways of combining two ranges: */
+ assert_valuelist_extend("[[1, 4], [6, 30], [40, 50], [53, 60]]",
+ "5",
+ "[[1, 30], [40, 50], [53, 60]]");
+ assert_valuelist_extend("[[1, 4], [6, 30], [40, 50], [53, 60]]",
+ "[[25, 45]]",
+ "[[1, 4], [6, 50], [53, 60]]");
+}
+
+static void test_slots_can_combine(void)
+{
+ DeviceSlotInfo *a = g_new0(DeviceSlotInfo, 1);
+ DeviceSlotInfo *b = g_new0(DeviceSlotInfo, 1);
+ const char *opt_name = NULL;
+
+ g_assert_true(slots_can_be_combined(a, b, &opt_name));
+ g_assert_null(opt_name);
+
+ slot_add_opt(a, "bus", JS("\"mybus.0\""));
+ g_assert_false(slots_can_be_combined(a, b, &opt_name));
+ slot_add_opt(b, "bus", JS("\"mybus.0\""));
+
+ g_assert_true(slots_can_be_combined(a, b, &opt_name));
+ g_assert_null(opt_name);
+
+ slot_add_opt(a, "addr", JS("[ 1, 3 ]"));
+ g_assert_false(slots_can_be_combined(a, b, &opt_name));
+ slot_add_opt(b, "addr", JS("5"));
+
+ g_assert_true(slots_can_be_combined(a, b, &opt_name));
+ g_assert_cmpstr(opt_name, ==, "addr");
+
+ slot_add_opt(a, "unit", JS("1"));
+ g_assert_false(slots_can_be_combined(a, b, &opt_name));
+ slot_add_opt(b, "unit", JS("1"));
+
+ g_assert_true(slots_can_be_combined(a, b, &opt_name));
+ g_assert_cmpstr(opt_name, ==, "addr");
+
+ a->hotpluggable = true;
+ g_assert_false(slots_can_be_combined(a, b, &opt_name));
+ a->hotpluggable = false;
+
+ a->has_device = true;
+ a->device = g_strdup("/machine/somedevice");
+ g_assert_false(slots_can_be_combined(a, b, &opt_name));
+ g_free(a->device);
+ a->has_device = false;
+ a->device = NULL;
+
+ slot_add_opt(a, "port", JS("10"));
+ g_assert_false(slots_can_be_combined(a, b, &opt_name));
+ slot_add_opt(b, "port", JS("20"));
+
+ g_assert_false(slots_can_be_combined(a, b, &opt_name));
+}
+
+static void test_slots_combine(void)
+{
+ DeviceSlotInfo *a = g_new0(DeviceSlotInfo, 1);
+ DeviceSlotInfo *b = g_new0(DeviceSlotInfo, 1);
+ SlotOption *o;
+
+ slot_add_opt(a, "bus", JS("\"mybus.0\""));
+ slot_add_opt(b, "bus", JS("\"mybus.0\""));
+
+ slot_add_opt(a, "addr", JS("[ 1, 3 ]"));
+ slot_add_opt(b, "addr", JS("5"));
+
+ slot_add_opt(a, "unit", JS("1"));
+ slot_add_opt(b, "unit", JS("1"));
+
+ g_assert_true(slots_try_combine(a, b));
+
+ o = a->opts->value;
+ g_assert_cmpstr(o->option, ==, "unit");
+ g_assert_cmpint(qobject_compare(o->values, JS("1")), ==, 0);
+
+ o = a->opts->next->value;
+ g_assert_cmpstr(o->option, ==, "addr");
+ g_assert_cmpint(qobject_compare(o->values, JS("[1, 3, 5]")), ==, 0);
+
+ o = a->opts->next->next->value;
+ g_assert_cmpstr(o->option, ==, "bus");
+ g_assert_cmpint(qobject_compare(o->values, JS("\"mybus.0\"")), ==, 0);
+}
+
+static void test_slot_list_collapse(void)
+{
+ DeviceSlotInfoList *l = NULL;
+ SlotOption *o;
+ int node, socket, core, thread;
+
+ for (node = 0; node < 4; node++) {
+ for (socket = 0; socket < 8; socket++) {
+ for (core = 0; core < 4; core++) {
+ for (thread = 0; thread < 2; thread++) {
+ DeviceSlotInfo *s = g_new0(DeviceSlotInfo, 1);
+ slot_add_opt_int(s, "node", node);
+ slot_add_opt_int(s, "socket", socket);
+ slot_add_opt_int(s, "core", core);
+ slot_add_opt_int(s, "thread", thread);
+ slot_list_add_slot(&l, s);
+ }
+ }
+ }
+ }
+
+ /*
+ * All the entries above should be merged in a single entry:
+ * node = [0, 3]
+ * socket = [0, 7]
+ * core = [0, 3]
+ * thread = [0, 1]
+ */
+ l = slot_list_collapse(l);
+ g_assert_nonnull(l);
+ g_assert_null(l->next);
+
+ o = slot_find_opt(l->value, "node");
+ g_assert_cmpint(qobject_compare(o->values, JS("[ [0, 3] ]")), ==, 0);
+
+ o = slot_find_opt(l->value, "socket");
+ g_assert_cmpint(qobject_compare(o->values, JS("[ [0, 7] ]")), ==, 0);
+
+ o = slot_find_opt(l->value, "core");
+ g_assert_cmpint(qobject_compare(o->values, JS("[ [0, 3] ]")), ==, 0);
+
+ o = slot_find_opt(l->value, "thread");
+ g_assert_cmpint(qobject_compare(o->values, JS("[ [0, 1] ]")), ==, 0);
+}
+
+int main(int argc, char *argv[])
+{
+ g_test_init(&argc, &argv, NULL);
+ g_test_add_func("/qapi/util/valuelist_contains", test_valuelist_contains);
+ g_test_add_func("/qapi/util/valuelist_extend", test_valuelist_extend);
+ g_test_add_func("/qapi/util/slots_can_combine", test_slots_can_combine);
+ g_test_add_func("/qapi/util/slots_combine", test_slots_combine);
+ g_test_add_func("/qapi/util/slot_list_collapse", test_slot_list_collapse);
+ g_test_run();
+ return 0;
+}
diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
index f8d7a4a..0f3a0a6 100644
--- a/hw/core/Makefile.objs
+++ b/hw/core/Makefile.objs
@@ -6,6 +6,7 @@ common-obj-y += fw-path-provider.o
common-obj-y += irq.o
common-obj-y += hotplug.o
common-obj-y += nmi.o
+common-obj-y += slotinfo.o
common-obj-$(CONFIG_EMPTY_SLOT) += empty_slot.o
common-obj-$(CONFIG_XILINX_AXI) += stream.o
@@ -21,3 +22,4 @@ common-obj-$(CONFIG_PLATFORM_BUS) += platform-bus.o
obj-$(CONFIG_SOFTMMU) += generic-loader.o
obj-$(CONFIG_SOFTMMU) += null-machine.o
+
diff --git a/tests/Makefile.include b/tests/Makefile.include
index eb4895f..493f9ce 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -131,6 +131,8 @@ check-unit-y += tests/ptimer-test$(EXESUF)
gcov-files-ptimer-test-y = hw/core/ptimer.c
check-unit-y += tests/test-qapi-util$(EXESUF)
gcov-files-test-qapi-util-y = qapi/qapi-util.c
+check-unit-y += tests/test-slotinfo$(EXESUF)
+gcov-files-test-slotinfo-y = hw/core/slotinfo.c
check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
@@ -535,6 +537,10 @@ test-qom-obj-y = $(qom-obj-y) $(test-util-obj-y)
test-qapi-obj-y = tests/test-qapi-visit.o tests/test-qapi-types.o \
tests/test-qapi-event.o tests/test-qmp-introspect.o \
$(test-qom-obj-y)
+test-qdev-obj-y = hw/core/qdev.o hw/core/qdev-properties.o \
+ hw/core/hotplug.o hw/core/bus.o hw/core/irq.o \
+ hw/core/fw-path-provider.o hw/core/reset.o \
+ hw/core/slotinfo.o
benchmark-crypto-obj-y = $(crypto-obj-y) $(test-qom-obj-y)
test-crypto-obj-y = $(crypto-obj-y) $(test-qom-obj-y)
test-io-obj-y = $(io-obj-y) $(test-crypto-obj-y)
@@ -573,12 +579,7 @@ tests/test-bufferiszero$(EXESUF): tests/test-bufferiszero.o $(test-util-obj-y)
tests/atomic_add-bench$(EXESUF): tests/atomic_add-bench.o $(test-util-obj-y)
tests/test-qdev-global-props$(EXESUF): tests/test-qdev-global-props.o \
- hw/core/qdev.o hw/core/qdev-properties.o hw/core/hotplug.o\
- hw/core/bus.o \
- hw/core/irq.o \
- hw/core/fw-path-provider.o \
- hw/core/reset.o \
- $(test-qapi-obj-y)
+ $(test-qdev-obj-y) $(test-qapi-obj-y)
tests/test-vmstate$(EXESUF): tests/test-vmstate.o \
migration/vmstate.o migration/vmstate-types.o migration/qemu-file.o \
migration/qemu-file-channel.o migration/qjson.o \
@@ -767,6 +768,7 @@ tests/vhost-user-bridge$(EXESUF): tests/vhost-user-bridge.o contrib/libvhost-use
tests/test-uuid$(EXESUF): tests/test-uuid.o $(test-util-obj-y)
tests/test-arm-mptimer$(EXESUF): tests/test-arm-mptimer.o
tests/test-qapi-util$(EXESUF): tests/test-qapi-util.o $(test-util-obj-y)
+tests/test-slotinfo$(EXESUF): tests/test-slotinfo.o hw/core/slotinfo.o $(test-qdev-obj-y) $(test-qom-obj-y)
tests/numa-test$(EXESUF): tests/numa-test.o
tests/vmgenid-test$(EXESUF): tests/vmgenid-test.o tests/boot-sector.o tests/acpi-utils.o
--
2.9.4
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [RFC v4 05/13] query-device-slots: Collapse similar entries
2017-08-14 21:57 [Qemu-devel] [RFC v4 00/13] qmp: query-device-slots command Eduardo Habkost
` (3 preceding siblings ...)
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 04/13] qdev: Slot info helpers Eduardo Habkost
@ 2017-08-14 21:57 ` Eduardo Habkost
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 06/13] qdev core: generic enumerate_slots implementation Eduardo Habkost
` (9 subsequent siblings)
14 siblings, 0 replies; 26+ messages in thread
From: Eduardo Habkost @ 2017-08-14 21:57 UTC (permalink / raw)
To: Eric Blake, qemu-devel, Markus Armbruster, Michael S. Tsirkin,
Marcel Apfelbaum, Laine Stump
Use slot_list_collapse() on the final list returned by
query-device-slots. This way we combine similar entries from
different buses.
For example, a machine with multiple IDE buses with no devices
would return a single entry like this:
{
"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
}
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
qdev-monitor.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/qdev-monitor.c b/qdev-monitor.c
index 785f4af..edc6e34 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -673,7 +673,8 @@ DeviceSlotInfoList *qmp_query_device_slots(Error **errp)
SlotListState s = { .next = &s.result };
object_child_foreach_recursive(qdev_get_machine(), enumerate_bus, &s);
- return s.result;
+
+ return slot_list_collapse(s.result);
}
--
2.9.4
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [RFC v4 06/13] qdev core: generic enumerate_slots implementation
2017-08-14 21:57 [Qemu-devel] [RFC v4 00/13] qmp: query-device-slots command Eduardo Habkost
` (4 preceding siblings ...)
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 05/13] query-device-slots: Collapse similar entries Eduardo Habkost
@ 2017-08-14 21:57 ` Eduardo Habkost
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 07/13] qdev: Enumerate CPU slots on query-device-slots Eduardo Habkost
` (8 subsequent siblings)
14 siblings, 0 replies; 26+ messages in thread
From: Eduardo Habkost @ 2017-08-14 21:57 UTC (permalink / raw)
To: Eric Blake, qemu-devel, Markus Armbruster, Michael S. Tsirkin,
Marcel Apfelbaum, Laine Stump
Implement a generic enumerate_slots method on TYPE_BUS
The generic implementation will just report a opts-complete=false
entry, listing the "bus" option. This will let clients know that
the bus is available, but that detailed slot information is not
available by the running QEMU version.
We disable the implementation on TYPE_SYS_BUS as sysbus is an
exceptional case where only a handful of device types are
actually user-creatable. This will be handled later by
sysbus-specific slot enumeration code.
TODO: write sysbus-specific slot enumeration code, based on
sysbus device whitelist.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
hw/core/bus.c | 42 ++++++++++++++++++++++++++++++++++++++++++
hw/core/sysbus.c | 7 +++++++
2 files changed, 49 insertions(+)
diff --git a/hw/core/bus.c b/hw/core/bus.c
index 4651f24..227c092 100644
--- a/hw/core/bus.c
+++ b/hw/core/bus.c
@@ -20,6 +20,7 @@
#include "qemu/osdep.h"
#include "qemu-common.h"
#include "hw/qdev.h"
+#include "hw/qdev-slotinfo.h"
#include "qapi/error.h"
static void qbus_set_hotplug_handler_internal(BusState *bus, Object *handler,
@@ -208,12 +209,53 @@ static char *default_bus_get_fw_dev_path(DeviceState *dev)
return g_strdup(object_get_typename(OBJECT(dev)));
}
+static void class_check_user_creatable(ObjectClass *oc, void *opaque)
+{
+ DeviceClass *dc = DEVICE_CLASS(oc);
+ bool *r = opaque;
+
+ if (dc->user_creatable) {
+ *r = true;
+ }
+}
+
+static bool bus_class_is_user_pluggable(BusClass *bc)
+{
+ bool r = false;
+
+ object_class_foreach(class_check_user_creatable,
+ bc->device_type, false, &r);
+ return r;
+}
+
+static DeviceSlotInfoList *bus_generic_enumerate_slots(BusState *bus)
+{
+ DeviceSlotInfoList *r = NULL;
+ BusChild *kid = NULL;
+
+ if (!bus_class_is_user_pluggable(BUS_GET_CLASS(bus))) {
+ return NULL;
+ }
+
+ QTAILQ_FOREACH(kid, &bus->children, sibling) {
+ DeviceSlotInfo *slot = make_slot(bus);
+ slot->available = false;
+ slot->has_device = true;
+ slot->device = object_get_canonical_path(OBJECT(kid->child));
+ slot_list_add_slot(&r, slot);
+ }
+
+ slot_list_add_slot(&r, make_slot(bus));
+ return r;
+}
+
static void bus_class_init(ObjectClass *class, void *data)
{
BusClass *bc = BUS_CLASS(class);
class->unparent = bus_unparent;
bc->get_fw_dev_path = default_bus_get_fw_dev_path;
+ bc->enumerate_slots = bus_generic_enumerate_slots;
}
static void qbus_finalize(Object *obj)
diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
index 9259931..06cb4d8 100644
--- a/hw/core/sysbus.c
+++ b/hw/core/sysbus.c
@@ -77,6 +77,13 @@ static void system_bus_class_init(ObjectClass *klass, void *data)
k->print_dev = sysbus_dev_print;
k->get_fw_dev_path = sysbus_get_fw_dev_path;
k->device_type = TYPE_SYS_BUS_DEVICE;
+ /*
+ * We won't return sysbus devices on query-device-slots by now.
+ *
+ * TODO: return user-creatable whitelisted sybus devices
+ * if the machine supports dynamic sysbus devices.
+ */
+ k->enumerate_slots = NULL;
}
static const TypeInfo system_bus_info = {
--
2.9.4
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [RFC v4 07/13] qdev: Enumerate CPU slots on query-device-slots
2017-08-14 21:57 [Qemu-devel] [RFC v4 00/13] qmp: query-device-slots command Eduardo Habkost
` (5 preceding siblings ...)
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 06/13] qdev core: generic enumerate_slots implementation Eduardo Habkost
@ 2017-08-14 21:57 ` Eduardo Habkost
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 08/13] ide: enumerate_slots implementation Eduardo Habkost
` (7 subsequent siblings)
14 siblings, 0 replies; 26+ messages in thread
From: Eduardo Habkost @ 2017-08-14 21:57 UTC (permalink / raw)
To: Eric Blake, qemu-devel, Markus Armbruster, Michael S. Tsirkin,
Marcel Apfelbaum, Laine Stump
Cc: Igor Mammedov, David Gibson, Bharata B Rao
Use machine_query_hotpluggable_cpus() to build the slot list for
CPU devices.
Example output: the following command line:
$ qemu-system-x86_64 -machine q35,accel=kvm \
-cpu Haswell -smp 2,maxcpus=4
will generate the following entries:
{
"available": false,
"count": 1,
"device": "/machine/unattached/device[0]",
"device-types": [ "Haswell-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/unattached/device[2]",
"device-types": [ "Haswell-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": true,
"count": 2,
"device-types": [ "Haswell-x86_64-cpu" ],
"hotpluggable": true,
"opts": [
{ "option": "socket-id", "values": [ [ 2, 3 ] ] },
{ "option": "thread-id", "values": 0 },
{ "option": "core-id", "values": 0 }
],
"opts-complete": true
}
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: David Gibson <david@gibson.dropbear.id.au>
Cc: Bharata B Rao <bharata@linux.vnet.ibm.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
qdev-monitor.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 70 insertions(+)
diff --git a/qdev-monitor.c b/qdev-monitor.c
index edc6e34..7ed41df 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -32,6 +32,8 @@
#include "migration/misc.h"
#include "qapi/qobject-output-visitor.h"
#include "hw/boards.h"
+#include "hw/qdev-slotinfo.h"
+#include "qapi-visit.h"
/*
* Aliases were a bad idea from the start. Let's keep them
@@ -668,11 +670,79 @@ static int enumerate_bus(Object *obj, void *opaque)
return 0;
}
+static void add_option(const char *key, QObject *obj, void *opaque)
+{
+ DeviceSlotInfo *slot = opaque;
+
+ qobject_incref(obj);
+ slot_add_opt(slot, key, obj);
+}
+
+/* Add all key/values in @d to as options to @slot */
+static void qdict_to_slot_props(QDict *d, DeviceSlotInfo *slot)
+{
+ qdict_iter(d, add_option, slot);
+}
+
+static QDict *cpu_instance_props_to_qdict(CpuInstanceProperties *src)
+{
+ QObject *qobj = NULL;
+ Visitor *v = qobject_output_visitor_new(&qobj);
+
+ visit_type_CpuInstanceProperties(v, "unused", &src, &error_abort);
+ visit_complete(v, &qobj);
+ visit_free(v);
+ return qobject_to_qdict(qobj);
+}
+
+static void enumerate_cpu_slots(SlotListState *s)
+{
+ DeviceSlotInfoList *r = NULL;
+ MachineState *ms = MACHINE(qdev_get_machine());
+ MachineClass *mc = MACHINE_GET_CLASS(ms);
+ HotpluggableCPUList *hcl;
+ HotpluggableCPUList *i;
+
+ if (!mc->has_hotpluggable_cpus) {
+ return;
+ }
+
+ hcl = machine_query_hotpluggable_cpus(ms);
+ for (i = hcl; i; i = i->next) {
+ DeviceSlotInfo *slot = g_new0(DeviceSlotInfo, 1);
+ HotpluggableCPU *hc = i->value;
+ QDict *props;
+
+ slot->device_types = g_new0(strList, 1);
+ slot->device_types->value = g_strdup(hc->type);
+ slot->available = !hc->has_qom_path;
+ if (hc->has_qom_path) {
+ slot->has_device = true;
+ slot->device = g_strdup(hc->qom_path);
+ }
+ slot->has_count = true;
+ slot->count = 1;
+ slot->opts_complete = true;
+ /*TODO: should 'hotpluggable' be always true? */
+ slot->hotpluggable = true;
+
+ props = cpu_instance_props_to_qdict(hc->props);
+ qdict_to_slot_props(props, slot);
+ QDECREF(props);
+
+ slot_list_add_slot(&r, slot);
+ }
+ qapi_free_HotpluggableCPUList(hcl);
+
+ append_slots(s, r);
+}
+
DeviceSlotInfoList *qmp_query_device_slots(Error **errp)
{
SlotListState s = { .next = &s.result };
object_child_foreach_recursive(qdev_get_machine(), enumerate_bus, &s);
+ enumerate_cpu_slots(&s);
return slot_list_collapse(s.result);
}
--
2.9.4
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [RFC v4 08/13] ide: enumerate_slots implementation
2017-08-14 21:57 [Qemu-devel] [RFC v4 00/13] qmp: query-device-slots command Eduardo Habkost
` (6 preceding siblings ...)
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 07/13] qdev: Enumerate CPU slots on query-device-slots Eduardo Habkost
@ 2017-08-14 21:57 ` Eduardo Habkost
2017-08-16 21:46 ` John Snow
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 09/13] pci: pci_bus_has_pcie_upstream_port() function Eduardo Habkost
` (6 subsequent siblings)
14 siblings, 1 reply; 26+ messages in thread
From: Eduardo Habkost @ 2017-08-14 21:57 UTC (permalink / raw)
To: Eric Blake, qemu-devel, Markus Armbruster, Michael S. Tsirkin,
Marcel Apfelbaum, Laine Stump
Cc: John Snow, qemu-block
Example output when using "-machine q35":
{
"available": true,
"count": 1,
"device-types": [
"ide-device"
],
"hotpluggable": false,
"opts": [
{ "option": "unit", "values": 0 },
{ "option": "bus", "values": "ide.2" }
],
"opts-complete": true
}
{
"available": false,
"count": 1,
"device": "/machine/unattached/device[19]",
"device-types": [
"ide-device"
],
"hotpluggable": false,
"opts": [
{ "option": "unit", "values": 1 },
{ "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
}
Cc: John Snow <jsnow@redhat.com>
Cc: qemu-block@nongnu.org
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
hw/ide/qdev.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index f17da1f..cc96f6f 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -25,6 +25,7 @@
#include "sysemu/block-backend.h"
#include "sysemu/blockdev.h"
#include "hw/block/block.h"
+#include "hw/qdev-slotinfo.h"
#include "sysemu/sysemu.h"
#include "qapi/visitor.h"
@@ -38,6 +39,30 @@ static Property ide_props[] = {
DEFINE_PROP_END_OF_LIST(),
};
+static DeviceSlotInfoList *ide_bus_enumerate_slots(BusState *bus)
+{
+ int unit;
+ DeviceSlotInfoList *r = NULL;
+ IDEBus *ib = IDE_BUS(bus);
+
+ for (unit = 0; unit < 2; unit++) {
+ DeviceSlotInfo *s = make_slot(bus);
+ IDEDevice *dev = (unit ? ib->master : ib->slave);
+ slot_add_opt_int(s, "unit", unit);
+ s->opts_complete = true;
+ s->has_count = true;
+ s->count = 1;
+ if (dev) {
+ s->available = false;
+ s->has_device = true;
+ s->device = object_get_canonical_path(OBJECT(dev));
+ }
+ slot_list_add_slot(&r, s);
+ }
+
+ return r;
+}
+
static void ide_bus_class_init(ObjectClass *klass, void *data)
{
BusClass *k = BUS_CLASS(klass);
@@ -45,6 +70,7 @@ static void ide_bus_class_init(ObjectClass *klass, void *data)
k->get_fw_dev_path = idebus_get_fw_dev_path;
k->unrealize = idebus_unrealize;
k->device_type = TYPE_IDE_DEVICE;
+ k->enumerate_slots = ide_bus_enumerate_slots;
}
static void idebus_unrealize(BusState *bus, Error **errp)
--
2.9.4
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [RFC v4 09/13] pci: pci_bus_has_pcie_upstream_port() function
2017-08-14 21:57 [Qemu-devel] [RFC v4 00/13] qmp: query-device-slots command Eduardo Habkost
` (7 preceding siblings ...)
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 08/13] ide: enumerate_slots implementation Eduardo Habkost
@ 2017-08-14 21:57 ` Eduardo Habkost
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 10/13] pci: device-number & function properties Eduardo Habkost
` (5 subsequent siblings)
14 siblings, 0 replies; 26+ messages in thread
From: Eduardo Habkost @ 2017-08-14 21:57 UTC (permalink / raw)
To: Eric Blake, qemu-devel, Markus Armbruster, Michael S. Tsirkin,
Marcel Apfelbaum, Laine Stump
The new helper will be useful when enumerating free slots on PCI
buses.
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Marcel Apfelbaum <marcel@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
hw/pci/pci.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 31b4d69..ead9cbf 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2591,9 +2591,9 @@ void pci_bus_get_w64_range(PCIBus *bus, Range *range)
pci_for_each_device_under_bus(bus, pci_dev_get_w64, range);
}
-static bool pcie_has_upstream_port(PCIDevice *dev)
+static bool pci_bus_has_pcie_upstream_port(PCIBus *bus)
{
- PCIDevice *parent_dev = pci_bridge_get_device(dev->bus);
+ PCIDevice *parent_dev = pci_bridge_get_device(bus);
/* Device associated with an upstream port.
* As there are several types of these, it's easier to check the
@@ -2609,7 +2609,7 @@ static bool pcie_has_upstream_port(PCIDevice *dev)
PCIDevice *pci_get_function_0(PCIDevice *pci_dev)
{
- if(pcie_has_upstream_port(pci_dev)) {
+ if(pci_bus_has_pcie_upstream_port(pci_dev->bus)) {
/* With an upstream PCIe port, we only support 1 device at slot 0 */
return pci_dev->bus->devices[0];
} else {
--
2.9.4
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [RFC v4 10/13] pci: device-number & function properties
2017-08-14 21:57 [Qemu-devel] [RFC v4 00/13] qmp: query-device-slots command Eduardo Habkost
` (8 preceding siblings ...)
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 09/13] pci: pci_bus_has_pcie_upstream_port() function Eduardo Habkost
@ 2017-08-14 21:57 ` Eduardo Habkost
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 11/13] pci: enumerate_slots implementation Eduardo Habkost
` (4 subsequent siblings)
14 siblings, 0 replies; 26+ messages in thread
From: Eduardo Habkost @ 2017-08-14 21:57 UTC (permalink / raw)
To: Eric Blake, qemu-devel, Markus Armbruster, Michael S. Tsirkin,
Marcel Apfelbaum, Laine Stump
The "addr" property on PCI devices has some magic for parsing it
as a single PCI device number (called "slot" internally), or a
"<slot>.<function>" string. Add simple integer properties that
can represent the device address with no special parsing.
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Marcel Apfelbaum <marcel@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
hw/pci/pci.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 63 insertions(+)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index ead9cbf..5753af3 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -41,6 +41,7 @@
#include "hw/hotplug.h"
#include "hw/boards.h"
#include "qemu/cutils.h"
+#include "qapi-visit.h"
//#define DEBUG_PCI
#ifdef DEBUG_PCI
@@ -2504,6 +2505,55 @@ MemoryRegion *pci_address_space_io(PCIDevice *dev)
return dev->bus->address_space_io;
}
+static void pci_device_get_devnr(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+ PCIDevice *dev = PCI_DEVICE(obj);
+ uint32_t devnr = PCI_SLOT(dev->devfn);
+
+ visit_type_uint32(v, "device-number", &devnr, errp);
+}
+
+static void pci_device_set_devnr(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+ PCIDevice *dev = PCI_DEVICE(obj);
+ uint32_t devnr;
+ Error *local_err = NULL;
+
+ visit_type_uint32(v, "device-number", &devnr, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+
+ dev->devfn = PCI_DEVFN(devnr, PCI_FUNC(dev->devfn));
+}
+
+static void pci_device_get_function(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+ PCIDevice *dev = PCI_DEVICE(obj);
+ uint32_t function = PCI_FUNC(dev->devfn);
+
+ visit_type_uint32(v, "function", &function, errp);
+}
+
+static void pci_device_set_function(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+ PCIDevice *dev = PCI_DEVICE(obj);
+ uint32_t function;
+ Error *local_err = NULL;
+
+ visit_type_uint32(v, "function", &function, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+ dev->devfn = PCI_DEVFN(PCI_SLOT(dev->devfn), function);
+}
+
static void pci_device_class_init(ObjectClass *klass, void *data)
{
DeviceClass *k = DEVICE_CLASS(klass);
@@ -2514,6 +2564,19 @@ static void pci_device_class_init(ObjectClass *klass, void *data)
k->bus_type = TYPE_PCI_BUS;
k->props = pci_props;
pc->realize = pci_default_realize;
+
+ /* Internally, bits 3:8 of devfn are called "slots", but:
+ * - they can be confused with physical slot numbers;
+ * - TYPE_PCIE_SLOT objects already have a "slot" property.
+ * So we use the terminology used in the PCI specifiction:
+ * "device number".
+ */
+ object_class_property_add(klass, "device-number", "uint32",
+ pci_device_get_devnr, pci_device_set_devnr,
+ NULL, NULL, &error_abort);
+ object_class_property_add(klass, "function", "uint32",
+ pci_device_get_function, pci_device_set_function,
+ NULL, NULL, &error_abort);
}
AddressSpace *pci_device_iommu_address_space(PCIDevice *dev)
--
2.9.4
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [RFC v4 11/13] pci: enumerate_slots implementation
2017-08-14 21:57 [Qemu-devel] [RFC v4 00/13] qmp: query-device-slots command Eduardo Habkost
` (9 preceding siblings ...)
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 10/13] pci: device-number & function properties Eduardo Habkost
@ 2017-08-14 21:57 ` Eduardo Habkost
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 12/13] usb: " Eduardo Habkost
` (3 subsequent siblings)
14 siblings, 0 replies; 26+ messages in thread
From: Eduardo Habkost @ 2017-08-14 21:57 UTC (permalink / raw)
To: Eric Blake, qemu-devel, Markus Armbruster, Michael S. Tsirkin,
Marcel Apfelbaum, Laine Stump
Enumerate each PCI function as a separate slots, because that's
how QEMU treats multi-function PCI devices.
Example output using "-machine q35" is below.
{
"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": false,
"count": 1,
"device": "/machine/unattached/device[30]",
"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/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": 1,
"device": "/machine/unattached/device[29]",
"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[3]",
"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": 1,
"device": "/machine/unattached/device[18]",
"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[20]",
"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": 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": 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
}
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Marcel Apfelbaum <marcel@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
hw/pci/pci.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 50 insertions(+)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 5753af3..ae268a9 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -27,6 +27,7 @@
#include "hw/pci/pci_bridge.h"
#include "hw/pci/pci_bus.h"
#include "hw/pci/pci_host.h"
+#include "hw/qdev-slotinfo.h"
#include "monitor/monitor.h"
#include "net/net.h"
#include "sysemu/sysemu.h"
@@ -144,6 +145,54 @@ static uint16_t pcibus_numa_node(PCIBus *bus)
return NUMA_NODE_UNASSIGNED;
}
+
+static bool pci_bus_has_pcie_upstream_port(PCIBus *bus);
+
+static DeviceSlotInfoList *pci_bus_enumerate_slots(BusState *bus)
+{
+ PCIBus *pb = PCI_BUS(bus);
+ int devnr, devnrs;
+ DeviceSlotInfoList *r = NULL;
+
+ if (pci_bus_has_pcie_upstream_port(pb)) {
+ devnrs = 1;
+ } else {
+ devnrs = PCI_SLOT_MAX;
+ }
+
+ /* Each PCI devfn (device number + function) is a separate slot,
+ * because we implement multi-function PCI devices as separate
+ * device objects.
+ */
+ for(devnr = PCI_SLOT(pb->devfn_min); devnr < devnrs; devnr++) {
+ PCIDevice *dev0 = pb->devices[PCI_DEVFN(devnr, 0)];
+ int func;
+
+ for (func = 0; func < PCI_FUNC_MAX; func++) {
+ /*TODO: add info about accepting only bridges on extra PCI root buses */
+ PCIDevice *dev = pb->devices[PCI_DEVFN(devnr, func)];
+ DeviceSlotInfo *s = make_slot(bus);
+ slot_add_opt_int(s, "device-number", devnr);
+ slot_add_opt_int(s, "function", func);
+ s->opts_complete = true;
+ s->has_count = true;
+ s->count = 1;
+ /* Conditions that make a devnr unavailable:
+ * - function already occupied
+ * - function 0 already occupied by a device
+ */
+ s->available &= !dev0 && !dev;
+ if (dev) {
+ s->has_device = true;
+ s->device = object_get_canonical_path(OBJECT(dev));
+ }
+ slot_list_add_slot(&r, s);
+ }
+ }
+
+ return r;
+}
+
static void pci_bus_class_init(ObjectClass *klass, void *data)
{
BusClass *k = BUS_CLASS(klass);
@@ -156,6 +205,7 @@ static void pci_bus_class_init(ObjectClass *klass, void *data)
k->unrealize = pci_bus_unrealize;
k->reset = pcibus_reset;
k->device_type = TYPE_PCI_DEVICE;
+ k->enumerate_slots = pci_bus_enumerate_slots;
pbc->is_root = pcibus_is_root;
pbc->bus_num = pcibus_num;
--
2.9.4
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [RFC v4 12/13] usb: enumerate_slots implementation
2017-08-14 21:57 [Qemu-devel] [RFC v4 00/13] qmp: query-device-slots command Eduardo Habkost
` (10 preceding siblings ...)
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 11/13] pci: enumerate_slots implementation Eduardo Habkost
@ 2017-08-14 21:57 ` Eduardo Habkost
2017-08-21 11:44 ` Gerd Hoffmann
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 13/13] tests: Experimental query-device-slots test code Eduardo Habkost
` (2 subsequent siblings)
14 siblings, 1 reply; 26+ messages in thread
From: Eduardo Habkost @ 2017-08-14 21:57 UTC (permalink / raw)
To: Eric Blake, qemu-devel, Markus Armbruster, Michael S. Tsirkin,
Marcel Apfelbaum, Laine Stump
Cc: Gerd Hoffmann
Example output for QEMU command-line:
$ qemu-system-x86_64 -machine q35,usb=on -S \
-device usb-hub,port=2 -device usb-tablet,port=2.4
Output:
{"available":true,"count":12,
"device-types":["usb-device"],"hotpluggable":true,
"opts":[
{"option":"port","values":["1","3","4","5","6","2.1","2.2","2.3","2.5","2.6","2.7","2.8"]},
{"option":"bus","values":"usb-bus.0"}],
"opts-complete":true}
{"available":false,"count":1,"device":"/machine/peripheral-anon/device[1]",
"device-types":["usb-device"],"hotpluggable":true,
"opts":[{"option":"port","values":"2"},{"option":"bus","values":"usb-bus.0"}],
"opts-complete":true}
{"available":false,"count":1,"device":"/machine/peripheral-anon/device[2]",
"device-types":["usb-device"],"hotpluggable":true,
"opts":[{"option":"port","values":"2.4"},{"option":"bus","values":"usb-bus.0"}],
"opts-complete":true}
TODO: Introduce USBPortList typedef in a separate patch
TODO: decide if "bus" should be included in the output, or we
should make "port" the only parameter required to identify where
the device will be plugged.
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
include/hw/usb.h | 6 ++++--
hw/usb/bus.c | 36 ++++++++++++++++++++++++++++++++++++
2 files changed, 40 insertions(+), 2 deletions(-)
diff --git a/include/hw/usb.h b/include/hw/usb.h
index eb28655..f8119c9 100644
--- a/include/hw/usb.h
+++ b/include/hw/usb.h
@@ -509,14 +509,16 @@ void musb_set_size(MUSBState *s, int epnum, int size, int is_tx);
#define TYPE_USB_BUS "usb-bus"
#define USB_BUS(obj) OBJECT_CHECK(USBBus, (obj), TYPE_USB_BUS)
+typedef QTAILQ_HEAD(, USBPort) USBPortList;
+
struct USBBus {
BusState qbus;
USBBusOps *ops;
int busnr;
int nfree;
int nused;
- QTAILQ_HEAD(, USBPort) free;
- QTAILQ_HEAD(, USBPort) used;
+ USBPortList free;
+ USBPortList used;
QTAILQ_ENTRY(USBBus) next;
};
diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index 42f85a7..fd00269 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -2,6 +2,7 @@
#include "hw/hw.h"
#include "hw/usb.h"
#include "hw/qdev.h"
+#include "hw/qdev-slotinfo.h"
#include "qapi/error.h"
#include "qemu/error-report.h"
#include "sysemu/sysemu.h"
@@ -25,6 +26,40 @@ static Property usb_props[] = {
DEFINE_PROP_END_OF_LIST()
};
+static void usb_bus_enumerate_slot_list(DeviceSlotInfoList **r, USBBus *bus, USBPortList *l)
+{
+ USBPort *port;
+
+ QTAILQ_FOREACH(port, l, next) {
+ DeviceSlotInfo *slot = make_slot(BUS(bus));
+ /*
+ * TODO: should the "bus" option be included, or is
+ * "port" enough to identify the USB bus + port?
+ */
+ slot_add_opt_str(slot, "port", port->path);
+ slot->opts_complete = slot;
+ if (port->dev) {
+ slot->has_device = true;
+ slot->device = object_get_canonical_path(OBJECT(port->dev));
+ slot->available = false;
+ }
+ slot->has_count = 1;
+ slot->count = 1;
+
+ slot_list_add_slot(r, slot);
+ }
+}
+
+static DeviceSlotInfoList *usb_bus_enumerate_slots(BusState *bus)
+{
+ USBBus *ub = USB_BUS(bus);
+ DeviceSlotInfoList *r = NULL;
+
+ usb_bus_enumerate_slot_list(&r, ub, &ub->free);
+ usb_bus_enumerate_slot_list(&r, ub, &ub->used);
+ return r;
+}
+
static void usb_bus_class_init(ObjectClass *klass, void *data)
{
BusClass *k = BUS_CLASS(klass);
@@ -34,6 +69,7 @@ static void usb_bus_class_init(ObjectClass *klass, void *data)
k->get_dev_path = usb_get_dev_path;
k->get_fw_dev_path = usb_get_fw_dev_path;
k->device_type = TYPE_USB_DEVICE;
+ k->enumerate_slots = usb_bus_enumerate_slots;
hc->unplug = qdev_simple_device_unplug_cb;
}
--
2.9.4
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [RFC v4 13/13] tests: Experimental query-device-slots test code
2017-08-14 21:57 [Qemu-devel] [RFC v4 00/13] qmp: query-device-slots command Eduardo Habkost
` (11 preceding siblings ...)
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 12/13] usb: " Eduardo Habkost
@ 2017-08-14 21:57 ` Eduardo Habkost
2017-08-14 22:37 ` [Qemu-devel] [libvirt] [RFC v4 00/13] qmp: query-device-slots command no-reply
2017-08-15 18:57 ` [Qemu-devel] " Eric Blake
14 siblings, 0 replies; 26+ messages in thread
From: Eduardo Habkost @ 2017-08-14 21:57 UTC (permalink / raw)
To: Eric Blake, qemu-devel, Markus Armbruster, Michael S. Tsirkin,
Marcel Apfelbaum, Laine Stump
The test script is crude and full of hacks. Included in the RFC
just in case somebody wants to try it.
The script uses environment variables as arguments to make it
easier to be invoked from the tests/ Makefile later.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
tests/qmp-machine-info.py | 300 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 300 insertions(+)
create mode 100755 tests/qmp-machine-info.py
diff --git a/tests/qmp-machine-info.py b/tests/qmp-machine-info.py
new file mode 100755
index 0000000..ad4a0ac
--- /dev/null
+++ b/tests/qmp-machine-info.py
@@ -0,0 +1,300 @@
+#!/usr/bin/env python
+import sys, os
+MY_DIR = os.path.dirname(__file__)
+sys.path.append(os.path.join(MY_DIR, '..', 'scripts'))
+import qtest
+import unittest
+import logging
+import argparse
+import itertools
+import operator
+import re
+
+logger = logging.getLogger('qemu.tests.machineinfo')
+
+# machines that we can't easily test because they can't run on all hosts:
+BLACKLIST = set(['xenpv', 'xenfv'])
+
+# machines known to be broken when using -nodefaults:
+NODEFAULTS_BLACKLIST = set([
+ 'cubieboard', # segfaults
+ 'petalogix-ml605', # segfaults
+ 'or32-sim', # segfaults
+ 'virtex-ml507', # segfaults
+ 'Niagara', # segfaults
+ 'akita', # "qemu: missing SecureDigital device"
+ 'borzoi', # "qemu: missing SecureDigital device"
+ 'cheetah', # "qemu: missing SecureDigital device"
+ 'connex', # "qemu: missing SecureDigital device"
+ 'mainstone', # "qemu: missing SecureDigital device"
+ 'n800', # "qemu: missing SecureDigital device"
+ 'n810', # "qemu: missing SecureDigital device"
+ 'spitz', # "qemu: missing SecureDigital device"
+ 'sx1', # "qemu: missing SecureDigital device"
+ 'sx1-v1', # "qemu: missing SecureDigital device"
+ 'terrier', # "qemu: missing SecureDigital device"
+ 'tosa', # "qemu: missing SecureDigital device"
+ 'verdex', # "qemu: missing SecureDigital device"
+ 'z2', # "qemu: missing SecureDigital device"
+])
+
+# iterators for QAPI ValueSets:
+# all of the iterators below should support iter() and len()
+
+class InvalidValueSet(Exception):
+ pass
+
+class ElementIterator:
+ def __init__(self, e):
+ self._e = e
+
+ def _data(self):
+ e = self._e
+ if type(e) is not list:
+ e = [e]
+ if len(e) == 1:
+ return e
+ elif len(e) == 2:
+ return xrange(e[0], e[1] + 1)
+ else:
+ raise InvalidValueSet
+
+ def __iter__(self):
+ return iter(self._data())
+
+ def __len__(self):
+ return len(self._data())
+
+class ValuesIterator:
+ def __init__(self, values):
+ if type(values) is not list:
+ values = [values]
+ self._values = values
+
+ def _iterators(self):
+ return map(ElementIterator, self._values)
+
+ def __iter__(self):
+ return itertools.chain(self._iterators())
+
+ def __len__(self):
+ return sum(map(len, self._iterators()))
+
+
+#TODO: move to common code
+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 QueryMachinesTest(unittest.TestCase):
+ def setUp(self):
+ self.vm = None
+
+ def tearDown(self):
+ if self.vm is not None:
+ self.vm.shutdown()
+
+ def walkQOMTree(self, vm, path):
+ """Walk QOM tree recusrively, starting at path"""
+ children = vm.qmp('qom-list', path=path)['return']
+ for c in children:
+ logging.debug('walking %s. child: %s', path, c)
+ if not c['type'].startswith('child<'):
+ continue
+
+ cp = '%s/%s' % (path, c['name'])
+ yield cp
+
+ for gc in self.walkQOMTree(vm, cp):
+ yield gc
+
+ def findAllBuses(self, vm):
+ """Find all bus objects in the QOM tree"""
+ r = vm.qmp('qom-list-types', implements='bus')
+ bus_types = set([b['name'] for b in r['return']])
+ for cp in self.walkQOMTree(vm, '/machine'):
+ t = vm.qmp('qom-get', path=cp, property='type')['return']
+ if t in bus_types:
+ yield dict(path=cp, type=t)
+
+ def getSubtypes(self, implements, **kwargs):
+ """Get full list of typenames of subtypes of @implements"""
+ types = self.vm.command('qom-list-types', implements=implements, **kwargs)
+ return [st['name'] for st in types]
+
+
+ def getUserCreatableSubtypes(self, devtype):
+ alldevs = set(self.getSubtypes(devtype, abstract=False))
+ # there's no way to query DeviceClass::user_creatable using QMP,
+ # so use 'info qdm':
+ no_user_devs = set([d['name'] for d in infoQDM(self.vm, ) if d['no-user']])
+ user_devs = alldevs.difference(no_user_devs)
+ return user_devs
+
+ def typeImplements(self, t, i):
+ """Returns True if type @t implements type @i"""
+ stypes = self.getSubtypes(i)
+ return t in stypes
+
+ def validateBus(self, bus, slots):
+ """Check if the bus identified by the slot matches the information returned
+ for the slot.
+
+ TODO: check if it is really a bus
+ TODO: check if device-types matches device-types
+ property in the bus
+ """
+
+ ##we could do this:
+ #bustype = self.vm.command('qom-get', path=bus, property='type')
+ #self.assertTrue(self.typeImplements(bustype, 'bus'))
+ ## but the bus _name_ (accepted by qbus_find()) does not necessarily matches the bus _path_
+
+ pass
+
+ def checkSlotProps(self, slots):
+ """check if all properties on "props" are valid properties
+ that appear on device-list-properties for all accepted device types
+ """
+ types_to_check = {}
+ buses_to_check = {}
+ for slot in slots:
+ for prop in slot['opts']:
+ if prop['option'] == 'bus':
+ values = ValuesIterator(bus['values'])
+ self.assertEquals(len(values), 1)
+ bus = values[0]
+ buses_to_check.setdefault(v, []).append(slot)
+
+ for t in slot['device-types']:
+ types_to_check.setdefault(t, set()).update(slot['opts'].keys())
+
+ for bus,slots in buses_to_check.items():
+ self.validateBus(bus, slots)
+
+ for t, props in types_to_check.items():
+ props.discard('bus') # 'bus' is handled by device_add directly
+ for st in self.vm.command('qom-list-types', implements=t, abstract=False):
+ dprops = self.vm.command('device-list-properties', typename=st['name'])
+ dpropnames = set([p['name'] for p in dprops])
+ for p in props:
+ self.assertIn(p, dpropnames)
+
+ def checkAvailableField(self, slots):
+ for slot in slots:
+ if slot.has_key('max-devices') and len(slot['devices']) >= slot['max-devices']:
+ self.assertFalse(slot['available'])
+
+ def checkSlotInfo(self, args):
+ #TODO: check if:
+ # * -device works with at least one device type
+ # * query-hotpluggable-cpus matches what's in query-device-slots
+ # * device-types match the property on the bus
+ # * available=false if hotpluggable=false
+ # * 'count' is always set if not incomplete
+ # * slot count is <= set of possible values for @props
+ self.vm = qtest.QEMUQtestMachine(self.binary, args=args)
+ self.vm.launch()
+
+ slots = self.vm.command('query-device-slots')
+ #self.checkSlotProps(slots)
+ #self.checkSlotDevices(slots)
+ #self.checkAvailableField(slots)
+
+ for slot in slots:
+ logging.debug('slot: %r', slot)
+ if 'device' in slot:
+ dev = slot['device']
+ self.assertFalse(slot['available'])
+ dtype = self.vm.command('qom-get', path=dev, property='type')
+ self.assertTrue(any(self.typeImplements(dtype, t) for t in slot['device-types']))
+
+ for dt in slot['device-types']:
+ self.assertTrue(len(self.getUserCreatableSubtypes(dt)) > 0, "There's no user-creatable subtype of %s" % (dt))
+
+ if slot['opts-complete']:
+ self.assertTrue('count' in slot)
+
+ all_counts = [len(ValuesIterator(p['values'])) for p in slot['opts']]
+ total_count = reduce(operator.mul, all_counts, 1)
+ logging.debug('%d possible values', total_count)
+ self.assertEquals(total_count, slot['count'])
+
+ self.vm.shutdown()
+
+ def machineTestSlotInfo(self):
+ if self.machine['name'] in BLACKLIST:
+ self.skipTest("machine %s on BLACKLIST" % (self.machine['name']))
+
+ args = ['-S', '-machine', self.machine['name']]
+ self.checkSlotInfo(args)
+
+ #TODO: also test using full config from docs/config/q35-*.cfg
+
+ @classmethod
+ def addMachineTest(klass, method_name, binary, machine):
+ """Dynamically add a testMachine_<arch>_<name>_<machine> method to the class"""
+ method = getattr(klass, method_name)
+ def testMachine(self):
+ self.binary = binary
+ self.machine = machine
+ return method(self)
+ machine_name = machine['name'].replace('-', '_').replace('.', '_')
+ method_name = 'test_%s_%s_%s' % (method_name, machine['arch'], machine_name)
+ setattr(klass, method_name, testMachine)
+ return method_name
+
+
+ @classmethod
+ def discoverMachines(klass, binary):
+ """Run query-machines
+
+ This method is run before test cases are started, so we
+ can dynamically add test cases for each machine supported
+ by the binary.
+ """
+ vm = qtest.QEMUQtestMachine(binary=binary, args=['-S', '-machine', 'none'])
+ vm.launch()
+ try:
+ arch = vm.qmp('query-target')['return']['arch']
+ machines = vm.qmp('query-machines')['return']
+ for m in machines:
+ m['arch'] = arch
+ finally:
+ vm.shutdown()
+ return machines
+
+ @classmethod
+ def addMachineTests(klass, binary):
+ """Dynamically add test methods for each machine found on QEMU binary
+
+ Look for all methods with "machineTest" prefix, and add
+ custom test methods that will test them, for each machine-type
+ found on QEMU binary 'binary'.
+ """
+ method_names = unittest.loader.getTestCaseNames(klass, prefix='machineTest')
+ machines = klass.discoverMachines(binary)
+ for machine in machines:
+ for mname in method_names:
+ klass.addMachineTest(mname, binary, machine)
+
+
+if os.getenv('QTEST_QEMU_BINARY'):
+ QueryMachinesTest.addMachineTests(os.getenv('QTEST_QEMU_BINARY'))
+
+if __name__ == '__main__':
+ if os.getenv('QTEST_LOG_LEVEL'):
+ logging.basicConfig(level=int(os.getenv('QTEST_LOG_LEVEL')))
+ else:
+ logging.basicConfig(level=logging.WARN)
+ unittest.main()
--
2.9.4
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [libvirt] [RFC v4 00/13] qmp: query-device-slots command
2017-08-14 21:57 [Qemu-devel] [RFC v4 00/13] qmp: query-device-slots command Eduardo Habkost
` (12 preceding siblings ...)
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 13/13] tests: Experimental query-device-slots test code Eduardo Habkost
@ 2017-08-14 22:37 ` no-reply
2017-08-15 18:57 ` [Qemu-devel] " Eric Blake
14 siblings, 0 replies; 26+ messages in thread
From: no-reply @ 2017-08-14 22:37 UTC (permalink / raw)
To: ehabkost
Cc: famz, eblake, qemu-devel, armbru, mst, marcel, laine,
libvir-list, imammedo
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
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [RFC v4 02/13] qapi: qobject_compare() helper
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 02/13] qapi: qobject_compare() helper Eduardo Habkost
@ 2017-08-15 16:16 ` Eric Blake
2017-08-15 17:59 ` Eduardo Habkost
0 siblings, 1 reply; 26+ messages in thread
From: Eric Blake @ 2017-08-15 16:16 UTC (permalink / raw)
To: Eduardo Habkost, qemu-devel, Markus Armbruster,
Michael S. Tsirkin, Marcel Apfelbaum, Laine Stump, Max Reitz
[-- Attachment #1: Type: text/plain, Size: 2635 bytes --]
On 08/14/2017 04:57 PM, Eduardo Habkost wrote:
> The helper function will be useful when writing support code to
> deal with device slot information.
>
> TODO: documentation is incomplete and unclear, needs to be
> improved.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> include/qapi/util.h | 39 +++++++++++++++++++++++++++++
> qapi/qapi-util.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++
> tests/test-qapi-util.c | 53 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 158 insertions(+)
>
> +/**
> + * qobject_compare:
> + *
> + * Compare the value of @a and @b.
> + *
> + * If @a and @b have the same type and the same value (see list
> + * of supported types below), return 0.
> + *
> + * If @a and @b are both strings, return strcmp(a, b).
> + *
> + * If @a and @b are numbers, return a negative value if a < b,
> + * and a positive value if a > b.
> + *
> + * Otherwise (if @a and @b are not the same, have different types,
> + * are of an unsupported type, or are different), return a non-zero value.
Is this number going to be commutative and distributive, in order to
provide stable qsort()ing? That is, if comparing a and b gives a
positive number, then comparing b and a should give a negative number;
and if comparing a and b then b and c results in two positive numbers,
then comparing a and c should also give a positive number. It is
unclear from the documentation whether you are able to make this
guarantee; and without it, it is unsafe to use this comparator in places
that require stability.
> + *
> + * Note that this function doesn't support some types, and may
> + * return false if the types are unsupported, or if the types don't
> + * match exactly.
How is a return of false (== 0, which also means equivalent) correct?
> + *
> + * Supported types:
> + * - QTYPE_QNULL
> + * - QTYPE_QSTRING
> + * - QTYPE_QBOOL
> + * - QTYPE_QNUM (integers only)
> + * - QTYPE_QLIST
> + *
> + * Unsupported (always return false):
> + * - QTYPE_QNUM (non-integer values)
> + * - QTYPE_QDICT
> + *
> + * TODO: rewrite documentation to be clearer.
> + * TODO: support non-integer QTYPE_NUM values and QTYPE_QDICT.
There's another patch series pending for qobject_is_equal(); should
these two patches share approaches or even code?
https://lists.gnu.org/archive/html/qemu-devel/2017-07/msg01134.html
https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg02459.html
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [RFC v4 02/13] qapi: qobject_compare() helper
2017-08-15 16:16 ` Eric Blake
@ 2017-08-15 17:59 ` Eduardo Habkost
0 siblings, 0 replies; 26+ messages in thread
From: Eduardo Habkost @ 2017-08-15 17:59 UTC (permalink / raw)
To: Eric Blake
Cc: qemu-devel, Markus Armbruster, Michael S. Tsirkin,
Marcel Apfelbaum, Laine Stump, Max Reitz
On Tue, Aug 15, 2017 at 11:16:57AM -0500, Eric Blake wrote:
> On 08/14/2017 04:57 PM, Eduardo Habkost wrote:
> > The helper function will be useful when writing support code to
> > deal with device slot information.
> >
> > TODO: documentation is incomplete and unclear, needs to be
> > improved.
> >
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> > include/qapi/util.h | 39 +++++++++++++++++++++++++++++
> > qapi/qapi-util.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++
> > tests/test-qapi-util.c | 53 ++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 158 insertions(+)
> >
>
> > +/**
> > + * qobject_compare:
> > + *
> > + * Compare the value of @a and @b.
> > + *
> > + * If @a and @b have the same type and the same value (see list
> > + * of supported types below), return 0.
> > + *
> > + * If @a and @b are both strings, return strcmp(a, b).
> > + *
> > + * If @a and @b are numbers, return a negative value if a < b,
> > + * and a positive value if a > b.
> > + *
> > + * Otherwise (if @a and @b are not the same, have different types,
> > + * are of an unsupported type, or are different), return a non-zero value.
>
> Is this number going to be commutative and distributive, in order to
> provide stable qsort()ing? That is, if comparing a and b gives a
> positive number, then comparing b and a should give a negative number;
> and if comparing a and b then b and c results in two positive numbers,
> then comparing a and c should also give a positive number. It is
> unclear from the documentation whether you are able to make this
> guarantee; and without it, it is unsafe to use this comparator in places
> that require stability.
No, I don't make that guarantee when the types don't match or in
the case of unsupported types. That's a limitation of this
implementation.
Guaranteeing it when types don't match should be easy.
Guaranteeing it in the case of QTYPE_QDICT looks a bit harder.
Probably it's easier to simply implement full dictionary
comparison.
>
> > + *
> > + * Note that this function doesn't support some types, and may
> > + * return false if the types are unsupported, or if the types don't
> > + * match exactly.
>
> How is a return of false (== 0, which also means equivalent) correct?
This is a documentation bug. Leftover from a version that
returned a boolean value (true for equal, false for not equal) in
the past.
>
> > + *
> > + * Supported types:
> > + * - QTYPE_QNULL
> > + * - QTYPE_QSTRING
> > + * - QTYPE_QBOOL
> > + * - QTYPE_QNUM (integers only)
> > + * - QTYPE_QLIST
> > + *
> > + * Unsupported (always return false):
> > + * - QTYPE_QNUM (non-integer values)
> > + * - QTYPE_QDICT
> > + *
> > + * TODO: rewrite documentation to be clearer.
> > + * TODO: support non-integer QTYPE_NUM values and QTYPE_QDICT.
>
> There's another patch series pending for qobject_is_equal(); should
> these two patches share approaches or even code?
>
> https://lists.gnu.org/archive/html/qemu-devel/2017-07/msg01134.html
> https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg02459.html
I will take a look. Thanks!
--
Eduardo
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [RFC v4 00/13] qmp: query-device-slots command
2017-08-14 21:57 [Qemu-devel] [RFC v4 00/13] qmp: query-device-slots command Eduardo Habkost
` (13 preceding siblings ...)
2017-08-14 22:37 ` [Qemu-devel] [libvirt] [RFC v4 00/13] qmp: query-device-slots command no-reply
@ 2017-08-15 18:57 ` Eric Blake
2017-08-15 19:44 ` Eduardo Habkost
14 siblings, 1 reply; 26+ messages in thread
From: Eric Blake @ 2017-08-15 18:57 UTC (permalink / raw)
To: Eduardo Habkost, qemu-devel, Markus Armbruster,
Michael S. Tsirkin, Marcel Apfelbaum, Laine Stump
Cc: libvir-list, Igor Mammedov
[-- Attachment #1: Type: text/plain, Size: 1728 bytes --]
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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [RFC v4 00/13] qmp: query-device-slots command
2017-08-15 18:57 ` [Qemu-devel] " Eric Blake
@ 2017-08-15 19:44 ` Eduardo Habkost
0 siblings, 0 replies; 26+ messages in thread
From: Eduardo Habkost @ 2017-08-15 19:44 UTC (permalink / raw)
To: Eric Blake
Cc: qemu-devel, Markus Armbruster, Michael S. Tsirkin,
Marcel Apfelbaum, Laine Stump, libvir-list, Igor Mammedov
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
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [RFC v4 08/13] ide: enumerate_slots implementation
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 08/13] ide: enumerate_slots implementation Eduardo Habkost
@ 2017-08-16 21:46 ` John Snow
2017-08-17 4:54 ` Markus Armbruster
2017-08-18 16:57 ` Eduardo Habkost
0 siblings, 2 replies; 26+ messages in thread
From: John Snow @ 2017-08-16 21:46 UTC (permalink / raw)
To: Eduardo Habkost, Eric Blake, qemu-devel, Markus Armbruster,
Michael S. Tsirkin, Marcel Apfelbaum, Laine Stump
Cc: qemu-block
On 08/14/2017 05:57 PM, Eduardo Habkost wrote:
> Example output when using "-machine q35":
>
> {
> "available": true,
> "count": 1,
> "device-types": [
> "ide-device"
> ],
> "hotpluggable": false,
> "opts": [
> { "option": "unit", "values": 0 },
> { "option": "bus", "values": "ide.2" }
> ],
> "opts-complete": true
> }
> {
> "available": false,
> "count": 1,
> "device": "/machine/unattached/device[19]",
> "device-types": [
> "ide-device"
> ],
> "hotpluggable": false,
> "opts": [
> { "option": "unit", "values": 1 },
> { "option": "bus", "values": "ide.2" } ],
> "opts-complete": true
> }
> {
> "available": true,
> "count": 10,
> "device-types": [
> "ide-device"
> ],
> "hotpluggable": false,
> "opts": [
> { "option": "unit", "values": [ [ 0, 1 ] ] },
Hm, these unit values aren't really correct -- we do not support
primary/secondary semantics for IDE buses on the AHCI device. I guess
they technically exist, but you cannot use them for anything.
Should I do something to "disable" or otherwise hide the unusable
secondary unit slots for AHCI devices?
--js
> { "option": "bus", "values": [ "ide.4", "ide.3", "ide.5", "ide.0", "ide.1" ] }
> ],
> "opts-complete": true
> }
>
> Cc: John Snow <jsnow@redhat.com>
> Cc: qemu-block@nongnu.org
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> hw/ide/qdev.c | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
> index f17da1f..cc96f6f 100644
> --- a/hw/ide/qdev.c
> +++ b/hw/ide/qdev.c
> @@ -25,6 +25,7 @@
> #include "sysemu/block-backend.h"
> #include "sysemu/blockdev.h"
> #include "hw/block/block.h"
> +#include "hw/qdev-slotinfo.h"
> #include "sysemu/sysemu.h"
> #include "qapi/visitor.h"
>
> @@ -38,6 +39,30 @@ static Property ide_props[] = {
> DEFINE_PROP_END_OF_LIST(),
> };
>
> +static DeviceSlotInfoList *ide_bus_enumerate_slots(BusState *bus)
> +{
> + int unit;
> + DeviceSlotInfoList *r = NULL;
> + IDEBus *ib = IDE_BUS(bus);
> +
> + for (unit = 0; unit < 2; unit++) {
> + DeviceSlotInfo *s = make_slot(bus);
> + IDEDevice *dev = (unit ? ib->master : ib->slave);
> + slot_add_opt_int(s, "unit", unit);
> + s->opts_complete = true;
> + s->has_count = true;
> + s->count = 1;
> + if (dev) {
> + s->available = false;
> + s->has_device = true;
> + s->device = object_get_canonical_path(OBJECT(dev));
> + }
> + slot_list_add_slot(&r, s);
> + }
> +
> + return r;
> +}
> +
> static void ide_bus_class_init(ObjectClass *klass, void *data)
> {
> BusClass *k = BUS_CLASS(klass);
> @@ -45,6 +70,7 @@ static void ide_bus_class_init(ObjectClass *klass, void *data)
> k->get_fw_dev_path = idebus_get_fw_dev_path;
> k->unrealize = idebus_unrealize;
> k->device_type = TYPE_IDE_DEVICE;
> + k->enumerate_slots = ide_bus_enumerate_slots;
> }
>
> static void idebus_unrealize(BusState *bus, Error **errp)
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [RFC v4 08/13] ide: enumerate_slots implementation
2017-08-16 21:46 ` John Snow
@ 2017-08-17 4:54 ` Markus Armbruster
2017-08-17 18:40 ` John Snow
2017-08-18 16:57 ` Eduardo Habkost
1 sibling, 1 reply; 26+ messages in thread
From: Markus Armbruster @ 2017-08-17 4:54 UTC (permalink / raw)
To: John Snow
Cc: Eduardo Habkost, Eric Blake, qemu-devel, Michael S. Tsirkin,
Marcel Apfelbaum, Laine Stump, qemu-block
John Snow <jsnow@redhat.com> writes:
> On 08/14/2017 05:57 PM, Eduardo Habkost wrote:
>> Example output when using "-machine q35":
>>
>> {
>> "available": true,
>> "count": 1,
>> "device-types": [
>> "ide-device"
>> ],
>> "hotpluggable": false,
>> "opts": [
>> { "option": "unit", "values": 0 },
>> { "option": "bus", "values": "ide.2" }
>> ],
>> "opts-complete": true
>> }
>> {
>> "available": false,
>> "count": 1,
>> "device": "/machine/unattached/device[19]",
>> "device-types": [
>> "ide-device"
>> ],
>> "hotpluggable": false,
>> "opts": [
>> { "option": "unit", "values": 1 },
>> { "option": "bus", "values": "ide.2" } ],
>> "opts-complete": true
>> }
>> {
>> "available": true,
>> "count": 10,
>> "device-types": [
>> "ide-device"
>> ],
>> "hotpluggable": false,
>> "opts": [
>> { "option": "unit", "values": [ [ 0, 1 ] ] },
>
> Hm, these unit values aren't really correct -- we do not support
> primary/secondary semantics for IDE buses on the AHCI device. I guess
> they technically exist, but you cannot use them for anything.
>
> Should I do something to "disable" or otherwise hide the unusable
> secondary unit slots for AHCI devices?
I think that would make this introspection more useful.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [RFC v4 08/13] ide: enumerate_slots implementation
2017-08-17 4:54 ` Markus Armbruster
@ 2017-08-17 18:40 ` John Snow
0 siblings, 0 replies; 26+ messages in thread
From: John Snow @ 2017-08-17 18:40 UTC (permalink / raw)
To: Markus Armbruster
Cc: Eduardo Habkost, Eric Blake, qemu-devel, Michael S. Tsirkin,
Marcel Apfelbaum, Laine Stump, qemu-block
On 08/17/2017 12:54 AM, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:
>
>> On 08/14/2017 05:57 PM, Eduardo Habkost wrote:
>>> Example output when using "-machine q35":
>>>
>>> {
>>> "available": true,
>>> "count": 1,
>>> "device-types": [
>>> "ide-device"
>>> ],
>>> "hotpluggable": false,
>>> "opts": [
>>> { "option": "unit", "values": 0 },
>>> { "option": "bus", "values": "ide.2" }
>>> ],
>>> "opts-complete": true
>>> }
>>> {
>>> "available": false,
>>> "count": 1,
>>> "device": "/machine/unattached/device[19]",
>>> "device-types": [
>>> "ide-device"
>>> ],
>>> "hotpluggable": false,
>>> "opts": [
>>> { "option": "unit", "values": 1 },
>>> { "option": "bus", "values": "ide.2" } ],
>>> "opts-complete": true
>>> }
>>> {
>>> "available": true,
>>> "count": 10,
>>> "device-types": [
>>> "ide-device"
>>> ],
>>> "hotpluggable": false,
>>> "opts": [
>>> { "option": "unit", "values": [ [ 0, 1 ] ] },
>>
>> Hm, these unit values aren't really correct -- we do not support
>> primary/secondary semantics for IDE buses on the AHCI device. I guess
>> they technically exist, but you cannot use them for anything.
>>
>> Should I do something to "disable" or otherwise hide the unusable
>> secondary unit slots for AHCI devices?
>
> I think that would make this introspection more useful.
>
OK, let me look into how to meaningfully disable them. I didn't think
they were addressable in any tangible way currently, you certainly can't
put devices on them. I'm not sure where these values are coming from --
I'll play with the patches.
The only reason they even technically exist is because IDEBus comes
indelibly with two IDEStates baked in -- it can't be changed currently,
because everything's all wonky in IDE land.
I really ought to just rip off the bandaid and do an aggressive
refactoring ...... :|
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [RFC v4 08/13] ide: enumerate_slots implementation
2017-08-16 21:46 ` John Snow
2017-08-17 4:54 ` Markus Armbruster
@ 2017-08-18 16:57 ` Eduardo Habkost
2017-08-21 21:46 ` John Snow
1 sibling, 1 reply; 26+ messages in thread
From: Eduardo Habkost @ 2017-08-18 16:57 UTC (permalink / raw)
To: John Snow
Cc: Eric Blake, qemu-devel, Markus Armbruster, Michael S. Tsirkin,
Marcel Apfelbaum, Laine Stump, qemu-block
On Wed, Aug 16, 2017 at 05:46:18PM -0400, John Snow wrote:
>
>
> On 08/14/2017 05:57 PM, Eduardo Habkost wrote:
> > Example output when using "-machine q35":
> >
> > {
> > "available": true,
> > "count": 1,
> > "device-types": [
> > "ide-device"
> > ],
> > "hotpluggable": false,
> > "opts": [
> > { "option": "unit", "values": 0 },
> > { "option": "bus", "values": "ide.2" }
> > ],
> > "opts-complete": true
> > }
> > {
> > "available": false,
> > "count": 1,
> > "device": "/machine/unattached/device[19]",
> > "device-types": [
> > "ide-device"
> > ],
> > "hotpluggable": false,
> > "opts": [
> > { "option": "unit", "values": 1 },
> > { "option": "bus", "values": "ide.2" } ],
> > "opts-complete": true
> > }
> > {
> > "available": true,
> > "count": 10,
> > "device-types": [
> > "ide-device"
> > ],
> > "hotpluggable": false,
> > "opts": [
> > { "option": "unit", "values": [ [ 0, 1 ] ] },
>
> Hm, these unit values aren't really correct -- we do not support
> primary/secondary semantics for IDE buses on the AHCI device. I guess
> they technically exist, but you cannot use them for anything.
>
> Should I do something to "disable" or otherwise hide the unusable
> secondary unit slots for AHCI devices?
If the device is already rejecting -device ...,unit=1, then the
bug is in my implementation of enumerate_devices. Maybe it
should just look at IDEBus::max_units to find that out?
--
Eduardo
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [RFC v4 12/13] usb: enumerate_slots implementation
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 12/13] usb: " Eduardo Habkost
@ 2017-08-21 11:44 ` Gerd Hoffmann
2017-08-23 17:17 ` Eduardo Habkost
0 siblings, 1 reply; 26+ messages in thread
From: Gerd Hoffmann @ 2017-08-21 11:44 UTC (permalink / raw)
To: Eduardo Habkost, Eric Blake, qemu-devel, Markus Armbruster,
Michael S. Tsirkin, Marcel Apfelbaum, Laine Stump
Hi,
-ENOCONTEXT
What is the use case?
> + QTAILQ_FOREACH(port, l, next) {
> + DeviceSlotInfo *slot = make_slot(BUS(bus));
> + /*
> + * TODO: should the "bus" option be included, or is
> + * "port" enough to identify the USB bus + port?
> + */
Yes, bus must be included, port alone isn't enough in case multiple usb
host adapters are present.
cheers,
Gerd
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [RFC v4 08/13] ide: enumerate_slots implementation
2017-08-18 16:57 ` Eduardo Habkost
@ 2017-08-21 21:46 ` John Snow
0 siblings, 0 replies; 26+ messages in thread
From: John Snow @ 2017-08-21 21:46 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-block, Michael S. Tsirkin, qemu-devel, Markus Armbruster,
Laine Stump, Marcel Apfelbaum
On 08/18/2017 12:57 PM, Eduardo Habkost wrote:
> On Wed, Aug 16, 2017 at 05:46:18PM -0400, John Snow wrote:
>>
>>
>> On 08/14/2017 05:57 PM, Eduardo Habkost wrote:
>>> Example output when using "-machine q35":
>>>
>>> {
>>> "available": true,
>>> "count": 1,
>>> "device-types": [
>>> "ide-device"
>>> ],
>>> "hotpluggable": false,
>>> "opts": [
>>> { "option": "unit", "values": 0 },
>>> { "option": "bus", "values": "ide.2" }
>>> ],
>>> "opts-complete": true
>>> }
>>> {
>>> "available": false,
>>> "count": 1,
>>> "device": "/machine/unattached/device[19]",
>>> "device-types": [
>>> "ide-device"
>>> ],
>>> "hotpluggable": false,
>>> "opts": [
>>> { "option": "unit", "values": 1 },
>>> { "option": "bus", "values": "ide.2" } ],
>>> "opts-complete": true
>>> }
>>> {
>>> "available": true,
>>> "count": 10,
>>> "device-types": [
>>> "ide-device"
>>> ],
>>> "hotpluggable": false,
>>> "opts": [
>>> { "option": "unit", "values": [ [ 0, 1 ] ] },
>>
>> Hm, these unit values aren't really correct -- we do not support
>> primary/secondary semantics for IDE buses on the AHCI device. I guess
>> they technically exist, but you cannot use them for anything.
>>
>> Should I do something to "disable" or otherwise hide the unusable
>> secondary unit slots for AHCI devices?
>
> If the device is already rejecting -device ...,unit=1, then the
> bug is in my implementation of enumerate_devices. Maybe it
> should just look at IDEBus::max_units to find that out?
>
jhuston@probe (master) ~/s/q/b/git>
./x86_64-softmmu/qemu-system-x86_64 \
-M q35 \
-nodefaults \
-device ich9-ahci,id=ahci0 \
-drive id=bar,file=/media/ext/img/f25.qcow2,if=none \
-device ide-hd,id=foo,bus=ahci0.0,drive=bar,unit=1 \
-vga std
qemu-system-x86_64: -device ide-hd,id=foo,bus=ahci0.0,drive=bar,unit=1:
Can't create IDE unit 1, bus supports only 1 units
qemu-system-x86_64: -device ide-hd,id=foo,bus=ahci0.0,drive=bar,unit=1:
Device initialization failed.
based on;
IDEBus.max_units
as seen in:
hw/ide/qdev.c line 93.
Seems a bit like a hack on IDE's end -- but if your enumerate devices
code has a call-in inside of the IDE layer, please do check the
IDEBus::max_units property.
--js
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [RFC v4 12/13] usb: enumerate_slots implementation
2017-08-21 11:44 ` Gerd Hoffmann
@ 2017-08-23 17:17 ` Eduardo Habkost
0 siblings, 0 replies; 26+ messages in thread
From: Eduardo Habkost @ 2017-08-23 17:17 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: Eric Blake, qemu-devel, Markus Armbruster, Michael S. Tsirkin,
Marcel Apfelbaum, Laine Stump
On Mon, Aug 21, 2017 at 01:44:19PM +0200, Gerd Hoffmann wrote:
> Hi,
>
> -ENOCONTEXT
>
> What is the use case?
I want to let libvirt know what are the available
slots/ports/addresses for devices in a machine, before it uses
device_add or -device. This is useful when the machine creates
some devices implicitly, when different machine-types provide
different bus topologies, or when a device was created with no
explicit slot/port/address options.
Currently libvirt work around the lack of such interface by
hardcoding information on the implicitly-created devices/buses
and available slots for each machine-type. See, for example,
src/qemu/qemu_domain_address.c in the libvirt source tree.
>
> > + QTAILQ_FOREACH(port, l, next) {
> > + DeviceSlotInfo *slot = make_slot(BUS(bus));
> > + /*
> > + * TODO: should the "bus" option be included, or is
> > + * "port" enough to identify the USB bus + port?
> > + */
>
> Yes, bus must be included, port alone isn't enough in case multiple usb
> host adapters are present.
Thanks!
--
Eduardo
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2017-08-23 17:18 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-14 21:57 [Qemu-devel] [RFC v4 00/13] qmp: query-device-slots command Eduardo Habkost
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 01/13] qmp: Define " Eduardo Habkost
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 02/13] qapi: qobject_compare() helper Eduardo Habkost
2017-08-15 16:16 ` Eric Blake
2017-08-15 17:59 ` Eduardo Habkost
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 03/13] qdev: Add BusClass::device_type field Eduardo Habkost
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 04/13] qdev: Slot info helpers Eduardo Habkost
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 05/13] query-device-slots: Collapse similar entries Eduardo Habkost
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 06/13] qdev core: generic enumerate_slots implementation Eduardo Habkost
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 07/13] qdev: Enumerate CPU slots on query-device-slots Eduardo Habkost
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 08/13] ide: enumerate_slots implementation Eduardo Habkost
2017-08-16 21:46 ` John Snow
2017-08-17 4:54 ` Markus Armbruster
2017-08-17 18:40 ` John Snow
2017-08-18 16:57 ` Eduardo Habkost
2017-08-21 21:46 ` John Snow
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 09/13] pci: pci_bus_has_pcie_upstream_port() function Eduardo Habkost
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 10/13] pci: device-number & function properties Eduardo Habkost
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 11/13] pci: enumerate_slots implementation Eduardo Habkost
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 12/13] usb: " Eduardo Habkost
2017-08-21 11:44 ` Gerd Hoffmann
2017-08-23 17:17 ` Eduardo Habkost
2017-08-14 21:57 ` [Qemu-devel] [RFC v4 13/13] tests: Experimental query-device-slots test code Eduardo Habkost
2017-08-14 22:37 ` [Qemu-devel] [libvirt] [RFC v4 00/13] qmp: query-device-slots command no-reply
2017-08-15 18:57 ` [Qemu-devel] " Eric Blake
2017-08-15 19:44 ` Eduardo Habkost
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.