[CRIU] [PATCH] Dismantle cgyard in non-detached restore mode.

[Tycho Andersen]

Hi Saied,

On Wed, Mar 18, 2015 at 11:14:31AM -0700, Saied Kazemi wrote:
> Tycho,
> 
> Thanks for looking into this.  I knew that we can't fini_cgroup() here but
> because I am not familiar with the internals of cgroup restoration I just
> put it there as a quick hack to get criu successfully dump and restore my
> test container multiple times (didn't care about properties, etc.).
> 
> For a permanent fix, I have to defer to your judgement on the best way to
> dismantle the cgyard in the non-detached mode.  I agree with you that it's
> a good idea to always fini_cgroup() as soon as we've fully restored
> cgroups, regardless of detached/non-detached mode.

Ok, I think the right thing to do is something like this:

https://github.com/tych0/criu/commit/63e138e7b4b1b183d864fb08923176c9cd9ba545

I vaguely recall some discussion about why this wasn't possible
before, but I don't remember exactly what it was. I'll try to look
through my old mail and see what that is about; I'll also test the
above patch to make sure it really does what I think it does, and then
send to the list.

Tycho

> --Saied
> 
> 
> On Wed, Mar 18, 2015 at 10:59 AM, Tycho Andersen <
> tycho.andersen at canonical.com> wrote:
> 
> > Hi Saied,
> >
> > On Mon, Mar 16, 2015 at 12:03:37PM -0700, Saied Kazemi wrote:
> > > If the --restore-detached command line option is not specified during
> > > restore, CRIU should unmount and remove the temporary cgyard directory
> > > tree before waiting for the restored process to exit.  Otherwise, all
> > > the temporary cgyard mount points will remain mounted and visible.
> > >
> > > Signed-off-by: Saied Kazemi <saied at google.com>
> > > ---
> > >  cr-restore.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/cr-restore.c b/cr-restore.c
> > > index 4a1fe4e..24d9f8b 100644
> > > --- a/cr-restore.c
> > > +++ b/cr-restore.c
> > > @@ -1845,8 +1845,10 @@ static int restore_root_task(struct pstree_item
> > *init)
> > >
> > >       write_stats(RESTORE_STATS);
> > >
> > > -     if (!opts.restore_detach && !opts.exec_cmd)
> > > +     if (!opts.restore_detach && !opts.exec_cmd) {
> > > +             fini_cgroup();
> > >               wait(NULL);
> > > +     }
> >
> > Good catch.
> >
> > I don't think we can do this here, because immediately after
> > restore_root_task() is called, prepare_cgroup_properties() is called,
> > which needs the cg yard to do its work. This brings up an interesting
> > point though: currently if we don't --restore-detached, the cg
> > properties aren't (attempted to be) restored until after the task
> > exits, which is not good. Should the prepare_cgroup_properties() call
> > be moved into restore_root_task()?
> >
> > Also, is there any reason not to fini_cgroup() all the time once we've
> > prepare_cgroup_properties(), not just in non-detached mode?
> >
> > Tycho
> >
> > >       return 0;
> > >
> > > --
> > > 2.2.0.rc0.207.ga3a616c
> > >
> >