From 4ff9ec7dae74ff6cc0f5ce1ae46a90f3a057be94 Mon Sep 17 00:00:00 2001 From: Laine Stump Date: Jun 26 2016 23:33:10 +0000 Subject: 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 703b0d6..0d5f16c 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,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 376d4ad..01d4867 100644 --- 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);