On Fri, Sep 19, 2025 at 08:28:38 -0500, Andrea Bolognani wrote:
On Thu, Sep 18, 2025 at 03:04:28PM +0200, Peter Krempa wrote:
On Tue, Aug 19, 2025 at 18:22:22 +0200, Andrea Bolognani via Devel wrote:
+ /* Sanity check. If the machine type supports PCI, we need to reflect + * that fact in the XML or other parts of the machine handling code + * might misbehave */
This one is a bit borderline. IMO you can have machine with no default PCI but the possibility to explicily add such a bus.
Can you point to a specific example? I'm not aware of any.
I don't have a specific example for this, but I have a example where the sanity check breaks loading config of one of VMs I had defined before: <domain type='kvm'> <name>microvm</name> <uuid>e739ac15-61b5-48c2-acc8-e7fb2b79774f</uuid> <memory unit='KiB'>1024000</memory> <currentMemory unit='KiB'>1024000</currentMemory> <vcpu placement='static'>1</vcpu> <os> <type arch='x86_64' machine='microvm'>hvm</type> <boot dev='hd'/> </os> <cpu mode='custom' match='exact' check='none'> <model fallback='forbid'>qemu64</model> </cpu> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> <emulator>/usr/bin/qemu-system-x86_64</emulator> <disk type='file' device='disk'> <driver name='qemu' type='qcow2'/> <source file='/var/lib/libvirt/images/alpine.qcow2'/> <target dev='vda' bus='virtio'/> <address type='virtio-mmio'/> </disk> <controller type='usb' index='0' model='none'/> <audio id='1' type='none'/> <memballoon model='none'/> </devices> </domain> ... will after this patch be rejected at load time: 2025-09-19 14:16:28.132+0000: 214862: info : virDomainObjListLoadAllConfigs:601 : Loading config file 'microvm.xml' 2025-09-19 14:16:28.132+0000: 214862: debug : virDomainControllerDefParseXML:9008 : Ignoring device address for none model usb controller 2025-09-19 14:16:28.132+0000: 214862: debug : virDomainMemballoonDefParseXML:12948 : Ignoring device address for none model Memballoon 2025-09-19 14:16:28.132+0000: 214862: debug : virCPUDataIsIdentical:1285 : a=0x7fffac30c7f0, b=0x7fffac523430 2025-09-19 14:16:28.132+0000: 214862: debug : virArchFromHost:236 : Mapped x86_64 to 35 (x86_64) 2025-09-19 14:16:28.132+0000: 214862: debug : virQEMUCapsCacheLookup:5996 : Returning caps 0x7fffac022ad0 for /usr/bin/qemu-system-x86_64 2025-09-19 14:16:28.132+0000: 214862: error : qemuDomainDefAddDefaultDevices:1343 : internal error: Machine type 'microvm' supports PCI but no PCI controller added 2025-09-19 14:16:28.132+0000: 214862: error : virDomainObjListLoadAllConfigs:622 : Failed to load config for domain 'microvm'
To the best of my knowledge, all machines that are PCI-capable come with a root PCI controller out of the gate.
There is no QEMU device corresponding to the root PCI controller either, so I don't know how you would even go about adding it if it were opt-in. Perhaps a machine type flag but again, I'm not aware of that actually being a thing.
IIUC 'microvm' doesn't support PCI, which shows that this check itself is broken not that there isn't merit in the check itself.
+ if (qemuDomainSupportsPCI(def) && + !addPCIRoot && + !addPCIeRoot) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Machine type '%1$s' supports PCI but no PCI controller added"), + def->os.machine); + return -1; + } + + /* Sanity check. USB controllers are PCI devices, so asking for a u> > > + * USB controller to be added but not for a PCI(e) root to be + * added at the same time is likely an oversight */
I'm fairly sure there are non-PCI USB controllers so this one is definitely false. It's just that libvirt supports only USB controllers which are on PCI.
IMO this assumption should be validated with the USB controller itself.
I'm not aware of any non-PCI USB controller out there, certainly not in QEMU. Can you point to one?
One example is the USB controller in older raspberry-pis, which is embedded in the SoC. I presume it's accessed via MMIO. QEMU claims support for rpis at least. As said this is something that ought to be validated when validating the USB controller device, rather than here.