[libvirt] [RFC][PATCH] LXC allow a container to have ethN named interfaces
Note that this patch is not mature yet and still proof- of-concept prototype, although it actually works. Current implementation of lxc driver creates vethN named interface(s) in the host and passes as it is to a container. The reason why it doesn't use ethN is due to the limitation that one namespace cannot have multiple iterfaces that have an identical name so that we give up creating ethN named interface in the host for the container. However, we should be able to allow the container to have ethN by changing the name after clone(CLONE_NEWNET) in controller. To this end, this patch extends --veth argument of controller to be able to have two names; one is the name of a created veth, and the other is a new name. The format is: --veth veth1,eth0 --veth veth3,eth1 ... where veth1 is old name and eth0 is new one. The implementation also allows a single name, in that case, controller does not do nothing for the interface, which is the original behavior. The numbering of the new names is simply ascending order from zero. So if there is one interface, its name will be eth0. --- src/lxc/lxc_container.c | 23 ++++++++++++++++++----- src/lxc/lxc_container.h | 1 + src/lxc/lxc_controller.c | 30 ++++++++++++++++++++++++++++-- src/lxc/lxc_driver.c | 30 +++++++++++++++++++++++++----- src/lxc/veth.c | 20 ++++++++++++++++++++ src/lxc/veth.h | 1 + 6 files changed, 93 insertions(+), 12 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index f4381e7..f065788 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -84,6 +84,7 @@ struct __lxc_child_argv { virDomainDefPtr config; unsigned int nveths; char **veths; + char **newveths; int monitor; char *ttyPath; }; @@ -233,14 +234,23 @@ static int lxcContainerWaitForContinue(int control) * Returns 0 on success or nonzero in case of error */ static int lxcContainerEnableInterfaces(unsigned int nveths, - char **veths) + char **veths, char **newveths) { int rc = 0; unsigned int i; for (i = 0 ; i < nveths ; i++) { - DEBUG("Enabling %s", veths[i]); - rc = vethInterfaceUpOrDown(veths[i], 1); + char *veth = veths[i]; + if (newveths[i] && !STREQ(veths[i], newveths[i])) { + DEBUG("changing %s", veths[i]); + rc = setInterfaceName(veths[i], newveths[i]); + if (0 != rc) { + goto error_out; + } + veth = newveths[i]; + } + DEBUG("Enabling %s", veth); + rc = vethInterfaceUpOrDown(veth, 1); if (0 != rc) { goto error_out; } @@ -758,7 +768,9 @@ static int lxcContainerChild( void *data ) return -1; /* enable interfaces */ - if (lxcContainerEnableInterfaces(argv->nveths, argv->veths) < 0) + if (lxcContainerEnableInterfaces(argv->nveths, + argv->veths, + argv->newveths) < 0) return -1; /* drop a set of root capabilities */ @@ -786,6 +798,7 @@ static int userns_supported(void) int lxcContainerStart(virDomainDefPtr def, unsigned int nveths, char **veths, + char **newveths, int control, char *ttyPath) { @@ -793,7 +806,7 @@ int lxcContainerStart(virDomainDefPtr def, int flags; int stacksize = getpagesize() * 4; char *stack, *stacktop; - lxc_child_argv_t args = { def, nveths, veths, control, ttyPath }; + lxc_child_argv_t args = { def, nveths, veths, newveths, control, ttyPath }; /* allocate a stack for the container */ if (VIR_ALLOC_N(stack, stacksize) < 0) { diff --git a/src/lxc/lxc_container.h b/src/lxc/lxc_container.h index a1dd5a1..d796380 100644 --- a/src/lxc/lxc_container.h +++ b/src/lxc/lxc_container.h @@ -49,6 +49,7 @@ int lxcContainerSendContinue(int control); int lxcContainerStart(virDomainDefPtr def, unsigned int nveths, char **veths, + char **newveths, int control, char *ttyPath); diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 545f718..75dd73a 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -493,6 +493,7 @@ static int lxcControllerRun(virDomainDefPtr def, unsigned int nveths, char **veths, + char **newveths, int monitor, int client, int appPty) @@ -603,6 +604,7 @@ lxcControllerRun(virDomainDefPtr def, if ((container = lxcContainerStart(def, nveths, veths, + newveths, control[1], containerPtyPath)) < 0) goto cleanup; @@ -649,6 +651,7 @@ int main(int argc, char *argv[]) char *name = NULL; int nveths = 0; char **veths = NULL; + char **newveths = NULL; int monitor = -1; int appPty = -1; int bg = 0; @@ -691,10 +694,33 @@ int main(int argc, char *argv[]) virReportOOMError(NULL); goto cleanup; } - if ((veths[nveths++] = strdup(optarg)) == NULL) { + if (VIR_REALLOC_N(newveths, nveths+1) < 0) { virReportOOMError(NULL); goto cleanup; } + { + char *sep = strchr(optarg, ','); + if (sep) { + *sep = '\0'; + sep++; + if ((veths[nveths] = strdup(optarg)) == NULL) { + virReportOOMError(NULL); + goto cleanup; + } + if ((newveths[nveths] = strdup(sep)) == NULL) { + virReportOOMError(NULL); + goto cleanup; + } + nveths++; + } else { + if ((veths[nveths] = strdup(optarg)) == NULL) { + virReportOOMError(NULL); + goto cleanup; + } + newveths[nveths] = NULL; + nveths++; + } + } break; case 'c': @@ -803,7 +829,7 @@ int main(int argc, char *argv[]) goto cleanup; } - rc = lxcControllerRun(def, nveths, veths, monitor, client, appPty); + rc = lxcControllerRun(def, nveths, veths, newveths, monitor, client, appPty); cleanup: diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 4f0787b..1323cff 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -693,13 +693,15 @@ static int lxcVMCleanup(virConnectPtr conn, static int lxcSetupInterfaces(virConnectPtr conn, virDomainDefPtr def, unsigned int *nveths, - char ***veths) + char ***veths, + char ***newveths) { int rc = -1, i; char *bridge = NULL; char parentVeth[PATH_MAX] = ""; char containerVeth[PATH_MAX] = ""; brControl *brctl = NULL; + char *newveth; if (brInit(&brctl) != 0) return -1; @@ -744,10 +746,20 @@ static int lxcSetupInterfaces(virConnectPtr conn, if (NULL == def->nets[i]->ifname) { def->nets[i]->ifname = strdup(parentVeth); } + if (VIR_REALLOC_N(*veths, (*nveths)+1) < 0) goto error_exit; - if (((*veths)[(*nveths)++] = strdup(containerVeth)) == NULL) + if (VIR_REALLOC_N(*newveths, (*nveths)+1) < 0) + goto error_exit; + + if (((*veths)[*nveths] = strdup(containerVeth)) == NULL) + goto error_exit; + if (virAsprintf(&newveth, "eth%d", i) < 0) goto error_exit; + if (((*newveths)[*nveths] = strdup(newveth)) == NULL) + goto error_exit; + (*nveths)++; + VIR_FREE(newveth); if (NULL == def->nets[i]->ifname) { lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, @@ -906,6 +918,7 @@ static int lxcControllerStart(virConnectPtr conn, virDomainObjPtr vm, int nveths, char **veths, + char **newveths, int appPty, int logfd) { @@ -1029,7 +1042,13 @@ static int lxcControllerStart(virConnectPtr conn, for (i = 0 ; i < nveths ; i++) { ADD_ARG_LIT("--veth"); - ADD_ARG_LIT(veths[i]); + if (newveths[i]) { + if (virAsprintf(&tmp, "%s,%s", veths[i], newveths[i]) < 0) + goto no_memory; + ADD_ARG_LIT(tmp); + } else { + ADD_ARG_LIT(veths[i]); + } } ADD_ARG(NULL); @@ -1107,6 +1126,7 @@ static int lxcVmStart(virConnectPtr conn, int logfd = -1; unsigned int nveths = 0; char **veths = NULL; + char **newveths = NULL; if ((r = virFileMakePath(driver->logDir)) < 0) { virReportSystemError(conn, r, @@ -1135,7 +1155,7 @@ static int lxcVmStart(virConnectPtr conn, VIR_FREE(parentTtyPath); } - if (lxcSetupInterfaces(conn, vm->def, &nveths, &veths) != 0) + if (lxcSetupInterfaces(conn, vm->def, &nveths, &veths, &newveths) != 0) goto cleanup; /* Persist the live configuration now we have veth & tty info */ @@ -1152,7 +1172,7 @@ static int lxcVmStart(virConnectPtr conn, if (lxcControllerStart(conn, vm, - nveths, veths, + nveths, veths, newveths, parentTty, logfd) < 0) goto cleanup; diff --git a/src/lxc/veth.c b/src/lxc/veth.c index b15df8d..fbd1195 100644 --- a/src/lxc/veth.c +++ b/src/lxc/veth.c @@ -247,3 +247,23 @@ int setMacAddr(const char* iface, const char* macaddr) error_out: return rc; } + +int setInterfaceName(const char* iface, const char* new) +{ + int rc = -1; + const char *argv[] = { + "ip", "link", "set", iface, "name", new, NULL + }; + int cmdResult; + + if (NULL == iface || NULL == new) { + goto error_out; + } + + rc = virRun(NULL, argv, &cmdResult); + if (0 == rc) + rc = cmdResult; + +error_out: + return rc; +} diff --git a/src/lxc/veth.h b/src/lxc/veth.h index 8f2f514..8075a5e 100644 --- a/src/lxc/veth.h +++ b/src/lxc/veth.h @@ -21,5 +21,6 @@ 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); #endif /* VETH_H */ -- 1.6.2.5
On Tue, Nov 03, 2009 at 12:23:26AM +0900, Ryota Ozaki wrote:
Note that this patch is not mature yet and still proof- of-concept prototype, although it actually works.
Current implementation of lxc driver creates vethN named interface(s) in the host and passes as it is to a container. The reason why it doesn't use ethN is due to the limitation that one namespace cannot have multiple iterfaces that have an identical name so that we give up creating ethN named interface in the host for the container.
However, we should be able to allow the container to have ethN by changing the name after clone(CLONE_NEWNET) in controller.
To this end, this patch extends --veth argument of controller to be able to have two names; one is the name of a created veth, and the other is a new name. The format is:
--veth veth1,eth0 --veth veth3,eth1 ...
Do we need to pass the names around on command line args ? Could we simply make libvirt_lxc auto-assign the names starting from eth0 instead of passing the names from libvirtd THe patch is a good idea though regardless - having them named eth0 is much more familiar for users Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Wed, Nov 4, 2009 at 6:02 AM, Daniel P. Berrange <berrange@redhat.com> wrote:
On Tue, Nov 03, 2009 at 12:23:26AM +0900, Ryota Ozaki wrote:
Note that this patch is not mature yet and still proof- of-concept prototype, although it actually works.
Current implementation of lxc driver creates vethN named interface(s) in the host and passes as it is to a container. The reason why it doesn't use ethN is due to the limitation that one namespace cannot have multiple iterfaces that have an identical name so that we give up creating ethN named interface in the host for the container.
However, we should be able to allow the container to have ethN by changing the name after clone(CLONE_NEWNET) in controller.
To this end, this patch extends --veth argument of controller to be able to have two names; one is the name of a created veth, and the other is a new name. The format is:
--veth veth1,eth0 --veth veth3,eth1 ...
Do we need to pass the names around on command line args ? Could we simply make libvirt_lxc auto-assign the names starting from eth0 instead of passing the names from libvirtd
I'm worrying two things; Is it ok that libvirt_lxc has such a decision? and it seems a bit inflexible, is it acceptable? Nevertheless, your suggestion makes code simple and sounds good idea if the above things are not big deal. So I will rewrite my patch anyway. Thank you for your review, ozaki-r
THe patch is a good idea though regardless - having them named eth0 is much more familiar for users
Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Wed, Nov 04, 2009 at 09:46:01PM +0900, Ryota Ozaki wrote:
On Wed, Nov 4, 2009 at 6:02 AM, Daniel P. Berrange <berrange@redhat.com> wrote:
On Tue, Nov 03, 2009 at 12:23:26AM +0900, Ryota Ozaki wrote:
Note that this patch is not mature yet and still proof- of-concept prototype, although it actually works.
Current implementation of lxc driver creates vethN named interface(s) in the host and passes as it is to a container. The reason why it doesn't use ethN is due to the limitation that one namespace cannot have multiple iterfaces that have an identical name so that we give up creating ethN named interface in the host for the container.
However, we should be able to allow the container to have ethN by changing the name after clone(CLONE_NEWNET) in controller.
To this end, this patch extends --veth argument of controller to be able to have two names; one is the name of a created veth, and the other is a new name. The format is:
--veth veth1,eth0 --veth veth3,eth1 ...
Do we need to pass the names around on command line args ? Could we simply make libvirt_lxc auto-assign the names starting from eth0 instead of passing the names from libvirtd
I'm worrying two things; Is it ok that libvirt_lxc has such a decision? and it seems a bit inflexible, is it acceptable?
I think it is ok, because the administrator inside the container can easily rename interfaces to whatever they wish later. So a simple fixed naming policy in libvirt_lxc is sufficient Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Wed, Nov 4, 2009 at 9:49 PM, Daniel P. Berrange <berrange@redhat.com> wrote:
On Wed, Nov 04, 2009 at 09:46:01PM +0900, Ryota Ozaki wrote:
On Wed, Nov 4, 2009 at 6:02 AM, Daniel P. Berrange <berrange@redhat.com> wrote:
On Tue, Nov 03, 2009 at 12:23:26AM +0900, Ryota Ozaki wrote:
Note that this patch is not mature yet and still proof- of-concept prototype, although it actually works.
Current implementation of lxc driver creates vethN named interface(s) in the host and passes as it is to a container. The reason why it doesn't use ethN is due to the limitation that one namespace cannot have multiple iterfaces that have an identical name so that we give up creating ethN named interface in the host for the container.
However, we should be able to allow the container to have ethN by changing the name after clone(CLONE_NEWNET) in controller.
To this end, this patch extends --veth argument of controller to be able to have two names; one is the name of a created veth, and the other is a new name. The format is:
--veth veth1,eth0 --veth veth3,eth1 ...
Do we need to pass the names around on command line args ? Could we simply make libvirt_lxc auto-assign the names starting from eth0 instead of passing the names from libvirtd
I'm worrying two things; Is it ok that libvirt_lxc has such a decision? and it seems a bit inflexible, is it acceptable?
I think it is ok, because the administrator inside the container can easily rename interfaces to whatever they wish later. So a simple fixed naming policy in libvirt_lxc is sufficient
Indeed, right. I was worrying too much ;-) Thanks! ozaki-r
Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
participants (2)
-
Daniel P. Berrange -
Ryota Ozaki