[CRIU] [PATCH v2] Extend the parser to accept negative options

Veronika Kabatova vkabatov at redhat.com
Wed Apr 5 08:01:29 PDT 2017


Introducing negative options for true / false values. The original
getopt_long parser is kept since it is able to set flag-like values
(instead of setting these values in the switch when it's not needed).
The type of the options needed to be changed to integers for getopt_long
to accept flag-like value settings (as per getopt_long documentation,
the address of integer variable has to be passed).

Corresponding negative options are not added for deprecated options.

This patch is a preparation for the addition of configuration files
(GitHub issue #278). General idea of this feature is to have global
configuration files in /etc/criu.d/ directory and user-specific
configuration files in $HOME/.criu.d/ directory, with the possibility
of specifying a chosen file to be used (default files will be used if
none is specified, or none in case the default ones are not present,
to not break compatibility). The options in configuration files should
be possible to be overriden by the options specified on command line,
hence the negative options addition.

The whole feature of configuration files will remove the need of
specifying all the options on command line, with the possibility of
reusing a file for different use case with only overriding some of the
values specified there.

In case both types of option (negative and positive) are passed, the
later one will be applied -- this works with the philosophy of
overriding the "earlier" options from configuration files.

Changes since v1:
- Describe the --no- option prefix in the beginning of OPTIONS section in
  both man page and --help instead of mentioning it at every eligible line
  (this also fixes line length issue with --help)
- Fix the accidental removal of check_only case caused by bad rebase
- Use a macro for getopt_long struct option generating instead of additional
  defines and hardcoded lines

Signed-off-by: Veronika Kabatova <vkabatov at redhat.com>
---
 Documentation/criu.txt    |   4 ++
 criu/crtools.c            | 133 ++++++++++++++++++----------------------------
 criu/include/cr_options.h |  44 +++++++--------
 3 files changed, 78 insertions(+), 103 deletions(-)

diff --git a/Documentation/criu.txt b/Documentation/criu.txt
index 1d64f7f..0a2f65b 100644
--- a/Documentation/criu.txt
+++ b/Documentation/criu.txt
@@ -24,6 +24,10 @@ on a different system, or both.
 OPTIONS
 -------
 
+Most of the true / false long options (the ones without arguments) can be
+prefixed with *--no-* to negate the option (example: *--display-stats*
+and *--no-display-stats*).
+
 Common options
 ~~~~~~~~~~~~~~
 Common options are applicable to any 'command'.
diff --git a/criu/crtools.c b/criu/crtools.c
index 5a039a8..013a138 100644
--- a/criu/crtools.c
+++ b/criu/crtools.c
@@ -212,6 +212,11 @@ bool deprecated_ok(char *what)
 
 int main(int argc, char *argv[], char *envp[])
 {
+
+#define BOOL_OPT(OPT_NAME, SAVE_TO) \
+		{OPT_NAME, no_argument, SAVE_TO, true},\
+		{"no-" OPT_NAME, no_argument, SAVE_TO, false}
+
 	pid_t pid = 0, tree_id = 0;
 	int ret = -1;
 	bool usage_error = true;
@@ -226,9 +231,9 @@ int main(int argc, char *argv[], char *envp[])
 		{ "pid",			required_argument,	0, 'p'	},
 		{ "leave-stopped",		no_argument,		0, 's'	},
 		{ "leave-running",		no_argument,		0, 'R'	},
-		{ "restore-detached",		no_argument,		0, 'd'	},
-		{ "restore-sibling",		no_argument,		0, 'S'	},
-		{ "daemon",			no_argument,		0, 'd'	},
+		BOOL_OPT("restore-detached", &opts.restore_detach),
+		BOOL_OPT("restore-sibling", &opts.restore_sibling),
+		BOOL_OPT("daemon", &opts.restore_detach),
 		{ "contents",			no_argument,		0, 'c'	},
 		{ "file",			required_argument,	0, 'f'	},
 		{ "fields",			required_argument,	0, 'F'	},
@@ -239,27 +244,27 @@ int main(int argc, char *argv[], char *envp[])
 		{ "root",			required_argument,	0, 'r'	},
 		{ USK_EXT_PARAM,		optional_argument,	0, 'x'	},
 		{ "help",			no_argument,		0, 'h'	},
-		{ SK_EST_PARAM,			no_argument,		0, 1042	},
+		BOOL_OPT(SK_EST_PARAM, &opts.tcp_established_ok),
 		{ "close",			required_argument,	0, 1043	},
-		{ "log-pid",			no_argument,		0, 1044	},
+		BOOL_OPT("log-pid", &opts.log_file_per_pid),
 		{ "version",			no_argument,		0, 'V'	},
-		{ "evasive-devices",		no_argument,		0, 1045	},
+		BOOL_OPT("evasive-devices", &opts.evasive_devices),
 		{ "pidfile",			required_argument,	0, 1046	},
 		{ "veth-pair",			required_argument,	0, 1047	},
 		{ "action-script",		required_argument,	0, 1049	},
-		{ LREMAP_PARAM,			no_argument,		0, 1041	},
-		{ OPT_SHELL_JOB,		no_argument,		0, 'j'	},
-		{ OPT_FILE_LOCKS,		no_argument,		0, 'l'	},
-		{ "page-server",		no_argument,		0, 1050	},
+		BOOL_OPT(LREMAP_PARAM, &opts.link_remap_ok),
+		BOOL_OPT(OPT_SHELL_JOB, &opts.shell_job),
+		BOOL_OPT(OPT_FILE_LOCKS, &opts.handle_file_locks),
+		BOOL_OPT("page-server", &opts.use_page_server),
 		{ "address",			required_argument,	0, 1051	},
 		{ "port",			required_argument,	0, 1052	},
 		{ "prev-images-dir",		required_argument,	0, 1053	},
 		{ "ms",				no_argument,		0, 1054	},
-		{ "track-mem",			no_argument,		0, 1055	},
-		{ "auto-dedup",			no_argument,		0, 1056	},
+		BOOL_OPT("track-mem", &opts.track_mem),
+		BOOL_OPT("auto-dedup", &opts.auto_dedup),
 		{ "libdir",			required_argument,	0, 'L'	},
 		{ "cpu-cap",			optional_argument,	0, 1057	},
-		{ "force-irmap",		no_argument,		0, 1058	},
+		BOOL_OPT("force-irmap", &opts.force_irmap),
 		{ "ext-mount-map",		required_argument,	0, 'M'	},
 		{ "exec-cmd",			no_argument,		0, 1059	},
 		{ "manage-cgroups",		optional_argument,	0, 1060	},
@@ -268,8 +273,8 @@ int main(int argc, char *argv[], char *envp[])
 		{ "feature",			required_argument,	0, 1063	},
 		{ "skip-mnt",			required_argument,	0, 1064 },
 		{ "enable-fs",			required_argument,	0, 1065 },
-		{ "enable-external-sharing", 	no_argument, 		0, 1066 },
-		{ "enable-external-masters", 	no_argument, 		0, 1067 },
+		{ "enable-external-sharing", 	no_argument, 		&opts.enable_external_sharing, true	},
+		{ "enable-external-masters", 	no_argument, 		&opts.enable_external_masters, true	},
 		{ "freeze-cgroup",		required_argument,	0, 1068 },
 		{ "ghost-limit",		required_argument,	0, 1069 },
 		{ "irmap-scan-path",		required_argument,	0, 1070 },
@@ -278,22 +283,24 @@ int main(int argc, char *argv[], char *envp[])
 		{ "external",			required_argument,	0, 1073	},
 		{ "empty-ns",			required_argument,	0, 1074	},
 		{ "lazy-pages",			no_argument,		0, 1076 },
-		{ "extra",			no_argument,		0, 1077	},
-		{ "experimental",		no_argument,		0, 1078	},
+		BOOL_OPT("extra", &opts.check_extra_features),
+		BOOL_OPT("experimental", &opts.check_experimental_features),
 		{ "all",			no_argument,		0, 1079	},
 		{ "cgroup-props",		required_argument,	0, 1080	},
 		{ "cgroup-props-file",		required_argument,	0, 1081	},
 		{ "cgroup-dump-controller",	required_argument,	0, 1082	},
-		{ SK_INFLIGHT_PARAM,		no_argument,		0, 1083	},
-		{ "deprecated",			no_argument,		0, 1084 },
-		{ "check-only",			no_argument,		0, 1085 },
-		{ "display-stats",		no_argument,		0, 1086 },
-		{ "weak-sysctls",		no_argument,		0, 1087 },
+		BOOL_OPT(SK_INFLIGHT_PARAM, &opts.tcp_skip_in_flight),
+		BOOL_OPT("deprecated", &opts.deprecated_ok),
+		BOOL_OPT("check-only", &opts.check_only),
+		BOOL_OPT("display-stats", &opts.display_stats),
+		BOOL_OPT("weak-sysctls", &opts.weak_sysctls),
 		{ "status-fd",			required_argument,	0, 1088 },
-		{ "remote",			no_argument,		0, 1089 },
+		BOOL_OPT("remote", &opts.remote),
 		{ },
 	};
 
+#undef BOOL_OPT
+
 	BUILD_BUG_ON(PAGE_SIZE != PAGE_IMAGE_SIZE);
 	BUILD_BUG_ON(CTL_32 != SYSCTL_TYPE__CTL_32);
 	BUILD_BUG_ON(__CTL_STR != SYSCTL_TYPE__CTL_STR);
@@ -330,6 +337,8 @@ int main(int argc, char *argv[], char *envp[])
 		opt = getopt_long(argc, argv, short_opts, long_opts, &idx);
 		if (opt == -1)
 			break;
+		if (!opt)
+			continue;
 
 		switch (opt) {
 		case 's':
@@ -394,14 +403,6 @@ int main(int argc, char *argv[], char *envp[])
 			} else
 				log_level++;
 			break;
-		case 1041:
-			pr_info("Will allow link remaps on FS\n");
-			opts.link_remap_ok = true;
-			break;
-		case 1042:
-			pr_info("Will dump TCP connections\n");
-			opts.tcp_established_ok = true;
-			break;
 		case 1043: {
 			int fd;
 
@@ -410,12 +411,6 @@ int main(int argc, char *argv[], char *envp[])
 			close(fd);
 			break;
 		}
-		case 1044:
-			opts.log_file_per_pid = 1;
-			break;
-		case 1045:
-			opts.evasive_devices = true;
-			break;
 		case 1046:
 			opts.pidfile = optarg;
 			break;
@@ -437,9 +432,6 @@ int main(int argc, char *argv[], char *envp[])
 				return 1;
 
 			break;
-		case 1050:
-			opts.use_page_server = true;
-			break;
 		case 1051:
 			opts.addr = optarg;
 			break;
@@ -457,12 +449,6 @@ int main(int argc, char *argv[], char *envp[])
 		case 1053:
 			opts.img_parent = optarg;
 			break;
-		case 1055:
-			opts.track_mem = true;
-			break;
-		case 1056:
-			opts.auto_dedup = true;
-			break;
 		case 1057:
 			if (parse_cpu_cap(&opts, optarg))
 				goto usage;
@@ -520,12 +506,6 @@ int main(int argc, char *argv[], char *envp[])
 			if (!add_fsname_auto(optarg))
 				return 1;
 			break;
-		case 1066:
-			opts.enable_external_sharing = true;
-			break;
-		case 1067:
-			opts.enable_external_masters = true;
-			break;
 		case 1068:
 			opts.freeze_cgroup = optarg;
 			break;
@@ -577,12 +557,6 @@ int main(int argc, char *argv[], char *envp[])
 				return 1;
 			}
 			break;
-		case 1077:
-			opts.check_extra_features = true;
-			break;
-		case 1078:
-			opts.check_experimental_features = true;
-			break;
 		case 1079:
 			opts.check_extra_features = true;
 			opts.check_experimental_features = true;
@@ -597,35 +571,12 @@ int main(int argc, char *argv[], char *envp[])
 			if (!cgp_add_dump_controller(optarg))
 				return 1;
 			break;
-		case 1083:
-			pr_msg("Will skip in-flight TCP connections\n");
-			opts.tcp_skip_in_flight = true;
-			break;
-		case 1084:
-			pr_msg("Turn deprecated stuff ON\n");
-			opts.deprecated_ok = true;
-			break;
-		case 1085:
-			pr_msg("Only checking if requested operation will succeed\n");
-			opts.check_only = true;
-			opts.final_state = TASK_ALIVE;
-			break;
-		case 1086:
-			opts.display_stats = true;
-			break;
-		case 1087:
-			pr_msg("Will skip non-existant sysctls on restore\n");
-			opts.weak_sysctls = true;
-			break;
 		case 1088:
 			if (sscanf(optarg, "%d", &opts.status_fd) != 1) {
 				pr_err("Unable to parse a value of --status-fd\n");
 				return 1;
 			}
 			break;
-		case 1089:
-			opts.remote = true;
-			break;
 		case 'V':
 			pr_msg("Version: %s\n", CRIU_VERSION);
 			if (strcmp(CRIU_GITID, "0"))
@@ -639,6 +590,21 @@ int main(int argc, char *argv[], char *envp[])
 		}
 	}
 
