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

[Stanislav Kinsburskiy]

02.08.2016 19:08, 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

I did in a way, combined of two of us.
The reason is that dump_one_reg_file accepts const pointer to fd_params.
Please, take a look on "files-reg: discover file system type only if 
file is going to be dumped"

>>
>>>>>>>> +
>>>>>>>>          /*
>>>>>>>>           * 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