[CRIU] [PATCH v3 08/16] rst_info: artificial files list introduced

[Pavel Emelyanov]

On 12/14/2015 03:18 PM, Stanislav Kinsburskiy wrote:
> 
> 
> 14.12.2015 13:00, Pavel Emelyanov пишет:
>> On 12/14/2015 02:55 PM, Stanislav Kinsburskiy wrote:
>>>
>>> 14.12.2015 12:47, Pavel Emelyanov пишет:
>>>> On 12/14/2015 02:16 PM, Stanislav Kinsburskiy wrote:
>>>>> 14.12.2015 12:10, Pavel Emelyanov пишет:
>>>>>> On 12/10/2015 06:16 PM, Stanislav Kinsburskiy wrote:
>>>>>>> This list is used to collect and handle some service files, which an be used
>>>>>>> only during restore procedure for some service perations, and must be closed
>>>>>>> before process is released.
>>>>>> OK, but I see only opening code in here, where's the closing one?
>>>>> I do close in post_open() callback.
>>>>> If there is a better place, I would be glad to do it there.
>>>> In the post_open_fd() right after calling the ->post_open?
>>>>
>>> Emm. How to distinguish, which fd have to closed and which has not?
>>> By introducing another optional callback?
>> You already know which list you're scanning -- artificial or not -- so it's
>> possible to make this decision based on this knowledge.
>>
>> Otherwise I see no benefits of having the 4th list on rst_info over the
>> specially crafted desc_opts (as you do now anyway) that would just close the
>> descriptor itself in its post_open callback.
> 
> You, probably, thinking about something, which you are not exposing to me.
> Function post_open_fd() doesn't have information about the list.

It doesn't have list as an argument, yes, but simply because the list is
dropped from arg list one call before it.

> But it has information about desc. I can check it for typeof ARTIFICIAL, 
> but you tend to remove this type, don't you?

No I don't I ask you (if we keep the artificial files concept at all) just
to move this constant out of protobuf files.

>>>>>>> AutoFS will use them to fix up mount points druing restore.
>>>>>> Why can't we use existing fds list?
>>>>> We can, if we put them at the end on the list.
>>>> Why at the end of the list?
>>> Because artificial files is just a way to operate on existent ones. Thus
>>> existent ones have to be processes (opened) befoe artificial ones.
>> No, it's not so. In the current set artificial files are the same ones (pipes)
>> that are anyway owned be the processes we restore. So you can pretend that
>> the process that _wants_ autofs pipe for mountpoint final fixup just shares
>> the pipe with the process that actually _owns_ one in its fdt.
> 
> I'm sorry, If I understand you wrong, but _artificial_ files are "owned" 
> by a process only in terms that they are traversed within process 
> address space.
> There are no real opened files descriptors for them. They are pure virtual.

You close() one at the end of the post_open callback.

> The main intention to introduce them was to have some object, which can 
> be used to call a callback in the proper time with proper _real_ files 
> already opened.
> By having this object, all AutoFS hacks can be isolated within Autofs 
> code, instead of introducing some callbacks or checks in generic pipes code.
> And pipe write end itself is _not_ aritificial. It's real fd in fds 
> list. But it have to be closed after fixup.
> Fixup and close of write end are done in post_open() callback of 
> _artificial_ fd.

And artificial fd is created at the task which has the real pipe in his fdt
to set this real fd into the superblock. I had an impression that the artificial
fd is just another descriptor for the real write pipe. OK, I was wrong reading 
the patchset.

Why can't we make the write-end setting to look like I described above? In
details:

1. Find the file object that corresponds to the write-end of the pipe that should
   sit in the kernel.

1a. If the write end is not there -- add one and use it.

Note, that I don't propose to create artificial files, I propose to use existing
pipe stuff, i.e. -- the pipe_info object.

2. Find the file descriptor that corresponds to the pipe object from step 1.

2a. If the file descriptor is absent _or_ sits in the wrong task -- add fake
    file descriptor

3. In pipe_post_open (add such) fixup the mountpoint and close() the descriptor
   if required.

> Did I miss something from you question?

-- Pavel