[CRIU] [PATCH v4 8/9] mount: do remaps for child-overmount of another overmount

[Andrey Vagin]

On Thu, Sep 27, 2018 at 10:02:05AM +0300, Pavel Tikhomirov wrote:
> 
> 
> On 09/27/2018 12:22 AM, Andrey Vagin wrote:
> > On Mon, Dec 11, 2017 at 07:07:25PM +0300, Pavel Tikhomirov wrote:
> > > In case we have mounts:
> > > 
> > > 1 /mnt/
> > > 2 /mnt/a with parent 1
> > > 3 /mnt/a/b with parent 1
> > > 4 /mnt/a with parent 2
> > > 
> > > We determine 2 as needing remap with does_mnt_overmount() and remap it.
> > 
> > Why do we determine that 2 has to be remmapped? It overmounts nothing.
> 
> 2 is a sibling-overmount for 3, that's why
> 
> > 
> > The test passes without this patch. Could you explain why do we need
> > this patch? I think we need to improve the test to check the cases what
> > is fixed by this patch.
> 
> For me if I revert the patch, test fails with lost mount. (If we don't remap
> 4, the mount 2 will be lost.

Probably, I forgot to compile criu after reverting this commit. I'm
sorry about this. I have played with the test and now I think I
understand how it works. Thank you!

> 
> Run criu restore
> 1: Old mounts lost: [(u'/', u'/zdtm/static/tempfs_overmounted.test/a', None,
> None)]
> 1: New mounts appeared: []
> ########## Test zdtm/static/tempfs_overmounted FAIL at mounts compare
> ##########
> 
> > 
> > 
> > > Next we mount 4 on top of 2. Next in fixup_remap_mounts() we want to
> > > move 2 back to it's parent 1, but instead move 4 there. So in these case
> > > children-overmounts need to be remapped too.
> > > 
> > > Signed-off-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>
> > > ---
> > >   criu/mount.c | 12 ++++++++++--
> > >   1 file changed, 10 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/criu/mount.c b/criu/mount.c
> > > index bacb24222..cdf1c0432 100644
> > > --- a/criu/mount.c
> > > +++ b/criu/mount.c
> > > @@ -630,7 +630,7 @@ static struct mount_info *find_fsroot_mount_for(struct mount_info *bm)
> > >   	return NULL;
> > >   }
> > > -static bool does_mnt_overmount(struct mount_info *m)
> > > +static bool mnt_needs_remap(struct mount_info *m)
> > >   {
> > >   	struct mount_info *t;
> > > @@ -644,6 +644,14 @@ static bool does_mnt_overmount(struct mount_info *m)
> > >   			return true;
> > >   	}
> > > +	/*
> > > +	 * If we are children-overmount and parent is remapped, we should be
> > > +	 * remapped too, else fixup_remap_mounts() won't be able to move parent
> > > +	 * to it's real place, it will move child instead.
> > > +	 */
> > > +	if (!strcmp(m->parent->mountpoint, m->mountpoint))
> > > +		return mnt_needs_remap(m->parent);
> > > +
> > >   	return false;
> > >   }
> > > @@ -2488,7 +2496,7 @@ static int try_remap_mount(struct mount_info *m)
> > >   {
> > >   	struct mnt_remap_entry *r;
> > > -	if (!does_mnt_overmount(m))
> > > +	if (!mnt_needs_remap(m))
> > >   		return 0;
> > >   	BUG_ON(!m->parent);
> > > -- 
> > > 2.14.3
> > > 
> 
> -- 
> Best regards, Tikhomirov Pavel
> Software Developer, Virtuozzo.