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.

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;
 }
 
 

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);