[CRIU] Sync TODO-s for mount.c work

[Pavel Emelyanov]

On 04/23/2015 07:22 PM, Tycho Andersen wrote:
> On Thu, Apr 23, 2015 at 07:15:47PM +0300, Pavel Emelyanov wrote:
>> On 04/23/2015 07:05 PM, Tycho Andersen wrote:
>>> On Thu, Apr 23, 2015 at 06:51:26PM +0300, Pavel Emelyanov wrote:
>>>> On 04/23/2015 06:09 PM, Tycho Andersen wrote:
>>>>> On Thu, Apr 23, 2015 at 06:04:35PM +0300, Pavel Emelyanov wrote:
>>>>>> On 04/23/2015 05:31 PM, Tycho Andersen wrote:
>>>>>>> On Thu, Apr 23, 2015 at 02:52:35PM +0300, Pavel Emelyanov wrote:
>>>>>>>> On 04/23/2015 02:17 PM, Oleg Nesterov wrote:
>>>>>>>>
>>>>>>>>> Now what? Obviously we can't just add
>>>>>>>>>
>>>>>>>>> 	if (is_not_external_mount(m))
>>>>>>>>> 		continue;
>>>>>>>>
>>>>>>>> Why not? The is_not_external_mount() is
>>>>>>>>
>>>>>>>> fsroot_mounted(m) || /* mount's root is visible from chroot */
>>>>>>>
>>>>>>> But what if the mount's root is /? e.g with pstore in lxc, the
>>>>>>> mountinfo from inside looks like:
>>>>>>>
>>>>>>> 110 104 0:23 / /sys/fs/pstore rw,nosuid,nodev,noexec,relatime master:11 - pstore pstore rw
>>>>>>>
>>>>>>> which is a bind mount from the host's:
>>>>>>>
>>>>>>> 29 17 0:23 / /sys/fs/pstore rw,nosuid,nodev,noexec,relatime shared:11 - pstore pstore rw
>>>>>>
>>>>>> Ah, so sharing takes place. OK, then (sorry, maybe I'm replaying your
>>>>>> conversation again, but I'm still blind about it) the check should be
>>>>>>
>>>>>>    (fsroot_mounted(m) || root_is_visible(m)) && master_is_external(m)
>>>>>
>>>>> I think not, because if we didn't have this as a shared mount it would
>>>>> still be an external bind mount, just without sharing. An external
>>>>> master peer is an indication that it's a bind mount, but if we don't
>>>>> have one of those, it could still be a bind mount.
>>>>
>>>> Ah, sure. It should be NOT before master_is_external, like this
>>>>
>>>>    (fsroot_mounted(m) || root_is_visible(m)) && !master_is_external()
>>>>
>>>> Provided master_is_external() is false for private mounts, this should
>>>> handle Oleg's case with mount --make-rprivate, but in suboptimal way --
>>>> all mountpoints will be bind-mounted from host with MS_SLAVE flag.
>>>
>>> But this only works for mounts that have external masters. Doesn't it
>>> fail to exclude mounts which aren't in a peer group at all (since
>>> !master_is_external() will always be true for them)?
>>
>> Yes, !master_is_external() will be true and then the 2nd part of the
>> check will take place -- fsroot || root_is_visible. If _this_ will be
>> false then mountpoint will be still considered to be external.
> 
> Oh, sorry, I was off by one in my negation :). Doesn't this exclude my
> pstore example if the mounts are all private?

The master_is_external() will be false, fsroot_mounted() will be true, so
this mountpoint will be classified as "not external" and criu will try to
just mount a new FS instance.

Since pstore is single-sb filesystem we'll get what we need. If it some
day will be make multi-sb (virtualized) we will notice this by devices
being different.

Hmm... I think I'm getting the point. Let's take your example with pstore
but relpace pstore with some multi-sb fs. E.g. the proc one. So if we have
proc privately inherited from host and proc privately mounted inside then
my checks will classify both as "private". While the former one should be
bind-mounted and the latter one should be just mounted. So we also need to
compare the super blocks, i.e. the devices. %)

So...

int is_not_external()
{
       if (master_is_external())
             return false;

       if (root_is_accessible())
             return true;

       return false;
}

where root_is_accessible() should also care for the superblocks, i.e.

int root_is_accessible()
{
       if (device_is_visible())
             return true;

       if (root_path_is_visible())
             return true;

       return false;
}

-- Pavel