[lxc-devel] [PATCH 7/9] container: introduce two functions named as {at/de}tach_interface().

[Serge Hallyn]

Quoting Dongsheng Yang (yangds.fnst at cn.fujitsu.com):
> Currently, we depends on ip command to attach interface to container.
> It means we only implemented it by python.
> 
> This patch implement adding and removing interface by c and added
> them in struct container.
> 
> Signed-off-by: Dongsheng Yang <yangds.fnst at cn.fujitsu.com>

Overall looks ok, but please see below.

Please don't resend the whole patchset, only send updated patches in
reply in this thread.

> ---
>  src/lxc/lxccontainer.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/lxc/lxccontainer.h | 19 ++++++++++++
>  2 files changed, 97 insertions(+)
> 
> diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> index f210a88..fedcf97 100644
> --- a/src/lxc/lxccontainer.c
> +++ b/src/lxc/lxccontainer.c
> @@ -39,6 +39,7 @@
>  
>  #include <lxc/lxccontainer.h>
>  #include <lxc/version.h>
> +#include <lxc/network.h>
>  
>  #include "config.h"
>  #include "lxc.h"
> @@ -3455,6 +3456,81 @@ static bool lxcapi_remove_device_node(struct lxc_container *c, const char *src_p
>  	return add_remove_device_node(c, src_path, dest_path, false);
>  }
>  
> +static bool lxcapi_attach_interface(struct lxc_container *c, const char *ifname,
> +				const char *dst_ifname)
> +{
> +	int ret = 0;
> +
> +	ret = lxc_netdev_isup(ifname);
> +	if (ret < 0)
> +		goto err;

This'll need tweaking based on your update to the lxc_netdev_isup patch.

I guess since you care about EINVAl it should return an int after
all.

> +
> +	/* netdev of ifname is up. */
> +	if (ret) {
> +		ret = lxc_netdev_down(ifname);
> +		if (ret)
> +			goto err;
> +	}
> +
> +	ret = lxc_netdev_move_by_name(ifname, c->init_pid(c), dst_ifname);
> +	if (ret)
> +		goto err;
> +
> +	return true;
> +err:
> +	/* -EINVAL means there is no netdev named as ifanme. */
> +	if (ret == -EINVAL) {
> +		ERROR("No network device named as %s.", ifname);
> +	}
> +	return false;
> +}
> +
> +static bool lxcapi_detach_interface(struct lxc_container *c, const char *ifname,
> +					const char *dst_ifname)
> +{
> +	pid_t pid, pid_outside;

Might want to check here whether the caller is root, and give a
clearer error msg.

> +	pid_outside = getpid();
> +
> +	pid = fork();
> +	if (pid < 0) {
> +		ERROR("failed to fork task to get interfaces information");
> +		return false;
> +	}
> +
> +	if (pid == 0) { // child
> +		int ret = 0;
> +		if (!enter_to_ns(c)) {
> +			ERROR("failed to enter namespace");
> +			exit(-1);
> +		}
> +
> +		ret = lxc_netdev_isup(ifname);
> +		if (ret < 0)
> +			exit(ret);
> +
> +		/* netdev of ifname is up. */
> +		if (ret) {
> +			ret = lxc_netdev_down(ifname);
> +			if (ret)
> +				exit(ret);
> +		}
> +
> +		ret = lxc_netdev_move_by_name(ifname, pid_outside, dst_ifname);
> +
> +		/* -EINVAL means there is no netdev named as ifanme. */
> +		if (ret == -EINVAL) {
> +			ERROR("No network device named as %s.", ifname);
> +		}
> +		exit(ret);
> +	}
> +
> +	if (wait_for_pid(pid) != 0)
> +		return false;
> +
> +	return true;
> +}
> +
>  struct criu_opts {
>  	/* The type of criu invocation, one of "dump" or "restore" */
>  	char *action;
> @@ -4050,6 +4126,8 @@ struct lxc_container *lxc_container_new(const char *name, const char *configpath
>  	c->may_control = lxcapi_may_control;
>  	c->add_device_node = lxcapi_add_device_node;
>  	c->remove_device_node = lxcapi_remove_device_node;
> +	c->attach_interface = lxcapi_attach_interface;
> +	c->detach_interface = lxcapi_detach_interface;
>  	c->checkpoint = lxcapi_checkpoint;
>  	c->restore = lxcapi_restore;
>  
> diff --git a/src/lxc/lxccontainer.h b/src/lxc/lxccontainer.h
> index 6344f3d..f9feeba 100644
> --- a/src/lxc/lxccontainer.h
> +++ b/src/lxc/lxccontainer.h
> @@ -762,6 +762,25 @@ struct lxc_container {
>  	bool (*remove_device_node)(struct lxc_container *c, const char *src_path, const char *dest_path);
>  
>  	/*!
> +	 * \brief Add specified netdev to the container.
> +	 *
> +	 * \param c Container.
> +	 * \param dev name of net device.
> +	 *
> +	 * \return \c true on success, else \c false.
> +	 */
> +	bool (*attach_interface)(struct lxc_container *c, const char *dev, const char *dst_dev);
> +
> +	/*!
> +	 * \brief Remove specified netdev from the container.
> +	 *
> +	 * \param c Container.
> +	 * \param dev name of net device.
> +	 *
> +	 * \return \c true on success, else \c false.
> +	 */
> +	bool (*detach_interface)(struct lxc_container *c, const char *dev, const char *dst_dev);
> +	/*!
>  	 * \brief Checkpoint a container.
>  	 *
>  	 * \param c Container.
> -- 
> 1.8.4.2
>