[CRIU] [PATCH 2/2] cr-check: add compat_restore check

[Pavel Emelyanov]

On 01/10/2017 04:26 PM, Dmitry Safonov wrote:
> On 01/10/2017 04:23 PM, Pavel Emelyanov wrote:
>> On 01/10/2017 04:08 PM, Dmitry Safonov wrote:
>>> On 01/10/2017 04:06 PM, Dmitry Safonov wrote:
>>>> On 01/10/2017 03:34 PM, Pavel Emelyanov wrote:
>>>>> On 01/09/2017 07:59 PM, Dmitry Safonov wrote:
>>>>>> Initialy, I thought to name it "compat_restore", but after I've dropped
>>>>>> the second 32-bit parasite (which surely made compat code lesser and
>>>>>> easier), our parasite works in 64-bit in 32-bit task and ptrace()
>>>>>> for setting registers in this long-jumped situation will work correctly
>>>>>> only after v4.9 kernel. Maybe it can be work-arounded if needed,
>>>>>> but yet no compatible dump for pre-v4.9 kernels.
>>>>>>
>>>>>> Requested-by: Andrei Vagin <avagin at virtuozzo.com>
>>>>>> Signed-off-by: Dmitry Safonov <dsafonov at virtuozzo.com>
>>>>>> ---
>>>>>>  criu/cr-check.c | 10 ++++++++++
>>>>>>  1 file changed, 10 insertions(+)
>>>>>>
>>>>>> diff --git a/criu/cr-check.c b/criu/cr-check.c
>>>>>> index 60279e135dfa..94c5074557ce 100644
>>>>>> --- a/criu/cr-check.c
>>>>>> +++ b/criu/cr-check.c
>>>>>> @@ -48,6 +48,7 @@
>>>>>>  #include "libnetlink.h"
>>>>>>  #include "net.h"
>>>>>>  #include "linux/userfaultfd.h"
>>>>>> +#include "restorer.h"
>>>>>>
>>>>>>  static char *feature_name(int (*func)());
>>>>>>
>>>>>> @@ -1134,6 +1135,14 @@ static int check_loginuid(void)
>>>>>>      return 0;
>>>>>>  }
>>>>>>
>>>>>> +static int check_compat_cr(void)
>>>>>
>>>>> Call for this should also be added to cr_check()
>>>>
>>>> Hmm, and what about tun/userns/loginuid features?
>>>> Should them be also in cr_check()?
>>>> I don't completely get the policy why some are(not) there.
>>>
>>> Tun is actually checked with other parameter.
>>> Should userns and loginuid also be in cr_check()?
>>
>> Well, there are three things with cr-check.
>>
>> 1. Any kernel functionality that may be or may be not in the
>>    kernel should be checked with 'criu check' action.
>>
>> 2. Depending on the how bad the functionality is required it
>>    can be in one of 3 groups -- must have, optional and
>>    experimental.
>>
>> 3. Individual features can be added to --feature <name> checking
>>    if this is needed by zdtm
> 
> Hmm, so what kind of features should be only in (3), but not in (1)?

None ;)

>>
>> So as far as userns and loginuid are concerned -- if there's anything
>> in the kernel that is needed to dump OR restore one and that can be
>> in or out (e.g. appeared after 3.11) -- it should be in cr-check.
> 
> Ok, I'll resend v2 for this and send separately a patch for adding 
> userns and loginuid to cr_check().

OK, but note, that if API to _use_ a feature and _C/R_ it is the same
there's not need in adding it to criu check.

>>
>>>>
>>>>>
>>>>>> +{
>>>>>> +    if (kdat_compat_sigreturn_test())
>>>>>> +        return 0;
>>>>>> +    pr_err("compat_cr is not supported. Requires kernel >= v4.9\n");
>>>>>> +    return -1;
>>>>>> +}
>>>>>> +
>>>>>>  struct feature_list {
>>>>>>      char *name;
>>>>>>      int (*func)();
>>>>>> @@ -1154,6 +1163,7 @@ static struct feature_list feature_list[] = {
>>>>>>      { "autofs", check_autofs },
>>>>>>      { "tcp_half_closed", check_tcp_halt_closed },
>>>>>>      { "lazy_pages", check_uffd },
>>>>>> +    { "compat_cr", check_compat_cr },
>>>>>>      { NULL, NULL },
>>>>>>  };
>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>
>>>
>>
> 
>