1. Patchew
  2. Libvirt
  3. View series

[PATCH] conf: Restrict model type to enum members

Adam Julis posted 1 patch 1 week, 5 days ago 
src/conf/domain_conf.c     | 23 +++++++++++++++++++++++
src/conf/domain_conf.h     | 23 +++++++++++++++++++++++
src/conf/domain_validate.c |  7 +++++++
3 files changed, 53 insertions(+)
[PATCH] conf: Restrict model type to enum members
Posted by Adam Julis 1 week, 5 days ago 
Historically, we supported any string as the model type for
network devices. This approach allowed us to stay up-to-date with
QEMU's frequent introduction of new model types. However, this
solution now causes more problems than benefits. A clearly
nonsensical model name can pass validation but result in a QEMU
internal error when the VM is started.

This patch restricts model types to members of the
'virDomainNetModelType' enum. The enum has been extended to include
all model types currently supported by QEMU. If QEMU introduces
new models in the future, the enum must be updated to support
them.

Resolves: https://issues.redhat.com/browse/RHEL-72082
Signed-off-by: Adam Julis <ajulis@redhat.com>
---

I'm not sure if the virtio-net-pci should be in this enum, since its not
located in qemu repo in hw/net/ but in hw/virtio/. When I manually
tested it, the qemu this option supported. The last two members have not
been supported for several years but I wanted to keep backward compatibility.

src/conf/domain_conf.c     | 23 +++++++++++++++++++++++
 src/conf/domain_conf.h     | 23 +++++++++++++++++++++++
 src/conf/domain_validate.c |  7 +++++++
 3 files changed, 53 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index af88d0bcfd..5e8ca0015d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -613,6 +613,29 @@ VIR_ENUM_IMPL(virDomainNetModel,
               "82540EM",
               "82545EM",
               "82543GC",
+              "dp8393x",
+              "eepro100",
+              "ftgmac100",
+              "igbvf",
+              "lasi_i82596",
+              "mcf_fec",
+              "mipsnet",
+              "ne2000-isa",
+              "ne2000-pci",
+              "npcm7xx_emc",
+              "npcm_gmac",
+              "opencores_eth",
+              "pcnet-pci",
+              "rocker",
+              "spapr_llan",
+              "sungem",
+              "sunhme",
+              "tulip",
+              "virtio-net",
+              "xen_nic",
+              "virtio-net-pci",
+              "etraxfs_eth",
+              "milkymist-minimac2",
 );
 
 VIR_ENUM_IMPL(virDomainNetDriver,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 9f7c28343f..d7f0073ba5 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -951,6 +951,29 @@ typedef enum {
     VIR_DOMAIN_NET_MODEL_82540EM,
     VIR_DOMAIN_NET_MODEL_82545EM,
     VIR_DOMAIN_NET_MODEL_82543GC,
+    VIR_DOMAIN_NET_MODEL_DP8393X,
+    VIR_DOMAIN_NET_MODEL_EEPRO100,
+    VIR_DOMAIN_NET_MODEL_FTGMAC100,
+    VIR_DOMAIN_NET_MODEL_IGBVF,
+    VIR_DOMAIN_NET_MODEL_LASI_I82596,
+    VIR_DOMAIN_NET_MODEL_MCF_FEC,
+    VIR_DOMAIN_NET_MODEL_MIPSNET,
+    VIR_DOMAIN_NET_MODEL_NE2000_ISA,
+    VIR_DOMAIN_NET_MODEL_NE2000_PCI,
+    VIR_DOMAIN_NET_MODEL_NPCM7XX_EMC,
+    VIR_DOMAIN_NET_MODEL_NPCM_GMAC,
+    VIR_DOMAIN_NET_MODEL_OPENCORES_ETH,
+    VIR_DOMAIN_NET_MODEL_PCNET_PCI,
+    VIR_DOMAIN_NET_MODEL_ROCKER,
+    VIR_DOMAIN_NET_MODEL_SPARP_LLAN,
+    VIR_DOMAIN_NET_MODEL_SUNGEM,
+    VIR_DOMAIN_NET_MODEL_SUNHME,
+    VIR_DOMAIN_NET_MODEL_TULIP,
+    VIR_DOMAIN_NET_MODEL_VIRTIO_NET,
+    VIR_DOMAIN_NET_MODEL_XEN_NIC,
+    VIR_DOMAIN_NET_MODEL_VIRTIO_NET_PCI,
+    VIR_DOMAIN_NET_MODEL_ETRAXFS_ETH,
+    VIR_DOMAIN_NET_MODEL_MILKYMIST_MINIMAC2,
 
     VIR_DOMAIN_NET_MODEL_LAST
 } virDomainNetModelType;
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index 1034bb57f5..fed013835d 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -2218,6 +2218,13 @@ virDomainNetDefValidate(const virDomainNetDef *net)
         break;
 
     case VIR_DOMAIN_NET_TYPE_NETWORK:
+        if (net->modelstr) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("Unsupported model type '%s'"),
+                           net->modelstr);
+            return -1;
+        }
+        break;
     case VIR_DOMAIN_NET_TYPE_VDPA:
     case VIR_DOMAIN_NET_TYPE_BRIDGE:
     case VIR_DOMAIN_NET_TYPE_CLIENT:
