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

[Pavel Emelyanov]

On 04/14/2016 05:41 PM, Tycho Andersen wrote:
> Hi Cyrill,
> 
> On Tue, Apr 12, 2016 at 12:40:40PM +0300, Cyrill Gorcunov wrote:
>> We have some common predefined properties such as
>> "cpuset.cpus" and etc gathered in @cgp_predefined
>> set, but there might be situation when only predefined
>> ones are not enough so add ability to specify additional
>> properties via --cgroup-props option.
>>
>> This option takes either path to a file or plain JSON
>> stream which describe the properties to dump.
>>
>> For example one may pass
>>
>>   --cgroup-props "{\"controller name 1\" : [ \"property-1\", \"property-2\", ... ]}"
>>
>> to dump custom properties.
>>
>> At moment we require Jansson library to be present in the
>> system to support JSON parsing of --cgroup-props. But if
>> someone doesn't need custom properties lets don't force
>> him to setup Jansson library for nothing, simply compile
>> criu without this feature support.
> 
> Not related to this patch, but it seems like I've let the list of
> predefined properties get fairly out of date (e.g. the entire pids
> cgroups, and a bunch of others are missing). It seems to me like we
> should have CRIU have a reasonably comprehensive list of cgroup props,
> and then leave this for other stuff (like custom cgroups in the vz
> kernel or whatever). Does that make sense?

It does, but do you have any ideas how to keep this list up-to-date 
without recompiling criu for every new property? %)

Maybe a simple plain text file with the list of options to dump (instead
of char *xxx[] in sources) that criu reads on start? This would at least
let use update the file on demand w/o reinstalling criu package itself.

-- Pavel

> If so, I can send a patch,
> but probably not until after ubuntu release :)
> 
>> +	/*
>> +	 * It's either plain JSON stream, then it must
>> +	 * start with '{', or it's path to a file to parse.
>> +	 */
>> +	if (arg[0] == '{') {
> 
> Minor thing, but can we do this check the "other" way, i.e. test for
> the first char == '/'? It would be legal json to have leading
> whitespace, so if some unparser somewhere emitted with a leading space
> or \n or something this might break. The filename has to start with a
> / though, so it seems slightly safer.
> 
> Tycho
> .
>