[lxc-devel] [PATCH] add lxc.network.script(.pre|.post|) configuration hooks
Daniel Lezcano
daniel.lezcano at free.fr
Fri Oct 8 14:52:17 UTC 2010
On 10/08/2010 01:23 PM, Stefan Tomanek wrote:
> This commit adds an configuration option to specify a script to be
> executed before, during and after creating or configuring the pair of
> veth devices. The name of the host sided device is passed as an
> argument, so the script can be used to configures routes or firewall
> rules related to the container. The interface can be extended to service
> other network types, and as well can be used to introduce scriptable
> hooks into other sections of the configuration.
>
Michael is not totally wrong when he said we don't need so much hooks I
think.
Are we sure, we want to add these hooks (pre and post) ? I am not
against adding them, but IMO it is more sane to add them if needed
rather than adding something which may not be used.
Wouldn't preferable to have these two hooks:
lxc.network.script.up
lxc.network.script.down
(script parameter will need 'name', 'conf section' 'up' | 'down' ...
If there is a need for a pre or post hook, we can easily add later:
lxc.network.script.pre-up
lxc.network.script.post-up
lxc.network.script.pre-down
lxc.network.script.post-down
No ?
That won't add unnecessary changes, will reduce the patch and will
provide the feature you need.
Sorry for arguing again with your patch but as that is part of the user
interface, when merged that will be difficult to change or to remove :)
Except a few nits, the patch looks ok.
> ---
> src/lxc/conf.c | 103 ++++++++++++++++++++++++++++++++++++++++++++++-------
> src/lxc/conf.h | 23 +++++++-----
> src/lxc/confile.c | 35 ++++++++++++++++++
> src/lxc/start.c | 2 +-
> 4 files changed, 140 insertions(+), 23 deletions(-)
>
> diff --git a/src/lxc/conf.c b/src/lxc/conf.c
> index adfe862..2993bce 100644
> --- a/src/lxc/conf.c
> +++ b/src/lxc/conf.c
> @@ -24,11 +24,13 @@
> #include<stdio.h>
> #undef _GNU_SOURCE
> #include<stdlib.h>
> +#include<stdarg.h>
> #include<errno.h>
> #include<string.h>
> #include<dirent.h>
> #include<mntent.h>
> #include<unistd.h>
> +#include<sys/wait.h>
> #include<pty.h>
>
> #include<sys/types.h>
> @@ -92,7 +94,7 @@ lxc_log_define(lxc_conf, lxc);
>
> extern int pivot_root(const char * new_root, const char * put_old);
>
> -typedef int (*instanciate_cb)(struct lxc_netdev *);
> +typedef int (*instanciate_cb)(struct lxc_handler *, struct lxc_netdev *);
>
> struct mount_opt {
> char *name;
> @@ -105,11 +107,11 @@ struct caps_opt {
> int value;
> };
>
> -static int instanciate_veth(struct lxc_netdev *);
> -static int instanciate_macvlan(struct lxc_netdev *);
> -static int instanciate_vlan(struct lxc_netdev *);
> -static int instanciate_phys(struct lxc_netdev *);
> -static int instanciate_empty(struct lxc_netdev *);
> +static int instanciate_veth(struct lxc_handler *, struct lxc_netdev *);
> +static int instanciate_macvlan(struct lxc_handler *, struct lxc_netdev *);
> +static int instanciate_vlan(struct lxc_handler *, struct lxc_netdev *);
> +static int instanciate_phys(struct lxc_handler *, struct lxc_netdev *);
> +static int instanciate_empty(struct lxc_handler *, struct lxc_netdev *);
>
> static instanciate_cb netdev_conf[LXC_NET_MAXCONFTYPE + 1] = {
> [LXC_NET_VETH] = instanciate_veth,
> @@ -184,6 +186,52 @@ static struct caps_opt caps_opt[] = {
> { "mac_admin", CAP_MAC_ADMIN },
> };
>
> +static int run_script(const char *name, const char *section, const char *script, ...)
> +{
> + va_list argp;
> + int vargc = 4;
> + /* count variable arguments and add 4 for script, container
> + * and section name as well as the terminating NULL
> + */
> + va_start(argp, script);
> + while (va_arg(argp, char*)) vargc++;
> + va_end(argp);
> + INFO("Executing script '%s' for section '%s'", script, section);
> +
> + int pid = fork();
> + if (pid< 0) {
> + ERROR("Error forking");
> + } else if (pid == 0) {
> + /* prepare command line arguments */
> + char *args[vargc];
> + args[0] = strdup(script);
> + args[1] = strdup(name);
> + args[2] = strdup(section);
> + args[vargc-1] = (char*) NULL;
> + va_start(argp, script);
> + int i;
> + for (i=3; i<vargc; i++) {
> + args[i] = va_arg(argp, char*);
> + }
> + va_end(argp);
> +
> + execv(script, args);
> + /* if we cannot exex, we exit this fork */
> + exit(1);
>
I think it would be nicer to use 'popen', so we can redirect the output
to the log file (DEBUG and ERROR).
(No need to take care of that now, I will do it after your final patch
will be pushed to git).
> + } else {
> + int status = 0;
> + waitpid( pid,&status, 0 );
> + if (status != 0) {
> + /* something weird happened */
> + SYSERROR("Script '%s' terminated with non-zero exitcode %d", name, status);
> + return -1;
> + } else {
> + return 1; /* all is well */
>
The convention is '0' means 'no error', why do you return 1 here ?
> + }
> + }
> + return -1;
> +}
> +
> static int find_fstype_cb(char* buffer, void *data)
> {
> struct cbarg {
> @@ -1204,7 +1252,7 @@ struct lxc_conf *lxc_conf_init(void)
> return new;
> }
>
> -static int instanciate_veth(struct lxc_netdev *netdev)
> +static int instanciate_veth(struct lxc_handler *handler, struct lxc_netdev *netdev)
> {
> char veth1buf[IFNAMSIZ], *veth1;
> char veth2buf[IFNAMSIZ], *veth2;
> @@ -1267,6 +1315,16 @@ static int instanciate_veth(struct lxc_netdev *netdev)
> }
> }
>
> + if (netdev->script) {
> + err = run_script(handler->name, "net", netdev->script, "veth",
> + veth1, (char*) NULL);
> + if (err) {
> + ERROR("failed to run script '%s' for interface '%s'",
> + netdev->script, veth1);
> + goto out_delete;
> + }
>
You returned '1' on success, that will raise an error, no ?
> + }
> +
> DEBUG("instanciated veth '%s/%s', index is '%d'",
> veth1, veth2, netdev->ifindex);
>
> @@ -1277,7 +1335,7 @@ out_delete:
> return -1;
> }
>
> -static int instanciate_macvlan(struct lxc_netdev *netdev)
> +static int instanciate_macvlan(struct lxc_handler *handler, struct lxc_netdev *netdev)
> {
> char peerbuf[IFNAMSIZ], *peer;
> int err;
> @@ -1317,7 +1375,7 @@ static int instanciate_macvlan(struct lxc_netdev *netdev)
> }
>
> /* XXX: merge with instanciate_macvlan */
> -static int instanciate_vlan(struct lxc_netdev *netdev)
> +static int instanciate_vlan(struct lxc_handler *handler, struct lxc_netdev *netdev)
> {
> char peer[IFNAMSIZ];
> int err;
> @@ -1349,7 +1407,7 @@ static int instanciate_vlan(struct lxc_netdev *netdev)
> return 0;
> }
>
> -static int instanciate_phys(struct lxc_netdev *netdev)
> +static int instanciate_phys(struct lxc_handler *handler, struct lxc_netdev *netdev)
> {
> if (!netdev->link) {
> ERROR("no link specified for the physical interface");
> @@ -1365,17 +1423,20 @@ static int instanciate_phys(struct lxc_netdev *netdev)
> return 0;
> }
>
> -static int instanciate_empty(struct lxc_netdev *netdev)
> +static int instanciate_empty(struct lxc_handler *handler, struct lxc_netdev *netdev)
> {
> netdev->ifindex = 0;
> return 0;
> }
>
> -int lxc_create_network(struct lxc_list *network)
> +int lxc_create_network(struct lxc_handler *handler)
> {
> + struct lxc_list *network =&handler->conf->network;
> struct lxc_list *iterator;
> struct lxc_netdev *netdev;
>
> + char *cname = handler->name;
> +
> lxc_list_for_each(iterator, network) {
>
> netdev = iterator->elem;
> @@ -1386,10 +1447,26 @@ int lxc_create_network(struct lxc_list *network)
> return -1;
> }
>
> - if (netdev_conf[netdev->type](netdev)) {
> + /* run pre conf hook */
> + if (netdev->prescript) {
> + if (!run_script(cname, "net", netdev->prescript, (char*) NULL)) {
> + ERROR("failed to run network pre-conf script '%s'", netdev->prescript);
> + return -1;
> + }
>
run_script never returns '0', AFAICS.
> + }
> +
> + if (netdev_conf[netdev->type](handler, netdev)) {
> ERROR("failed to create netdev");
> return -1;
> }
> +
> + if (netdev->postscript) {
> + if (!run_script(cname, "net", netdev->postscript, (char*) NULL)) {
> + ERROR("failed to run network post-conf script '%s'", netdev->postscript);
> + return -1;
> + }
> + }
>
Same comment.
> +
> }
>
> return 0;
> diff --git a/src/lxc/conf.h b/src/lxc/conf.h
> index b12a346..90f5496 100644
> --- a/src/lxc/conf.h
> +++ b/src/lxc/conf.h
> @@ -28,6 +28,8 @@
>
> #include<lxc/list.h>
>
> +#include<start.h> /* for lxc_handler */
> +
> enum {
> LXC_NET_EMPTY,
> LXC_NET_VETH,
> @@ -94,11 +96,14 @@ 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
> + * @script : a script filename to be executed during configuration
> + * @prescript : a script filename to be executed before configuration
> + * @postscript : a script filename to be executed after configuration
> */
> struct lxc_netdev {
> int type;
> @@ -111,6 +116,9 @@ struct lxc_netdev {
> union netdev_p priv;
> struct lxc_list ipv4;
> struct lxc_list ipv6;
> + char *script;
> + char *prescript;
> + char *postscript;
> };
>
> /*
> @@ -210,7 +218,7 @@ struct lxc_conf {
> */
> extern struct lxc_conf *lxc_conf_init(void);
>
> -extern int lxc_create_network(struct lxc_list *networks);
> +extern int lxc_create_network(struct lxc_handler *handler);
> extern void lxc_delete_network(struct lxc_list *networks);
> extern int lxc_assign_network(struct lxc_list *networks, pid_t pid);
>
> @@ -221,8 +229,5 @@ extern void lxc_delete_tty(struct lxc_tty_info *tty_info);
> * Configure the container from inside
> */
>
> -struct lxc_handler;
> -
> extern int lxc_setup(const char *name, struct lxc_conf *lxc_conf);
> -
> #endif
> diff --git a/src/lxc/confile.c b/src/lxc/confile.c
> index 610ca15..b150fa9 100644
> --- a/src/lxc/confile.c
> +++ b/src/lxc/confile.c
> @@ -63,6 +63,7 @@ static int config_network_hwaddr(const char *, char *, struct lxc_conf *);
> static int config_network_vlan_id(const char *, char *, struct lxc_conf *);
> static int config_network_mtu(const char *, char *, struct lxc_conf *);
> static int config_network_ipv4(const char *, char *, struct lxc_conf *);
> +static int config_network_script(const char *, char *, struct lxc_conf *);
> static int config_network_ipv6(const char *, char *, struct lxc_conf *);
> static int config_cap_drop(const char *, char *, struct lxc_conf *);
> static int config_console(const char *, char *, struct lxc_conf *);
> @@ -91,6 +92,9 @@ static struct config config[] = {
> { "lxc.network.name", config_network_name },
> { "lxc.network.macvlan.mode", config_network_macvlan_mode },
> { "lxc.network.veth.pair", config_network_veth_pair },
> + { "lxc.network.script", config_network_script },
> + { "lxc.network.script.pre", config_network_script },
> + { "lxc.network.script.post", config_network_script },
> { "lxc.network.hwaddr", config_network_hwaddr },
> { "lxc.network.mtu", config_network_mtu },
> { "lxc.network.vlan.id", config_network_vlan_id },
> @@ -478,6 +482,37 @@ static int config_network_ipv6(const char *key, char *value,
> return 0;
> }
>
> +static int config_network_script(const char *key, char *value,
> + struct lxc_conf *lxc_conf)
> +{
> + struct lxc_netdev *netdev;
> +
> + netdev = network_netdev(key, value,&lxc_conf->network);
> + if (!netdev)
> + return -1;
> +
> + char *copy = strdup(value);
> + if (!copy) {
> + SYSERROR("failed to dup string '%s'", value);
> + return -1;
> + }
> + if (strcmp(key, "lxc.network.script") == 0) {
> + netdev->script = copy;
> + return 0;
> + }
> + if (strcmp(key, "lxc.network.script.pre") == 0) {
> + netdev->prescript = copy;
> + return 0;
> + }
> + if (strcmp(key, "lxc.network.script.post") == 0) {
> + netdev->postscript = copy;
> + return 0;
> + }
> + SYSERROR("Unknown key: %s", key);
> + free(copy);
> + return 1;
>
return -1;
> +}
> +
> static int config_personality(const char *key, char *value,
> struct lxc_conf *lxc_conf)
> {
> diff --git a/src/lxc/start.c b/src/lxc/start.c
> index 1d4087c..b963b85 100644
> --- a/src/lxc/start.c
> +++ b/src/lxc/start.c
> @@ -487,7 +487,7 @@ int lxc_spawn(struct lxc_handler *handler)
> /* that should be done before the clone because we will
> * fill the netdev index and use them in the child
> */
> - if (lxc_create_network(&handler->conf->network)) {
> + if (lxc_create_network(handler)) {
> ERROR("failed to create the network");
> lxc_sync_fini(handler);
> return -1;
>
More information about the lxc-devel
mailing list