These checks enforce some expectations that were, until now,
documented solely through comments or not spelled out at all.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_postparse.c | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_postparse.c b/src/qemu/qemu_postparse.c
index d5708fbff9..cf08e6e07c 100644
--- a/src/qemu/qemu_postparse.c
+++ b/src/qemu/qemu_postparse.c
@@ -1384,6 +1384,29 @@ qemuDomainDefAddDefaultDevices(virQEMUDriver *driver,
         break;
     }
 
+    /* Sanity check. If the machine type does not support PCI, asking
+     * for PCI(e) root to be added is an obvious mistake */
+    if ((addPCIRoot ||
+         addPCIeRoot) &&
+        !qemuDomainSupportsPCI(def)) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Machine type '%1$s' wants PCI but PCI is not supported"),
+                       def->os.machine);
+        return -1;
+    }
+
+    /* 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 */
+    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;
+    }
+
     if (addDefaultUSB && virDomainControllerFind(def, VIR_DOMAIN_CONTROLLER_TYPE_USB, 0) < 0)
         virDomainDefAddUSBController(def, 0, usbModel);
 
@@ -1392,9 +1415,6 @@ qemuDomainDefAddDefaultDevices(virQEMUDriver *driver,
 
     pciRoot = virDomainControllerFind(def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 0);
 
-    /* NB: any machine that sets addPCIRoot to true must also return
-     * true from the function qemuDomainSupportsPCI().
-     */
     if (addPCIRoot) {
         if (pciRoot >= 0) {
             if (def->controllers[pciRoot]->model != VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) {
-- 
2.51.0

On Thu, Sep 25, 2025 at 20:07:05 +0200, Andrea Bolognani via Devel wrote:
> These checks enforce some expectations that were, until now,
> documented solely through comments or not spelled out at all.
> 
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/qemu/qemu_postparse.c | 26 +++++++++++++++++++++++---
>  1 file changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/src/qemu/qemu_postparse.c b/src/qemu/qemu_postparse.c
> index d5708fbff9..cf08e6e07c 100644
> --- a/src/qemu/qemu_postparse.c
> +++ b/src/qemu/qemu_postparse.c
> @@ -1384,6 +1384,29 @@ qemuDomainDefAddDefaultDevices(virQEMUDriver *driver,
>          break;
>      }
>  
> +    /* Sanity check. If the machine type does not support PCI, asking
> +     * for PCI(e) root to be added is an obvious mistake */
> +    if ((addPCIRoot ||
> +         addPCIeRoot) &&
> +        !qemuDomainSupportsPCI(def)) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("Machine type '%1$s' wants PCI but PCI is not supported"),
> +                       def->os.machine);
> +        return -1;
> +    }
> +
> +    /* 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 */
> +    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;
> +    }

I still don't really like that we add sanity checks into (effectively)
the XML parser, but for now I don' thave any better suggestion.

Please don't add any other.

Reviewed-by: Peter Krempa <pkrempa@redhat.com>

The summary is wrong; it mentions USB which was removed.

On Fri, Oct 03, 2025 at 08:56:37 +0200, Peter Krempa via Devel wrote:
> On Thu, Sep 25, 2025 at 20:07:05 +0200, Andrea Bolognani via Devel wrote:
> > These checks enforce some expectations that were, until now,
> > documented solely through comments or not spelled out at all.
> > 
> > Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> > Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
> > ---

With the commit message fixed:

> 
> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
>