[CRIU] [PATCH v5 2/3] Do not error out in RPC mode with wrong config file entries

[Andrei Vagin]

On Thu, Dec 20, 2018 at 04:18:12PM +0000, Adrian Reber wrote:
> From: Adrian Reber <areber at redhat.com>
> 
> Relates: https://github.com/checkpoint-restore/criu/issues/578
> 
> If the config parser finds a unknown option in the configuration file,
> the wrong option is printed out and CRIU exits.
> 
> In RPC mode this is not the best thing to do, as CRIU might not be able
> to print the message to the user.

It is a bad explanation, because with your first patch the user will see
errors. I am not sure why we need to ignore unknown options in config
files. It looks dangerous, because the user can do a typo and will not
notice it.

> 
> This changes CRIU's behaviour in RPC mode to write a wrong configuration
> option to the log file. In CLI mode nothing changes:
> 
> $ echo test >> /etc/criu/default.conf
> $ criu check
> criu: unrecognized option '--test'
> $ runc checkpoint <container>
> $ grep Unknown dump.log
> Warn  (criu/config.c:812): Unknown option encountered: --test
> $ echo test-runc >> /etc/criu/runc.conf
> $ runc restore -d <container>
> $ grep Unknown restore.log
> Warn  (criu/config.c:812): Unknown option encountered: --test
> Warn  (criu/config.c:812): Unknown option encountered: --test-runc
> 
> This way unknown configuration file entries do not break RPC mode, but
> they are reported.
>
> Reported-by: Radostin Stoyanov <rstoyanov1 at gmail.com>
> Signed-off-by: Adrian Reber <areber at redhat.com>
> ---
>  criu/config.c | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 
> diff --git a/criu/config.c b/criu/config.c
> index f4fb39b86..3c54fa72a 100644
> --- a/criu/config.c
> +++ b/criu/config.c
> @@ -31,6 +31,8 @@
>  
>  struct cr_options opts;
>  
> +static bool rpc_mode = false;
> +
>  static int count_elements(char **to_count)
>  {
>  	int count = 0;
> @@ -235,6 +237,20 @@ static int pre_parse(int argc, char **argv, bool *usage_error, bool *no_default_
>  		} else if (strstr(argv[i], "--config=") != NULL) {
>  			*cfg_file = argv[i] + strlen("--config=");
>  			*no_default_config = true;
> +		} else if (!strcmp(argv[i], "swrk")) {
> +			/*
> +			 * In RPC mode we do not want to error out if we
> +			 * encounter unknown options. The options can only
> +			 * be from a configuration file. To not error out
> +			 * because of wrong lines in the configuration file
> +			 * this just prints the wrong option into the log.
> +			 */
> +			rpc_mode = true;
> +			/*
> +			 * This is only needed so that getopt() does not
> +			 * print invalid options to stderr.
> +			 */
> +			opterr = 0;
>  		}
>  	}
>  
> @@ -787,6 +803,28 @@ int parse_options(int argc, char **argv, bool *usage_error,
>  		case 'h':
>  			*usage_error = false;
>  			return 2;
> +		case '?':
> +			/*
> +			 * In RPC mode we do not want to
> +			 * error out if an unknown option is found.
> +			 * This writes it to the log file and continues.
> +			 */
> +			if (rpc_mode) {
> +				pr_warn("Unknown option encountered: %s\n", _argv[optind - 1]);
> +				break;
> +			} else {
> +				/*
> +				 * Only an unknown option that starts with '-' needs to be
> +				 * reported to the user. getopt() knows nothing about our
> +				 * commands (dump, check, swrk, ...). Those should be
> +				 * ignored.
> +				 */
> +				if (_argv[optind - 1][0] == '-') {
> +					*usage_error = true;
> +					return 2;
> +				}
> +			}
> +			break;
>  		default:
>  			return 2;
>  		}
> -- 
> 2.18.0
>