Re: [libvirt] [Qemu-devel] [RFC] qmp: query-device-slots command

On Tue, Dec 13, 2016 at 04:15:18PM +0200, Marcel Apfelbaum wrote: > On 12/13/2016 02:42 PM, Eduardo Habkost wrote: >> On Tue, Dec 13, 2016 at 12:04:17PM +0100, Markus Armbruster wrote: >>> Quick interface review only: >>> >>> Eduardo Habkost <ehabkost(a)redhat.com&gt; writes: >>> >>>> 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 per bus in the case of the other buses; >>> >>> Umm, that doesn't seem right. For instance, a SCSI bus has multiple >>> slots. The slot address is the SCSI ID. An IDE bus may have one (SATA) >>> or two (PATA). For more examples, see qdev-device-use.txt section >>> "Specifying Bus and Address on Bus". >> >> Yes, I should have clarified that: this version changes only PCI >> to expose multiple slots, but other buses also need be changed to >> implement BusClass::enumerate_slots() properly, too. >> >>> >>>> * One slot for each entry from query-hotpluggable-cpus. >>>> In the example below, I am not sure if the PCIe ports are all >>>> supposed to report all slots, but I didn't find any existing >>>> field in PCIBus that would help me figure out the actual number >>>> of slots in a given PCI bus. >>>> >>>> Git tree >>>> -------- >>>> >>>> This patch needs the previous query-machines series I am working >>>> on. The full tree can be found on the git tree at: >>>> >>>> git://github.com/ehabkost/qemu-hacks.git work/query-machines-bus-info >>>> >>>> Example output >>>> -------------- >>>> >>>> The following output was returned by QEMU when running it as: >>>> >>>> $ qemu-system-x86_64 -machine q35 \ >>>> -readconfig docs/q35-chipset.cfg \ >>>> -smp 4,maxcpus=8,sockets=2,cores=2,threads=2 >>>> >>>> { >>>> "return": [ >>> >>> Are you sure >3000 lines of example output make sense here? >> >> I'm not. :) >> >> That's why I need feedback from the PCI experts. I believe most >> of the PCI ports on q35 accept only one device, but I see no code >> implementing that restriction, and no obvious PCIBus or >> PCIBusClass field indicating that. > > Hi, > > Sadly there is no such restriction today as far as I know. > We trust the user that he knows what he is doing :) > and we let libvirt implement the restriction. > > I do agree we should fix that. OK, this matches what I saw on the current code. But I would like to understand what exactly should be the rules if we fix that. Should we report only 1 slot if pci_bus_has_pcie_upstream_port(bus)?

(Note that pci_bus_has_pcie_upstream_port() doesn't exist in the current code, it is added by my patch)