-- 
2.47.1
Re: [PATCH] conf: Restrict model type to enum members
Posted by Ján Tomko 1 week, 5 days ago 
On a Wednesday in 2025, Adam Julis wrote:
>Historically, we supported any string as the model type for
>network devices. This approach allowed us to stay up-to-date with
>QEMU's frequent introduction of new model types. However, this
>solution now causes more problems than benefits. A clearly
>nonsensical model name can pass validation but result in a QEMU
>internal error when the VM is started.
>
>This patch restricts model types to members of the
>'virDomainNetModelType' enum. The enum has been extended to include
>all model types currently supported by QEMU. If QEMU introduces
>new models in the future, the enum must be updated to support
>them.
>
>Resolves: https://issues.redhat.com/browse/RHEL-72082

I apologize for suggesting that issue to you, it looked easy at
the first glance but then I looked into it and wrote the comment
that I wrote.

Personally, I'm inclined to close the issue as WONTFIX, or at least
without adding any models we don't know to work for sure with libvirt.

>Signed-off-by: Adam Julis <ajulis@redhat.com>
>---
>
>I'm not sure if the virtio-net-pci should be in this enum, since its not
>located in qemu repo in hw/net/ but in hw/virtio/. When I manually
>tested it, the qemu this option supported. The last two members have not
>been supported for several years but I wanted to keep backward compatibility.
>

I think neither "virtio-net" nor "virtio-net-pci" should be in this
list. For virtio devices, libvirt expects the model to be just "virtio";
then we have logic based on the model being VIR_DOMAIN_NET_MODEL_VIRTIO
which adds the correct suffix to "virtio-net" based on whether the
address is PCI or CCW (on s390x)

>src/conf/domain_conf.c     | 23 +++++++++++++++++++++++
> src/conf/domain_conf.h     | 23 +++++++++++++++++++++++
> src/conf/domain_validate.c |  7 +++++++
> 3 files changed, 53 insertions(+)
>
>diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>index af88d0bcfd..5e8ca0015d 100644
>--- a/src/conf/domain_conf.c
>+++ b/src/conf/domain_conf.c
>@@ -613,6 +613,29 @@ VIR_ENUM_IMPL(virDomainNetModel,
>               "82540EM",
>               "82545EM",
>               "82543GC",
>+              "dp8393x",
>+              "eepro100",

That is not a valid device name. If you derived the list from the
filenames in QEMU's hw/net, that's not going to work.

hw/net/eepro100.c seems to implement various models named i8255*

>+              "ftgmac100",
>+              "igbvf",
>+              "lasi_i82596",
>+              "mcf_fec",
>+              "mipsnet",
>+              "ne2000-isa",

This also won't work because libvirt tries to assign a PCI address to
any net device that
1) should not use a S390 or CCW model
2) is not usb-net

