[libvirt] [PATCH 20/28] lxc: move debug/error log when adding IP addresses to virNetDevIPAddrAdd

Laine Stump laine at laine.org
Wed Jun 22 17:37:19 UTC 2016


It makes more sense to have the logging at the lower level so other
callers can share the goodness.

While removing so much stuff from / touching so many lines in
lxcContainerRenameAndEnableInterfaces() (which used to have this
debug/error logging), label names were changed and it was updated to
use the now-more-common method of initializing ret to -1 (failure),
then setting to 0 right before the cleanup label.
---
 src/lxc/lxc_container.c | 60 +++++++++++++++++++++----------------------------
 src/util/virnetdevip.c  | 32 ++++++++++++++++++++++----
 2 files changed, 53 insertions(+), 39 deletions(-)

diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index a5ced92..09ad8cb 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -484,33 +484,32 @@ lxcContainerGetNetDef(virDomainDefPtr vmDef, const char *devName)
  *
  * Returns 0 on success or nonzero in case of error
  */
-static int lxcContainerRenameAndEnableInterfaces(virDomainDefPtr vmDef,
-                                                 size_t nveths,
-                                                 char **veths)
+static int
+lxcContainerRenameAndEnableInterfaces(virDomainDefPtr vmDef,
+                                      size_t nveths,
+                                      char **veths)
 {
-    int rc = 0;
+    int ret = -1;
     size_t i, j;
     const char *newname;
-    char *toStr = NULL;
-    char *viaStr = NULL;
     virDomainNetDefPtr netDef;
     bool privNet = vmDef->features[VIR_DOMAIN_FEATURE_PRIVNET] ==
                    VIR_TRISTATE_SWITCH_ON;
 
     for (i = 0; i < nveths; i++) {
         if (!(netDef = lxcContainerGetNetDef(vmDef, veths[i])))
-            return -1;
+            goto cleanup;
 
         newname = netDef->ifname_guest;
         if (!newname) {
-            rc = -1;
-            goto error_out;
+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                           _("Missing device name for container-side veth"));
+            goto cleanup;
         }
 
         VIR_DEBUG("Renaming %s to %s", veths[i], newname);
-        rc = virNetDevSetName(veths[i], newname);
-        if (rc < 0)
-            goto error_out;
+        if (virNetDevSetName(veths[i], newname) < 0)
+           goto cleanup;
 
         for (j = 0; j < netDef->guestIP.nips; j++) {
             virNetDevIPAddrPtr ip = netDef->guestIP.ips[j];
@@ -519,30 +518,23 @@ static int lxcContainerRenameAndEnableInterfaces(virDomainDefPtr vmDef,
 
             if ((prefix = virSocketAddrGetIPPrefix(&ip->address,
                                                    NULL, ip->prefix)) < 0) {
+                ipStr = virSocketAddrFormat(&ip->address);
                 virReportError(VIR_ERR_INTERNAL_ERROR,
                                _("Failed to determine prefix for IP address '%s'"),
                                ipStr);
-                goto error_out;
-            }
-
-            VIR_DEBUG("Adding IP address '%s/%d' to '%s'",
-                      ipStr, prefix, newname);
-            if (virNetDevIPAddrAdd(newname, &ip->address, NULL, prefix) < 0) {
-                virReportError(VIR_ERR_SYSTEM_ERROR,
-                               _("Failed to set IP address '%s' on %s"),
-                               ipStr, newname);
                 VIR_FREE(ipStr);
-                goto error_out;
+                goto cleanup;
             }
-            VIR_FREE(ipStr);
+
+            if (virNetDevIPAddrAdd(newname, &ip->address, NULL, prefix) < 0)
+                goto cleanup;
         }
 
         if (netDef->guestIP.nips ||
             netDef->linkstate == VIR_DOMAIN_NET_INTERFACE_LINK_STATE_UP) {
             VIR_DEBUG("Enabling %s", newname);
-            rc = virNetDevSetOnline(newname, true);
-            if (rc < 0)
-                goto error_out;
+            if (virNetDevSetOnline(newname, true) < 0)
+                goto cleanup;
 
             /* Set the routes */
             for (j = 0; j < netDef->guestIP.nroutes; j++) {
@@ -553,22 +545,20 @@ static int lxcContainerRenameAndEnableInterfaces(virDomainDefPtr vmDef,
                                         virNetDevIPRouteGetPrefix(route),
                                         virNetDevIPRouteGetGateway(route),
                                         virNetDevIPRouteGetMetric(route)) < 0) {
-                    goto error_out;
+                    goto cleanup;
                 }
-                VIR_FREE(toStr);
-                VIR_FREE(viaStr);
             }
         }
     }
 
     /* enable lo device only if there were other net devices */
-    if (veths || privNet)
-        rc = virNetDevSetOnline("lo", true);
+    if ((veths || privNet) &&
+        virNetDevSetOnline("lo", true) < 0)
+       goto cleanup;
 
- error_out:
-    VIR_FREE(toStr);
-    VIR_FREE(viaStr);
-    return rc;
+    ret = 0;
+ cleanup:
+    return ret;
 }
 
 
diff --git a/src/util/virnetdevip.c b/src/util/virnetdevip.c
index 376d4ad..dad342f 100644
--- a/src/util/virnetdevip.c
+++ b/src/util/virnetdevip.c
@@ -173,6 +173,13 @@ virNetDevIPAddrAdd(const char *ifname,
     struct nl_msg *nlmsg = NULL;
     struct nlmsghdr *resp = NULL;
     unsigned int recvbuflen;
+    char *ipStr = NULL;
+    char *peerStr = NULL;
+    char *bcastStr = NULL;
+
+    ipStr = virSocketAddrFormat(addr);
+    if (peer && VIR_SOCKET_ADDR_VALID(peer))
+       peerStr = virSocketAddrFormat(peer);
 
     /* The caller needs to provide a correct address */
     if (VIR_SOCKET_ADDR_FAMILY(addr) == AF_INET &&
@@ -181,28 +188,45 @@ virNetDevIPAddrAdd(const char *ifname,
         if (VIR_ALLOC(broadcast) < 0)
             return -1;
 
-        if (virSocketAddrBroadcastByPrefix(addr, prefix, broadcast) < 0)
+        if (virSocketAddrBroadcastByPrefix(addr, prefix, broadcast) < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Failed to determine broadcast address for '%s/%d'"),
+                       ipStr, prefix);
             goto cleanup;
+        }
+        bcastStr = virSocketAddrFormat(broadcast);
     }
 
+    VIR_DEBUG("Adding IP address %s/%d%s%s%s%s to %s", ipStr, prefix,
+              peerStr ? " peer " : "", peerStr ? peerStr : "",
+              bcastStr ? " bcast " : "", bcastStr ? bcastStr : "",
+              ifname);
+
     if (!(nlmsg = virNetDevCreateNetlinkAddressMessage(RTM_NEWADDR, ifname,
                                                        addr, prefix,
                                                        broadcast, peer)))
         goto cleanup;
 
-    if (virNetlinkCommand(nlmsg, &resp, &recvbuflen, 0, 0,
-                          NETLINK_ROUTE, 0) < 0)
+    if (virNetlinkCommand(nlmsg, &resp, &recvbuflen,
+                          0, 0, NETLINK_ROUTE, 0) < 0)
         goto cleanup;
 
 
     if (virNetlinkGetErrorCode(resp, recvbuflen) < 0) {
         virReportError(VIR_ERR_SYSTEM_ERROR,
-                       _("Error adding IP address to %s"), ifname);
+                       _("Failed to add IP address %s/%d%s%s%s%s to %s"),
+                       ipStr, prefix,
+                       peerStr ? " peer " : "", peerStr ? peerStr : "",
+                       bcastStr ? " bcast " : "", bcastStr ? bcastStr : "",
+                       ifname);
         goto cleanup;
     }
 
     ret = 0;
  cleanup:
+    VIR_FREE(ipStr);
+    VIR_FREE(peerStr);
+    VIR_FREE(bcastStr);
     nlmsg_free(nlmsg);
     VIR_FREE(resp);
     VIR_FREE(broadcast);
-- 
2.5.5




More information about the libvir-list mailing list