[lxc-devel] [PATCH 1/5] cgroup: minor bugfixes so start and attach work again

[Serge Hallyn]

Quoting Christian Seiler (christian at iwakd.de):
> This fixes some minor bugs in the cgroup logic that made start and
> attach fail (at least when all cgroup controllers were mounted
> together).
>
> Signed-off-by: Christian Seiler <christian at iwakd.de>

Thanks, a few comments.

> ---
>  src/lxc/cgroup.c   |   31 ++++++++++++++++++++++++++++---
>  src/lxc/commands.c |    3 ++-
>  2 files changed, 30 insertions(+), 4 deletions(-)
> 
> diff --git a/src/lxc/cgroup.c b/src/lxc/cgroup.c
> index d2737ea..2dad706 100644
> --- a/src/lxc/cgroup.c
> +++ b/src/lxc/cgroup.c
> @@ -555,6 +555,26 @@ static int in_subsys_list(const char *s, const char *list)
>  	return 0;
>  }
>  
> +static int subsys_lists_match(const char *list1, const char *list2)
> +{
> +	char *token, *str, *saveptr = NULL;
> +
> +	if (!list1 || !list2)
> +		return 0;
> +
> +        if (strlen(list1) != strlen(list2))
> +                return 0;
> +
> +	str = alloca(strlen(list1)+1);
> +	strcpy(str, list1);
> +	for (; (token = strtok_r(str, ",", &saveptr)); str = NULL) {
> +		if (in_subsys_list(token, list2) == 0)
> +			return 0;
> +	}
> +
> +	return 1;
> +}
> +
>  static void set_clone_children(struct mntent *m)
>  {
>  	char path[MAXPATHLEN];
> @@ -620,6 +640,7 @@ static char *record_visited(char *opts, char *all_subsystems)
>  		else
>  			*visited = '\0';
>  		strcat(visited, token);
> +		oldlen += toklen + (oldlen ? 1 : 0);

This needs to just be

	oldlen = newlen;

>  	}
>  
>  	return visited;
> @@ -768,9 +789,12 @@ static bool find_real_cgroup(struct cgroup_desc *d, char *path)
>  		if (!(p2 = index(++p, ':')))
>  			continue;
>  		*p2 = '\0';
> +		// remove trailing newlines
> +		if (*(p2 + 1) && p2[strlen(p2 + 1)] == '\n')
> +		        p2[strlen(p2 + 1)] = '\0';
>  		// in case of multiple mounts it may be more correct to
>  		// insist all subsystems be the same
> -		if (in_subsys_list(p, d->subsystems))
> +		if (subsys_lists_match(p, d->subsystems))
>  			goto found;
>         }
>  
> @@ -1128,7 +1152,7 @@ void lxc_cgroup_destroy_desc(struct cgroup_desc *cgroups)
>  int lxc_cgroup_attach(pid_t pid, const char *name, const char *lxcpath)
>  {
>  	FILE *f;
> -	char *line = NULL, ret = -1;
> +	char *line = NULL, ret = 0;
>  	size_t len = 0;
>  	int first = 1;
>  	char *dirpath;

Note you are returning 0 here if /proc/groups couldn't be opened.  Was
that your intent?  Should I just add a 'return -1' on that failure?

> @@ -1156,8 +1180,9 @@ int lxc_cgroup_attach(pid_t pid, const char *name, const char *lxcpath)
>  			continue;
>  
>  		INFO("joining pid %d to cgroup %s", pid, dirpath);
> -		if (lxc_cgroup_enter_one(dirpath, pid)) {
> +		if (!lxc_cgroup_enter_one(dirpath, pid)) {
>  			ERROR("Failed joining %d to %s\n", pid, dirpath);
> +			ret = -1;
>  			continue;
>  		}
>  	}
> diff --git a/src/lxc/commands.c b/src/lxc/commands.c
> index f1f9fcc..a7981ba 100644
> --- a/src/lxc/commands.c
> +++ b/src/lxc/commands.c
> @@ -393,12 +393,13 @@ static int lxc_cmd_get_cgroup_callback(int fd, struct lxc_cmd_req *req,
>  
>  	if (req->datalen < 1)
>  		return -1;
> -
> +        
>  	path = cgroup_get_subsys_path(handler, req->data);
>  	if (!path)
>  		return -1;
>  	rsp.datalen = strlen(path) + 1,
>  	rsp.data = path;
> +	rsp.ret = 0;
>  
>  	return lxc_cmd_rsp_send(fd, &rsp);
>  }
> -- 
> 1.7.10.4
> 
> 
> ------------------------------------------------------------------------------
> Get 100% visibility into Java/.NET code with AppDynamics Lite!
> It's a free troubleshooting tool designed for production.
> Get down to code-level detail for bottlenecks, with <2% overhead. 
> Download for free and get started troubleshooting in minutes. 
> http://pubads.g.doubleclick.net/gampad/clk?id=48897031&iu=/4140/ostg.clktrk
> _______________________________________________
> Lxc-devel mailing list
> Lxc-devel at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/lxc-devel