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

[Dmitry Safonov]

2017-03-30 3:17 GMT+03:00 Kir Kolyshkin <kir at openvz.org>:
> On 03/29/2017 05:10 PM, Dmitry Safonov wrote:
>>
>> 2017-03-30 2:51 GMT+03:00 Kir Kolyshkin <kir at openvz.org>:
>>>
>>> On 03/28/2017 12:24 PM, 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.
>>>> 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.
>>>>
>>>> Signed-off-by: Veronika Kabatova <vkabatov at redhat.com>
>>>> ---
[...]
>>>> @@ -597,35 +584,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;
>>>
>>>
>>> I don't see where are you recreating this removed functionality.
>>
>> Oh, that's actually, quite nice part of the patch: instead of setting
>> bools inside of opts by the hands, it passes a pointer as option::flag
>> to getopt_long(). Though, that could has been described in the
>> changelog..
>
>
> I like this part a lot, too, but I'm talking about something quite different
> here:
> setting opts.final_state to TASK_ALIVE in case opts.check_only is set.

Oh, I haven't caught it. Good catch.

*grumbling*
Maybe that's why there is one-thing-per-patch rule.

-- 
             Dmitry