From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55585) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c9adq-0007w8-Qe for qemu-devel@nongnu.org; Wed, 23 Nov 2016 11:43:29 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c9adn-0007uN-KH for qemu-devel@nongnu.org; Wed, 23 Nov 2016 11:43:26 -0500 Received: from mx1.redhat.com ([209.132.183.28]:42654) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c9adn-0007tL-BA for qemu-devel@nongnu.org; Wed, 23 Nov 2016 11:43:23 -0500 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 533367AE92 for ; Wed, 23 Nov 2016 16:43:22 +0000 (UTC) References: <1479777133-23567-1-git-send-email-ehabkost@redhat.com> From: Marcel Apfelbaum Message-ID: <47c2fb2a-7785-b7b9-bf7d-5743cbc5f4be@redhat.com> Date: Wed, 23 Nov 2016 18:43:16 +0200 MIME-Version: 1.0 In-Reply-To: <1479777133-23567-1-git-send-email-ehabkost@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC 00/15] qmp: Report supported device types on 'query-machines' List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost , qemu-devel@nongnu.org, Markus Armbruster , "Michael S. Tsirkin" Cc: libvir-list@redhat.com, Laine Stump On 11/22/2016 03:11 AM, Eduardo Habkost wrote: > The Problem > =========== > > Currently management software has no way to find out which device > types can be plugged in a machine, unless the machine is already > initialized. > Hi Eduardo, Thank you for this interesting series. I think this is a problem worth addressing. > Even after the machine is initialized, there's no way to map > existing bus types to supported device types unless management > software hardcodes the mapping between bus types and device > types. > Here I am a little lost. We are going for machine => supported devices or bus-type => supported devices? > Example: floppy support on q35 vs i440fx > ---------------------------------------- > > There's no way for libvirt to find out that there's no floppy > controller on pc-q35-* machine-types by default. > Again "by default". So do we want to query the init state of a machine? What devices are there? Or what devices *can be* there? > With this series, pc-i440fx-* will report "floppy" as a supported > device type, but pc-q35-* will not. > > Example: Legacy PCI vs vs PCIe devices > -------------------------------------- > > Some devices require a PCIe bus to be available, others work on > both legacy PCI and PCIe, while others work only on a legacy PCI > bus. > > Currently management software has no way to know which devices > can be added to a given machine, unless it hardcodes machine-type > names and device-types names. > Again it seems a double problem, machine => devices vs pci/pcie bus => devices. The bus => devices match is not related to a machine type. > The Proposed Interface > ====================== > > This series adds a new field to the output of 'query-machines': > 'supported-device-types'. It will contain a list of QOM type > names, that can be used to find the list of device types that can > be plugged in the machine by default. What do you mean "by default"? Without bridges or part of the machine itself? The type names reported on > the new field can then be used as the 'implements' argument on > the 'qom-list-types' command, to find out which device types can > be plugged on the machine. > > Example output > -------------- > > (QEMU) query-machines > { > "return": [ > [...] > { > "supported-device-types": [ > "sys-bus-device" I don't know how "sys-bus-device" can help us... :) > ], > "cpu-max": 1, > "hotpluggable-cpus": false, > "name": "none" > }, > [...] > { > "supported-device-types": [ > "sys-bus-device" > ], > "cpu-max": 1, > "hotpluggable-cpus": false, > "name": "xenpv" > }, > [...] > { > "supported-device-types": [ > "sys-bus-device", > "floppy", > "i2c-slave", > "pci-device", > "isa-device", > "ide-device" Is don't know is this high level classification is useful, here is an example: pvi-device is supported => then we look for all pci devices? But what if some pci devices make sense on a machine type, but not on another? > ], > "name": "pc-i440fx-2.8", > "alias": "pc", > "is-default": true, > "cpu-max": 255, > "hotpluggable-cpus": true > }, > [...] > { > "supported-device-types": [ > "sys-bus-device", > "floppy", > "isa-device", > "ide-device" > ], > "cpu-max": 1, > "hotpluggable-cpus": true, > "name": "isapc" > }, > [...] > { > "supported-device-types": [ > "sys-bus-device", > "floppy", > "i2c-slave", > "pci-device", > "isa-device", > "ide-device" > ], > "cpu-max": 128, > "hotpluggable-cpus": true, > "name": "xenfv" > }, > [...] > { > "alias": "q35", > "supported-device-types": [ > "sys-bus-device", > "i2c-slave", > "PCIE-device", > "isa-device", > "ide-device" > ], > "cpu-max": 288, > "hotpluggable-cpus": true, > "name": "pc-q35-2.8" > }, > [...] > ] > } > > Considered alternatives > ======================= > > Indirect mapping (machine => bus => device) > ------------------------------------------- > > This RFC implements a mechanism to implement ax > machine-type => supported-device-types > mapping. An alternative solution I considered was to expose an > indirect mapping: > machine-type => default-bus-types > followed by > bus-type => supported-device-types. > As I previously stated, I don't know if it helps. bus-type can support different devices on different archs. > But exposing only the resulting supported device-types list > imposes less restrictions on how the device and bus type > hierarchy is implemented inside QEMU. There's still a > machine-type => bus-type => device-type > mapping implemented internally, but it is an implementation > detail on the current version, and not part of the > externally-visible interface. > Good, I personally don't like this "pass". > The Implementation > ================== > > This add a new field to MachineClass: default_bus_types, and a > new field to BusClass: supported_device_type. > > The two fields are used to build the list of supported device > types for a given machine. On most cases, the normal QOM type > hierarchy is used to define the set of supported devices for a > bus. On the case of PCIe buses, a INTERFACE_PCIE_DEVICE interface > name was introduced, to indicate PCIe-capable devices. > > This means we are duplicating information in some cases: > > * BusClass::supported_device_type duplicates knowlege that is > already encoded in DeviceClass::bus_type. > > To make sure both fields agree with each other, a new > device_class_set_bus_type() wrapper was added, to perform > additional validation. > > * MachineClass::default_bus_type duplicates knowledge that is > already encoded in the machine init function. > > To make sure the information is correct, a qmp-machine-info.py > test case is added, that will validate the > supported-device-types field based on the buses created by the > machine. > > * PCIDeviceClass::is_express and INTERFACE_PCIE_DEVICE > both encode the same information about a PCI device class. > > A future version of this series may include a > class_base_post_init hook that will allow TYPE_PCI_DEVICE to > validate/update is_express and the interface list to ensure > both are always consistent. > > Test Code > --------- > > qdev-based test code for the new field was implemented in a > Python script. Some extra support was implemented in > tests/Makefile.include, scripts/qemu.py and scripts/qtest.py to > allow the test to be implemented. > > Limitations > =========== > > TYPE_SYS_BUS_DEVICE is too generic > ---------------------------------- > > Currently all machines have a TYPE_SYS_BUS bus, meaning all > TYPE_SYS_BUS_DEVICE subclasses are reported as supported. > Agreed, this is a problem. > The current solution in this series is to report > TYPE_SYS_BUS_DEVICE as supported by all machines. But we could > gradually add arch-specific or machine-family-specific interface > names that can be used on devices that are meant to work with > only a subset of TYPE_SYS_BUS_DEVICE subclasses. > > A future version of this series may remove TYPE_SYS_BUS_DEVICE > from the supported-device-types output, and return a > arch-specific or machine-family-specific interface name to > restrict management software to a subset of TYPE_SYS_BUS_DEVICE > subclasses. > > PCI vs PCIe > ----------- > > Machines with PCIe buses will report INTERFACE_PCIE_DEVICE on > supported-device-types. > > Machines with legacy PCI buses will report TYPE_PCI_DEVICE on > supported-device-types. > > The problem with the current approach is that PCIe devices are > TYPE_PCI_DEVICE subclasses. The allows PCI device classes to > indicate they are PCIe-capable, but there's no obvious way to > indicate that a device is PCIe-only. This needs to be addressed > in a future version of this series. > > Suggestions are welcome. As we talked offline, I personally like an interface IPCIType with a field having 3 possible values {pci/pcie/hybrid} To understand how hybrid works we need some rules, like "pci on pci bus/pcie on pcie bus" "pcie on a non-root pcie bus/pcie otherwise I don't think we'll have a lot of rules, simple boolean fields for the interface should be enough. This still does not solve the problem that some devices makes sense only on a specific arch. > > Incomplete bus lists on some machines > ------------------------------------- > > With this series, not all machines classes are changed to add the > full list of device types on the 'supported-device-types'. To > allow the code to be updated gradually, qmp-machine-info.py has a > STRICT_ARCHES variable, that will make the test code require a > complete device type list only on some architectures. > > Out of scope: Configurable buses > -------------------------------- > > There's no way to map machine options like "usb=on|off" to > device-types or buses. I plan to propose a new interface that > allows machine options to be mapped to buses/device-types later. > > Out of scope: Deciding where to plug devices > -------------------------------------------- > > Once management software discovers which devices can be plugged > to a machine, it still has to discover or define where devices > can/should/will be plugged. This is out of the scope of this > series. > That's a pitty :( I was hoping this series will solve this issue. But if it prepares the grounds for it is also a good step . Thanks, Marcel > Out of scope: Hotplug > --------------------- > > The proposed interface is supposed to help management software > find which device types can be used when creating the VM. Device > hotplug is out of the scope of this series. However, the new > 'device-types' QOM property on bus objects could be used to find > out which device types can be plugged on the existing buses. > > --- > Cc: libvir-list@redhat.com > Cc: Laine Stump > > Eduardo Habkost (15): > qemu.py: Make logging optional > qtest.py: Support QTEST_LOG environment variable > qtest.py: make logging optional > qtest.py: Make 'binary' parameter optional > tests: Add rules to non-gtester qtest test cases > qdev: Add device_type field to BusClass > machine: Add MachineClass::default_buses field > qmp: Add 'supported-device-types' field to 'query-machines' > pci: Introduce INTERFACE_PCIE_DEVICE interface name > pc: Initialize default bus lists > s390x: Initialize default bus lists > arm: Initialize default bus lists > mips: Initialize default bus lists > ppc: Initialize default bus lists > qdev: Add device_class_set_bus_type() function > > hw/arm/aspeed.c | 2 + > hw/arm/collie.c | 1 + > hw/arm/cubieboard.c | 1 + > hw/arm/exynos4_boards.c | 5 ++ > hw/arm/gumstix.c | 7 ++ > hw/arm/highbank.c | 4 ++ > hw/arm/imx25_pdk.c | 1 + > hw/arm/kzm.c | 1 + > hw/arm/musicpal.c | 1 + > hw/arm/nseries.c | 2 + > hw/arm/palm.c | 1 + > hw/arm/realview.c | 1 + > hw/arm/spitz.c | 10 +++ > hw/arm/stellaris.c | 4 ++ > hw/audio/intel-hda.c | 9 ++- > hw/block/fdc.c | 17 +++-- > hw/block/nvme.c | 4 ++ > hw/char/virtio-serial-bus.c | 3 +- > hw/core/bus.c | 9 +++ > hw/core/machine.c | 18 ++++- > hw/core/qdev.c | 8 +++ > hw/core/sysbus.c | 3 +- > hw/i2c/core.c | 9 ++- > hw/i386/pc_piix.c | 13 ++++ > hw/i386/pc_q35.c | 4 ++ > hw/ide/qdev.c | 3 +- > hw/input/adb.c | 9 ++- > hw/ipack/ipack.c | 9 ++- > hw/isa/isa-bus.c | 3 +- > hw/mips/mips_malta.c | 7 ++ > hw/mips/mips_r4k.c | 2 + > hw/misc/auxbus.c | 3 +- > hw/net/e1000e.c | 4 ++ > hw/net/vmxnet3.c | 4 ++ > hw/pci-bridge/ioh3420.c | 4 ++ > hw/pci-bridge/xio3130_downstream.c | 4 ++ > hw/pci/pci.c | 16 ++++- > hw/ppc/e500plat.c | 3 + > hw/ppc/mac_newworld.c | 4 ++ > hw/ppc/mac_oldworld.c | 3 + > hw/ppc/mpc8544ds.c | 4 ++ > hw/ppc/ppc440_bamboo.c | 1 + > hw/ppc/prep.c | 4 ++ > hw/ppc/spapr_vio.c | 3 +- > hw/s390x/css-bridge.c | 2 + > hw/s390x/event-facility.c | 3 +- > hw/s390x/s390-pci-bus.c | 9 ++- > hw/s390x/s390-virtio-ccw.c | 6 ++ > hw/s390x/virtio-ccw.c | 2 +- > hw/scsi/megasas.c | 7 ++ > hw/scsi/scsi-bus.c | 3 +- > hw/scsi/vmw_pvscsi.c | 1 + > hw/sd/core.c | 7 ++ > hw/sd/sd.c | 2 +- > hw/ssi/ssi.c | 9 ++- > hw/usb/bus.c | 3 +- > hw/usb/dev-smartcard-reader.c | 9 ++- > hw/usb/hcd-xhci.c | 4 ++ > hw/vfio/pci.c | 4 ++ > hw/virtio/virtio-bus.c | 1 + > hw/virtio/virtio-pci.c | 4 ++ > hw/virtio/virtio.c | 2 +- > include/hw/boards.h | 5 ++ > include/hw/pci/pci.h | 3 + > include/hw/qdev-core.h | 4 ++ > qapi-schema.json | 9 ++- > scripts/qemu.py | 25 +++++-- > scripts/qtest.py | 15 +++- > tests/Makefile.include | 39 ++++++++++- > tests/qmp-machine-info.py | 138 +++++++++++++++++++++++++++++++++++++ > vl.c | 11 +++ > 71 files changed, 518 insertions(+), 37 deletions(-) > create mode 100755 tests/qmp-machine-info.py >