1. Patchew
  2. Libvirt
  3. View series

[PATCH] vmx: Do not require all ID data for VMWare Distributed Switch

Martin Kletzander posted 1 patch 3 months, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/7fa1180fb609c08e47caab74aa68b67e8f241c55.1720443170.git.mkletzan@redhat.com
src/conf/domain_conf.c                        | 11 ++++-----
src/conf/schemas/domaincommon.rng             | 24 ++++++++++++-------
src/vmx/vmx.c                                 | 24 ++++++++++++-------
...-portid.vmx => ethernet-vds-no-params.vmx} |  2 --
...-portid.xml => ethernet-vds-no-params.xml} |  2 +-
5 files changed, 37 insertions(+), 26 deletions(-)
rename tests/vmx2xmldata/{ethernet-vds-no-portid.vmx => ethernet-vds-no-params.vmx} (76%)
rename tests/vmx2xmldata/{ethernet-vds-no-portid.xml => ethernet-vds-no-params.xml} (82%)
[PATCH] vmx: Do not require all ID data for VMWare Distributed Switch
Posted by Martin Kletzander 3 months, 1 week ago 
Similarly to commit 2482801608b8 we can safely ignore connectionId,
portId and portgroupId in both XML and VMX as they are only a blind
pass-through between XML and VMX and an ethernet without such parameters
was spotted in the wild.  On top of that even our documentation says the
whole VMWare Distrubuted Switch configuration is a best-effort.

Resolves: https://issues.redhat.com/browse/RHEL-46099

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 src/conf/domain_conf.c                        | 11 ++++-----
 src/conf/schemas/domaincommon.rng             | 24 ++++++++++++-------
 src/vmx/vmx.c                                 | 24 ++++++++++++-------
 ...-portid.vmx => ethernet-vds-no-params.vmx} |  2 --
 ...-portid.xml => ethernet-vds-no-params.xml} |  2 +-
 5 files changed, 37 insertions(+), 26 deletions(-)
 rename tests/vmx2xmldata/{ethernet-vds-no-portid.vmx => ethernet-vds-no-params.vmx} (76%)
 rename tests/vmx2xmldata/{ethernet-vds-no-portid.xml => ethernet-vds-no-params.xml} (82%)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6080f4f90a54..bfef89e1beae 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -9593,15 +9593,14 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt,
                            def->data.vds.switch_id) < 0)
             return NULL;
 
-        if (virXMLPropLongLong(source_node, "portid", 0, VIR_XML_PROP_REQUIRED,
-                               &def->data.vds.port_id, def->data.vds.port_id) < 0)
+        if (virXMLPropLongLong(source_node, "portid", 0, VIR_XML_PROP_NONE,
+                               &def->data.vds.port_id, 0) < 0)
             return NULL;
 
-        if (!(def->data.vds.portgroup_id = virXMLPropStringRequired(source_node, "portgroupid")))
-            return NULL;
+        def->data.vds.portgroup_id = virXMLPropString(source_node, "portgroupid");
 
-        if (virXMLPropLongLong(source_node, "connectionid", 0, VIR_XML_PROP_REQUIRED,
-                               &def->data.vds.connection_id, def->data.vds.connection_id) < 0)
+        if (virXMLPropLongLong(source_node, "connectionid", 0, VIR_XML_PROP_NONE,
+                               &def->data.vds.connection_id, 0) < 0)
             return NULL;
 
         break;
diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng
index b163e4eece09..2d23fcf12375 100644
--- a/src/conf/schemas/domaincommon.rng
+++ b/src/conf/schemas/domaincommon.rng
@@ -3684,15 +3684,21 @@
               <attribute name="switchid">
                 <ref name="UUID"/>
               </attribute>
-              <attribute name="portid">
-                <data type="long"/>
-              </attribute>
-              <attribute name="portgroupid">
-                <data type="string"/>
-              </attribute>
-              <attribute name="connectionid">
-                <data type="long"/>
-              </attribute>
+              <optional>
+                <attribute name="portid">
+                  <data type="long"/>
+                </attribute>
+              </optional>
+              <optional>
+                <attribute name="portgroupid">
+                  <data type="string"/>
+                </attribute>
+              </optional>
+              <optional>
+                <attribute name="connectionid">
+                  <data type="long"/>
+                </attribute>
+              </optional>
             </element>
             <ref name="interface-options"/>
           </interleave>
diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index d082a0766010..e5bc2d793c66 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -2896,7 +2896,7 @@ virVMXParseEthernet(virConf *conf, int controller, virDomainNetDef **def)
         if (virVMXGetConfigString(conf,
                                   portgroupId_name,
                                   &(*def)->data.vds.portgroup_id,
-                                  false) < 0 ||
+                                  true) < 0 ||
             virVMXGetConfigLong(conf,
                                 portId_name,
                                 &(*def)->data.vds.port_id,
@@ -2906,7 +2906,7 @@ virVMXParseEthernet(virConf *conf, int controller, virDomainNetDef **def)
                                 connectionId_name,
                                 &(*def)->data.vds.connection_id,
                                 0,
-                                false) < 0)
+                                true) < 0)
             goto cleanup;
     } else if (connectionType == NULL && networkName == NULL) {
         (*def)->type = VIR_DOMAIN_NET_TYPE_NULL;
@@ -4038,14 +4038,22 @@ virVMXFormatEthernet(virDomainNetDef *def, int controller,
                           uuid[5], uuid[6], uuid[7], uuid[8], uuid[9], uuid[10],
                           uuid[11], uuid[12], uuid[13], uuid[14], uuid[15]);
 
-        virBufferAsprintf(buffer, "ethernet%d.dvs.portId = \"%lld\"\n",
-                          controller, def->data.vds.port_id);
+        if (def->data.vds.port_id) {
+            virBufferAsprintf(buffer, "ethernet%d.dvs.portId = \"%lld\"\n",
+                              controller, def->data.vds.port_id);
+        }
+
+        if (def->data.vds.portgroup_id) {
+            virBufferAsprintf(buffer, "ethernet%d.dvs.", controller);
+            virBufferEscapeString(buffer, "portgroupId = \"%s\"\n",
+                                  def->data.vds.portgroup_id);
+        }
 
-        virBufferAsprintf(buffer, "ethernet%d.dvs.", controller);
-        virBufferEscapeString(buffer, "portgroupId = \"%s\"\n", def->data.vds.portgroup_id);
+        if (def->data.vds.connection_id) {
+            virBufferAsprintf(buffer, "ethernet%d.dvs.connectionId = \"%lld\"\n",
+                              controller, def->data.vds.connection_id);
+        }
 
-        virBufferAsprintf(buffer, "ethernet%d.dvs.connectionId = \"%lld\"\n",
-                          controller, def->data.vds.connection_id);
         break;
     }
 
diff --git a/tests/vmx2xmldata/ethernet-vds-no-portid.vmx b/tests/vmx2xmldata/ethernet-vds-no-params.vmx
similarity index 76%
rename from tests/vmx2xmldata/ethernet-vds-no-portid.vmx
rename to tests/vmx2xmldata/ethernet-vds-no-params.vmx
index 7761accb3abc..90afbdac30cd 100644
--- a/tests/vmx2xmldata/ethernet-vds-no-portid.vmx
+++ b/tests/vmx2xmldata/ethernet-vds-no-params.vmx
@@ -5,6 +5,4 @@ ethernet0.virtualDev = "e1000e"
 ethernet0.addressType = "vpx"
 ethernet0.generatedAddress = "00:50:56:87:65:43"
 ethernet0.dvs.switchId = "50 34 26 b2 94 e9 3b 16-1d 68 87 bf ff 4a 54 40"
-ethernet0.dvs.portgroupId = "dvportgroup-1285"
-ethernet0.dvs.connectionId = "408217997"
 displayName = "test"
diff --git a/tests/vmx2xmldata/ethernet-vds-no-portid.xml b/tests/vmx2xmldata/ethernet-vds-no-params.xml
similarity index 82%
rename from tests/vmx2xmldata/ethernet-vds-no-portid.xml
rename to tests/vmx2xmldata/ethernet-vds-no-params.xml
index 60fd9c99feb9..0011ba471a50 100644
--- a/tests/vmx2xmldata/ethernet-vds-no-portid.xml
+++ b/tests/vmx2xmldata/ethernet-vds-no-params.xml
@@ -14,7 +14,7 @@
   <devices>
     <interface type='vds'>
       <mac address='00:50:56:87:65:43' type='generated'/>
-      <source switchid='503426b2-94e9-3b16-1d68-87bfff4a5440' portid='0' portgroupid='dvportgroup-1285' connectionid='408217997'/>
+      <source switchid='503426b2-94e9-3b16-1d68-87bfff4a5440'/>
       <model type='e1000e'/>
     </interface>
     <video>
-- 
2.45.1
Re: [PATCH] vmx: Do not require all ID data for VMWare Distributed Switch
Posted by Jiri Denemark 3 months, 1 week ago 
On Mon, Jul 08, 2024 at 14:52:51 +0200, Martin Kletzander wrote:
> Similarly to commit 2482801608b8 we can safely ignore connectionId,
> portId and portgroupId in both XML and VMX as they are only a blind
> pass-through between XML and VMX and an ethernet without such parameters
> was spotted in the wild.  On top of that even our documentation says the
> whole VMWare Distrubuted Switch configuration is a best-effort.
> 
> Resolves: https://issues.redhat.com/browse/RHEL-46099
> 
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
>  src/conf/domain_conf.c                        | 11 ++++-----
>  src/conf/schemas/domaincommon.rng             | 24 ++++++++++++-------
>  src/vmx/vmx.c                                 | 24 ++++++++++++-------
>  ...-portid.vmx => ethernet-vds-no-params.vmx} |  2 --
>  ...-portid.xml => ethernet-vds-no-params.xml} |  2 +-
>  5 files changed, 37 insertions(+), 26 deletions(-)
>  rename tests/vmx2xmldata/{ethernet-vds-no-portid.vmx => ethernet-vds-no-params.vmx} (76%)
>  rename tests/vmx2xmldata/{ethernet-vds-no-portid.xml => ethernet-vds-no-params.xml} (82%)

Reviewed-by: Jiri Denemark <jdenemar@redhat.com>

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.