Re: [lxc-devel] [PATCH] add lxc.network.veth.script configuration hook
On 10/07/2010 09:30 AM, Stefan Tomanek wrote:
This commit adds an lxc.network.veth.script configuration option to
specify a script to be executed after creating or configuring the pair
of veth devices. The name of the host sided device is passed as first
argument, so the script can be used to configures routes or firewall
rules related to the container.
---
Hi Stefan,
Thanks for your patch. Adding some possibility to hook the configuration
with scripts is a good idea.
I think your patch is too focused on a specific desired feature.
As Michael suggested, a generic option could be used for each network
section, not a veth specific one.
As you pointed, you need to run the script from the instanciate_veth
because it is the only place where the name is used.
I suggest you add a lxc.network.script section where it will be called
from each instanciate_*
Depending of the function you will pass the parameters making sense for
the script.
The function prototype could be with va_args:
static int run_script(const char *name, const char *section, const char
*script, ...)
{
...
execl(script, args, VA_ARGS);
...
}
The script should receive always the two parameters:
$1 : container name
$2 : configuration section : network, pts, etc ...
And the optional parameters depending of the hooks caller:
In your case:
$3 : network type veth, ...
$4 : network link
$5 : guest ifname
$6 : host ifname (in case of veth)
If you can respin your patch to follow that way, that will be nice and
will open the door for more hooks.
But no need to implement more than what you need :)
A few comments below:
src/lxc/conf.c| 30 ++
src/lxc/conf.h| 12 +++-
src/lxc/confile.c | 20
3 files changed, 57 insertions(+), 5 deletions(-)
diff --git a/src/lxc/conf.c b/src/lxc/conf.c
index adfe862..be12499 100644
--- a/src/lxc/conf.c
+++ b/src/lxc/conf.c
@@ -29,6 +29,7 @@
#includedirent.h
#includemntent.h
#includeunistd.h
+#includesys/wait.h
#includepty.h
#includesys/types.h
@@ -1061,6 +1062,26 @@ static int setup_ipv6_addr(struct lxc_list *ip, int
ifindex)
return 0;
}
+static int run_network_script(char *script, const char *ifname)
+{
+ INFO(Executing network script '%s' for interface '%s', script,
ifname);
+ int pid = fork();
+ if (pid 0) {
+ ERROR(Error forking);
+ } else if (pid == 0) {
+ // child
use the /* */ format to conform to the Coding Style please.
+ execl(script, script, ifname, (char *) NULL);
A SYSERROR log will help the user to understand why it's script was not
execed.
+ // if an error occurs, we terminate
+ exit(1);
+ } else {
+ // parent
+ int status = 0;
+ waitpid( pid,status, 0 );
Hmm, I am wondering if the return value shouldn't be checked here,
especially for the eintr.
+ return status;
Do we assume the script returns always 0 on success ? and we don't care
about the WIFSIGNALED, ... ?
+ }
+ return 1;
+}
+
static int setup_netdev(struct lxc_netdev *netdev)
{
char ifname[IFNAMSIZ];
@@ -1267,6 +1288,15 @@ static int instanciate_veth(struct lxc_netdev *netdev)
}
}
+ if (netdev-vethscript) {
+ err = run_network_script(netdev-vethscript, veth1);
+ if (err) {
+ ERROR(failed to run script '%s' for interface '%s',
+ veth1, netdev-vethscript);
+ goto out_delete;
+ }
+ }
+
DEBUG(instanciated veth '%s/%s', index is '%d',
veth1, veth2, netdev-ifindex);
diff --git a/src/lxc/conf.h b/src/lxc/conf.h
index b12a346..23cf9f8 100644
--- a/src/lxc/conf.h
+++ b/src/lxc/conf.h
@@ -94,11 +94,12 @@ union netdev_p {
/*
* Defines a structure to configure a network device
- * @link : lxc.network.link, name of bridge or host iface to attach if any
- * @name : lxc.network.name, name of iface on the container side
- * @flags : flag of the network device (IFF_UP, ... )
- * @ipv4 : a list of ipv4 addresses to be set on the network device
- * @ipv6 : a list of ipv6 addresses to be set on the network device
+ * @link : lxc.network.link, name of bridge or host iface to attach if
any
+ * @name : lxc.network.name, name of iface on the container side
+ * @flags : flag of the network device (IFF_UP, ... )
+ * @ipv4 : a list of ipv4 addresses to be set on the network device
+ * @ipv6 : a list of ipv6 addresses to be set on the network device
+ * @vethscript : a script filename to be executed on veth configuration
*/
struct lxc_netdev {
int type;
@@ -111,6 +112,7 @@ struct lxc_netdev {
union netdev_p priv;
struct lxc_list
Re: [lxc-devel] [PATCH] add lxc.network.veth.script configuration hook
Dies schrieb Daniel Lezcano (daniel.lezc...@free.fr): I suggest you add a lxc.network.script section where it will be called from each instanciate_* OK, that's what an earlier version of the patch did, so I partially reverted :-) The script should receive always the two parameters: $1 : container name $2 : configuration section : network, pts, etc ... What is the easiest way to retrieve the container name from inside instanciate_*? All it gets passed have is the lxc_netdev struct, and I'd like to avoid changing several signatures. And the optional parameters depending of the hooks caller: In your case: $3 : network type veth, ... $4 : network link $5 : guest ifname $6 : host ifname (in case of veth) Is the guest ifname really useful? The script runs in the context of the host, what can be achieved with it? use the /* */ format to conform to the Coding Style please. OK, that was an easy change... +return status; Do we assume the script returns always 0 on success ? and we don't care about the WIFSIGNALED, ... ? I'd suggest that a non-zero exit value indicates some kind of problem with the network configuration and should abort the network setup. Thanks for your feedback, I'll see what I can do about it :) -- Beautiful is writing same markup. Internet Explorer 9 supports standards for HTML5, CSS3, SVG 1.1, ECMAScript5, and DOM L2 L3. Spend less time writing and rewriting code and more time creating great experiences on the web. Be a part of the beta today. http://p.sf.net/sfu/beautyoftheweb ___ Lxc-devel mailing list Lxc-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/lxc-devel
Re: [lxc-devel] [PATCH] add lxc.network.veth.script configuration hook
Dies schrieb Daniel Lezcano (daniel.lezc...@free.fr): * lxc.network.script.pre: IMO, it does not make sense because that means it is the host itself which should be modified, so that fall under the host network configuration umbrella = administrator job :P I cannot think of a fitting example, but I'd like to point at Debian's /etc/network/interfaces that has an even wider variety of hooks: pre-up up post-up pre-down down post-down And all of them are useful in one or another way :-) * lxc.network.script.create Ok * lxc.network.script.post Do you have an example of use case. Does it hurt if we 'merge' the 'post' and 'create' hooks and put the 'create' right after the virtual devices are created ? If it is done before, will fall in the same 'pre' hook case, no ? The post script can be generic for all types of network configuration, while script.create is highly dependent on the type of network setup. PS: No need to CC every message to me, that way, mutt does not recognize the mailing list and makes responding awkward :-) -- ste...@pico.ruhr.de GPG available and welcome http://stefans.datenbruch.de/ -- Beautiful is writing same markup. Internet Explorer 9 supports standards for HTML5, CSS3, SVG 1.1, ECMAScript5, and DOM L2 L3. Spend less time writing and rewriting code and more time creating great experiences on the web. Be a part of the beta today. http://p.sf.net/sfu/beautyoftheweb ___ Lxc-devel mailing list Lxc-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/lxc-devel
Re: [lxc-devel] [PATCH] add lxc.network.veth.script configuration hook
On 10/07/2010 03:06 PM, Stefan Tomanek wrote: Dies schrieb Daniel Lezcano (daniel.lezc...@free.fr): * lxc.network.script.pre: IMO, it does not make sense because that means it is the host itself which should be modified, so that fall under the host network configuration umbrella = administrator job :P I cannot think of a fitting example, but I'd like to point at Debian's /etc/network/interfaces that has an even wider variety of hooks: pre-up up post-up pre-down down post-down And all of them are useful in one or another way :-) * lxc.network.script.create Ok * lxc.network.script.post Do you have an example of use case. Does it hurt if we 'merge' the 'post' and 'create' hooks and put the 'create' right after the virtual devices are created ? If it is done before, will fall in the same 'pre' hook case, no ? The post script can be generic for all types of network configuration, while script.create is highly dependent on the type of network setup. Ok, I will play a bit with your patchset when it will be ready to check if there is no something we missed. PS: No need to CC every message to me, that way, mutt does not recognize the mailing list and makes responding awkward :-) Oh, sure. I am so used to reply-all, I can not guarantee that won't happen again :) -- Daniel -- Beautiful is writing same markup. Internet Explorer 9 supports standards for HTML5, CSS3, SVG 1.1, ECMAScript5, and DOM L2 L3. Spend less time writing and rewriting code and more time creating great experiences on the web. Be a part of the beta today. http://p.sf.net/sfu/beautyoftheweb ___ Lxc-devel mailing list Lxc-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/lxc-devel
