[CRIU] [PATCH 3/4] remap: add a dead pid /proc remap

[Tycho Andersen]

Hi Pavel,

On Thu, Sep 18, 2014 at 08:07:52PM +0400, Pavel Emelyanov wrote:
> On 09/18/2014 05:41 PM, Tycho Andersen wrote:
> > On Thu, Sep 18, 2014 at 03:03:15PM +0400, Pavel Emelyanov wrote:
> >> On 09/18/2014 12:32 AM, Tycho Andersen wrote:
> >>> If a file like /proc/20/mountinfo is open, but 20 is a zombie (or doesn't exist
> >>> any more), we can't read this file at all, so a link remap won't work. Instead,
> >>> we add a new remap, called the dead process remap, which forks a TASK_HELPER as
> >>> that dead pid so that the restore task can open the new /proc/20/mountinfo
> >>> instead.
> >>>
> >>> This commit also adds a new stage CR_STATE_RESTORE_SHARED. Since new
> >>> TASK_HELPERS are added when loading the shared resource images, we need to wait
> >>> to start forking tasks until after these resources are loaded.
> >>
> >> I don't quite understand this explanation. CRIU forks root task, which calls
> >> root_prepare_shared() in which all the remaps are collected. At that point any
> >> proc remap is lined into the pstree. After this root task starts forking
> >> whatever entries it finds in there. Why do we need one more stage?
> > 
> > Because of restore_root_task. Think of the case when after the
> > CR_STATE_RESTORE_NS, the restore_root_task process runs until it
> > starts waiting for CR_STATE_FORKING. It enters that wait looking for
> > task_entries->nr_tasks + task_entries->nr_helpers. But then the init
> > task runs, parses the shared state and adds some more helpers. The
> > first thread was waiting on the wrong number of helpers.
> > 
> > Another option would be to get rid of the CR_STATE_RESTORE_NS point
> > and just synchronize after restoring the shared state. Then the
> > ACT_SETUP_NS scripts wouldn't necessarily run before the tasks are
> > forked, though. I didn't know if that was a problem or not, so I left
> > it the way it was.
> 
> I think we can just move the call to root_prepare_shared() to be above
> the _NS synchronization point. IIRC there's nothing that requires namespaces
> to be set up.
> 
> Can you try that?

With that [1], I get a failure running all the tests:

Execute zdtm/live/static/cwd01
./cwd01 --pidfile=cwd01.pid --outfile=cwd01.out --dirname=cwd01.test
/tmp/criu/test
Dump 9484
Restore
Check results 9515
Waiting...
cat: zdtm/live/static/cwd01.out: No such file or directory
cat: zdtm/live/static/cwd01.out: No such file or directory
Test: zdtm/live/static/cwd01, Result: FAIL
==================================== ERROR ====================================
Test: zdtm/live/static/cwd01, Namespace: 1
Dump log   : /tmp/criu/test/dump/static/cwd01/9484/1/dump.log
--------------------------------- grep Error ---------------------------------
------------------------------------- END -------------------------------------
Restore log: /tmp/criu/test/dump/static/cwd01/9484/1/restore.log
--------------------------------- grep Error ---------------------------------
(00.123315)      1: Error (files-reg.c:97): Can't make ghost dir: No such file or directory
(00.123442) Error (cr-restore.c:1785): Restoring FAILED.
------------------------------------- END -------------------------------------
================================= ERROR OVER =================================

I guess this must depend on the results of prepare_namespace, which has to
come after the RESTORE_NS synchronize call?

[1]: https://github.com/tych0/criu/commit/5bbc76d6e6c67b5dc7e7496a19e5034a7fea82e6

Tycho