[CRIU] [PATCH] criu: dump and restore a thread name

[Dmitry Safonov]

2018-02-01 19:29 GMT+00:00 Andrei Vagin <avagin at openvz.org>:
> From: Andrei Vagin <avagin at virtuozzo.com>
>
> Reported-by: 志平 林 <larry.lin at outlook.com>
> Signed-off-by: Andrei Vagin <avagin at virtuozzo.com>

Looks good.
We could get it from procfs on pre-dump...
But I guess, it's a trade-off between 3 syscalls from criu
and 1 syscall in parasite, so I don't feel the difference.

How about your rule for 1 new feature = 1 new test? ;-)
Also some small nits below.

>         ret = sys_prctl(PR_GET_PDEATHSIG, (unsigned long)&ti->pdeath_sig, 0, 0, 0);
> -       if (ret)
> +       if (ret) {
> +               pr_err("Unable to get the parent death signal: %d\n", ret);
> +               goto out;
> +       }
> +
> +       ret = sys_prctl(PR_GET_NAME, (unsigned long) &ti->comm, 0, 0, 0);
> +       if (ret) {
> +               pr_err("Unable to get the thread name: %d\n", ret);
>                 goto out;
> +       }

We could omit getting thread's name if it's a thread leader.
(just less on a syscall per each task on the dump).
We can live without thread's name if it's the same as leader
(potentialy saving a bit of space in image, removing allocation on
dump, not bothering on restore about the name).

> @@ -549,6 +549,12 @@ long __export_restore_thread(struct thread_restore_args *args)
>         if (ret)
>                 goto core_restore_end;
>
> +       ret = sys_prctl(PR_SET_NAME, (unsigned long) &args->comm, 0, 0, 0);
> +       if (ret) {
> +               pr_err("Unable to set a thread name: %d\n", ret);
> +               goto core_restore_end;
> +       }
> +
>         pr_info("%ld: Restored\n", sys_gettid());
>
>         restore_finish_stage(task_entries_local, CR_STATE_RESTORE);

We could omit restoring:
1. Thread leader (we already restored that)
2. If threads have the same name as the leader.
Potentially saving a syscall per-restored thread.

-- 
             Dmitry