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;
- if (qemuDomainSupportsPCI(def)) {
if (qemuDomainValidateDevicePCISlotsChipsets(def, addrs) < 0)
goto cleanup;
@@ -3290,7 +3298,7 @@ qemuDomainAssignAddresses(virDomainDef *def,
qemuDomainAssignVirtioMMIOAddresses(def, qemuCaps);
- if (qemuDomainAssignPCIAddresses(def, qemuCaps, driver, obj) < 0)
+ if (qemuDomainAssignPCIAddresses(def, qemuCaps, driver, obj, newDomain) < 0)
return -1;
if (qemuDomainAssignUSBAddresses(def, obj, newDomain) < 0)
diff --git a/tests/qemuxmlconfdata/usb-controller-default-fallback-nousb.aarch64-latest.args b/tests/qemuxmlconfdata/usb-controller-default-fallback-nousb.aarch64-latest.args
deleted file mode 100644
index 0fb2909dd2..0000000000
--- a/tests/qemuxmlconfdata/usb-controller-default-fallback-nousb.aarch64-latest.args
+++ /dev/null
@@ -1,32 +0,0 @@
-LC_ALL=C \
-PATH=/bin \
-HOME=/var/lib/libvirt/qemu/domain--1-aarch64test \
-USER=test \
-LOGNAME=test \
-XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-aarch64test/.local/share \
-XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-aarch64test/.cache \
-XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-aarch64test/.config \
-/usr/bin/qemu-system-aarch64 \
--name guest=aarch64test,debug-threads=on \
--S \
--object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-aarch64test/master-key.aes"}' \
--machine collie,usb=off,dump-guest-core=off,memory-backend=strongarm.sdram \
--accel kvm \
--cpu host \
--m size=1048576k \
--object '{"qom-type":"memory-backend-ram","id":"strongarm.sdram","size":1073741824}' \
--overcommit mem-lock=off \
--smp 1,sockets=1,cores=1,threads=1 \
--uuid 6ba410c5-1e5c-4d57-bee7-2228e7ffa32f \
--display none \
--no-user-config \
--nodefaults \
--chardev socket,id=charmonitor,fd=1729,server=on,wait=off \
--mon chardev=charmonitor,id=monitor,mode=control \
--rtc base=utc \
--no-shutdown \
--boot strict=on \
--device '{"driver":"qemu-xhci","id":"usb"}' \
--audiodev '{"id":"audio1","driver":"none"}' \
--sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
--msg timestamp=on
diff --git a/tests/qemuxmlconfdata/usb-controller-default-fallback-nousb.aarch64-latest.xml b/tests/qemuxmlconfdata/usb-controller-default-fallback-nousb.aarch64-latest.xml
deleted file mode 100644
index fa258c5671..0000000000
--- a/tests/qemuxmlconfdata/usb-controller-default-fallback-nousb.aarch64-latest.xml
+++ /dev/null
@@ -1,22 +0,0 @@
-<domain type='kvm'>
- <name>aarch64test</name>
- <uuid>6ba410c5-1e5c-4d57-bee7-2228e7ffa32f</uuid>
- <memory unit='KiB'>1048576</memory>
- <currentMemory unit='KiB'>1048576</currentMemory>
- <vcpu placement='static'>1</vcpu>
- <os>
- <type arch='aarch64' machine='collie'>hvm</type>
- <boot dev='hd'/>
- </os>
- <cpu mode='host-passthrough' check='none'/>
- <clock offset='utc'/>
- <on_poweroff>destroy</on_poweroff>
- <on_reboot>restart</on_reboot>
- <on_crash>destroy</on_crash>
- <devices>
- <emulator>/usr/bin/qemu-system-aarch64</emulator>
- <controller type='usb' index='0' model='qemu-xhci'/>
- <audio id='1' type='none'/>
- <memballoon model='none'/>
- </devices>
-</domain>
diff --git a/tests/qemuxmlconfdata/usb-controller-default-fallback-nousb.xml b/tests/qemuxmlconfdata/usb-controller-default-fallback-nousb.xml
deleted file mode 120000
index 9cba7c3a06..0000000000
--- a/tests/qemuxmlconfdata/usb-controller-default-fallback-nousb.xml
+++ /dev/null
@@ -1 +0,0 @@
-usb-controller-default-nousb.xml
\ No newline at end of file
diff --git a/tests/qemuxmlconfdata/usb-controller-default-nousb.aarch64-latest.abi-update.err b/tests/qemuxmlconfdata/usb-controller-default-nousb.aarch64-latest.abi-update.err
new file mode 100644
index 0000000000..a0ca4fba5d
--- /dev/null
+++ b/tests/qemuxmlconfdata/usb-controller-default-nousb.aarch64-latest.abi-update.err
@@ -0,0 +1 @@
+XML error: No PCI buses available
diff --git a/tests/qemuxmlconfdata/usb-controller-default-nousb.aarch64-latest.args b/tests/qemuxmlconfdata/usb-controller-default-nousb.aarch64-latest.args
deleted file mode 100644
index 0fb2909dd2..0000000000
--- a/tests/qemuxmlconfdata/usb-controller-default-nousb.aarch64-latest.args
+++ /dev/null
@@ -1,32 +0,0 @@
-LC_ALL=C \
-PATH=/bin \
-HOME=/var/lib/libvirt/qemu/domain--1-aarch64test \
-USER=test \
-LOGNAME=test \
-XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-aarch64test/.local/share \
-XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-aarch64test/.cache \
-XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-aarch64test/.config \
-/usr/bin/qemu-system-aarch64 \
--name guest=aarch64test,debug-threads=on \
--S \
--object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-aarch64test/master-key.aes"}' \
--machine collie,usb=off,dump-guest-core=off,memory-backend=strongarm.sdram \
--accel kvm \
--cpu host \
--m size=1048576k \
--object '{"qom-type":"memory-backend-ram","id":"strongarm.sdram","size":1073741824}' \
--overcommit mem-lock=off \
--smp 1,sockets=1,cores=1,threads=1 \
--uuid 6ba410c5-1e5c-4d57-bee7-2228e7ffa32f \
--display none \
--no-user-config \
--nodefaults \
--chardev socket,id=charmonitor,fd=1729,server=on,wait=off \
--mon chardev=charmonitor,id=monitor,mode=control \
--rtc base=utc \
--no-shutdown \
--boot strict=on \
--device '{"driver":"qemu-xhci","id":"usb"}' \
--audiodev '{"id":"audio1","driver":"none"}' \
--sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
--msg timestamp=on
diff --git a/tests/qemuxmlconfdata/usb-controller-default-nousb.aarch64-latest.err b/tests/qemuxmlconfdata/usb-controller-default-nousb.aarch64-latest.err
new file mode 100644
index 0000000000..a0ca4fba5d
--- /dev/null
+++ b/tests/qemuxmlconfdata/usb-controller-default-nousb.aarch64-latest.err
@@ -0,0 +1 @@
+XML error: No PCI buses available
diff --git a/tests/qemuxmlconfdata/usb-controller-default-unavailable-nousb.aarch64-latest.err b/tests/qemuxmlconfdata/usb-controller-default-unavailable-nousb.aarch64-latest.err
deleted file mode 100644
index cac4e8e760..0000000000
--- a/tests/qemuxmlconfdata/usb-controller-default-unavailable-nousb.aarch64-latest.err
+++ /dev/null
@@ -1 +0,0 @@
-internal error: Unable to determine model for USB controller idx=0
diff --git a/tests/qemuxmlconfdata/usb-controller-default-unavailable-nousb.aarch64-latest.xml b/tests/qemuxmlconfdata/usb-controller-default-unavailable-nousb.aarch64-latest.xml
deleted file mode 100644
index ac5f270a3a..0000000000
--- a/tests/qemuxmlconfdata/usb-controller-default-unavailable-nousb.aarch64-latest.xml
+++ /dev/null
@@ -1,22 +0,0 @@
-<domain type='kvm'>
- <name>aarch64test</name>
- <uuid>6ba410c5-1e5c-4d57-bee7-2228e7ffa32f</uuid>
- <memory unit='KiB'>1048576</memory>
- <currentMemory unit='KiB'>1048576</currentMemory>
- <vcpu placement='static'>1</vcpu>
- <os>
- <type arch='aarch64' machine='collie'>hvm</type>
- <boot dev='hd'/>
- </os>
- <cpu mode='host-passthrough' check='none'/>
- <clock offset='utc'/>
- <on_poweroff>destroy</on_poweroff>
- <on_reboot>restart</on_reboot>
- <on_crash>destroy</on_crash>
- <devices>
- <emulator>/usr/bin/qemu-system-aarch64</emulator>
- <controller type='usb' index='0'/>
- <audio id='1' type='none'/>
- <memballoon model='none'/>
- </devices>
-</domain>
diff --git a/tests/qemuxmlconfdata/usb-controller-default-unavailable-nousb.xml b/tests/qemuxmlconfdata/usb-controller-default-unavailable-nousb.xml
deleted file mode 120000
index 9cba7c3a06..0000000000
--- a/tests/qemuxmlconfdata/usb-controller-default-unavailable-nousb.xml
+++ /dev/null
@@ -1 +0,0 @@
-usb-controller-default-nousb.xml
\ No newline at end of file
diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c
index e2f7d3dc83..b3e1e37abb 100644
--- a/tests/qemuxmlconftest.c
+++ b/tests/qemuxmlconftest.c
@@ -2064,18 +2064,8 @@ mymain(void)
ARG_END);
/* The '-nousb' test case tests machine without a built-in USB controller */
- DO_TEST_CAPS_ARCH_LATEST("usb-controller-default-nousb", "aarch64");
- DO_TEST_FULL("usb-controller-default-fallback-nousb", ".aarch64-latest",
- ARG_CAPS_ARCH, "aarch64",
- ARG_CAPS_VER, "latest",
- ARG_QEMU_CAPS_DEL, QEMU_CAPS_PIIX3_USB_UHCI, QEMU_CAPS_LAST,
- ARG_END);
- DO_TEST_FULL("usb-controller-default-unavailable-nousb", ".aarch64-latest",
- ARG_CAPS_ARCH, "aarch64",
- ARG_CAPS_VER, "latest",
- ARG_FLAGS, FLAG_EXPECT_FAILURE,
- ARG_QEMU_CAPS_DEL, QEMU_CAPS_DEVICE_QEMU_XHCI, QEMU_CAPS_NEC_USB_XHCI, QEMU_CAPS_PIIX3_USB_UHCI, QEMU_CAPS_PCI_OHCI, QEMU_CAPS_LAST,
- ARG_END);
+ DO_TEST_CAPS_ARCH_LATEST_FAILURE("usb-controller-default-nousb", "aarch64");
+ DO_TEST_CAPS_ARCH_LATEST_ABI_UPDATE_PARSE_ERROR("usb-controller-default-nousb", "aarch64");
DO_TEST_FULL("usb-controller-default-fallback-g3beige", ".ppc64-latest",
ARG_CAPS_ARCH, "ppc64",
--
2.50.1On 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.
On Thu, Sep 18, 2025 at 02:53:00PM +0200, Peter Krempa wrote:
> On Tue, Aug 19, 2025 at 18:22:19 +0200, Andrea Bolognani via Devel wrote:
> > @@ -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?
Basically the way we perform address allocation and device validation
makes this incredibly tricky to put anywhere else.
Ideally you'd just interate over all PCI devices at validate time
and, if any exists but the machine type doesn't support PCI, you'd
report an error.
However the single source of truth for whether or not a device is PCI
are the virDomainPCIConnectFlags that get assigned to it. We don't
want to duplicate that information elsewhere.
Plus the fact that virtio devices exist make things even more
complicated. Is virtio-blk a PCI device? Well, yes. Unless you are on
Arm/RISC-V and you explicitly asked for it not to be by assigning a
virtio-mmio address to it. Or you're on s390x :)
Putting this check here means that the problematic scenarios are
naturally filtered out as part of the PCI address allocation process,
with no code duplication or possibility of something falling through
the cracks.
> > +++ 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.
The error is not necessarily great but you will only hit it when
attempting to do something genuinely invalid and PCI-related, such as
adding a USB or PCI controller to a machine that doesn't support PCI.
--
Andrea Bolognani / Red Hat / Virtualization
On Fri, Sep 19, 2025 at 08:21:50 -0500, Andrea Bolognani wrote:
> On Thu, Sep 18, 2025 at 02:53:00PM +0200, Peter Krempa wrote:
> > On Tue, Aug 19, 2025 at 18:22:19 +0200, Andrea Bolognani via Devel wrote:
> > > @@ -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?
>
> Basically the way we perform address allocation and device validation
> makes this incredibly tricky to put anywhere else.
>
> Ideally you'd just interate over all PCI devices at validate time
> and, if any exists but the machine type doesn't support PCI, you'd
> report an error.
>
> However the single source of truth for whether or not a device is PCI
> are the virDomainPCIConnectFlags that get assigned to it. We don't
> want to duplicate that information elsewhere.
>
> Plus the fact that virtio devices exist make things even more
> complicated. Is virtio-blk a PCI device? Well, yes. Unless you are on
> Arm/RISC-V and you explicitly asked for it not to be by assigning a
> virtio-mmio address to it. Or you're on s390x :)
>
> Putting this check here means that the problematic scenarios are
> naturally filtered out as part of the PCI address allocation process,
> with no code duplication or possibility of something falling through
> the cracks.
I wanted to complain that there actually are multiple places that still
need fixing, namely hotplug, as that allocates addresses in a different
way (for each device individually). But looking a bit deeper it results
in the same code being called.
I still think that validation ought to be together with validation but
your explanation makes sense.
> > > +++ 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.
>
> The error is not necessarily great but you will only hit it when
> attempting to do something genuinely invalid and PCI-related, such as
> adding a USB or PCI controller to a machine that doesn't support PCI.
>
> --
> Andrea Bolognani / Red Hat / Virtualization
>
© 2016 - 2026 Red Hat, Inc.