[libvirt] [PATCH 12/33] Fix error reporting in port profile parsing/formatting APIs

Daniel P. Berrange berrange at redhat.com
Thu Nov 3 17:30:08 UTC 2011


From: "Daniel P. Berrange" <berrange at redhat.com>

The virtual port profile parsing/formatting APIs do not
correctly handle unknown profile type strings/numbers.
They behave as a no-op, instead of raising an error

* src/util/network.c, src/util/network.h: Fix error
  handling of port profile APIs
* src/conf/domain_conf.c, src/conf/network_conf.c: Update
  for API changes
---
 src/conf/domain_conf.c  |   16 +++++++-----
 src/conf/network_conf.c |   20 +++++++-------
 src/util/network.c      |   61 ++++++++++++++++++++++++-----------------------
 src/util/network.h      |    8 +++---
 4 files changed, 54 insertions(+), 51 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c1f8950..b1e47a3 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3112,10 +3112,9 @@ virDomainActualNetDefParseXML(xmlNodePtr node,
 
         virtPortNode = virXPathNode("./virtualport", ctxt);
         if (virtPortNode &&
-            virNetDevVPortProfileParse(virtPortNode,
-                                       &actual->data.direct.virtPortProfile) < 0) {
+            (!(actual->data.direct.virtPortProfile =
+               virNetDevVPortProfileParse(virtPortNode))))
             goto error;
-        }
     }
 
     bandwidth_node = virXPathNode("./bandwidth", ctxt);
@@ -3221,7 +3220,7 @@ virDomainNetDefParseXML(virCapsPtr caps,
                        ((def->type == VIR_DOMAIN_NET_TYPE_DIRECT) ||
                         (def->type == VIR_DOMAIN_NET_TYPE_NETWORK)) &&
                        xmlStrEqual(cur->name, BAD_CAST "virtualport")) {
-                if (virNetDevVPortProfileParse(cur, &virtPort) < 0)
+                if (!(virtPort = virNetDevVPortProfileParse(cur)))
                     goto error;
             } else if ((network == NULL) &&
                        ((def->type == VIR_DOMAIN_NET_TYPE_SERVER) ||
@@ -9721,7 +9720,8 @@ virDomainActualNetDefFormat(virBufferPtr buf,
         }
         virBufferAsprintf(buf, " mode='%s'/>\n", mode);
         virBufferAdjustIndent(buf, 8);
-        virNetDevVPortProfileFormat(def->data.direct.virtPortProfile, buf);
+        if (virNetDevVPortProfileFormat(def->data.direct.virtPortProfile, buf) < 0)
+            goto error;
         virBufferAdjustIndent(buf, -8);
         break;
     default:
@@ -9768,7 +9768,8 @@ virDomainNetDefFormat(virBufferPtr buf,
                               def->data.network.portgroup);
         virBufferAddLit(buf, "/>\n");
         virBufferAdjustIndent(buf, 6);
-        virNetDevVPortProfileFormat(def->data.network.virtPortProfile, buf);
+        if (virNetDevVPortProfileFormat(def->data.network.virtPortProfile, buf) < 0)
+            return -1;
         virBufferAdjustIndent(buf, -6);
         if ((flags & VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET) &&
             (virDomainActualNetDefFormat(buf, def->data.network.actual) < 0))
@@ -9818,7 +9819,8 @@ virDomainNetDefFormat(virBufferPtr buf,
                           virMacvtapModeTypeToString(def->data.direct.mode));
         virBufferAddLit(buf, "/>\n");
         virBufferAdjustIndent(buf, 6);
-        virNetDevVPortProfileFormat(def->data.direct.virtPortProfile, buf);
+        if (virNetDevVPortProfileFormat(def->data.direct.virtPortProfile, buf) < 0)
+            return -1;
         virBufferAdjustIndent(buf, -6);
         break;
 
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 023eb9f..5e38bee 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -790,10 +790,8 @@ virNetworkPortGroupParseXML(virPortGroupDefPtr def,
 
     virtPortNode = virXPathNode("./virtualport", ctxt);
     if (virtPortNode &&
-        (virNetDevVPortProfileParse(virtPortNode,
-                                    &def->virtPortProfile) < 0)) {
+        (!(def->virtPortProfile = virNetDevVPortProfileParse(virtPortNode))))
         goto error;
-    }
 
     bandwidth_node = virXPathNode("./bandwidth", ctxt);
     if (bandwidth_node &&
@@ -894,10 +892,8 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
 
     virtPortNode = virXPathNode("./virtualport", ctxt);
     if (virtPortNode &&
-        (virNetDevVPortProfileParse(virtPortNode,
-                                    &def->virtPortProfile) < 0)) {
+        (!(def->virtPortProfile = virNetDevVPortProfileParse(virtPortNode))))
         goto error;
-    }
 
     nPortGroups = virXPathNodeSet("./portgroup", ctxt, &portGroupNodes);
     if (nPortGroups < 0)
@@ -1258,7 +1254,7 @@ error:
     return result;
 }
 
-static void
+static int
 virPortGroupDefFormat(virBufferPtr buf,
                       const virPortGroupDefPtr def)
 {
@@ -1268,10 +1264,12 @@ virPortGroupDefFormat(virBufferPtr buf,
     }
     virBufferAddLit(buf, ">\n");
     virBufferAdjustIndent(buf, 4);
-    virNetDevVPortProfileFormat(def->virtPortProfile, buf);
+    if (virNetDevVPortProfileFormat(def->virtPortProfile, buf) < 0)
+        return -1;
     virNetDevBandwidthFormat(def->bandwidth, buf);
     virBufferAdjustIndent(buf, -4);
     virBufferAddLit(buf, "  </portgroup>\n");
+    return 0;
 }
 
 char *virNetworkDefFormat(const virNetworkDefPtr def)
@@ -1354,11 +1352,13 @@ char *virNetworkDefFormat(const virNetworkDefPtr def)
     }
 
     virBufferAdjustIndent(&buf, 2);
