On Tue, Jul 03, 2012 at 16:58:44 +0100, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Move the veth device name state into the virLXCControllerPtr object and stop passing it around. Also use size_t instead of unsigned int for the array length parameters.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/lxc/lxc_container.c | 10 +++--- src/lxc/lxc_container.h | 2 +- src/lxc/lxc_controller.c | 79 ++++++++++++++++++++++++++++++---------------- 3 files changed, 57 insertions(+), 34 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 910e82b..4f8703c 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -93,7 +93,7 @@ typedef struct __lxc_child_argv lxc_child_argv_t; struct __lxc_child_argv { virDomainDefPtr config; virSecurityManagerPtr securityDriver; - unsigned int nveths; + size_t nveths; char **veths; int monitor; char **ttyPaths; @@ -262,15 +262,15 @@ int lxcContainerWaitForContinue(int control) * Returns 0 on success or nonzero in case of error */ static int lxcContainerRenameAndEnableInterfaces(bool privNet, - unsigned int nveths, + size_t nveths, char **veths) { int rc = 0; - unsigned int i; + size_t i; char *newname = NULL;
for (i = 0 ; i < nveths ; i++) { - if (virAsprintf(&newname, "eth%d", i) < 0) { + if (virAsprintf(&newname, "eth%zu", i) < 0) { virReportOOMError(); rc = -1; goto error_out; @@ -1775,7 +1775,7 @@ const char *lxcContainerGetAlt32bitArch(const char *arch) */ int lxcContainerStart(virDomainDefPtr def, virSecurityManagerPtr securityDriver, - unsigned int nveths, + size_t nveths, char **veths, int control, int handshakefd, diff --git a/src/lxc/lxc_container.h b/src/lxc/lxc_container.h index 77fb9b2..6f1cc8b 100644 --- a/src/lxc/lxc_container.h +++ b/src/lxc/lxc_container.h @@ -51,7 +51,7 @@ int lxcContainerWaitForContinue(int control);
int lxcContainerStart(virDomainDefPtr def, virSecurityManagerPtr securityDriver, - unsigned int nveths, + size_t nveths, char **veths, int control, int handshakefd, diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 7447f6c..ad11307 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -85,6 +85,9 @@ typedef virLXCController *virLXCControllerPtr; struct _virLXCController { char *name; virDomainDefPtr def; + + size_t nveths; + char **veths; };
static void virLXCControllerFree(virLXCControllerPtr ctrl); @@ -129,15 +132,35 @@ no_memory:
static void virLXCControllerFree(virLXCControllerPtr ctrl) { + size_t i; + if (!ctrl) return;
+ for (i = 0 ; i < ctrl->nveths ; i++) + VIR_FREE(ctrl->veths[i]); + VIR_FREE(ctrl->veths); + virDomainDefFree(ctrl->def); VIR_FREE(ctrl->name);
VIR_FREE(ctrl); }
+ +static int virLXCControllerValidateNICs(virLXCControllerPtr ctrl) +{ + if (ctrl->def->nnets != ctrl->nveths) { + lxcError(VIR_ERR_INTERNAL_ERROR, + _("expecting %d veths, but got %zu"), + ctrl->def->nnets, ctrl->nveths); + return -1; + } + + return 0; +} + + static int lxcGetLoopFD(char **dev_name) { int fd = -1; @@ -1307,7 +1330,7 @@ cleanup2:
/** - * lxcControllerMoveInterfaces + * virLXCControllerMoveInterfaces * @nveths: number of interfaces * @veths: interface names * @container: pid of container @@ -1316,13 +1339,13 @@ cleanup2: * * Returns 0 on success or -1 in case of error */ -static int lxcControllerMoveInterfaces(unsigned int nveths, - char **veths, - pid_t container) +static int virLXCControllerMoveInterfaces(virLXCControllerPtr ctrl, + pid_t container) { - unsigned int i; - for (i = 0 ; i < nveths ; i++) - if (virNetDevSetNamespace(veths[i], container) < 0) + size_t i; + + for (i = 0 ; i < ctrl->nveths ; i++) + if (virNetDevSetNamespace(ctrl->veths[i], container) < 0) return -1;
Since you are touching this, I think it would be a good idea to add {} around the content of this for loop.
return 0; @@ -1330,24 +1353,26 @@ static int lxcControllerMoveInterfaces(unsigned int nveths,
/** - * lxcCleanupInterfaces: - * @nveths: number of interfaces - * @veths: interface names + * virLXCControllerDeleteInterfaces: + * @ctrl: the LXC controller * * Cleans up the container interfaces by deleting the veth device pairs. * * Returns 0 on success or -1 in case of error */ -static int lxcControllerCleanupInterfaces(unsigned int nveths, - char **veths) +static int virLXCControllerDeleteInterfaces(virLXCControllerPtr ctrl) { - unsigned int i; - for (i = 0 ; i < nveths ; i++) - ignore_value(virNetDevVethDelete(veths[i])); + size_t i; + int ret = 0;
- return 0; + for (i = 0 ; i < ctrl->nveths ; i++) + if (virNetDevVethDelete(ctrl->veths[i]) < 0) + ret = -1;
And here as well...
+ + return ret; }
+ static int lxcSetPersonality(virDomainDefPtr def) { struct utsname utsname; @@ -1422,8 +1447,6 @@ cleanup: static int virLXCControllerRun(virLXCControllerPtr ctrl, virSecurityManagerPtr securityDriver, - unsigned int nveths, - char **veths, int monitor, int client, int *ttyFDs, @@ -1588,8 +1611,8 @@ virLXCControllerRun(virLXCControllerPtr ctrl,
if ((container = lxcContainerStart(ctrl->def, securityDriver, - nveths, - veths, + ctrl->nveths, + ctrl->veths, control[1], containerhandshake[1], containerTtyPaths, @@ -1598,7 +1621,7 @@ virLXCControllerRun(virLXCControllerPtr ctrl, VIR_FORCE_CLOSE(control[1]); VIR_FORCE_CLOSE(containerhandshake[1]);
- if (lxcControllerMoveInterfaces(nveths, veths, container) < 0) + if (virLXCControllerMoveInterfaces(ctrl, container) < 0) goto cleanup;
if (lxcContainerSendContinue(control[0]) < 0) { @@ -1684,7 +1707,7 @@ int main(int argc, char *argv[]) int rc = 1; int client; char *name = NULL; - int nveths = 0; + size_t nveths = 0; char **veths = NULL; int monitor = -1; int handshakefd = -1; @@ -1833,11 +1856,11 @@ int main(int argc, char *argv[]) NULLSTR(ctrl->def->seclabel.label), NULLSTR(ctrl->def->seclabel.imagelabel));
- if (ctrl->def->nnets != nveths) { - fprintf(stderr, "%s: expecting %d veths, but got %d\n", - argv[0], ctrl->def->nnets, nveths); + ctrl->veths = veths; + ctrl->nveths = nveths;
I was a bit afraid of double free after the two lines above but since we didn't actually free veths array before this patch (which was a little bit wrong), we are safe.
+ + if (virLXCControllerValidateNICs(ctrl) < 0) goto cleanup; - }
if ((sockpath = lxcMonitorPath(ctrl)) == NULL) goto cleanup; @@ -1885,12 +1908,12 @@ int main(int argc, char *argv[]) }
rc = virLXCControllerRun(ctrl, securityDriver, - nveths, veths, monitor, client, + monitor, client, ttyFDs, nttyFDs, handshakefd);
cleanup: virPidFileDelete(LXC_STATE_DIR, name); - lxcControllerCleanupInterfaces(nveths, veths); + virLXCControllerDeleteInterfaces(ctrl); if (sockpath) unlink(sockpath); VIR_FREE(sockpath);
ACK Jirka