On Tue, Aug 19, 2025 at 18:22:18 +0200, Andrea Bolognani via Devel wrote:
This centralizes the knowledge about which network interface models are PCI devices and thus need to have a PCI address allocated by libvirt, and expands said knowledge to include the fact that models such as spapr-vlan and smc91c111 are not PCI devices.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_domain_address.c | 62 +++++++++++++++++++++++++++++++--- 1 file changed, 57 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 96a9ca9b14..07d6366b1b 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -499,6 +499,58 @@ qemuDomainDeviceCalculatePCIAddressExtensionFlags(virQEMUCaps *qemuCaps, }
+static bool +qemuDomainNetIsPCI(const virDomainNetDef *net) +{ + switch ((virDomainNetModelType)net->model) { + case VIR_DOMAIN_NET_MODEL_USB_NET: + case VIR_DOMAIN_NET_MODEL_SPAPR_VLAN: + case VIR_DOMAIN_NET_MODEL_LAN9118: + case VIR_DOMAIN_NET_MODEL_SMC91C111: + /* The model above are not PCI devices */ + return false;
This returns false and no error ...
+ + case VIR_DOMAIN_NET_MODEL_RTL8139: + case VIR_DOMAIN_NET_MODEL_VIRTIO: + case VIR_DOMAIN_NET_MODEL_E1000: + case VIR_DOMAIN_NET_MODEL_E1000E: + case VIR_DOMAIN_NET_MODEL_IGB: + case VIR_DOMAIN_NET_MODEL_VIRTIO_TRANSITIONAL: + case VIR_DOMAIN_NET_MODEL_VIRTIO_NON_TRANSITIONAL: + case VIR_DOMAIN_NET_MODEL_VMXNET3: + /* The models above are PCI devices */ + return true; + + case VIR_DOMAIN_NET_MODEL_NETFRONT: + case VIR_DOMAIN_NET_MODEL_VMXNET: + case VIR_DOMAIN_NET_MODEL_VMXNET2: + case VIR_DOMAIN_NET_MODEL_VLANCE: + case VIR_DOMAIN_NET_MODEL_AM79C970A: + case VIR_DOMAIN_NET_MODEL_AM79C973: + case VIR_DOMAIN_NET_MODEL_82540EM: + case VIR_DOMAIN_NET_MODEL_82545EM: + case VIR_DOMAIN_NET_MODEL_82543GC: + case VIR_DOMAIN_NET_MODEL_UNKNOWN: + /* The models above are probably not PCI devices, and in fact + * some of them are not even relevant to the QEMU driver, but + * historically we've defaulted to considering all network + * interfaces to be PCI so we preserve that behavior here */ + return true; + + case VIR_DOMAIN_NET_MODEL_LAST: + default: + virReportEnumRangeError(virDomainNetModelType, net->model); + return false;
And this returns false and raises an error. The caller can't know whether there is an error to raise or not.
+ } + + /* Due to historical reasons, model names for network interfaces + * are not validated as strictly as other devices. When in doubt, + * assume that network interfaces are PCI devices, as that has + * the highest chance of working correctly */ + return true;
Also this is dead code based on the fact that you have 'default' above.
+} + + /** * qemuDomainDeviceCalculatePCIConnectFlags: * @@ -669,10 +721,11 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDef *dev, * address is assigned when we're assigning the * addresses for other hostdev devices. */ - if (net->type == VIR_DOMAIN_NET_TYPE_HOSTDEV || - net->model == VIR_DOMAIN_NET_MODEL_USB_NET) { + if (net->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) + return 0; + + if (!qemuDomainNetIsPCI(net)) return 0; - }
if (net->model == VIR_DOMAIN_NET_MODEL_VIRTIO || net->model == VIR_DOMAIN_NET_MODEL_VIRTIO_NON_TRANSITIONAL) @@ -2110,9 +2163,8 @@ qemuDomainAssignDevicePCISlots(virDomainDef *def, for (i = 0; i < def->nnets; i++) { virDomainNetDef *net = def->nets[i];
- if (net->model == VIR_DOMAIN_NET_MODEL_USB_NET) { + if (!qemuDomainNetIsPCI(net)) continue; - }
/* type='hostdev' network devices might be USB, and are also * in hostdevs list anyway, so handle them with other hostdevs
This part looks reasonable but as you can see callers ignore the error raised by virReportEnumRangeError. I suggest you remove the error reported and also merge the 'default' case with the return at the end of the function with the comment explaining why you return true. Reviewed-by: Peter Krempa <pkrempa@redhat.com>