-    virNetDevVPortProfileFormat(def->virtPortProfile, &buf);
+    if (virNetDevVPortProfileFormat(def->virtPortProfile, &buf) < 0)
+        goto error;
     virBufferAdjustIndent(&buf, -2);
 
     for (ii = 0; ii < def->nPortGroups; ii++)
-        virPortGroupDefFormat(&buf, &def->portGroups[ii]);
+        if (virPortGroupDefFormat(&buf, &def->portGroups[ii]) < 0)
+            goto error;
 
     virBufferAddLit(&buf, "</network>\n");
 
diff --git a/src/util/network.c b/src/util/network.c
index f7f5d6c..c467121 100644
--- a/src/util/network.c
+++ b/src/util/network.c
@@ -684,11 +684,10 @@ VIR_ENUM_IMPL(virNetDevVPort, VIR_NETDEV_VPORT_PROFILE_LAST,
               "802.1Qbg",
               "802.1Qbh")
 
-int
-virNetDevVPortProfileParse(xmlNodePtr node,
-                           virNetDevVPortProfilePtr *def)
+
+virNetDevVPortProfilePtr
+virNetDevVPortProfileParse(xmlNodePtr node)
 {
-    int ret = -1;
     char *virtPortType;
     char *virtPortManagerID = NULL;
     char *virtPortTypeID = NULL;
@@ -700,13 +699,19 @@ virNetDevVPortProfileParse(xmlNodePtr node,
 
     if (VIR_ALLOC(virtPort) < 0) {
         virReportOOMError();
-        return -1;
+        return NULL;
     }
 
     virtPortType = virXMLPropString(node, "type");
     if (!virtPortType) {
         virSocketError(VIR_ERR_XML_ERROR, "%s",
-                             _("missing virtualportprofile type"));
+                       _("missing virtualportprofile type"));
+        goto error;
+    }
+
+    if ((virtPortType = virNetDevVPortTypeFromString(virtPortType)) <= 0) {
+        virSocketError(VIR_ERR_XML_ERROR,
+                       _("unknown virtualportprofile type %s"), virtPortType);
         goto error;
     }
 
@@ -725,10 +730,7 @@ virNetDevVPortProfileParse(xmlNodePtr node,
         cur = cur->next;
     }
 
-    virtPort->virtPortType = VIR_NETDEV_VPORT_PROFILE_NONE;
-
-    switch (virNetDevVPortTypeFromString(virtPortType)) {
-
+    switch (virtPort->virtPortType) {
     case VIR_NETDEV_VPORT_PROFILE_8021QBG:
         if (virtPortManagerID     != NULL && virtPortTypeID     != NULL &&
             virtPortTypeIDVersion != NULL) {
@@ -798,7 +800,7 @@ virNetDevVPortProfileParse(xmlNodePtr node,
                                          _("a parameter is missing for 802.1Qbg description"));
             goto error;
         }
-    break;
+        break;
 
     case VIR_NETDEV_VPORT_PROFILE_8021QBH:
         if (virtPortProfileID != NULL) {
@@ -815,23 +817,15 @@ virNetDevVPortProfileParse(xmlNodePtr node,
                                  _("profileid parameter is missing for 802.1Qbh descripion"));
             goto error;
         }
-    break;
-
+        break;
 
     default:
-    case VIR_NETDEV_VPORT_PROFILE_NONE:
-    case VIR_NETDEV_VPORT_PROFILE_LAST:
-         virSocketError(VIR_ERR_XML_ERROR, "%s",
-                              _("unknown virtualport type"));
+        virSocketError(VIR_ERR_XML_ERROR,
+                       _("unexpected virtualport type %d"), virtPort->virtPortType);
         goto error;
-    break;
     }
 
-    ret = 0;
-    *def = virtPort;
-    virtPort = NULL;
-error:
-    VIR_FREE(virtPort);
+cleanup:
     VIR_FREE(virtPortManagerID);
     VIR_FREE(virtPortTypeID);
     VIR_FREE(virtPortTypeIDVersion);
@@ -839,7 +833,11 @@ error:
     VIR_FREE(virtPortProfileID);
     VIR_FREE(virtPortType);
 
-    return ret;
+    return virtPort;
+
+error:
+    VIR_FREE(virtPort);
+    goto cleanup;
 }
 
 bool
@@ -879,23 +877,20 @@ virNetDevVPortProfileEqual(virNetDevVPortProfilePtr a, virNetDevVPortProfilePtr
     return true;
 }
 
-void
+
+int
 virNetDevVPortProfileFormat(virNetDevVPortProfilePtr virtPort,
                             virBufferPtr buf)
 {
     char uuidstr[VIR_UUID_STRING_BUFLEN];
 
     if (!virtPort || virtPort->virtPortType == VIR_NETDEV_VPORT_PROFILE_NONE)
-        return;
+        return 0;
 
     virBufferAsprintf(buf, "<virtualport type='%s'>\n",
                       virNetDevVPortTypeToString(virtPort->virtPortType));
 
     switch (virtPort->virtPortType) {
-    case VIR_NETDEV_VPORT_PROFILE_NONE:
-    case VIR_NETDEV_VPORT_PROFILE_LAST:
-        break;
-
     case VIR_NETDEV_VPORT_PROFILE_8021QBG:
         virUUIDFormat(virtPort->u.virtPort8021Qbg.instanceID,
                       uuidstr);
@@ -913,9 +908,15 @@ virNetDevVPortProfileFormat(virNetDevVPortProfilePtr virtPort,
                           "  <parameters profileid='%s'/>\n",
                           virtPort->u.virtPort8021Qbh.profileID);
         break;
+
+    default:
+        virSocketError(VIR_ERR_XML_ERROR,
+                       _("unexpected virtualport type %d"), virtPort->virtPortType);
+        return -1;
     }
 
     virBufferAddLit(buf, "</virtualport>\n");
+    return 0;
 }
 
 static int
diff --git a/src/util/network.h b/src/util/network.h
index c3ab623..98dfacc 100644
--- a/src/util/network.h
+++ b/src/util/network.h
@@ -142,11 +142,11 @@ struct _virNetDevVPortProfile {
     } u;
 };
 
-int
-virNetDevVPortProfileParse(xmlNodePtr node,
-                           virNetDevVPortProfilePtr *def);
 
-void
+virNetDevVPortProfilePtr
+virNetDevVPortProfileParse(xmlNodePtr node);
+
+int
 virNetDevVPortProfileFormat(virNetDevVPortProfilePtr virtPort,
                             virBufferPtr buf);
 
-- 
1.7.6.4




More information about the libvir-list mailing list