[Devel] Re: [PATCH 2/6] c/r: [pty 1/2] allow allocation of desired pty slave

[Oren Laadan]

Louis Rilling wrote:
> On 04/09/09 10:26 -0500, Serge E. Hallyn wrote:
>> Quoting Oren Laadan (orenl at librato.com):
>>> During restart, we need to allocate pty slaves with the same
>>> identifiers as recorded during checkpoint. Modify the allocation code
>>> to allow an in-kernel caller to request a specific slave identifier.
>>>
>>> For this, add a new field to task_struct - 'required_id'. It will
>>> hold the desired identifier when restoring a (master) pty.
>>>
>>> The code in ptmx_open() will use this value only for tasks that try to
>>> open /dev/ptmx that are restarting (PF_RESTARTING), and if the value
>>> isn't CKPT_REQUIRED_NONE (-1).
>> So noone has indicated any preference for this versus the ptmx_create()
>> approach...
>>
>> I'm satisfied knowing we have a working fallback in case task->required_id
>> is deemed unacceptable.
>>
>> However I'd like to not have linux-kernel folks think us morons for not
>> having considered that.  Can you add a message to the changelog saying
>> why we're going with this approach (namely, that it lets us re-use
>> filp_open() instead of having to do a custom alloc_file in a new code-path,
>> which introduces maintenance duplication for file permission checking
>> paths)?
> 
> As far as I am concerned, I do have a preference for the ptmx_create()
> approach. This task->required_id field reminds me the former approach taken for
> restarting pids and (and SYSV IPC ids IIRC) from userspace, that was proposed
> last year and actually deemed unacceptable [ IIRC, this was an argument in favor
> of a restart() syscall ]. I know that it's not the same since ->required_id is
> not set from userspace and used in a later syscall, but still ...
> 
> Moreover I see no reason whey there would be no way to refactorize ptmx code and
> have less duplicated code with the ptmx_create() approach.

I basically agree - I simply took the easiest/fastest path; if the ptmx
code is properly refactored, we should use that instead.

Did you have a chance to look at Serge's attempt to do exactly that ?
https://lists.linux-foundation.org/pipermail/containers/2009-September/020363.html

Thanks,

Oren.

_______________________________________________
Containers mailing list
Containers at lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers