1. Patchew
  2. Libvirt
  3. View series

[PATCH] conf: parse interface/source/@dev for all interface types (with backend type='passt')

Laine Stump posted 1 patch 1 day, 11 hours ago 
src/conf/domain_conf.c                                 | 10 +++++++---
src/conf/domain_validate.c                             |  5 +++++
.../net-vhostuser-passt.x86_64-latest.xml              |  2 ++
3 files changed, 14 insertions(+), 3 deletions(-)
[PATCH] conf: parse interface/source/@dev for all interface types (with backend type='passt')
Posted by Laine Stump 1 day, 11 hours ago 
The original implementation of the passt backend for vhost-user
interfaces erroneously forgot to parse:

  <source dev='blah'/>

for interface type='vhostuser', so it wasn't being added to the passt
commandline, and also wasn't being saved to the domain config. Now we
parse it no matter what the interface type, and then throw an error
during validation if source/@dev was specified and backend type !=
'passt' (or if interface type != 'user|vhostuser').

Fixes: 1e9054b9c79d721a55f413c2983c5370044f8f60
Resolves: https://issues.redhat.com/browse/RHEL-82539
Signed-off-by: Laine Stump <laine@redhat.com>
---
 src/conf/domain_conf.c                                 | 10 +++++++---
 src/conf/domain_validate.c                             |  5 +++++
 .../net-vhostuser-passt.x86_64-latest.xml              |  2 ++
 3 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d555873848..5daa1f89e8 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -9938,14 +9938,18 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt,
         break;
 
     case VIR_DOMAIN_NET_TYPE_USER:
-        def->sourceDev = virXMLPropString(source_node, "dev");
-        break;
-
     case VIR_DOMAIN_NET_TYPE_NULL:
     case VIR_DOMAIN_NET_TYPE_LAST:
         break;
     }
 
+    /* source/@dev is used for two different interface types and *not*
+     * stored inside the union, so we can parse it out always, and
+     * then log an error during validation if it was specified for one
+     * of the interface types that doesn't support it.
+     */
+    def->sourceDev = virXMLPropString(source_node, "dev");
+
     if ((virtualport_node = virXPathNode("./virtualport", ctxt))) {
         if (virtualport_flags == 0) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index f2a98f143d..9de442d7d7 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -2196,6 +2196,11 @@ virDomainNetDefValidate(const virDomainNetDef *net)
         }
     }
 
+    if (net->sourceDev && net->backend.type != VIR_DOMAIN_NET_BACKEND_PASST) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("The 'dev' attribute of the <source> element can only be used with interface type='user' or type='vhostuser' and backend='passt'."));
+    }
+
     if (net->nPortForwards > 0) {
         size_t p;
 
diff --git a/tests/qemuxmlconfdata/net-vhostuser-passt.x86_64-latest.xml b/tests/qemuxmlconfdata/net-vhostuser-passt.x86_64-latest.xml
index a1f9366722..529aff11f8 100644
--- a/tests/qemuxmlconfdata/net-vhostuser-passt.x86_64-latest.xml
+++ b/tests/qemuxmlconfdata/net-vhostuser-passt.x86_64-latest.xml
@@ -33,6 +33,7 @@
     <controller type='pci' index='0' model='pci-root'/>
     <interface type='vhostuser'>
       <mac address='00:11:22:33:44:55'/>
+      <source dev='eth42'/>
       <ip address='172.17.2.0' family='ipv4' prefix='24'/>
       <ip address='2001:db8:ac10:fd01::feed' family='ipv6'/>
       <portForward proto='tcp' address='2001:db8:ac10:fd01::1:10'>
@@ -63,6 +64,7 @@
     </interface>
     <interface type='vhostuser'>
       <mac address='00:11:22:33:44:11'/>
+      <source dev='eth43'/>
       <model type='virtio'/>
       <backend type='passt'/>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
-- 
2.48.1
Re: [PATCH] conf: parse interface/source/@dev for all interface types (with backend type='passt')
Posted by Peter Krempa 1 day, 10 hours ago 
On Mon, Mar 10, 2025 at 16:11:03 -0400, Laine Stump wrote:
> The original implementation of the passt backend for vhost-user
> interfaces erroneously forgot to parse:
> 
>   <source dev='blah'/>
> 
> for interface type='vhostuser', so it wasn't being added to the passt
> commandline, and also wasn't being saved to the domain config. Now we
> parse it no matter what the interface type, and then throw an error
> during validation if source/@dev was specified and backend type !=
> 'passt' (or if interface type != 'user|vhostuser').
> 
> Fixes: 1e9054b9c79d721a55f413c2983c5370044f8f60
> Resolves: https://issues.redhat.com/browse/RHEL-82539
> Signed-off-by: Laine Stump <laine@redhat.com>
> ---
>  src/conf/domain_conf.c                                 | 10 +++++++---
>  src/conf/domain_validate.c                             |  5 +++++
>  .../net-vhostuser-passt.x86_64-latest.xml              |  2 ++
>  3 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index d555873848..5daa1f89e8 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -9938,14 +9938,18 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt,
>          break;
>  
>      case VIR_DOMAIN_NET_TYPE_USER:
> -        def->sourceDev = virXMLPropString(source_node, "dev");
> -        break;
> -
>      case VIR_DOMAIN_NET_TYPE_NULL:
>      case VIR_DOMAIN_NET_TYPE_LAST:
>          break;
>      }
>  
> +    /* source/@dev is used for two different interface types and *not*
> +     * stored inside the union, so we can parse it out always, and
> +     * then log an error during validation if it was specified for one
> +     * of the interface types that doesn't support it.
> +     */
> +    def->sourceDev = virXMLPropString(source_node, "dev");
> +
>      if ((virtualport_node = virXPathNode("./virtualport", ctxt))) {
>          if (virtualport_flags == 0) {
>              virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
> index f2a98f143d..9de442d7d7 100644
> --- a/src/conf/domain_validate.c
> +++ b/src/conf/domain_validate.c
> @@ -2196,6 +2196,11 @@ virDomainNetDefValidate(const virDomainNetDef *net)
>          }
>      }
>  
> +    if (net->sourceDev && net->backend.type != VIR_DOMAIN_NET_BACKEND_PASST) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("The 'dev' attribute of the <source> element can only be used with interface type='user' or type='vhostuser' and backend='passt'."));