Jano

>+              "ne2000-pci",
>+              "npcm7xx_emc",
>+              "npcm_gmac",
>+              "opencores_eth",
>+              "pcnet-pci",
>+              "rocker",
>+              "spapr_llan",
>+              "sungem",
>+              "sunhme",
>+              "tulip",
>+              "virtio-net",
>+              "xen_nic",
>+              "virtio-net-pci",
>+              "etraxfs_eth",
>+              "milkymist-minimac2",
> );
>
> VIR_ENUM_IMPL(virDomainNetDriver,
>diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>index 9f7c28343f..d7f0073ba5 100644
>--- a/src/conf/domain_conf.h
>+++ b/src/conf/domain_conf.h
>@@ -951,6 +951,29 @@ typedef enum {
>     VIR_DOMAIN_NET_MODEL_82540EM,
>     VIR_DOMAIN_NET_MODEL_82545EM,
>     VIR_DOMAIN_NET_MODEL_82543GC,
>+    VIR_DOMAIN_NET_MODEL_DP8393X,
>+    VIR_DOMAIN_NET_MODEL_EEPRO100,
>+    VIR_DOMAIN_NET_MODEL_FTGMAC100,
>+    VIR_DOMAIN_NET_MODEL_IGBVF,
>+    VIR_DOMAIN_NET_MODEL_LASI_I82596,
>+    VIR_DOMAIN_NET_MODEL_MCF_FEC,
>+    VIR_DOMAIN_NET_MODEL_MIPSNET,
>+    VIR_DOMAIN_NET_MODEL_NE2000_ISA,
>+    VIR_DOMAIN_NET_MODEL_NE2000_PCI,
>+    VIR_DOMAIN_NET_MODEL_NPCM7XX_EMC,
>+    VIR_DOMAIN_NET_MODEL_NPCM_GMAC,
>+    VIR_DOMAIN_NET_MODEL_OPENCORES_ETH,
>+    VIR_DOMAIN_NET_MODEL_PCNET_PCI,
>+    VIR_DOMAIN_NET_MODEL_ROCKER,
>+    VIR_DOMAIN_NET_MODEL_SPARP_LLAN,
>+    VIR_DOMAIN_NET_MODEL_SUNGEM,
>+    VIR_DOMAIN_NET_MODEL_SUNHME,
>+    VIR_DOMAIN_NET_MODEL_TULIP,
>+    VIR_DOMAIN_NET_MODEL_VIRTIO_NET,
>+    VIR_DOMAIN_NET_MODEL_XEN_NIC,
>+    VIR_DOMAIN_NET_MODEL_VIRTIO_NET_PCI,
>+    VIR_DOMAIN_NET_MODEL_ETRAXFS_ETH,
>+    VIR_DOMAIN_NET_MODEL_MILKYMIST_MINIMAC2,
>
>     VIR_DOMAIN_NET_MODEL_LAST
> } virDomainNetModelType;
>diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
>index 1034bb57f5..fed013835d 100644
>--- a/src/conf/domain_validate.c
>+++ b/src/conf/domain_validate.c
>@@ -2218,6 +2218,13 @@ virDomainNetDefValidate(const virDomainNetDef *net)
>         break;
>
>     case VIR_DOMAIN_NET_TYPE_NETWORK:
>+        if (net->modelstr) {
>+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>+                           _("Unsupported model type '%s'"),
>+                           net->modelstr);
>+            return -1;
>+        }
>+        break;
>     case VIR_DOMAIN_NET_TYPE_VDPA:
>     case VIR_DOMAIN_NET_TYPE_BRIDGE:
>     case VIR_DOMAIN_NET_TYPE_CLIENT:
>-- 
>2.47.1
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.