[lxc-devel] [PATCH] add lxc.network.veth.script configuration hook

Daniel Lezcano daniel.lezcano at free.fr
Thu Oct 7 09:06:45 UTC 2010


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 @@
>   #include<dirent.h>
>   #include<mntent.h>
>   #include<unistd.h>
> +#include<sys/wait.h>
>   #include<pty.h>
>
>   #include<sys/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 ipv4;
>   	struct lxc_list ipv6;
> +	char *vethscript;
>   };
>
>   /*
> diff --git a/src/lxc/confile.c b/src/lxc/confile.c
> index 610ca15..dce76b6 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_veth_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,7 @@ 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.veth.script",  config_veth_script          },
>   	{ "lxc.network.hwaddr",       config_network_hwaddr       },
>   	{ "lxc.network.mtu",          config_network_mtu          },
>   	{ "lxc.network.vlan.id",      config_network_vlan_id      },
> @@ -478,6 +480,24 @@ static int config_network_ipv6(const char *key, char *value,
>   	return 0;
>   }
>
> +static int config_veth_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;
> +
> +	netdev->vethscript = strdup(value);
> +	if (!netdev->vethscript) {
> +		SYSERROR("failed to dup string '%s'", value);
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
>   static int config_personality(const char *key, char *value,
>   			      struct lxc_conf *lxc_conf)
>   {
>    


Thanks
   -- Daniel




More information about the lxc-devel mailing list