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

From: Louis Rilling <Louis.Rilling-aw0BnHfMbSpBDgjK7y7TUQ@public.gmane.org>
To: "Serge E. Hallyn" <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
Subject: Re: [PATCH 2/6] c/r: [pty 1/2] allow allocation of desired pty slave
Date: Tue, 8 Sep 2009 10:09:44 +0200	[thread overview]
Message-ID: <20090908080944.GP12824@hawkmoon.kerlabs.com> (raw)
In-Reply-To: <20090904152644.GA15253-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

[-- Attachment #1.1: Type: text/plain, Size: 2066 bytes --]

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.

Thanks,

Louis

-- 
Dr Louis Rilling			Kerlabs
Skype: louis.rilling			Batiment Germanium
Phone: (+33|0) 6 80 89 08 23		80 avenue des Buttes de Coesmes
http://www.kerlabs.com/			35700 Rennes

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 206 bytes --]

_______________________________________________
Containers mailing list
Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
https://lists.linux-foundation.org/mailman/listinfo/containers