On 07/23/2010 01:25 PM, Ryota Ozaki wrote:
Both may return a positive value when they fail. We should check if the value is not zero instead of checking if it's negative.
I notice that lxcSetupInterfaces has a comment saying that it returns -1 on failure, but it actually just passes on what is returned by vethInterfaceUpOrDown.
Would you be willing to consider instead just changing vethInterfaceUpOrDown and moveInterfaceToNetNs to return -1 in all error cases (and checking the return for < 0), rather than grabbing the exit code of the exec'ed command? None of the callers do anything with that extra information anyway, and it would help to make the return values more consistent (which makes it easier to catch bugs like this, or eliminates them altogether ;-)
(I was recently bitten by a similar bug...)
lxcContainerRenameAndEnableInterfaces is expected to return a negative value on a failure, so the patch changes the return value to -1 if vethInterfaceUpOrDown fails. Note that this patch may be related to the bug: https://bugzilla.redhat.com/show_bug.cgi?id=607496 . It would not fix the bug, but would unveil what happens. --- src/lxc/lxc_container.c | 7 ++++++- src/lxc/lxc_controller.c | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 4371dba..c77d262 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -273,8 +273,13 @@ static int lxcContainerRenameAndEnableInterfaces(unsigned int nveths, } /* enable lo device only if there were other net devices */ - if (veths) + if (veths) { rc = vethInterfaceUpOrDown("lo", 1); + if (0 != rc) { + VIR_ERROR(_("Failed to enable lo (%d)"), rc); + rc = -1; + } + } error_out: VIR_FREE(newname); diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index d8b7bc7..9829a69 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -477,7 +477,7 @@ static int lxcControllerMoveInterfaces(unsigned int nveths, { unsigned int i; for (i = 0 ; i< nveths ; i++) - if (moveInterfaceToNetNs(veths[i], container)< 0) { + if (moveInterfaceToNetNs(veths[i], container) != 0) { lxcError(VIR_ERR_INTERNAL_ERROR, _("Failed to move interface %s to ns %d"), veths[i], container);
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