+	if (opts.deprecated_ok)
+		pr_msg("Turn deprecated stuff ON\n");
+	if (opts.tcp_skip_in_flight)
+		pr_msg("Will skip in-flight TCP connections\n");
+	if (opts.tcp_established_ok)
+		pr_info("Will dump TCP connections\n");
+	if (opts.link_remap_ok)
+		pr_info("Will allow link remaps on FS\n");
+	if (opts.weak_sysctls)
+		pr_msg("Will skip non-existant sysctls on restore\n");
+	if (opts.check_only) {
+		pr_msg("Only checking if requested operation will succeed\n");
+		opts.final_state = TASK_ALIVE;
+	}
+
 	if (getenv("CRIU_DEPRECATED")) {
 		pr_msg("Turn deprecated stuff ON via env\n");
 		opts.deprecated_ok = true;
@@ -848,6 +814,11 @@ usage:
 	}
 
 	pr_msg("\n"
+
+"Most of the true / false long options (the ones without arguments) can be\n"
+"prefixed with --no- to negate the option (example: --display-stats and\n"
+"--no-display-stats).\n"
+"\n"
 "Dump/Restore options:\n"
 "\n"
 "* Generic:\n"
diff --git a/criu/include/cr_options.h b/criu/include/cr_options.h
index 518d2a3..94fc717 100644
--- a/criu/include/cr_options.h
+++ b/criu/include/cr_options.h
@@ -51,21 +51,21 @@ struct cr_options {
 	int			final_state;
 	char			*show_dump_file;
 	char			*show_fmt;
-	bool			check_extra_features;
-	bool			check_experimental_features;
+	int			check_extra_features;
+	int			check_experimental_features;
 	bool			show_pages_content;
 	union {
-		bool		restore_detach;
+		int		restore_detach;
 		bool		daemon_mode;
 	};
-	bool			restore_sibling;
+	int			restore_sibling;
 	bool			ext_unix_sk;
-	bool			shell_job;
-	bool			handle_file_locks;
-	bool			tcp_established_ok;
-	bool			evasive_devices;
-	bool			link_remap_ok;
-	bool			log_file_per_pid;
+	int			shell_job;
+	int			handle_file_locks;
+	int			tcp_established_ok;
+	int			evasive_devices;
+	int			link_remap_ok;
+	int			log_file_per_pid;
 	bool			swrk_restore;
 	char			*output;
 	char			*root;
@@ -76,15 +76,15 @@ struct cr_options {
 	struct list_head	external;
 	struct list_head	join_ns;
 	char			*libdir;
-	bool			use_page_server;
+	int			use_page_server;
 	unsigned short		port;
 	char			*addr;
 	int			ps_socket;
-	bool			track_mem;
+	int			track_mem;
 	char			*img_parent;
-	bool			auto_dedup;
+	int			auto_dedup;
 	unsigned int		cpu_cap;
-	bool			force_irmap;
+	int			force_irmap;
 	char			**exec_cmd;
 	unsigned int		manage_cgroups;
 	char			*new_global_cg_root;
@@ -92,8 +92,8 @@ struct cr_options {
 	char			*cgroup_props_file;
 	struct list_head	new_cgroup_roots;
 	bool			autodetect_ext_mounts;
-	bool			enable_external_sharing;
-	bool			enable_external_masters;
+	int			enable_external_sharing;
+	int			enable_external_masters;
 	bool			aufs;		/* auto-deteced, not via cli */
 	bool			overlayfs;
 #ifdef CONFIG_BINFMT_MISC_VIRTUALIZED
@@ -106,7 +106,7 @@ struct cr_options {
 	unsigned int		timeout;
 	unsigned int		empty_ns;
 	bool			lazy_pages;
-	bool			tcp_skip_in_flight;
+	int			tcp_skip_in_flight;
 	char			*work_dir;
 
 	/*
@@ -115,13 +115,13 @@ struct cr_options {
 	 * the deprecated stuff is not working, but it's still possible
 	 * to turn one ON while the code is in.
 	 */
-	bool			deprecated_ok;
-	bool			display_stats;
-	bool			weak_sysctls;
+	int			deprecated_ok;
+	int			display_stats;
+	int			weak_sysctls;
 	int			status_fd;
 	bool			orphan_pts_master;
-	bool			check_only;
-	bool			remote;
+	int			check_only;
+	int			remote;
 };
 
 extern struct cr_options opts;
-- 
2.7.4



More information about the CRIU mailing list