[CRIU] [PATCH 1/2] mnt: do_new_mount should restore sharing

[Andrey Wagin]

2015-04-20 19:15 GMT+03:00 Tycho Andersen <tycho.andersen at canonical.com>:
> On Mon, Apr 20, 2015 at 06:52:59PM +0300, Andrew Vagin wrote:
>> On Fri, Apr 17, 2015 at 11:07:49AM -0600, Tycho Andersen wrote:
>> > On Fri, Apr 17, 2015 at 06:58:34PM +0300, Andrew Vagin wrote:
>> > > On Wed, Apr 15, 2015 at 01:17:40PM -0600, Tycho Andersen wrote:
>> > > > The function masks off sharing in the initial setting, but doesn't correctly
>> > > > reset the flags afterwards.
>> > > >
>> > > > CC: Oleg Nesterov <oleg at redhat.com>
>> > > > Signed-off-by: Tycho Andersen <tycho.andersen at canonical.com>
>> > > > ---
>> > > >  mount.c | 4 +++-
>> > > >  1 file changed, 3 insertions(+), 1 deletion(-)
>> > > >
>> > > > diff --git a/mount.c b/mount.c
>> > > > index 1d8f486..17f97a2 100644
>> > > > --- a/mount.c
>> > > > +++ b/mount.c
>> > > > @@ -1789,7 +1789,9 @@ static int do_new_mount(struct mount_info *mi)
>> > > >                 return -1;
>> > > >         }
>> > > >
>> > > > -       if (restore_shared_options(mi, 0, mi->shared_id, 0))
>> > > > +       if (restore_shared_options(mi, !mi->shared_id && !mi->master_id,
>> > >
>> > > Could you explain this? If we talk about external mount, we need to give
>> > > true here if host_mi->shared_id isn't equal to mi->shared_id.
>> >
>> > All external mounts are restored via do_bind_mount(), so the sharing
>> > here is all internal.
>>
>> I mean auto external mounts.
>>
>> Lets imagine that we dump a mount point and find a pair on the host.
>> On restore we bind-mount the host point into a restored point and a new
>> mount point will inherit the host group. We know that these groups were
>> different on dump, so we need to reset a group in the new mount point.
>> For that we need to remount it as private and then as shared.
>
> Yes, that's what force_private_remount in do_bind_mount() is for.
> Autodetected external mounts aren't restored via do_new_mount().

Sorry for my stupidity. I had to read code before commenting.

Acked-by: Andrew Vagin <avagin at odin.com>

>
>> >
>> > However, I think if this mi->shared_id differs from
>> > mi->parent->shared_id we do need to remount private first, since the
>> > internal sharing is different. I can resend with this change if it
>> > makes sense.
>>
>> There is no correlation between mi->shared_id and mi->parent->shared_id.
>
> A mount inherits its parent's sharing when initially mounted, so if
> the IDs differ, don't we need to re-mount private to get a new id?
>
> Tycho
>
>> >
>> > Tycho
>> >
>> > > > +                                       mi->shared_id,
>> > > > +                                       mi->master_id))
>> > > >                 return -1;
>> > > >
>> > > >         mi->mounted = true;
>> > > > --
>> > > > 2.1.4
>> > > >
>> > > > _______________________________________________
>> > > > CRIU mailing list
>> > > > CRIU at openvz.org
>> > > > https://lists.openvz.org/mailman/listinfo/criu
> _______________________________________________
> CRIU mailing list
> CRIU at openvz.org
> https://lists.openvz.org/mailman/listinfo/criu