interface: clean up virInterfaceDefDevFormat

This modifies the formatting function of virInterface to be a proper
mirror of the parse function, including the addition of a
"parentIfType" arg so that we can decide whether or not it is
appropriate to emit the elements that are only in toplevel interfaces,
as well as the <link> element (which isn't allowed for bridge
interfaces).

Since the restructuring of the code necessarily changes the order of
some of the elements, some test case data had to be updated.

diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c
index 883053f92390cf20aed8be42aef86165fc7d7298..141b4a26320998dc7bc668323e13a7b10441ca2a 100644 (file)
--- a/src/conf/interface_conf.c
+++ b/src/conf/interface_conf.c
@@ -41,7 +41,8 @@ VIR_ENUM_IMPL(virInterface,
 static virInterfaceDefPtr
 virInterfaceDefParseXML(xmlXPathContextPtr ctxt, int parentIfType);
 static int
-virInterfaceDefDevFormat(virBufferPtr buf, const virInterfaceDef *def);
+virInterfaceDefDevFormat(virBufferPtr buf, const virInterfaceDef *def,
+                         virInterfaceType parentIfType);
 
 static
 void virInterfaceIpDefFree(virInterfaceIpDefPtr def)
@@ -870,7 +871,8 @@ virInterfaceBridgeDefFormat(virBufferPtr buf, const virInterfaceDef *def)
     virBufferAdjustIndent(buf, 2);
 
     for (i = 0; i < def->data.bridge.nbItf; i++) {
-        if (virInterfaceDefDevFormat(buf, def->data.bridge.itf[i]) < 0)
+        if (virInterfaceDefDevFormat(buf, def->data.bridge.itf[i],
+                                     VIR_INTERFACE_TYPE_BRIDGE) < 0)
             ret = -1;
     }
 
@@ -932,7 +934,8 @@ virInterfaceBondDefFormat(virBufferPtr buf, const virInterfaceDef *def)
         virBufferAddLit(buf, "/>\n");
     }
     for (i = 0; i < def->data.bond.nbItf; i++) {
-        if (virInterfaceDefDevFormat(buf, def->data.bond.itf[i]) < 0)
+        if (virInterfaceDefDevFormat(buf, def->data.bond.itf[i],
+                                     VIR_INTERFACE_TYPE_BOND) < 0)
             ret = -1;
     }
 
@@ -1035,7 +1038,8 @@ virInterfaceStartmodeDefFormat(virBufferPtr buf,
 }
 
 static int
