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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list