[lxc-devel] [PATCH 1/1] support use of 'all' containers when cgmanager supports it

Stéphane Graber stgraber at ubuntu.com
Fri Sep 19 21:31:44 UTC 2014


On Thu, Sep 18, 2014 at 09:20:02PM +0000, Serge Hallyn wrote:
> 
> Introduce a new list of controllers just containing "all".
> 
> Make the lists of controllers null-terminated.
> 
> If the cgmanager api version is high enough, use the 'all' controller
> rather than walking all controllers, which should greatly reduce the
> amount of dbus overhead.  This will be especially important for
> those going through a cgproxy.
> 
> Also remove the call to cleanup cgroups when a cgroup existed.  That
> usually fails (and failure is ignored) since the to-be-cleaned-up
> cgroup is busy, but we shouldn't even be trying.  Note this can
> create for extra un-cleanedup cgroups, however it's better than us
> accidentally removing a cgroup that someone else had created and was
> about to use.
> 
> Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>

Acked-by: Stéphane Graber <stgraber at ubuntu.com>

> ---
>  src/lxc/cgmanager.c | 94 +++++++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 69 insertions(+), 25 deletions(-)
> 
> diff --git a/src/lxc/cgmanager.c b/src/lxc/cgmanager.c
> index 97d19ca..a11d42f 100644
> --- a/src/lxc/cgmanager.c
> +++ b/src/lxc/cgmanager.c
> @@ -53,6 +53,7 @@
>  
>  #define CGM_SUPPORTS_GET_ABS 3
>  #define CGM_SUPPORTS_NAMED 4
> +#define CGM_SUPPORTS_MULT_CONTROLLERS 10
>  
>  #ifdef HAVE_CGMANAGER
>  lxc_log_define(lxc_cgmanager, lxc);
> @@ -114,7 +115,7 @@ static int32_t api_version;
>  
>  static struct cgroup_ops cgmanager_ops;
>  static int nr_subsystems;
> -static char **subsystems;
> +static char **subsystems, **subsystems_inone;
>  static bool dbus_threads_initialized = false;
>  static void cull_user_controllers(void);
>  
> @@ -181,6 +182,11 @@ static bool cgm_dbus_connect(void)
>  	return true;
>  }
>  
> +static inline bool cgm_supports_multiple_controllers(void)
> +{
> +	return api_version >= CGM_SUPPORTS_MULT_CONTROLLERS;
> +}
> +
>  static int send_creds(int sock, int rpid, int ruid, int rgid)
>  {
>  	struct msghdr msg = { 0 };
> @@ -242,15 +248,19 @@ static bool lxc_cgmanager_escape(void)
>  {
>  	bool ret = true;
>  	pid_t me = getpid();
> +	char **slist = subsystems;
>  	int i;
>  
> -	for (i = 0; i < nr_subsystems; i++) {
> +	if (cgm_supports_multiple_controllers())
> +		slist = subsystems_inone;
> +
> +	for (i = 0; slist[i]; i++) {
>  		if (cgmanager_move_pid_abs_sync(NULL, cgroup_manager,
> -					subsystems[i], "/", me) != 0) {
> +					slist[i], "/", me) != 0) {
>  			NihError *nerr;
>  			nerr = nih_error_get();
>  			ERROR("call to cgmanager_move_pid_abs_sync(%s) failed: %s",
> -					subsystems[i], nerr->message);
> +					slist[i], nerr->message);
>  			nih_free(nerr);
>  			ret = false;
>  			break;
> @@ -340,7 +350,8 @@ out:
>  static int chown_cgroup_wrapper(void *data)
>  {
>  	struct chown_data *arg = data;
> -	int i;
> +	char **slist = subsystems;
> +	int i, ret = -1;
>  	uid_t destuid;
>  
>  	if (setresgid(0,0,0) < 0)
> @@ -355,15 +366,21 @@ static int chown_cgroup_wrapper(void *data)
>  		return -1;
>  	}
>  	destuid = get_ns_uid(arg->origuid);
> -	for (i = 0; i < nr_subsystems; i++) {
> -		if (do_chown_cgroup(subsystems[i], arg->cgroup_path, destuid) < 0) {
> +
> +	if (cgm_supports_multiple_controllers())
> +		slist = subsystems_inone;
> +
> +	for (i = 0; slist[i]; i++) {
> +		if (do_chown_cgroup(slist[i], arg->cgroup_path, destuid) < 0) {
>  			ERROR("Failed to chown %s:%s to container root",
> -				subsystems[i], arg->cgroup_path);
> -			return -1;
> +				slist[i], arg->cgroup_path);
> +			goto fail;
>  		}
>  	}
> +	ret = 0;
> +fail:
>  	cgm_dbus_disconnect();
> -	return 0;
> +	return ret;
>  }
>  
>  /* Internal helper.  Must be called with the cgmanager dbus socket open */
> @@ -385,6 +402,7 @@ static bool lxc_cgmanager_chmod(const char *controller,
>  static bool chown_cgroup(const char *cgroup_path, struct lxc_conf *conf)
>  {
>  	struct chown_data data;
> +	char **slist = subsystems;
>  	int i;
>  
>  	if (lxc_list_empty(&conf->id_map))
> @@ -407,12 +425,15 @@ static bool chown_cgroup(const char *cgroup_path, struct lxc_conf *conf)
>  	 * This can't be done in the child namespace because it only group-owns
>  	 * the cgroup
>  	 */
> -	for (i = 0; i < nr_subsystems; i++) {
> -		if (!lxc_cgmanager_chmod(subsystems[i], cgroup_path, "", 0775))
> +	if (cgm_supports_multiple_controllers())
> +		slist = subsystems_inone;
> +
> +	for (i = 0; slist[i]; i++) {
> +		if (!lxc_cgmanager_chmod(slist[i], cgroup_path, "", 0775))
>  			return false;
> -		if (!lxc_cgmanager_chmod(subsystems[i], cgroup_path, "tasks", 0775))
> +		if (!lxc_cgmanager_chmod(slist[i], cgroup_path, "tasks", 0775))
>  			return false;
> -		if (!lxc_cgmanager_chmod(subsystems[i], cgroup_path, "cgroup.procs", 0775))
> +		if (!lxc_cgmanager_chmod(slist[i], cgroup_path, "cgroup.procs", 0775))
>  			return false;
>  	}
>  
> @@ -478,6 +499,7 @@ err1:
>  static void cgm_destroy(void *hdata)
>  {
>  	struct cgm_data *d = hdata;
> +	char **slist = subsystems;
>  	int i;
>  
>  	if (!d || !d->cgroup_path)
> @@ -486,8 +508,11 @@ static void cgm_destroy(void *hdata)
>  		ERROR("Error connecting to cgroup manager");
>  		return;
>  	}
> -	for (i = 0; i < nr_subsystems; i++)
> -		cgm_remove_cgroup(subsystems[i], d->cgroup_path);
> +
> +	if (cgm_supports_multiple_controllers())
> +		slist = subsystems_inone;
> +	for (i = 0; slist[i]; i++)
> +		cgm_remove_cgroup(slist[i], d->cgroup_path);
>  
>  	free(d->name);
>  	if (d->cgroup_path)
> @@ -503,13 +528,18 @@ static void cgm_destroy(void *hdata)
>  static inline void cleanup_cgroups(char *path)
>  {
>  	int i;
> -	for (i = 0; i < nr_subsystems; i++)
> -		cgm_remove_cgroup(subsystems[i], path);
> +	char **slist = subsystems;
> +
> +	if (cgm_supports_multiple_controllers())
> +		slist = subsystems_inone;
> +	for (i = 0; slist[i]; i++)
> +		cgm_remove_cgroup(slist[i], path);
>  }
>  
>  static inline bool cgm_create(void *hdata)
>  {
>  	struct cgm_data *d = hdata;
> +	char **slist = subsystems;
>  	int i, index=0, baselen, ret;
>  	int32_t existed;
>  	char result[MAXPATHLEN], *tmp, *cgroup_path;
> @@ -545,9 +575,13 @@ again:
>  			goto bad;
>  	}
>  	existed = 0;
> -	for (i = 0; i < nr_subsystems; i++) {
> -		if (!lxc_cgmanager_create(subsystems[i], tmp, &existed)) {
> -			ERROR("Error creating cgroup %s:%s", subsystems[i], result);
> +
> +	if (cgm_supports_multiple_controllers())
> +		slist = subsystems_inone;
> +
> +	for (i = 0; slist[i]; i++) {
> +		if (!lxc_cgmanager_create(slist[i], tmp, &existed)) {
> +			ERROR("Error creating cgroup %s:%s", slist[i], result);
>  			cleanup_cgroups(tmp);
>  			goto bad;
>  		}
> @@ -565,7 +599,6 @@ again:
>  	return true;
>  
>  next:
> -	cleanup_cgroups(tmp);
>  	index++;
>  	goto again;
>  bad:
> @@ -606,10 +639,14 @@ static bool lxc_cgmanager_enter(pid_t pid, const char *controller,
>  /* Internal helper, must be called with cgmanager dbus socket open */
>  static bool do_cgm_enter(pid_t pid, const char *cgroup_path, bool abs)
>  {
> +	char **slist = subsystems;
>  	int i;
>  
> -	for (i = 0; i < nr_subsystems; i++) {
> -		if (!lxc_cgmanager_enter(pid, subsystems[i], cgroup_path, abs))
> +	if (cgm_supports_multiple_controllers())
> +		slist = subsystems_inone;
> +
> +	for (i = 0; slist[i]; i++) {
> +		if (!lxc_cgmanager_enter(pid, slist[i], cgroup_path, abs))
>  			return false;
>  	}
>  	return true;
> @@ -996,6 +1033,12 @@ static bool collect_subsytems(void)
>  	if (subsystems) // already initialized
>  		return true;
>  
> +	subsystems_inone = malloc(2 * sizeof(char *));
> +	if (!subsystems_inone)
> +		return false;
> +	subsystems_inone[0] = "all";
> +	subsystems_inone[1] = NULL;
> +
>  	f = fopen_cloexec("/proc/self/cgroup", "r");
>  	if (!f) {
>  		f = fopen_cloexec("/proc/1/cgroup", "r");
> @@ -1022,12 +1065,13 @@ static bool collect_subsytems(void)
>  		for (p = strtok_r(slist, ",", &saveptr);
>  				p;
>  				p = strtok_r(NULL, ",", &saveptr)) {
> -			tmp = realloc(subsystems, (nr_subsystems+1)*sizeof(char *));
> +			tmp = realloc(subsystems, (nr_subsystems+2)*sizeof(char *));
>  			if (!tmp)
>  				goto out_free;
>  
>  			subsystems = tmp;
>  			tmp[nr_subsystems] = strdup(p);
> +			tmp[nr_subsystems+1] = NULL;
>  			if (!tmp[nr_subsystems])
>  				goto out_free;
>  			nr_subsystems++;
> -- 
> 2.1.0
> 
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel

-- 
Stéphane Graber
Ubuntu developer
http://www.ubuntu.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20140919/cdb22579/attachment.sig>


More information about the lxc-devel mailing list