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

Pavel Emelyanov xemul at virtuozzo.com
Thu Apr 14 04:56:41 PDT 2016


On 04/12/2016 12:40 PM, 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.
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
> ---
>  criu/Makefile.config        |   9 ++
>  criu/cgroup-props.c         | 196 ++++++++++++++++++++++++++++++++++++++++++++
>  criu/cr-dump.c              |   6 ++
>  criu/crtools.c              |   5 ++
>  criu/include/cgroup-props.h |   4 +
>  criu/include/cr_options.h   |   1 +
>  scripts/feature-tests.mak   |  26 ++++++
>  7 files changed, 247 insertions(+)
> 
> diff --git a/criu/Makefile.config b/criu/Makefile.config
> index 326356e1e220..ce0be7e21dbf 100644
> --- a/criu/Makefile.config
> +++ b/criu/Makefile.config
> @@ -18,6 +18,11 @@ ifeq ($(call try-cc,$(FEATURE_TEST_UFFD)),y)
>  	export UFFD := 1
>  endif
>  
> +ifeq ($(call try-cc,$(FEATURE_TEST_LIBJANSSON),-ljansson),y)
> +        LIBS    += -ljansson
> +        JANSSON := 1
> +endif
> +
>  FEATURES_LIST	:= TCP_REPAIR STRLCPY STRLCAT PTRACE_PEEKSIGINFO \
>  	SETPROCTITLE_INIT MEMFD
>  
> @@ -46,6 +51,10 @@ ifeq ($$(UFFD),1)
>  	$(Q) @echo '#define CONFIG_HAS_UFFD'				>> $$@
>  	$(Q) @echo ''							>> $$@
>  endif
> +ifeq ($$(JANSSON),1)
> +	$(Q) @echo '#define CONFIG_HAS_LIBJANSSON'			>> $$@
> +	$(Q) @echo ''							>> $$@
> +endif
>  ifeq ($$(piegen-y),y)
>  	$(Q) @echo '#define CONFIG_PIEGEN'				>> $$@
>  	$(Q) @echo ''							>> $$@
> diff --git a/criu/cgroup-props.c b/criu/cgroup-props.c
> index 2f6ed2594cb3..2b383727db8b 100644
> --- a/criu/cgroup-props.c
> +++ b/criu/cgroup-props.c
> @@ -1,9 +1,24 @@
>  #include <stdio.h>
>  #include <string.h>
>  #include <unistd.h>
> +#include <fcntl.h>
> +
> +#include <sys/mman.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
>  
>  #include "compiler.h"
>  #include "cgroup-props.h"
> +#include "config.h"
> +#include "xmalloc.h"
> +#include "util.h"
> +#include "list.h"
> +#include "log.h"
> +#include "bug.h"
> +
> +#ifdef CONFIG_HAS_LIBJANSSON
> +# include <jansson.h>
> +#endif
>  
>  #undef	LOG_PREFIX
>  #define LOG_PREFIX "cg-prop: "
> @@ -98,8 +113,175 @@ static const cgp_t cgp_predefined[] = {
>  	},
>  };
>  
> +typedef struct {
> +	struct list_head	list;
> +	cgp_t			cgp;
> +} cgp_list_t;
> +
> +static LIST_HEAD(cgp_list);
> +
> +static void cgp_free(cgp_list_t *p)
> +{
> +	size_t i;
> +
> +	if (p) {
> +		for (i = 0; i < p->cgp.nr_props; i++)
> +			xfree((void *)p->cgp.props[i]);
> +		xfree((void *)p->cgp.name);
> +		xfree((void *)p->cgp.props);
> +		xfree(p);
> +	}
> +}
> +
> +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.

