[CRIU] [PATCH v2] Extend the parser to accept negative options
Andrei Vagin
avagin at virtuozzo.com
Wed Apr 5 14:59:17 PDT 2017
Applied, thanks!
On Wed, Apr 05, 2017 at 05:01:29PM +0200, Veronika Kabatova wrote:
> 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
>
> _______________________________________________
> CRIU mailing list
> CRIU at openvz.org
> https://lists.openvz.org/mailman/listinfo/criu
More information about the CRIU
mailing list