Missing return/goto statement. Also config errors must not use
VIR_ERR_INTERNAL_ERROR. Use the XML_ERROR or XML_DETAIL codes that I
can't remember the exact spelling of.

> +    }
> +
>      if (net->nPortForwards > 0) {
>          size_t p;
>

Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Re: [PATCH] conf: parse interface/source/@dev for all interface types (with backend type='passt')
Posted by Laine Stump 1 day, 9 hours ago 
On 3/10/25 5:06 PM, Peter Krempa wrote:
> On Mon, Mar 10, 2025 at 16:11:03 -0400, Laine Stump wrote:
>> The original implementation of the passt backend for vhost-user
>> interfaces erroneously forgot to parse:
>>
>>    <source dev='blah'/>
>>
>> for interface type='vhostuser', so it wasn't being added to the passt
>> commandline, and also wasn't being saved to the domain config. Now we
>> parse it no matter what the interface type, and then throw an error
>> during validation if source/@dev was specified and backend type !=
>> 'passt' (or if interface type != 'user|vhostuser').
>>
>> Fixes: 1e9054b9c79d721a55f413c2983c5370044f8f60
>> Resolves: https://issues.redhat.com/browse/RHEL-82539
>> Signed-off-by: Laine Stump <laine@redhat.com>
>> ---
>>   src/conf/domain_conf.c                                 | 10 +++++++---
>>   src/conf/domain_validate.c                             |  5 +++++
>>   .../net-vhostuser-passt.x86_64-latest.xml              |  2 ++
>>   3 files changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index d555873848..5daa1f89e8 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -9938,14 +9938,18 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt,
>>           break;
>>   
>>       case VIR_DOMAIN_NET_TYPE_USER:
>> -        def->sourceDev = virXMLPropString(source_node, "dev");
>> -        break;
>> -
>>       case VIR_DOMAIN_NET_TYPE_NULL:
>>       case VIR_DOMAIN_NET_TYPE_LAST:
>>           break;
>>       }
>>   
>> +    /* source/@dev is used for two different interface types and *not*
>> +     * stored inside the union, so we can parse it out always, and
>> +     * then log an error during validation if it was specified for one
>> +     * of the interface types that doesn't support it.
>> +     */
>> +    def->sourceDev = virXMLPropString(source_node, "dev");
>> +
>>       if ((virtualport_node = virXPathNode("./virtualport", ctxt))) {
>>           if (virtualport_flags == 0) {
>>               virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
>> index f2a98f143d..9de442d7d7 100644
>> --- a/src/conf/domain_validate.c
>> +++ b/src/conf/domain_validate.c
>> @@ -2196,6 +2196,11 @@ virDomainNetDefValidate(const virDomainNetDef *net)
>>           }
>>       }
>>   
>> +    if (net->sourceDev && net->backend.type != VIR_DOMAIN_NET_BACKEND_PASST) {
>> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +                       _("The 'dev' attribute of the <source> element can only be used with interface type='user' or type='vhostuser' and backend='passt'."));
> 
> Missing return/goto statement. Also config errors must not use
> VIR_ERR_INTERNAL_ERROR. Use the XML_ERROR or XML_DETAIL codes that I
> can't remember the exact spelling of.

That's what happens when you *almost* finish something on Friday, spend 
the weekend away from the keyboard, and then come back Monday having 
forgotten exactly where you were :-/. Thanks for taking it easy on me :-)


> 
>> +    }
>> +
>>       if (net->nPortForwards > 0) {
>>           size_t p;
>>
> 
> Reviewed-by: Peter Krempa <pkrempa@redhat.com>

After I fix the above, of course!

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.