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

[Pavel Emelyanov]

On 12/14/2015 05:25 PM, Stanislav Kinsburskiу wrote:

>>> Yes, it can be done this way. The benefit I see here is avoiding of 
>>> introducing an object, which we can live without. Like artificial files. 
>>> Occam razor is nice, 
>>> but: 
>>> 1) It means, that AutoFS code and pipe code are interconnected. I 
>>> believe it's something, which should be avoided. Especially because 
>>> usually, in real life, pipes won't be related to AutoFS. 
>>
>> IRL pipes are related to AutoFS and you use this fact anyway. Moreover, in 
>> your patch you mess with generic fds layer from inside the autofs code -- the 
>> find_fle_by_fd() and the way you call it is all about it. 
>>
> 
> We can' avoid of calling pipes callbacks from autofs anyway.
> Autofs is related to pipes, but not vise versa. Isn't it better to keep all
> the hacks in Autofs code, which can't survive without pipes, then in generic pipes code?

It is.

>>> 2) I believe, that for debugging purposes, it's better to have 
>>> independent pieces of code be isolated as much, as possible. In case of 
>>> AutoFS - especially. 
>>> Current implementation can be modified, while "spaghetti" code approach 
>>> (like kernel CPT in OpenVZ) makes things harder to understand, develop 
>>> and debug. 
>>> IOW, in this particular case I believe, that introducing one more (quite 
>>> generic, btw) object like "artificial file" worth it. 
>>
>> We've just seen how this made things hard to understand :( I agree that it's 
>> handy to have the ability to call something after the fdt is populated and 
>> have this ability w/o messing hard with the pipes.c code. 
>>
> 
> I don't think that they are hard to understand. Simple virtual objects to do
> some tricks without introducing more connections between weakly connected pieces 
> of code. If you want to rename artificial fds into something else - this makes 
> some sense.
> 
>> Artificial files solve this by obfuscating the files opening engine.
> 
> By obfuscating you mean introducing one more list to iterate while opening?

By obfuscating I mean taking a list of files and a complex routine that does
several virtual calls and synchronizes files and tasks between each other
and adding a non-file object to this list to make the routine in question
call something at the very end.

>> If having a way to call something after fdt is populated, then make it be 
>> literally "something that is called after fdt is populated", i.e. a list 
>> of dedicated objects that are queued up to rst_info and called _once_ at 
>> the end of prepare_fds(). 
>>
> 
> This can be done. But in case of AutoFs they will be file-related object and
> operations anyway.
> If you think, that it's better to introduce another object, I can do it.
> 
>> On the other hand what I'm proposing above seems to be a single call to the 
>> collect_one_pipe() from autofs code. 
>>
>>> 3) You proposal means, that I have to drop almost all the code I wrote 
>>> it, and implement it, debug it, etc from scratch. 
>>
>> Yes, sometimes good code to perfect code change is via complete rewrite. 
>>
> 
> Nothing to object to such a statement. But I don't see any significant evidence 
> for now, why it should be done and how you proposal leads to perfectness.
> If we are talking about obfuscating, then pipes code is already obfuscated enough 
> by introducing one more place, where fds are send via unix socket. It took some
> time for me to find this place and recall, why it was done (but I still don't
> really get, why it was implemented _this_ way, instead of keeping "generic fds" 
> code, which you value a lot, simple and strict).

The files.c and related stuff is tricky indeed. Fixes to make pipes.c transporting
be more clear are always welcome.

> The thing you propose adds more obfuscating connections with other code pieces
> to pipe restore logic.
> In spite of it allows to reduce amount of code lines, I believe it makes pipes.c 
> even more harder to understand. And Autofs restore won't be clear from a brief 
> look as well.

My proposal is to either request an existing pipe object into autofs code via
existing API or to introduce explicit post-fdt-restore hooks. If either of them
leads to ugly hacks to either pipe.c or files.c this would only mean we'd have 
to thing again.

> But, frankly, I don't want to argue, because it's pointless.
> I can do it the way you want just to fiinish this task.

-- Pavel