> +	if (arg[0] == '{') {
> +		if (cgp_parse(arg, len)) {
> +			pr_err("Failed to parse stream `%s'\n", arg);
> +			return -1;
> +		}
> +		return 0;
> +	}
> +
> +	fd = open(arg, O_RDONLY);
> +	if (fd < 0) {
> +		pr_perror("Can't open file %s\n", arg);
> +		goto err;
> +	}
> +
> +	if (fstat(fd, &st)) {
> +		pr_perror("Can't stat file %s\n", arg);
> +		goto err;
> +	}
> +
> +	mem = mmap(NULL, st.st_size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_FILE, fd, 0);
> +	if (mem == MAP_FAILED) {
> +		pr_perror("Can't mmap file %s\n", arg);
> +		goto err;
> +	}
> +
> +	if (cgp_parse(mem, st.st_size)) {
> +		pr_err("Failed to parse file `%s'\n", arg);
> +		goto err;
> +	}
> +
> +	ret = 0;
> +err:
> +	if (mem != MAP_FAILED)
> +		munmap(mem, st.st_size);
> +	close_safe(&fd);

Why not just close?

> +	return ret;
> +}
> +
> +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.

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

Should be static

> +{
> +	json_t *root, *controller_obj;
> +	json_error_t error;
> +
> +	const char *controller;
> +	int ret = -1;
> +
> +	cgp_list_t *cgp_p = NULL;
> +
> +	root = json_loadb(stream, len, JSON_DISABLE_EOF_CHECK, &error);
> +	if (!root) {
> +		pr_err("json_loadb error: %s\n", error.text);
> +		goto err;
> +	}
> +
> +	/*
> +	 * The format we expect here is
> +	 * {
> +	 * 	"controller name 1" : [ "property-1", "property-2", ... ],
> +	 * 	"controller name 2" : [ "property-1", "property-2", ... ]
> +	 * }
> +	 */
> +
> +	json_object_foreach(root, controller, controller_obj) {
> +		json_t *v;
> +		size_t i;
> +
> +		if (!json_is_array(controller_obj)) {
> +			pr_err("Expected array props for controller %s\n", controller);
> +			goto err;
> +		}
> +
> +		if (!cgp_p) {
> +			cgp_p = xzalloc(sizeof(*cgp_p));
> +			if (!cgp_p)
> +				goto err;
> +			INIT_LIST_HEAD(&cgp_p->list);
> +		}
> +
> +		cgp_p->cgp.name = xstrdup(controller);
> +		if (!cgp_p->cgp.name)
> +			goto err;
> +
> +		pr_debug("Parsing dynamic controller '%s'\n", controller);
> +
> +		if (json_array_size(controller_obj) < 1) {
> +			pr_err("Expected array of props for controller %s\n", controller);
> +			goto err;
> +		}
> +
> +		cgp_p->cgp.props = xzalloc(json_array_size(controller_obj) * sizeof(cgp_p->cgp.props));
> +		if (!cgp_p->cgp.props)
> +			goto err;
> +		cgp_p->cgp.nr_props = json_array_size(controller_obj);
> +
> +		for (i = 0; i < json_array_size(controller_obj); i++) {
> +			v = json_array_get(controller_obj, i);
> +
> +			if (!json_is_string(v)) {
> +				pr_err("Expected strings array props for controller %s\n", controller);
> +				goto err;
> +			}
> +
> +			pr_debug("\tproperty '%s'\n", json_string_value(v));
> +
> +			cgp_p->cgp.props[i] = xstrdup(json_string_value(v));
> +			if (!cgp_p->cgp.props[i])
> +				goto err;
> +		}
> +
> +		list_add(&cgp_p->list, &cgp_list);
> +		cgp_p = NULL;
> +	}
> +	ret = 0;
> +
> +err:
> +	if (root)
> +		json_decref(root);
> +	cgp_free(cgp_p);
> +	return ret;
> +}
> +#else
> +int cgp_parse(char *stream, size_t len)
> +{
> +	pr_err("No JSON parser is compiled in. Feature unsupported\n");
> +	return -1;
> +}
> +#endif
> +
>  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.

> +	}
> +
>  	return NULL;
>  }
> +
> +void cgp_fini(void)
> +{
> +	cgp_list_t *p, *t;
> +
> +	list_for_each_entry_safe(p, t, &cgp_list, list)
> +		cgp_free(p);
> +	INIT_LIST_HEAD(&cgp_list);
> +}
> diff --git a/criu/cr-dump.c b/criu/cr-dump.c
> index 5ac9fd041e4e..6fdfe72bede7 100644
> --- a/criu/cr-dump.c
> +++ b/criu/cr-dump.c
> @@ -61,6 +61,7 @@
>  #include "cpu.h"
>  #include "elf.h"
>  #include "cgroup.h"
> +#include "cgroup-props.h"
>  #include "file-lock.h"
>  #include "page-xfer.h"
>  #include "kerndat.h"
> @@ -1554,6 +1555,7 @@ static int cr_dump_finish(int ret)
>  		ret = -1;
>  
>  	cr_plugin_fini(CR_PLUGIN_STAGE__DUMP, ret);
> +	cgp_fini();
>  
>  	if (!ret) {
>  		/*
> @@ -1653,6 +1655,10 @@ int cr_dump_tasks(pid_t pid)
>  	if (vdso_init())
>  		goto err;
>  
> +	if (cgp_init(opts.cgroup_props,
> +		     opts.cgroup_props ? strlen(opts.cgroup_props) : 0))
> +		goto err;
> +
>  	if (parse_cg_info())
>  		goto err;
>  
> diff --git a/criu/crtools.c b/criu/crtools.c
> index 5d9647ad25c8..c23a0a3ae6c1 100644
> --- a/criu/crtools.c
> +++ b/criu/crtools.c
> @@ -38,6 +38,7 @@
>  #include "mount.h"
>  #include "namespaces.h"
>  #include "cgroup.h"
> +#include "cgroup-props.h"
>  #include "cpu.h"
>  #include "action-scripts.h"
>  #include "irmap.h"
> @@ -323,6 +324,7 @@ int main(int argc, char *argv[], char *envp[])
>  		{ "extra",			no_argument,		0, 1077	},
>  		{ "experimental",		no_argument,		0, 1078	},
>  		{ "all",			no_argument,		0, 1079	},
> +		{ "cgroup-props",		required_argument,	0, 1080	},
>  		{ },
>  	};
>  
> @@ -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.

RPC bits are required too.

> diff --git a/criu/include/cgroup-props.h b/criu/include/cgroup-props.h
> index 1259e0cdc27f..07d5261b397f 100644
> --- a/criu/include/cgroup-props.h
> +++ b/criu/include/cgroup-props.h
> @@ -10,4 +10,8 @@ typedef struct {
>  extern cgp_t cgp_global;
>  extern const cgp_t *cgp_get_props(const char *name);
>  
> +extern int cgp_init(char *arg, size_t len);
> +extern void cgp_fini(void);
> +extern int cgp_parse(char *stream, size_t len);
> +
>  #endif /* __CR_CGROUP_PROPS_H__ */
> diff --git a/criu/include/cr_options.h b/criu/include/cr_options.h
> index b6ae3a146b8d..ff7abfec73cf 100644
> --- a/criu/include/cr_options.h
> +++ b/criu/include/cr_options.h
> @@ -97,6 +97,7 @@ struct cr_options {
>  	char			**exec_cmd;
>  	unsigned int		manage_cgroups;
>  	char			*new_global_cg_root;
> +	char			*cgroup_props;
>  	struct list_head	new_cgroup_roots;
>  	bool			autodetect_ext_mounts;
>  	bool			enable_external_sharing;
> diff --git a/scripts/feature-tests.mak b/scripts/feature-tests.mak
> index c48b52e7a7ad..55fe12c3d640 100644
> --- a/scripts/feature-tests.mak
> +++ b/scripts/feature-tests.mak
> @@ -105,3 +105,29 @@ int main(void)
>  }
>  
>  endef
> +
> +define FEATURE_TEST_LIBJANSSON
> +
> +#include <jansson.h>
> +
> +int main(int argc, char *argv[], char *envp[])
> +{
> +	json_error_t error;
> +	const char *v;
> +	json_t *obj;
> +
> +	(void)json_loadb(v, 0, 0, &error);
> +	json_object_foreach(obj, v, obj);
> +	(void)json_is_array(obj);
> +	(void)json_array_size(obj);
> +	(void)json_array_get(obj, 0);
> +	(void)json_is_string(obj);
> +	(void)json_decref(obj);
> +
> +	(void)error;
> +	(void)v;
> +	(void)obj;
> +
> +	return 0;
> +}
> +endef
> 



More information about the CRIU mailing list