[CRIU] [PATCH 2/2] cg: Add ability to dump custom cgroup properties

[Cyrill Gorcunov]

On Thu, Apr 14, 2016 at 02:56:41PM +0300, Pavel Emelyanov wrote:
> > +
> > +static int cgp_handle_option(char *arg, size_t len)
> > +{
> > +	void *mem = MAP_FAILED;
> > +	int fd = -1, ret = -1;
> > +	struct stat st;
> > +
> > +	/*
> > +	 * It's either plain JSON stream, then it must
> > +	 * start with '{', or it's path to a file to parse.
> > +	 */
> 
> Better introduce another option to distinguish CLI props from in-file props.

OK, will do.

> > +
> > +	ret = 0;
> > +err:
> > +	if (mem != MAP_FAILED)
> > +		munmap(mem, st.st_size);
> > +	close_safe(&fd);
> 
> Why not just close?

because fd might be < 0, the err label reached even when fd = open() failed

> > +
> > +int cgp_init(char *arg, size_t len)
> > +{
> > +	if (arg && len)
> > +		return cgp_handle_option(arg, len);
> > +	return 0;
> > +}
> > +
> > +#ifdef CONFIG_HAS_LIBJANSSON
> 
> Why only _parse is under ifndef? From my perspective the whole 
> _handle_option should be such.

ok

> 
> > +int cgp_parse(char *stream, size_t len)
> 
> Should be static

ok

> >  const cgp_t *cgp_get_props(const char *name)
> >  {
> > +	cgp_list_t *p;
> >  	size_t i;
> >  
> >  	for (i = 0; i < ARRAY_SIZE(cgp_predefined); i++) {
> > @@ -107,5 +289,19 @@ const cgp_t *cgp_get_props(const char *name)
> >  			return &cgp_predefined[i];
> >  	}
> >  
> > +	list_for_each_entry(p, &cgp_list, list) {
> > +		if (!strcmp(p->cgp.name, name))
> > +			return &p->cgp;
> 
> This makes it impossible to override existing built-in prop lists.

Do we really want to override predefined prop list?

> >  
> > @@ -627,6 +629,9 @@ int main(int argc, char *argv[], char *envp[])
> >  			opts.check_extra_features = true;
> >  			opts.check_experimental_features = true;
> >  			break;
> > +		case 1080:
> > +			opts.cgroup_props = optarg;
> > +			break;
> >  		case 'V':
> >  			pr_msg("Version: %s\n", CRIU_VERSION);
> >  			if (strcmp(CRIU_GITID, "0"))
> 
> Help text required.

OK, will do.

> 
> RPC bits are required too.

This one will be on top.

	Cyrill