-virInterfaceDefDevFormat(virBufferPtr buf, const virInterfaceDef *def)
+virInterfaceDefDevFormat(virBufferPtr buf, const virInterfaceDef *def,
+                         virInterfaceType parentIfType)
 {
     const char *type = NULL;
 
@@ -1063,39 +1067,33 @@ virInterfaceDefDevFormat(virBufferPtr buf, const virInterfaceDef *def)
     virBufferAddLit(buf, ">\n");
     virBufferAdjustIndent(buf, 2);
 
+    if (parentIfType == VIR_INTERFACE_TYPE_LAST) {
+        /* these elements are only valid on top-level interfaces - IP
+         * address info ("protocol") only makes sense for the
+         * top-level, and subordinate interfaces inherit the toplevel
+         * setting for mtu and start mode, which cannot be overridden.
+         */
+        virInterfaceStartmodeDefFormat(buf, def->startmode);
+        if (def->mtu)
+            virBufferAsprintf(buf, "<mtu size='%d'/>\n", def->mtu);
+        virInterfaceProtocolDefFormat(buf, def);
+    }
+
+    if (def->type != VIR_INTERFACE_TYPE_BRIDGE) {
+        virInterfaceLinkFormat(buf, &def->lnk);
+    }
     switch (def->type) {
         case VIR_INTERFACE_TYPE_ETHERNET:
-            virInterfaceStartmodeDefFormat(buf, def->startmode);
-            if (def->mac != NULL)
+            if (def->mac)
                 virBufferAsprintf(buf, "<mac address='%s'/>\n", def->mac);
-            virInterfaceLinkFormat(buf, &def->lnk);
-            if (def->mtu != 0)
-                virBufferAsprintf(buf, "<mtu size='%d'/>\n", def->mtu);
-            virInterfaceProtocolDefFormat(buf, def);
             break;
         case VIR_INTERFACE_TYPE_BRIDGE:
-            virInterfaceStartmodeDefFormat(buf, def->startmode);
-            if (def->mtu != 0)
-                virBufferAsprintf(buf, "<mtu size='%d'/>\n", def->mtu);
-            virInterfaceProtocolDefFormat(buf, def);
             virInterfaceBridgeDefFormat(buf, def);
             break;
         case VIR_INTERFACE_TYPE_BOND:
-            virInterfaceStartmodeDefFormat(buf, def->startmode);
-            virInterfaceLinkFormat(buf, &def->lnk);
-            if (def->mtu != 0)
-                virBufferAsprintf(buf, "<mtu size='%d'/>\n", def->mtu);
-            virInterfaceProtocolDefFormat(buf, def);
             virInterfaceBondDefFormat(buf, def);
             break;
         case VIR_INTERFACE_TYPE_VLAN:
-            virInterfaceStartmodeDefFormat(buf, def->startmode);
-            if (def->mac != NULL)
-                virBufferAsprintf(buf, "<mac address='%s'/>\n", def->mac);
-            virInterfaceLinkFormat(buf, &def->lnk);
-            if (def->mtu != 0)
-                virBufferAsprintf(buf, "<mtu size='%d'/>\n", def->mtu);
-            virInterfaceProtocolDefFormat(buf, def);
             virInterfaceVlanDefFormat(buf, def);
             break;
     }
@@ -1116,7 +1114,7 @@ char *virInterfaceDefFormat(const virInterfaceDef *def)
 {
     virBuffer buf = VIR_BUFFER_INITIALIZER;
 
-    if (virInterfaceDefDevFormat(&buf, def) < 0) {
+    if (virInterfaceDefDevFormat(&buf, def, VIR_INTERFACE_TYPE_LAST) < 0) {
         virBufferFreeAndReset(&buf);
         return NULL;
     }

diff --git a/tests/interfaceschemadata/bond.xml b/tests/interfaceschemadata/bond.xml
index cbc1dfa8aa4a0afd014c7da11ae5a035fc0d0f7e..3d24beae9f963b8bd136b210c6ce3dfc83c2a885 100644 (file)
--- a/tests/interfaceschemadata/bond.xml
+++ b/tests/interfaceschemadata/bond.xml
@@ -1,10 +1,10 @@
 <interface type='bond' name='bond0'>
   <start mode='none'/>
-  <link speed='1000' state='up'/>
   <protocol family='ipv4'>
     <ip address='192.168.50.7' prefix='24'/>
     <route gateway='192.168.50.1'/>
   </protocol>
+  <link speed='1000' state='up'/>
   <bond mode='active-backup'>
     <miimon freq='100' updelay='10' carrier='ioctl'/>
     <interface type='ethernet' name='eth1'>

diff --git a/tests/interfaceschemadata/bridge-no-address.xml b/tests/interfaceschemadata/bridge-no-address.xml
index 68b8c94bf1ca7dfc324e4d0e553459fd434b8b68..fd6c1a753ecd6eae79f070019c1e54f04a8c063f 100644 (file)
--- a/tests/interfaceschemadata/bridge-no-address.xml
+++ b/tests/interfaceschemadata/bridge-no-address.xml
@@ -3,8 +3,8 @@
   <mtu size='1500'/>
   <bridge stp='off'>
     <interface type='ethernet' name='eth0'>
-      <mac address='ab:bb:cc:dd:ee:ff'/>
       <link speed='1000' state='up'/>
+      <mac address='ab:bb:cc:dd:ee:ff'/>
     </interface>
     <interface type='ethernet' name='eth1'>
     </interface>

diff --git a/tests/interfaceschemadata/bridge.xml b/tests/interfaceschemadata/bridge.xml
index c865116548216ddf87bee2e7266da6e93d5e3f6b..ece087ea2ef5e3a85fd708ff44286acb15680a64 100644 (file)
--- a/tests/interfaceschemadata/bridge.xml
+++ b/tests/interfaceschemadata/bridge.xml
@@ -6,8 +6,8 @@
   </protocol>
   <bridge stp='off' delay='0.01'>
     <interface type='ethernet' name='eth0'>
-      <mac address='ab:bb:cc:dd:ee:ff'/>
       <link speed='10'/>
+      <mac address='ab:bb:cc:dd:ee:ff'/>
     </interface>
     <interface type='ethernet' name='eth1'>
     </interface>

diff --git a/tests/interfaceschemadata/ethernet-dhcp.xml b/tests/interfaceschemadata/ethernet-dhcp.xml
index c124372ff1efce5fd87409320bfeb86eb212af36..9a8a16032310f61b6324ef1319126834f1355684 100644 (file)
--- a/tests/interfaceschemadata/ethernet-dhcp.xml
+++ b/tests/interfaceschemadata/ethernet-dhcp.xml
@@ -1,9 +1,9 @@
 <interface type='ethernet' name='eth0'>
   <start mode='none'/>
-  <mac address='aa:bb:cc:dd:ee:ff'/>
-  <link state='down'/>
   <mtu size='1492'/>
   <protocol family='ipv4'>
     <dhcp peerdns='no'/>
   </protocol>
+  <link state='down'/>
+  <mac address='aa:bb:cc:dd:ee:ff'/>
 </interface>

diff --git a/tests/interfaceschemadata/vlan.xml b/tests/interfaceschemadata/vlan.xml
index 6432b9624001a5d6d53855eb1b093f82b697dcdb..301377788ae0ad1a92000603ea8736fdefd64341 100644 (file)
--- a/tests/interfaceschemadata/vlan.xml
+++ b/tests/interfaceschemadata/vlan.xml
@@ -1,9 +1,9 @@
 <interface type='vlan' name='eth0.42'>
   <start mode='onboot'/>
-  <link state='lowerlayerdown'/>
   <protocol family='ipv4'>
     <dhcp peerdns='no'/>
   </protocol>
+  <link state='lowerlayerdown'/>
   <vlan tag='42'>
     <interface name='eth0'/>
   </vlan>