mirror of
https://gitlab.com/libvirt/libvirt.git
synced 2025-01-03 11:35:19 +00:00
lxc: Fix return values of veth.c functions
Previously, the functions in src/lxc/veth.c could sometimes return positive values on failure rather than -1. This made accurate error reporting difficult, and led to one failure to catch an error in a calling function. This patch makes all the functions in veth.c consistently return 0 on success, and -1 on failure. It also fixes up the callers to the veth.c functions where necessary. Note that this patch may be related to the bug: https://bugzilla.redhat.com/show_bug.cgi?id=607496. It will not fix the bug, but should unveil what happens. * po/POTFILES.in - add veth.c, which previously had no translatable strings * src/lxc/lxc_controller.c * src/lxc/lxc_container.c * src/lxc/lxc_driver.c - fixup callers to veth.c, and remove error logs, as they are now done in veth.c * src/lxc/veth.c - make all functions consistently return -1 on error. * src/lxc/veth.h - use ATTRIBUTE_NONNULL to protect against NULL args.
This commit is contained in:
parent
1999e4f8f8
commit
938f2dbd9e
@ -33,6 +33,7 @@ src/lxc/lxc_container.c
|
||||
src/lxc/lxc_conf.c
|
||||
src/lxc/lxc_controller.c
|
||||
src/lxc/lxc_driver.c
|
||||
src/lxc/veth.c
|
||||
src/network/bridge_driver.c
|
||||
src/node_device/node_device_driver.c
|
||||
src/node_device/node_device_hal.c
|
||||
|
@ -249,26 +249,22 @@ static int lxcContainerRenameAndEnableInterfaces(unsigned int nveths,
|
||||
char *newname = NULL;
|
||||
|
||||
for (i = 0 ; i < nveths ; i++) {
|
||||
rc = virAsprintf(&newname, "eth%d", i);
|
||||
if (rc < 0)
|
||||
if (virAsprintf(&newname, "eth%d", i) < 0) {
|
||||
virReportOOMError();
|
||||
rc = -1;
|
||||
goto error_out;
|
||||
}
|
||||
|
||||
DEBUG("Renaming %s to %s", veths[i], newname);
|
||||
rc = setInterfaceName(veths[i], newname);
|
||||
if (0 != rc) {
|
||||
VIR_ERROR(_("Failed to rename %s to %s (%d)"),
|
||||
veths[i], newname, rc);
|
||||
rc = -1;
|
||||
if (rc < 0)
|
||||
goto error_out;
|
||||
}
|
||||
|
||||
DEBUG("Enabling %s", newname);
|
||||
rc = vethInterfaceUpOrDown(newname, 1);
|
||||
if (0 != rc) {
|
||||
VIR_ERROR(_("Failed to enable %s (%d)"), newname, rc);
|
||||
rc = -1;
|
||||
if (rc < 0)
|
||||
goto error_out;
|
||||
}
|
||||
|
||||
VIR_FREE(newname);
|
||||
}
|
||||
|
||||
|
@ -477,12 +477,8 @@ static int lxcControllerMoveInterfaces(unsigned int nveths,
|
||||
{
|
||||
unsigned int i;
|
||||
for (i = 0 ; i < nveths ; i++)
|
||||
if (moveInterfaceToNetNs(veths[i], container) < 0) {
|
||||
lxcError(VIR_ERR_INTERNAL_ERROR,
|
||||
_("Failed to move interface %s to ns %d"),
|
||||
veths[i], container);
|
||||
if (moveInterfaceToNetNs(veths[i], container) < 0)
|
||||
return -1;
|
||||
}
|
||||
|
||||
return 0;
|
||||
}
|
||||
@ -502,10 +498,7 @@ static int lxcControllerCleanupInterfaces(unsigned int nveths,
|
||||
{
|
||||
unsigned int i;
|
||||
for (i = 0 ; i < nveths ; i++)
|
||||
if (vethDelete(veths[i]) < 0)
|
||||
lxcError(VIR_ERR_INTERNAL_ERROR,
|
||||
_("Failed to delete veth: %s"), veths[i]);
|
||||
/* will continue to try to cleanup any other interfaces */
|
||||
vethDelete(veths[i]);
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
@ -722,7 +722,7 @@ cleanup:
|
||||
static int lxcVmCleanup(lxc_driver_t *driver,
|
||||
virDomainObjPtr vm)
|
||||
{
|
||||
int rc = -1;
|
||||
int rc = 0;
|
||||
int waitRc;
|
||||
int childStatus = -1;
|
||||
virCgroupPtr cgroup;
|
||||
@ -737,13 +737,10 @@ static int lxcVmCleanup(lxc_driver_t *driver,
|
||||
virReportSystemError(errno,
|
||||
_("waitpid failed to wait for container %d: %d"),
|
||||
vm->pid, waitRc);
|
||||
}
|
||||
|
||||
rc = 0;
|
||||
|
||||
if (WIFEXITED(childStatus)) {
|
||||
rc = WEXITSTATUS(childStatus);
|
||||
DEBUG("container exited with rc: %d", rc);
|
||||
rc = -1;
|
||||
} else if (WIFEXITED(childStatus)) {
|
||||
DEBUG("container exited with rc: %d", WEXITSTATUS(childStatus));
|
||||
rc = -1;
|
||||
}
|
||||
|
||||
/* now that we know it's stopped call the hook if present */
|
||||
@ -859,11 +856,9 @@ static int lxcSetupInterfaces(virConnectPtr conn,
|
||||
strcpy(parentVeth, def->nets[i]->ifname);
|
||||
}
|
||||
DEBUG("parentVeth: %s, containerVeth: %s", parentVeth, containerVeth);
|
||||
if (0 != (rc = vethCreate(parentVeth, PATH_MAX, containerVeth, PATH_MAX))) {
|
||||
lxcError(VIR_ERR_INTERNAL_ERROR,
|
||||
_("Failed to create veth device pair: %d"), rc);
|
||||
if (vethCreate(parentVeth, PATH_MAX, containerVeth, PATH_MAX) < 0)
|
||||
goto error_exit;
|
||||
}
|
||||
|
||||
if (NULL == def->nets[i]->ifname) {
|
||||
def->nets[i]->ifname = strdup(parentVeth);
|
||||
}
|
||||
@ -885,30 +880,22 @@ static int lxcSetupInterfaces(virConnectPtr conn,
|
||||
{
|
||||
char macaddr[VIR_MAC_STRING_BUFLEN];
|
||||
virFormatMacAddr(def->nets[i]->mac, macaddr);
|
||||
if (0 != (rc = setMacAddr(containerVeth, macaddr))) {
|
||||
virReportSystemError(rc,
|
||||
_("Failed to set %s to %s"),
|
||||
macaddr, containerVeth);
|
||||
if (setMacAddr(containerVeth, macaddr) < 0)
|
||||
goto error_exit;
|
||||
}
|
||||
}
|
||||
|
||||
if (0 != (rc = brAddInterface(brctl, bridge, parentVeth))) {
|
||||
virReportSystemError(rc,
|
||||
_("Failed to add %s device to %s"),
|
||||
parentVeth, bridge);
|
||||
rc = -1;
|
||||
goto error_exit;
|
||||
}
|
||||
|
||||
if (0 != (rc = vethInterfaceUpOrDown(parentVeth, 1))) {
|
||||
virReportSystemError(rc,
|
||||
_("Failed to enable %s device"),
|
||||
parentVeth);
|
||||
if (vethInterfaceUpOrDown(parentVeth, 1) < 0)
|
||||
goto error_exit;
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
rc = 0;
|
||||
|
||||
error_exit:
|
||||
|
129
src/lxc/veth.c
129
src/lxc/veth.c
@ -13,12 +13,21 @@
|
||||
|
||||
#include <string.h>
|
||||
#include <stdio.h>
|
||||
#include <sys/types.h>
|
||||
#include <sys/wait.h>
|
||||
|
||||
#include "veth.h"
|
||||
#include "internal.h"
|
||||
#include "logging.h"
|
||||
#include "memory.h"
|
||||
#include "util.h"
|
||||
#include "virterror_internal.h"
|
||||
|
||||
#define VIR_FROM_THIS VIR_FROM_LXC
|
||||
|
||||
#define vethError(code, ...) \
|
||||
virReportErrorHelper(NULL, VIR_FROM_LXC, code, __FILE__, \
|
||||
__FUNCTION__, __LINE__, __VA_ARGS__)
|
||||
|
||||
/* Functions */
|
||||
/**
|
||||
@ -76,17 +85,13 @@ static int getFreeVethName(char *veth, int maxLen, int startDev)
|
||||
int vethCreate(char* veth1, int veth1MaxLen,
|
||||
char* veth2, int veth2MaxLen)
|
||||
{
|
||||
int rc = -1;
|
||||
int rc;
|
||||
const char *argv[] = {
|
||||
"ip", "link", "add", veth1, "type", "veth", "peer", "name", veth2, NULL
|
||||
};
|
||||
int cmdResult;
|
||||
int cmdResult = 0;
|
||||
int vethDev = 0;
|
||||
|
||||
if ((NULL == veth1) || (NULL == veth2)) {
|
||||
goto error_out;
|
||||
}
|
||||
|
||||
DEBUG("veth1: %s veth2: %s", veth1, veth2);
|
||||
|
||||
while ((1 > strlen(veth1)) || STREQ(veth1, veth2)) {
|
||||
@ -104,11 +109,14 @@ int vethCreate(char* veth1, int veth1MaxLen,
|
||||
DEBUG("veth1: %s veth2: %s", veth1, veth2);
|
||||
rc = virRun(argv, &cmdResult);
|
||||
|
||||
if (0 == rc) {
|
||||
rc = cmdResult;
|
||||
if (rc != 0 ||
|
||||
(WIFEXITED(cmdResult) && WEXITSTATUS(cmdResult) != 0)) {
|
||||
vethError(VIR_ERR_INTERNAL_ERROR,
|
||||
_("Failed to create veth device pair '%s', '%s': %d"),
|
||||
veth1, veth2, WEXITSTATUS(cmdResult));
|
||||
rc = -1;
|
||||
}
|
||||
|
||||
error_out:
|
||||
return rc;
|
||||
}
|
||||
|
||||
@ -125,23 +133,25 @@ error_out:
|
||||
*/
|
||||
int vethDelete(const char *veth)
|
||||
{
|
||||
int rc = -1;
|
||||
int rc;
|
||||
const char *argv[] = {"ip", "link", "del", veth, NULL};
|
||||
int cmdResult;
|
||||
|
||||
if (NULL == veth) {
|
||||
goto error_out;
|
||||
}
|
||||
int cmdResult = 0;
|
||||
|
||||
DEBUG("veth: %s", veth);
|
||||
|
||||
rc = virRun(argv, &cmdResult);
|
||||
|
||||
if (0 == rc) {
|
||||
rc = cmdResult;
|
||||
if (rc != 0 ||
|
||||
(WIFEXITED(cmdResult) && WEXITSTATUS(cmdResult) != 0)) {
|
||||
/*
|
||||
* Prevent overwriting an error log which may be set
|
||||
* where an actual failure occurs.
|
||||
*/
|
||||
VIR_DEBUG(_("Failed to delete '%s' (%d)"),
|
||||
veth, WEXITSTATUS(cmdResult));
|
||||
rc = -1;
|
||||
}
|
||||
|
||||
error_out:
|
||||
return rc;
|
||||
}
|
||||
|
||||
@ -157,13 +167,9 @@ error_out:
|
||||
*/
|
||||
int vethInterfaceUpOrDown(const char* veth, int upOrDown)
|
||||
{
|
||||
int rc = -1;
|
||||
int rc;
|
||||
const char *argv[] = {"ifconfig", veth, NULL, NULL};
|
||||
int cmdResult;
|
||||
|
||||
if (NULL == veth) {
|
||||
goto error_out;
|
||||
}
|
||||
int cmdResult = 0;
|
||||
|
||||
if (0 == upOrDown)
|
||||
argv[2] = "down";
|
||||
@ -172,11 +178,22 @@ int vethInterfaceUpOrDown(const char* veth, int upOrDown)
|
||||
|
||||
rc = virRun(argv, &cmdResult);
|
||||
|
||||
if (0 == rc) {
|
||||
rc = cmdResult;
|
||||
if (rc != 0 ||
|
||||
(WIFEXITED(cmdResult) && WEXITSTATUS(cmdResult) != 0)) {
|
||||
if (0 == upOrDown)
|
||||
/*
|
||||
* Prevent overwriting an error log which may be set
|
||||
* where an actual failure occurs.
|
||||
*/
|
||||
VIR_DEBUG(_("Failed to disable '%s' (%d)"),
|
||||
veth, WEXITSTATUS(cmdResult));
|
||||
else
|
||||
vethError(VIR_ERR_INTERNAL_ERROR,
|
||||
_("Failed to enable '%s' (%d)"),
|
||||
veth, WEXITSTATUS(cmdResult));
|
||||
rc = -1;
|
||||
}
|
||||
|
||||
error_out:
|
||||
return rc;
|
||||
}
|
||||
|
||||
@ -193,26 +210,28 @@ error_out:
|
||||
*/
|
||||
int moveInterfaceToNetNs(const char* iface, int pidInNs)
|
||||
{
|
||||
int rc = -1;
|
||||
int rc;
|
||||
char *pid = NULL;
|
||||
const char *argv[] = {
|
||||
"ip", "link", "set", iface, "netns", NULL, NULL
|
||||
};
|
||||
int cmdResult;
|
||||
int cmdResult = 0;
|
||||
|
||||
if (NULL == iface) {
|
||||
goto error_out;
|
||||
if (virAsprintf(&pid, "%d", pidInNs) == -1) {
|
||||
virReportOOMError();
|
||||
return -1;
|
||||
}
|
||||
|
||||
if (virAsprintf(&pid, "%d", pidInNs) == -1)
|
||||
goto error_out;
|
||||
|
||||
argv[5] = pid;
|
||||
rc = virRun(argv, &cmdResult);
|
||||
if (0 == rc)
|
||||
rc = cmdResult;
|
||||
if (rc != 0 ||
|
||||
(WIFEXITED(cmdResult) && WEXITSTATUS(cmdResult) != 0)) {
|
||||
vethError(VIR_ERR_INTERNAL_ERROR,
|
||||
_("Failed to move '%s' into NS(pid=%d) (%d)"),
|
||||
iface, pidInNs, WEXITSTATUS(cmdResult));
|
||||
rc = -1;
|
||||
}
|
||||
|
||||
error_out:
|
||||
VIR_FREE(pid);
|
||||
return rc;
|
||||
}
|
||||
@ -230,21 +249,21 @@ error_out:
|
||||
*/
|
||||
int setMacAddr(const char* iface, const char* macaddr)
|
||||
{
|
||||
int rc = -1;
|
||||
int rc;
|
||||
const char *argv[] = {
|
||||
"ip", "link", "set", iface, "address", macaddr, NULL
|
||||
};
|
||||
int cmdResult;
|
||||
|
||||
if (NULL == iface) {
|
||||
goto error_out;
|
||||
}
|
||||
int cmdResult = 0;
|
||||
|
||||
rc = virRun(argv, &cmdResult);
|
||||
if (0 == rc)
|
||||
rc = cmdResult;
|
||||
if (rc != 0 ||
|
||||
(WIFEXITED(cmdResult) && WEXITSTATUS(cmdResult) != 0)) {
|
||||
vethError(VIR_ERR_INTERNAL_ERROR,
|
||||
_("Failed to set '%s' to '%s' (%d)"),
|
||||
macaddr, iface, WEXITSTATUS(cmdResult));
|
||||
rc = -1;
|
||||
}
|
||||
|
||||
error_out:
|
||||
return rc;
|
||||
}
|
||||
|
||||
@ -261,20 +280,20 @@ error_out:
|
||||
*/
|
||||
int setInterfaceName(const char* iface, const char* new)
|
||||
{
|
||||
int rc = -1;
|
||||
int rc;
|
||||
const char *argv[] = {
|
||||
"ip", "link", "set", iface, "name", new, NULL
|
||||
};
|
||||
int cmdResult;
|
||||
|
||||
if (NULL == iface || NULL == new) {
|
||||
goto error_out;
|
||||
}
|
||||
int cmdResult = 0;
|
||||
|
||||
rc = virRun(argv, &cmdResult);
|
||||
if (0 == rc)
|
||||
rc = cmdResult;
|
||||
if (rc != 0 ||
|
||||
(WIFEXITED(cmdResult) && WEXITSTATUS(cmdResult) != 0)) {
|
||||
vethError(VIR_ERR_INTERNAL_ERROR,
|
||||
_("Failed to set '%s' to '%s' (%d)"),
|
||||
new, iface, WEXITSTATUS(cmdResult));
|
||||
rc = -1;
|
||||
}
|
||||
|
||||
error_out:
|
||||
return rc;
|
||||
}
|
||||
|
@ -13,14 +13,21 @@
|
||||
# define VETH_H
|
||||
|
||||
# include <config.h>
|
||||
# include "internal.h"
|
||||
|
||||
/* Function declarations */
|
||||
int vethCreate(char* veth1, int veth1MaxLen, char* veth2,
|
||||
int veth2MaxLen);
|
||||
int vethDelete(const char* veth);
|
||||
int vethInterfaceUpOrDown(const char* veth, int upOrDown);
|
||||
int moveInterfaceToNetNs(const char *iface, int pidInNs);
|
||||
int setMacAddr(const char* iface, const char* macaddr);
|
||||
int setInterfaceName(const char* iface, const char* new);
|
||||
int veth2MaxLen)
|
||||
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3);
|
||||
int vethDelete(const char* veth)
|
||||
ATTRIBUTE_NONNULL(1);
|
||||
int vethInterfaceUpOrDown(const char* veth, int upOrDown)
|
||||
ATTRIBUTE_NONNULL(1);
|
||||
int moveInterfaceToNetNs(const char *iface, int pidInNs)
|
||||
ATTRIBUTE_NONNULL(1);
|
||||
int setMacAddr(const char* iface, const char* macaddr)
|
||||
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
|
||||
int setInterfaceName(const char* iface, const char* new)
|
||||
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
|
||||
|
||||
#endif /* VETH_H */
|
||||
|
Loading…
Reference in New Issue
Block a user