[CRIU] [PATCH] sk-unix: set mntinfo

[Pavel Emelyanov]

On 09/17/2016 02:19 AM, Christian Brauner wrote:
> On Thu, Sep 15, 2016 at 02:07:47PM +0300, Pavel Emelyanov wrote:
>> On 09/14/2016 12:13 AM, Christian Brauner wrote:
>>> On Tue, Sep 13, 2016 at 10:51:04PM +0200, Christian Brauner wrote:
>>>> On Tue, Sep 13, 2016 at 05:53:59PM +0300, Pavel Emelyanov wrote:
>>>>> On 09/07/2016 05:29 PM, Christian Brauner wrote:
>>>>>> phys_stat_resolve() call mount_resolve_path() which requires that mntinfo_tree
>>>>>> in the ns_id struct is initialized. This is a problem we observed with sockets
>>>>>> on btrfs volumes:
>>>>>
>>>>> Sorry for the late response. See my comment inline.
>>>>>
>>>>>> Signed-off-by: Christian Brauner <christian.brauner at canonical.com>
>>>>>> ---
>>>>>>  criu/sk-unix.c | 1 +
>>>>>>  1 file changed, 1 insertion(+)
>>>>>>
>>>>>> diff --git a/criu/sk-unix.c b/criu/sk-unix.c
>>>>>> index 99f0b08..a199ca4 100644
>>>>>> --- a/criu/sk-unix.c
>>>>>> +++ b/criu/sk-unix.c
>>>>>> @@ -525,6 +525,7 @@ static int unix_process_name(struct unix_sk_desc *d, const struct unix_diag_msg
>>>>>>  			ret = -ENOENT;
>>>>>>  			goto out;
>>>>>>  		}
>>>>>> +		ns->mnt.mntinfo_tree = mntinfo;
>>>>>
>>>>> I guess that's too late for this assignment. Since this is dump time, then
>>>>> ns->mnt.mntinfo_tree should have been initialized in collect_mntns() or
>>>>> nearby.
>>>>
>>>> It is in collect_mntinfo() but it seems calling lookup_ns_by_id() in
>>>> unix_process_name() does return an ns_id but ns.mnt->mntinfo_tree is not
>>>> initialized. Is mntinfo a global that is identical for all ns_ids in the linked
>>>> list? If not how should they be initialized?
>>>
>>> Scratch that, collect_mntinfo() is per namespace. The question is when to call
>>> it. One option is that as soon as you add a new ns_id you call collect_mntinfo()
>>> or you call it *first* when you need it, i.e. you check whether mntinfo_tree ==
>>> NULL and if so call collect_mntinfo() on the corresponding ns_id.
>>
>> Actually collect_mntinfo() is called for every namespace found already. Can you
>> trace back why the ns you see isn't yet collect_mntinfo()-ed at the time the
>> unix_process_name() is?
> 
> This mainly affects cr-check.c and there only collect_mntinfo() is called. I'd
> argue that when unix_process_name() is called and calls lookup_ns_by_id() a
> namespace is retrieved for which we did not yet collect_mntinfo() in cr-check.c.
> So one possibility is to use collect_mnt_namespaces() instead of
> collect_mntinfo().

If its only about cr-check, then of course, do whatever is needed, we don't fight
for performance in this action :)

-- Pavel