[libvirt] [PATCH 3.5/7] fixes to get 3/7 past make check

Laine Stump laine at laine.org
Fri Aug 17 04:07:58 UTC 2012


Patch 3/7 fails make check, so I need to sqaush in the following to
fix it. It seemed a bit too much to squash in without getting somebody
to review it first:


1) The indentation was off for the new use of
   virDevicePCIAddressFormat in network_conf.c. Rather than just
   hacking in even more tweaks of virBufferAdjustIndent(), I corrected
   virNetworkDefFormat and the other format functions it calls
   (i.e. the rest of the file) to properly use the buffer indent
   everywhere.

2) the accessor function virNetworkDefForwardIf() needed to be updated
   to not access the newly unionized member device.dev unless the type
   was NETDEV. This caused a segfault when non-pointer data was
   interpreted as a pointer.

3) Some of the network XML test cases didn't match the output format
   (PCI addresses are always printed in hex with a 0x at the beginning).

4) Previously it was legal to put both a <pf> and a list of
   <interface> elements inside a <forward> element. That has now been
   made illegal, so it can't show up even in the *input* XML.

---
 src/conf/network_conf.c                   | 83 +++++++++++++++++--------------
 src/conf/network_conf.h                   |  3 +-
 tests/networkxml2xmlin/hostdev-pf.xml     |  2 +-
 tests/networkxml2xmlin/hostdev.xml        | 10 ++--
 tests/networkxml2xmlin/passthrough-pf.xml |  2 -
 tests/networkxml2xmlout/hostdev-pf.xml    |  4 +-
 tests/networkxml2xmlout/hostdev.xml       | 10 ++--
 7 files changed, 61 insertions(+), 53 deletions(-)

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index ef519f6..9d53d8e 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -1388,17 +1388,18 @@ virNetworkDNSDefFormat(virBufferPtr buf,
     if (def == NULL)
         goto out;
 
-    virBufferAddLit(buf, "  <dns>\n");
+    virBufferAddLit(buf, "<dns>\n");
+    virBufferAdjustIndent(buf, 2);
 
     for (i = 0 ; i < def->ntxtrecords ; i++) {
-        virBufferAsprintf(buf, "    <txt name='%s' value='%s' />\n",
+        virBufferAsprintf(buf, "<txt name='%s' value='%s' />\n",
                               def->txtrecords[i].name,
                               def->txtrecords[i].value);
     }
 
     for (i = 0 ; i < def->nsrvrecords ; i++) {
         if (def->srvrecords[i].service && def->srvrecords[i].protocol) {
-            virBufferAsprintf(buf, "    <srv service='%s' protocol='%s'",
+            virBufferAsprintf(buf, "<srv service='%s' protocol='%s'",
                                   def->srvrecords[i].service,
                                   def->srvrecords[i].protocol);
 
@@ -1423,18 +1424,19 @@ virNetworkDNSDefFormat(virBufferPtr buf,
         for (ii = 0 ; ii < def->nhosts; ii++) {
             char *ip = virSocketAddrFormat(&def->hosts[ii].ip);
 
-            virBufferAsprintf(buf, "    <host ip='%s'>\n", ip);
-
+            virBufferAsprintf(buf, "<host ip='%s'>\n", ip);
+            virBufferAdjustIndent(buf, 2);
             for (j = 0; j < def->hosts[ii].nnames; j++)
-                virBufferAsprintf(buf, "      <hostname>%s</hostname>\n",
-                                               def->hosts[ii].names[j]);
+                virBufferAsprintf(buf, "<hostname>%s</hostname>\n",
+                                  def->hosts[ii].names[j]);
 
-            virBufferAsprintf(buf, "    </host>\n");
+            virBufferAdjustIndent(buf, -2);
+            virBufferAsprintf(buf, "</host>\n");
             VIR_FREE(ip);
         }
     }
-
-    virBufferAddLit(buf, "  </dns>\n");
+    virBufferAdjustIndent(buf, -2);
+    virBufferAddLit(buf, "</dns>\n");
 out:
     return result;
 }
@@ -1445,7 +1447,7 @@ virNetworkIpDefFormat(virBufferPtr buf,
 {
     int result = -1;
 
-    virBufferAddLit(buf, "  <ip");
+    virBufferAddLit(buf, "<ip");
 
     if (def->family) {
         virBufferAsprintf(buf, " family='%s'", def->family);
@@ -1468,14 +1470,17 @@ virNetworkIpDefFormat(virBufferPtr buf,
         virBufferAsprintf(buf," prefix='%u'", def->prefix);
     }
     virBufferAddLit(buf, ">\n");
+    virBufferAdjustIndent(buf, 2);
 
     if (def->tftproot) {
-        virBufferEscapeString(buf, "    <tftp root='%s' />\n",
+        virBufferEscapeString(buf, "<tftp root='%s' />\n",
                               def->tftproot);
     }
     if ((def->nranges || def->nhosts)) {
         int ii;
-        virBufferAddLit(buf, "    <dhcp>\n");
+        virBufferAddLit(buf, "<dhcp>\n");
+        virBufferAdjustIndent(buf, 2);
+
         for (ii = 0 ; ii < def->nranges ; ii++) {
             char *saddr = virSocketAddrFormat(&def->ranges[ii].start);
             if (!saddr)
@@ -1485,13 +1490,13 @@ virNetworkIpDefFormat(virBufferPtr buf,
                 VIR_FREE(saddr);
                 goto error;
             }
-            virBufferAsprintf(buf, "      <range start='%s' end='%s' />\n",
+            virBufferAsprintf(buf, "<range start='%s' end='%s' />\n",
                               saddr, eaddr);
             VIR_FREE(saddr);
             VIR_FREE(eaddr);
         }
         for (ii = 0 ; ii < def->nhosts ; ii++) {
-            virBufferAddLit(buf, "      <host ");
+            virBufferAddLit(buf, "<host ");
             if (def->hosts[ii].mac)
                 virBufferAsprintf(buf, "mac='%s' ", def->hosts[ii].mac);
             if (def->hosts[ii].name)
@@ -1506,7 +1511,7 @@ virNetworkIpDefFormat(virBufferPtr buf,
             virBufferAddLit(buf, "/>\n");
         }
         if (def->bootfile) {
-            virBufferEscapeString(buf, "      <bootp file='%s' ",
+            virBufferEscapeString(buf, "<bootp file='%s' ",
                                   def->bootfile);
             if (VIR_SOCKET_ADDR_VALID(&def->bootserver)) {
                 char *ipaddr = virSocketAddrFormat(&def->bootserver);
@@ -1516,12 +1521,15 @@ virNetworkIpDefFormat(virBufferPtr buf,
                 VIR_FREE(ipaddr);
             }
             virBufferAddLit(buf, "/>\n");
+
         }
 
-        virBufferAddLit(buf, "    </dhcp>\n");
+        virBufferAdjustIndent(buf, -2);
+        virBufferAddLit(buf, "</dhcp>\n");
     }
 
-    virBufferAddLit(buf, "  </ip>\n");
+    virBufferAdjustIndent(buf, -2);
+    virBufferAddLit(buf, "</ip>\n");
 
     result = 0;
 error:
@@ -1532,19 +1540,19 @@ static int
 virPortGroupDefFormat(virBufferPtr buf,
                       const virPortGroupDefPtr def)
 {
-    virBufferAsprintf(buf, "  <portgroup name='%s'", def->name);
+    virBufferAsprintf(buf, "<portgroup name='%s'", def->name);
     if (def->isDefault) {
         virBufferAddLit(buf, " default='yes'");
     }
     virBufferAddLit(buf, ">\n");
-    virBufferAdjustIndent(buf, 4);
+    virBufferAdjustIndent(buf, 2);
     if (virNetDevVlanFormat(&def->vlan, buf) < 0)
         return -1;
     if (virNetDevVPortProfileFormat(def->virtPortProfile, buf) < 0)
         return -1;
     virNetDevBandwidthFormat(def->bandwidth, buf);
-    virBufferAdjustIndent(buf, -4);
-    virBufferAddLit(buf, "  </portgroup>\n");
+    virBufferAdjustIndent(buf, -2);
+    virBufferAddLit(buf, "</portgroup>\n");
     return 0;
 }
 
@@ -1560,11 +1568,12 @@ char *virNetworkDefFormat(const virNetworkDefPtr def, unsigned int flags)
         virBufferAsprintf(&buf, " connections='%d'", def->connections);
     }
     virBufferAddLit(&buf, ">\n");
-    virBufferEscapeString(&buf, "  <name>%s</name>\n", def->name);
+    virBufferAdjustIndent(&buf, 2);
+    virBufferEscapeString(&buf, "<name>%s</name>\n", def->name);
 
     uuid = def->uuid;
     virUUIDFormat(uuid, uuidstr);
-    virBufferAsprintf(&buf, "  <uuid>%s</uuid>\n", uuidstr);
+    virBufferAsprintf(&buf, "<uuid>%s</uuid>\n", uuidstr);
 
     if (def->forwardType != VIR_NETWORK_FORWARD_NONE) {
         const char *dev = NULL;
@@ -1578,27 +1587,29 @@ char *virNetworkDefFormat(const virNetworkDefPtr def, unsigned int flags)
                            def->forwardType, def->name);
             goto error;
         }
-        virBufferAddLit(&buf, "  <forward");
+        virBufferAddLit(&buf, "<forward");
         virBufferEscapeString(&buf, " dev='%s'", dev);
+        virBufferAsprintf(&buf, " mode='%s'", mode);
         if (def->forwardType == VIR_NETWORK_FORWARD_HOSTDEV) {
             if (def->managed == 1)
                 virBufferAddLit(&buf, " managed='yes'");
             else
                 virBufferAddLit(&buf, " managed='no'");
         }
-        virBufferAsprintf(&buf, " mode='%s'%s>\n", mode,
+        virBufferAsprintf(&buf, "%s>\n",
                           (def->nForwardIfs || def->nForwardPfs) ? "" : "/");
+        virBufferAdjustIndent(&buf, 2);
 
         /* For now, hard-coded to at most 1 forwardPfs */
         if (def->nForwardPfs)
-            virBufferEscapeString(&buf, "    <pf dev='%s'/>\n",
+            virBufferEscapeString(&buf, "<pf dev='%s'/>\n",
                                   def->forwardPfs[0].dev);
 
         if (def->nForwardIfs &&
             (!def->nForwardPfs || !(flags & VIR_NETWORK_XML_INACTIVE))) {
             for (ii = 0; ii < def->nForwardIfs; ii++) {
                 if (def->forwardType != VIR_NETWORK_FORWARD_HOSTDEV) {
-                    virBufferEscapeString(&buf, "    <interface dev='%s'",
+                    virBufferEscapeString(&buf, "<interface dev='%s'",
                                           def->forwardIfs[ii].device.dev);
                     if (!(flags & VIR_NETWORK_XML_INACTIVE) &&
                         (def->forwardIfs[ii].connections > 0)) {
@@ -1617,15 +1628,16 @@ char *virNetworkDefFormat(const virNetworkDefPtr def, unsigned int flags)
                 }
             }
         }
+        virBufferAdjustIndent(&buf, -2);
         if (def->nForwardPfs || def->nForwardIfs)
-            virBufferAddLit(&buf, "  </forward>\n");
+            virBufferAddLit(&buf, "</forward>\n");
     }
 
     if (def->forwardType == VIR_NETWORK_FORWARD_NONE ||
          def->forwardType == VIR_NETWORK_FORWARD_NAT ||
          def->forwardType == VIR_NETWORK_FORWARD_ROUTE) {
 
-        virBufferAddLit(&buf, "  <bridge");
+        virBufferAddLit(&buf, "<bridge");
         if (def->bridge)
             virBufferEscapeString(&buf, " name='%s'", def->bridge);
         virBufferAsprintf(&buf, " stp='%s' delay='%ld' />\n",
@@ -1633,43 +1645,40 @@ char *virNetworkDefFormat(const virNetworkDefPtr def, unsigned int flags)
                           def->delay);
     } else if (def->forwardType == VIR_NETWORK_FORWARD_BRIDGE &&
                def->bridge) {
-       virBufferEscapeString(&buf, "  <bridge name='%s' />\n", def->bridge);
+       virBufferEscapeString(&buf, "<bridge name='%s' />\n", def->bridge);
     }
 
 
     if (def->mac_specified) {
         char macaddr[VIR_MAC_STRING_BUFLEN];
         virMacAddrFormat(&def->mac, macaddr);
-        virBufferAsprintf(&buf, "  <mac address='%s'/>\n", macaddr);
+        virBufferAsprintf(&buf, "<mac address='%s'/>\n", macaddr);
     }
 
     if (def->domain)
-        virBufferAsprintf(&buf, "  <domain name='%s'/>\n", def->domain);
+        virBufferAsprintf(&buf, "<domain name='%s'/>\n", def->domain);
 
     if (virNetworkDNSDefFormat(&buf, def->dns) < 0)
         goto error;
 
-    virBufferAdjustIndent(&buf, 2);
     if (virNetDevVlanFormat(&def->vlan, &buf) < 0)
         goto error;
     if (virNetDevBandwidthFormat(def->bandwidth, &buf) < 0)
         goto error;
-    virBufferAdjustIndent(&buf, -2);
 
     for (ii = 0; ii < def->nips; ii++) {
         if (virNetworkIpDefFormat(&buf, &def->ips[ii]) < 0)
             goto error;
     }
 
-    virBufferAdjustIndent(&buf, 2);
     if (virNetDevVPortProfileFormat(def->virtPortProfile, &buf) < 0)
         goto error;
-    virBufferAdjustIndent(&buf, -2);
 
     for (ii = 0; ii < def->nPortGroups; ii++)
         if (virPortGroupDefFormat(&buf, &def->portGroups[ii]) < 0)
             goto error;
 
+    virBufferAdjustIndent(&buf, -2);
     virBufferAddLit(&buf, "</network>\n");
 
     if (virBufferError(&buf))
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 2a2a0ba..f49c367 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -261,7 +261,8 @@ char *virNetworkDefFormat(const virNetworkDefPtr def, unsigned int flags);
 static inline const char *
 virNetworkDefForwardIf(const virNetworkDefPtr def, size_t n)
 {
-    return ((def->forwardIfs && (def->nForwardIfs > n))
+    return ((def->forwardIfs && (def->nForwardIfs > n) &&
+             def->forwardIfs[n].type == VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV)
             ? def->forwardIfs[n].device.dev : NULL);
 }
 
diff --git a/tests/networkxml2xmlin/hostdev-pf.xml b/tests/networkxml2xmlin/hostdev-pf.xml
index fc82c55..7bf857d 100644
--- a/tests/networkxml2xmlin/hostdev-pf.xml
+++ b/tests/networkxml2xmlin/hostdev-pf.xml
@@ -1,7 +1,7 @@
 <network>
   <name>hostdev</name>
   <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid>
-  <forward mode="hostdev" managed="yes">
+  <forward mode='hostdev' managed='yes'>
     <pf dev='eth2'/>
   </forward>
 </network>
diff --git a/tests/networkxml2xmlin/hostdev.xml b/tests/networkxml2xmlin/hostdev.xml
index 0ec52d2..03f1411 100644
--- a/tests/networkxml2xmlin/hostdev.xml
+++ b/tests/networkxml2xmlin/hostdev.xml
@@ -1,10 +1,10 @@
 <network>
   <name>hostdev</name>
   <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid>
-  <forward mode="hostdev" managed="yes">
-    <address type='pci' domain='0' bus='3' slot='0' function='1'/>
-    <address type='pci' domain='0' bus='3' slot='0' function='2'/>
-    <address type='pci' domain='0' bus='3' slot='0' function='3'/>
-    <address type='pci' domain='0' bus='3' slot='0' function='4'/>
+  <forward mode='hostdev' managed='yes'>
+    <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x1'/>
+    <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x2'/>
+    <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x3'/>
+    <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x4'/>
   </forward>
 </network>
diff --git a/tests/networkxml2xmlin/passthrough-pf.xml b/tests/networkxml2xmlin/passthrough-pf.xml
index e63aae0..ecdb953 100644
--- a/tests/networkxml2xmlin/passthrough-pf.xml
+++ b/tests/networkxml2xmlin/passthrough-pf.xml
@@ -3,8 +3,6 @@
   <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid>
   <forward mode="passthrough">
     <pf dev='eth0'/>
-    <interface dev='eth10'/>
-    <interface dev='eth11'/>
   </forward>
   <ip address="192.168.122.1" netmask="255.255.255.0"/>
 </network>
diff --git a/tests/networkxml2xmlout/hostdev-pf.xml b/tests/networkxml2xmlout/hostdev-pf.xml
index e955312..7bf857d 100644
--- a/tests/networkxml2xmlout/hostdev-pf.xml
+++ b/tests/networkxml2xmlout/hostdev-pf.xml
@@ -1,7 +1,7 @@
 <network>
   <name>hostdev</name>
   <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid>
-  <forward mode="hostdev" managed="yes">
+  <forward mode='hostdev' managed='yes'>
     <pf dev='eth2'/>
-   </forward>
+  </forward>
 </network>
diff --git a/tests/networkxml2xmlout/hostdev.xml b/tests/networkxml2xmlout/hostdev.xml
index 0ec52d2..03f1411 100644
--- a/tests/networkxml2xmlout/hostdev.xml
+++ b/tests/networkxml2xmlout/hostdev.xml
@@ -1,10 +1,10 @@
 <network>
   <name>hostdev</name>
   <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid>
-  <forward mode="hostdev" managed="yes">
-    <address type='pci' domain='0' bus='3' slot='0' function='1'/>
-    <address type='pci' domain='0' bus='3' slot='0' function='2'/>
-    <address type='pci' domain='0' bus='3' slot='0' function='3'/>
-    <address type='pci' domain='0' bus='3' slot='0' function='4'/>
+  <forward mode='hostdev' managed='yes'>
+    <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x1'/>
+    <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x2'/>
+    <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x3'/>
+    <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x4'/>
   </forward>
 </network>
-- 
1.7.11.4




More information about the libvir-list mailing list