On Tue, Aug 19, 2025 at 18:22:19 +0200, Andrea Bolognani via Devel wrote:
At the moment, we check whether the machine type supports PCI before attempting to allocate PCI addresses, and if it does not, we simply skip the step entirely.
This means that an attempt to use a PCI device with a machine type that has no PCI support won't be rejected by libvirt, and only once the QEMU process is started the problem will be made apparent.
Validate things ahead of time instead, rejecting any such configurations.
Note that we only do this for new domains, because otherwise existing domains that are configured incorrectly would disappear and we generally try really hard to avoid that.
A few tests start failing after this change, demonstrating that things are now working as desired.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_domain_address.c | 18 ++++++++--- ...default-fallback-nousb.aarch64-latest.args | 32 ------------------- ...-default-fallback-nousb.aarch64-latest.xml | 22 ------------- .../usb-controller-default-fallback-nousb.xml | 1 - ...efault-nousb.aarch64-latest.abi-update.err | 1 + ...ntroller-default-nousb.aarch64-latest.args | 32 ------------------- ...ontroller-default-nousb.aarch64-latest.err | 1 + ...fault-unavailable-nousb.aarch64-latest.err | 1 - ...fault-unavailable-nousb.aarch64-latest.xml | 22 ------------- ...b-controller-default-unavailable-nousb.xml | 1 - tests/qemuxmlconftest.c | 14 ++------ 11 files changed, 17 insertions(+), 128 deletions(-) delete mode 100644 tests/qemuxmlconfdata/usb-controller-default-fallback-nousb.aarch64-latest.args delete mode 100644 tests/qemuxmlconfdata/usb-controller-default-fallback-nousb.aarch64-latest.xml delete mode 120000 tests/qemuxmlconfdata/usb-controller-default-fallback-nousb.xml create mode 100644 tests/qemuxmlconfdata/usb-controller-default-nousb.aarch64-latest.abi-update.err delete mode 100644 tests/qemuxmlconfdata/usb-controller-default-nousb.aarch64-latest.args create mode 100644 tests/qemuxmlconfdata/usb-controller-default-nousb.aarch64-latest.err delete mode 100644 tests/qemuxmlconfdata/usb-controller-default-unavailable-nousb.aarch64-latest.err delete mode 100644 tests/qemuxmlconfdata/usb-controller-default-unavailable-nousb.aarch64-latest.xml delete mode 120000 tests/qemuxmlconfdata/usb-controller-default-unavailable-nousb.xml
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 07d6366b1b..312ed52bbd 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -2675,7 +2675,8 @@ static int qemuDomainAssignPCIAddresses(virDomainDef *def, virQEMUCaps *qemuCaps, virQEMUDriver *driver, - virDomainObj *obj) + virDomainObj *obj, + bool newDomain) { int ret = -1; virDomainPCIAddressSet *addrs = NULL; @@ -2843,10 +2844,17 @@ qemuDomainAssignPCIAddresses(virDomainDef *def, g_clear_pointer(&addrs, virDomainPCIAddressSetFree); }
- if (!(addrs = qemuDomainPCIAddressSetCreate(def, qemuCaps, nbuses, false))) - goto cleanup; + /* We're done collecting available information, now we're going + * to allocate PCI addresses for real. We normally skip this part + * for machine type that don't support PCI, but we run it for new + * domains to catch situation in which the user is incorrectly + * asking for PCI devices to be used. If that's the case, an + * error will naturally be raised when attempting to allocate a + * PCI address since no PCI buses exist */ + if (qemuDomainSupportsPCI(def) || newDomain) { + if (!(addrs = qemuDomainPCIAddressSetCreate(def, qemuCaps, nbuses, false))) + goto cleanup;
Any reason why this is here and not in e.g qemuValidateDomainDef() which based on the description seems like the proper place?
- if (qemuDomainSupportsPCI(def)) { if (qemuDomainValidateDevicePCISlotsChipsets(def, addrs) < 0) goto cleanup;
--- /dev/null +++ b/tests/qemuxmlconfdata/usb-controller-default-nousb.aarch64-latest.abi-update.err @@ -0,0 +1 @@ +XML error: No PCI buses available
At this point it's IMO borderline whether it's a XML error or not. It's a configuration error but some configurations which don't mention PCI do get it normally. Although this is pre-existing and cosmetic.