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

[Pavel Emelyanov]

On 01/10/2017 04:43 PM, Dmitry Safonov wrote:
> 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)?
> 
> I mean, my point is: maybe it's better to make 3 lists according to
> groups and in --feature just go over them searching for a feature?

It can be done so, but

a) we'd have to invent names for each
b) a bunch of stuff that is currently in group 1 is "must have", I see
   no reasons for testing all of them separately

> That will guarantee that any feature from groups can be tested
> separately and vice-versa.
> 
>>
>>>
>>> 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().
>>
>>>
>>>>>
>>>>>>
>>>>>>> +{
>>>>>>> +    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 },
>>>>>>>  };
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>>
> 
>