[CRIU] [PATCH v2] filemap: collect mapped file fs type

[Andrei Vagin]

On Tue, Aug 2, 2016 at 8:44 AM, Stanislav Kinsburskiy
<skinsbursky at virtuozzo.com> wrote:
>
>
> 01.08.2016 22:08, Andrew Vagin пишет:
>
>> On Mon, Aug 01, 2016 at 11:23:02AM +0200, Stanislav Kinsburskiy wrote:
>>>
>>>
>>> 29.07.2016 19:40, Andrew Vagin пишет:
>>>>
>>>> On Fri, Jul 29, 2016 at 11:33:43AM +0200, Stanislav Kinsburskiy wrote:
>>>>>
>>>>> 29.07.2016 04:37, Andrew Vagin пишет:
>>>>>>
>>>>>> On Thu, Jul 28, 2016 at 12:06:41PM +0300, Stanislav Kinsburskiy wrote:
>>>>>>>
>>>>>>> This is essential to define whether this file is located on NFS (and
>>>>>>> thus has
>>>>>>> to be "link-remapped") or not (see nfs_silly_renamed).
>>>>>>>
>>>>>>> v2:
>>>>>>> Compilation issue fixed
>>>>>>>
>>>>>>> Signed-off-by: Stanislav Kinsburskiy <skinsbursky at virtuozzo.com>
>>>>>>> ---
>>>>>>>     criu/cr-dump.c |    8 ++++++++
>>>>>>>     1 file changed, 8 insertions(+)
>>>>>>>
>>>>>>> diff --git a/criu/cr-dump.c b/criu/cr-dump.c
>>>>>>> index 06ff2d7..2288908 100644
>>>>>>> --- a/criu/cr-dump.c
>>>>>>> +++ b/criu/cr-dump.c
>>>>>>> @@ -365,12 +365,20 @@ static int dump_filemap(struct vma_area
>>>>>>> *vma_area, int fd)
>>>>>>>         struct fd_parms p = FD_PARMS_INIT;
>>>>>>>         VmaEntry *vma = vma_area->e;
>>>>>>>         int ret = 0;
>>>>>>> +       struct statfs fst;
>>>>>>>         u32 id;
>>>>>>>         BUG_ON(!vma_area->vmst);
>>>>>>>         p.stat = *vma_area->vmst;
>>>>>>>         p.mnt_id = vma_area->mnt_id;
>>>>>>> +       if (fstatfs(fd, &fst)) {
>>>>>>> +               pr_perror("Unable to statfs fd %d", fd);
>>>>>>> +               return -1;
>>>>>>> +       }
>>>>>>> +
>>>>>>> +       p.fs_type = fst.f_type;
>>>>>>
>>>>>> Can we get fs_type in handle_vma()? We get vmst and mnt_id there.
>>>>>
>>>>> What do you mean?
>>>>> Call fstatfs in handle_vma() or use of mnt_id?
>>>>> And why we should do it in handle_vma?
>>>>
>>>> There is an optimization when a previous vma and the current one use the
>>>> same file.
>>>>
>>>>
>>>> if (vma_area->file_borrowed) {
>>>> ...
>>>>                   vma_area->vmst = prev->vmst;
>>>>                   vma_area->mnt_id = prev->mnt_id;
>>>> } else {
>>>> ...
>>>> }
>>>>
>>> Ok, but then:
>>> 1) FS type has to be placed on vma_area struct to be able to use this
>>> optimization pass it to dump_filemap.
>>> 2) File path is dumped for a vma regardless of this optimization.
>>
>> Where is it dumped?
>>
>>> So, I understand, that we can optimize away some statfs calls. But it
>>> looks
>>> somehow wrong to me.
>>> If one vma has "file_borrowed" tag, then it probably means, that the same
>>> file shouldn't be dumped once more?
>>> Is it correct?
>>
>> Yes, it is. Now I think I was wrong when I suggested to move statfs in
>> handle_vma(). There we get properties which is used in
>> fd_id_generate_special(). You can call fstatfs() before
>> dump_one_reg_file(), but only when it is going to be called.
>>
>>          if (fd_id_generate_special(&p, &id)) {
>>                 ....
>>                  ret = dump_one_reg_file(fd, id, &p);
>>         }
>>
>
>
> But then, probably, it's better to call for statfs in dump_one_reg_file and
> remove it from fill_fd_params_special, which is called for each file
> occurrence.

Yes, you are right

>
>
>>>>>>> +
>>>>>>>         /*
>>>>>>>          * AUFS support to compensate for the kernel bug
>>>>>>>          * exposing branch pathnames in map_files.
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> 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