Linux Container Development
 help / color / mirror / Atom feed
From: Oren Laadan <orenl-RdfvBDnrOixBDgjK7y7TUQ@public.gmane.org>
To: "Serge E. Hallyn" <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>,
	Oren Laadan <orenl-RdfvBDnrOixBDgjK7y7TUQ@public.gmane.org>,
	containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
Subject: Re: [PATCH 2/6] c/r: [pty 1/2] allow allocation of desired pty	slave
Date: Tue, 08 Sep 2009 09:19:03 -0400	[thread overview]
Message-ID: <4AA659C7.5020700@librato.com> (raw)
In-Reply-To: <20090908080944.GP12824-Hu8+6S1rdjywhHL9vcZdMVaTQe2KTcn/@public.gmane.org>



Louis Rilling wrote:
> On 04/09/09 10:26 -0500, Serge E. Hallyn wrote:
>> Quoting Oren Laadan (orenl-RdfvBDnrOixBDgjK7y7TUQ@public.gmane.org):
>>> 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.

  parent reply	other threads:[~2009-09-08 13:19 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-09-04 14:20 Checkpoint/restart of ptys, pgids, and controlling tty Oren Laadan
     [not found] ` <1252074054-22241-1-git-send-email-orenl-RdfvBDnrOixBDgjK7y7TUQ@public.gmane.org>
2009-09-04 14:20   ` [PATCH 1/6] c/r: [objhash] add ckpt_obj_reserve() Oren Laadan
2009-09-04 14:20   ` [PATCH 2/6] c/r: [pty 1/2] allow allocation of desired pty slave Oren Laadan
     [not found]     ` <1252074054-22241-3-git-send-email-orenl-RdfvBDnrOixBDgjK7y7TUQ@public.gmane.org>
2009-09-04 15:26       ` Serge E. Hallyn
     [not found]         ` <20090904152644.GA15253-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2009-09-04 16:08           ` Oren Laadan
2009-09-08  8:09           ` Louis Rilling
     [not found]             ` <20090908080944.GP12824-Hu8+6S1rdjywhHL9vcZdMVaTQe2KTcn/@public.gmane.org>
2009-09-08 13:19               ` Oren Laadan [this message]
     [not found]                 ` <4AA659C7.5020700-RdfvBDnrOixBDgjK7y7TUQ@public.gmane.org>
2009-09-08 14:04                   ` Louis Rilling
2009-09-04 14:20   ` [PATCH 3/6] c/r: [pty 2/2] support for pseudo terminals Oren Laadan
     [not found]     ` <1252074054-22241-4-git-send-email-orenl-RdfvBDnrOixBDgjK7y7TUQ@public.gmane.org>
2009-09-04 16:21       ` Serge E. Hallyn
2009-09-08  8:50       ` Louis Rilling
     [not found]         ` <20090908085013.GQ12824-Hu8+6S1rdjywhHL9vcZdMVaTQe2KTcn/@public.gmane.org>
2009-09-08 13:19           ` Oren Laadan
2009-09-04 14:20   ` [PATCH 4/6] c/r: tighten logic to protect against bogus pids in input Oren Laadan
     [not found]     ` <1252074054-22241-5-git-send-email-orenl-RdfvBDnrOixBDgjK7y7TUQ@public.gmane.org>
2009-09-08  1:09       ` Serge E. Hallyn
2009-09-04 14:20   ` [PATCH 1/3] mktree: add support to ghost tasks (TASK_GHOST) Oren Laadan
2009-09-04 14:20   ` [PATCH 2/3] test/pgrp.c: add test case for process-groups Oren Laadan
2009-09-04 14:20   ` [PATCH 3/3] restart: correctly handle pgid/ppid/sid = 0 Oren Laadan
     [not found] ` <1252074054-22241-6-git-send-email-orenl@librato.com>
     [not found]   ` <1252074054-22241-6-git-send-email-orenl-RdfvBDnrOixBDgjK7y7TUQ@public.gmane.org>
2009-09-08  3:04     ` [PATCH 5/6] c/r: correctly restore pgid Serge E. Hallyn
     [not found]       ` <20090908030445.GB16460-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2009-09-08 13:33         ` Oren Laadan
     [not found]           ` <4AA65D10.6000702-RdfvBDnrOixBDgjK7y7TUQ@public.gmane.org>
2009-09-08 14:00             ` Serge E. Hallyn
     [not found]               ` <20090908140056.GA873-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2009-09-08 16:40                 ` Oren Laadan
     [not found] ` <1252074054-22241-7-git-send-email-orenl@librato.com>
     [not found]   ` <1252074054-22241-7-git-send-email-orenl-RdfvBDnrOixBDgjK7y7TUQ@public.gmane.org>
2009-09-08  3:21     ` [PATCH 6/6] c/r: support for controlling terminal and job control Serge E. Hallyn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4AA659C7.5020700@librato.com \
    --to=orenl-rdfvbdnroixbdgjk7y7tuq@public.gmane.org \
    --cc=containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org \
    --cc=serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox