lxc: move debug/error log when adding IP addresses to virNetDevIPAddrAdd

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.

diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 703b0d69ee1fcdb08adb6104056793c54bc0fc8b..0d5f16c517661b2c79bfa952810f0f16894a76f7 100644 (file)
--- 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,31 +518,24 @@ 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);
                 VIR_FREE(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++) {
@@ -554,22 +546,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 376d4adbdb31e03f77d67a59a3cf8b0198e97759..01d48671067019d7b59964d2a24c608d5d56e530 100644 (file)
--- a/src/util/virnetdevip.c
+++ b/src/util/virnetdevip.c
@@ -173,36 +173,61 @@ 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 &&
         !(peer && VIR_SOCKET_ADDR_VALID(peer))) {
         /* compute a broadcast address if this is IPv4 */
         if (VIR_ALLOC(broadcast) < 0)
-            return -1;
+            goto cleanup;
 
-        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",
+              NULLSTR(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);