OpenVZ Forum - RDF feed
type: channel
items: [PATCH] An attempt to have an unlimitedly extendable sys_clone
items: Re: [PATCH] An attempt to have an unlimitedly extendable sys_clone

[PATCH] An attempt to have an unlimitedly extendable sys_clone
type: item
description: We have one bit in the clone_flags left, so we won't be able to create more namespaces after we make it busy. Besides, for checkpoint/restart jobs we might want to create tasks with pre-defined pids (virtual of course). What else might be required from clone() - nobody knows. This is an attempt to create a extendable API for clone. I use the last bit in the clone_flags for CLONE_NEWCLONE. When set it will denote that the child_tidptr is not a pointer on the tid storage, but the pointer on the struct long_clone_struct which currently looks like this: struct long_clone_arg { int size; }; When we want to add a new argument for clone we just put it *at the end of this structure* and adjust the size. The binary compatibility with older long_clone_arg-s is facilitated with the clone_arg_has() macro. Sure, we lose the ability to clone tasks with extended argument and the CLONE_CHILD_SETTID/CLEARTID, but do we really need this? The same thing is about to be done for unshare - we can add the second argument for it and iff the CLONE_NEWCLONE is specified - try to use it. Binary compatibility with the old ushare will be kept. The new argument is pulled up to the create_new_namespaces so that later we can easily use it w/o sending additional patches. This is a final, but a pre-review patch for sys_clone() that I plan to send to Andrew before we go on developing new namespaces. Made against 2.6.24-rc5-mm1. Signed-off-by: Pavel Emelyanov <xemul@openzv.org> --- diff --git a/include/linux/nsproxy.h b/include/linux/nsproxy.h index 0e66b57..e01de56 100644 --- a/include/linux/nsproxy.h +++ b/include/linux/nsproxy.h @@ -62,7 +62,8 @@ static inline struct nsproxy *task_nsproxy(struct task_struct *tsk) return rcu_dereference(tsk->nsproxy); } -int copy_namespaces(unsigned long flags, struct task_struct *tsk); +int copy_namespaces(unsigned long flags, struct task_struct *tsk, + struct long_clone_arg *carg); void exit_task_namespaces(struct task_struct *tsk); void switch_task_namespaces(str…
creator: Pavel Emelianov
date: 2008-01-15T12:50:58-00:00

Re: [PATCH] An attempt to have an unlimitedly extendable sys_clone
type: item
description: Pavel Emelyanov wrote: > We have one bit in the clone_flags left, so we won't be > able to create more namespaces after we make it busy. > Besides, for checkpoint/restart jobs we might want to > create tasks with pre-defined pids (virtual of course). > What else might be required from clone() - nobody knows. > > This is an attempt to create a extendable API for clone. > > I use the last bit in the clone_flags for CLONE_NEWCLONE. > When set it will denote that the child_tidptr is not a > pointer on the tid storage, but the pointer on the struct > long_clone_struct which currently looks like this: > > struct long_clone_arg { > int size; > }; > > When we want to add a new argument for clone we just put > it *at the end of this structure* and adjust the size. > The binary compatibility with older long_clone_arg-s is > facilitated with the clone_arg_has() macro. hmm, I wonder how lkml@ will react to this. do we have similar apis in the kernel ? > Sure, we lose the ability to clone tasks with extended > argument and the CLONE_CHILD_SETTID/CLEARTID, but do we > really need this? not in the extended clone flag version. I think. > The same thing is about to be done for unshare - we can > add the second argument for it and iff the CLONE_NEWCLONE > is specified - try to use it. Binary compatibility with > the old ushare will be kept. > > The new argument is pulled up to the create_new_namespaces > so that later we can easily use it w/o sending additional > patches. > > This is a final, but a pre-review patch for sys_clone() > that I plan to send to Andrew before we go on developing > new namespaces. > > Made against 2.6.24-rc5-mm1. The patch looks good and I compiled it and booted on x64 and x86_64. I think we should add the unshare support before sending to andrew and also add an extended flag array to show how it will be used. I have a mq_namespace patchset pending we could use for that and send all together ? C. > Signed-off-by: Pavel Emelyanov <xemul@openzv.org> > > --- > …
creator: Cedric Le Goater
date: 2008-01-15T13:38:06-00:00

Re: [PATCH] An attempt to have an unlimitedly extendable sys_clone
type: item
description: > @@ -1132,8 +1161,15 @@ static struct task_struct *copy_process(unsigned long clone_flags, > /* Perform scheduler related setup. Assign this task to a CPU. */ > sched_fork(p, clone_flags); > > + if (clone_flags & CLONE_NEWCLONE) { > + carg = get_long_clone_arg(child_tidptr); > + retval = PTR_ERR(carg); > + if (IS_ERR(carg)) > + goto bad_fork_cleanup_policy; > + } > + it's probably better do to : carg = get_long_clone_arg(child_tidptr); if (IS_ERR(carg)) { retval = PTR_ERR(carg); goto bad_fork_cleanup_policy; } C. _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Cedric Le Goater
date: 2008-01-15T13:52:02-00:00

Re: [PATCH] An attempt to have an unlimitedly extendable sys_clone
type: item
description: Pavel Emelyanov wrote: > We have one bit in the clone_flags left, so we won't be > able to create more namespaces after we make it busy. > Besides, for checkpoint/restart jobs we might want to > create tasks with pre-defined pids (virtual of course). > What else might be required from clone() - nobody knows. > > This is an attempt to create a extendable API for clone. > > I use the last bit in the clone_flags for CLONE_NEWCLONE. > When set it will denote that the child_tidptr is not a > pointer on the tid storage, but the pointer on the struct > long_clone_struct which currently looks like this: > > struct long_clone_arg { > int size; > }; > > When we want to add a new argument for clone we just put > it *at the end of this structure* and adjust the size. > The binary compatibility with older long_clone_arg-s is > facilitated with the clone_arg_has() macro. > > Sure, we lose the ability to clone tasks with extended > argument and the CLONE_CHILD_SETTID/CLEARTID, but do we > really need this? > > The same thing is about to be done for unshare - we can > add the second argument for it and iff the CLONE_NEWCLONE > is specified - try to use it. Binary compatibility with > the old ushare will be kept. > > The new argument is pulled up to the create_new_namespaces > so that later we can easily use it w/o sending additional > patches. > > This is a final, but a pre-review patch for sys_clone() > that I plan to send to Andrew before we go on developing > new namespaces. > > Made against 2.6.24-rc5-mm1. > > Signed-off-by: Pavel Emelyanov <xemul@openzv.org> > > --- > > diff --git a/include/linux/nsproxy.h b/include/linux/nsproxy.h > index 0e66b57..e01de56 100644 > --- a/include/linux/nsproxy.h > +++ b/include/linux/nsproxy.h > @@ -62,7 +62,8 @@ static inline struct nsproxy *task_nsproxy(struct > task_struct *tsk) > return rcu_dereference(tsk->nsproxy); > } > > -int copy_namespaces(unsigned long flags, struct task_struct *tsk); > +int copy_namespaces(unsigned long flags, struc…
creator: Daniel Hokka Zakrisso
date: 2008-01-15T14:34:41-00:00

Re: [PATCH] An attempt to have an unlimitedly extendable sys_clone
type: item
description: Cedric Le Goater wrote: > Pavel Emelyanov wrote: >> We have one bit in the clone_flags left, so we won't be >> able to create more namespaces after we make it busy. >> Besides, for checkpoint/restart jobs we might want to >> create tasks with pre-defined pids (virtual of course). >> What else might be required from clone() - nobody knows. >> >> This is an attempt to create a extendable API for clone. >> >> I use the last bit in the clone_flags for CLONE_NEWCLONE. >> When set it will denote that the child_tidptr is not a >> pointer on the tid storage, but the pointer on the struct >> long_clone_struct which currently looks like this: >> >> struct long_clone_arg { >> int size; >> }; >> >> When we want to add a new argument for clone we just put >> it *at the end of this structure* and adjust the size. >> The binary compatibility with older long_clone_arg-s is >> facilitated with the clone_arg_has() macro. > > hmm, I wonder how lkml@ will react to this. do we have > similar apis in the kernel ? > >> Sure, we lose the ability to clone tasks with extended >> argument and the CLONE_CHILD_SETTID/CLEARTID, but do we >> really need this? > > not in the extended clone flag version. I think. > >> The same thing is about to be done for unshare - we can >> add the second argument for it and iff the CLONE_NEWCLONE >> is specified - try to use it. Binary compatibility with >> the old ushare will be kept. >> >> The new argument is pulled up to the create_new_namespaces >> so that later we can easily use it w/o sending additional >> patches. >> >> This is a final, but a pre-review patch for sys_clone() >> that I plan to send to Andrew before we go on developing >> new namespaces. >> >> Made against 2.6.24-rc5-mm1. > > The patch looks good and I compiled it and booted on x64 and > x86_64. > > I think we should add the unshare support before sending to > andrew and also add an extended flag array to show how it will > be used. I have a mq_namespace patchset pending we could use > for …
creator: Cedric Le Goater
date: 2008-01-15T14:48:13-00:00

Re: [PATCH] An attempt to have an unlimitedly extendable sys_clone
type: item
description: Pavel Emelyanov wrote: > We have one bit in the clone_flags left, so we won't be > able to create more namespaces after we make it busy. > Besides, for checkpoint/restart jobs we might want to > create tasks with pre-defined pids (virtual of course). > What else might be required from clone() - nobody knows. > > This is an attempt to create a extendable API for clone. > > I use the last bit in the clone_flags for CLONE_NEWCLONE. how about "CLONE_EXTEND" ? > When set it will denote that the child_tidptr is not a > pointer on the tid storage, but the pointer on the struct > long_clone_struct which currently looks like this: > > struct long_clone_arg { > int size; > }; how about "ext_clone_arg" ? (both suggestion make the use more explicit and are more consistent with each other; but definitely a nit ...) > > When we want to add a new argument for clone we just put > it *at the end of this structure* and adjust the size. > The binary compatibility with older long_clone_arg-s is > facilitated with the clone_arg_has() macro. Since the same kernel version (maj/min) can be compiled with different config options, need to ensure that not only are args added at the end, but also without #if/#ifdef around them. That also means, that if a feature isn't compile (but the size argument remains larger because of fields required for other arguments, a user program currently has no way to figure out what's available and supported by the kernel. (see also comment below about the code). > > Sure, we lose the ability to clone tasks with extended > argument and the CLONE_CHILD_SETTID/CLEARTID, but do we > really need this? not necessarily. the relevant field can be added (even already now) inside long_clone_arg, e.g. child_tidptr; so if the extra bit is set, _and_ ....CLEARTID is set, the pointer is found inside the extra struct. in other words we don't give up the functionality. > > The same thing is about to be done for unshare - we can > add the second argument for it and iff the CLON…
creator: Oren Laadan
date: 2008-01-15T15:17:21-00:00

Re: [PATCH] An attempt to have an unlimitedly extendable sys_clone
type: item
description: Oren Laadan wrote: > > Pavel Emelyanov wrote: >> We have one bit in the clone_flags left, so we won't be >> able to create more namespaces after we make it busy. >> Besides, for checkpoint/restart jobs we might want to >> create tasks with pre-defined pids (virtual of course). >> What else might be required from clone() - nobody knows. >> >> This is an attempt to create a extendable API for clone. >> >> I use the last bit in the clone_flags for CLONE_NEWCLONE. > > how about "CLONE_EXTEND" ? > >> When set it will denote that the child_tidptr is not a >> pointer on the tid storage, but the pointer on the struct >> long_clone_struct which currently looks like this: >> >> struct long_clone_arg { >> int size; >> }; > > how about "ext_clone_arg" ? > > (both suggestion make the use more explicit and are more > consistent with each other; but definitely a nit ...) yeah I agree. The naming can be improved but let's just wait for the patch to be sent on lkml@. I'm sure we will have plenty of feedback. however, the last clone flag name should be consistent with the structure name. CLONE_NEWCLONE is not. C. _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Cedric Le Goater
date: 2008-01-15T15:56:39-00:00

Re: [PATCH] An attempt to have an unlimitedly extendable sys_clone
type: item
description: On Tue, 2008-01-15 at 15:50 +0300, Pavel Emelyanov wrote: > +static struct long_clone_arg *get_long_clone_arg(int __user > *child_tidptr) > +{ > + int size; > + struct long_clone_arg *carg; > + > + if (get_user(size, child_tidptr)) > + return ERR_PTR(-EFAULT); > + > + if (size > sizeof(struct long_clone_arg)) > + return ERR_PTR(-EINVAL); This little bit means that any newer app (with a large long_clone_arg->size) trying to run on an older kernel (with a smaller struct) would simply fail to run clone(). Perhaps it shouldn't be _so_ generic as to allow anything in the struct and should stick to bits. That way, we can actually go look to see whether there are any _unknown_ bits set just like we do now with clone flags. The more I think about this, the more nervous I get about it. It is really neat, but has a bit of the stink of ioctl()s on it. I'd personally rather see a new system call. But, this seems like a good Linus question. Want to keep us on cc, but run it by him (and the rest of LKML)? -- Dave _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Dave Hansen
date: 2008-01-15T17:54:20-00:00

Re: [PATCH] An attempt to have an unlimitedly extendable sys_clone
type: item
description: Quoting Pavel Emelyanov (xemul@openvz.org): > We have one bit in the clone_flags left, so we won't be > able to create more namespaces after we make it busy. > Besides, for checkpoint/restart jobs we might want to > create tasks with pre-defined pids (virtual of course). > What else might be required from clone() - nobody knows. > > This is an attempt to create a extendable API for clone. > > I use the last bit in the clone_flags for CLONE_NEWCLONE. > When set it will denote that the child_tidptr is not a > pointer on the tid storage, but the pointer on the struct > long_clone_struct which currently looks like this: > > struct long_clone_arg { > int size; > }; > > When we want to add a new argument for clone we just put > it *at the end of this structure* and adjust the size. > The binary compatibility with older long_clone_arg-s is > facilitated with the clone_arg_has() macro. > > Sure, we lose the ability to clone tasks with extended > argument and the CLONE_CHILD_SETTID/CLEARTID, but do we > really need this? > > The same thing is about to be done for unshare - we can > add the second argument for it and iff the CLONE_NEWCLONE > is specified - try to use it. Binary compatibility with > the old ushare will be kept. > > The new argument is pulled up to the create_new_namespaces > so that later we can easily use it w/o sending additional > patches. > > This is a final, but a pre-review patch for sys_clone() > that I plan to send to Andrew before we go on developing > new namespaces. > > Made against 2.6.24-rc5-mm1. If we're going to go this route, the patch does look correct to me. > > Signed-off-by: Pavel Emelyanov <xemul@openzv.org> Reviewed-by: Serge Hallyn <serue@us.ibm.com> > > --- > > diff --git a/include/linux/nsproxy.h b/include/linux/nsproxy.h > index 0e66b57..e01de56 100644 > --- a/include/linux/nsproxy.h > +++ b/include/linux/nsproxy.h > @@ -62,7 +62,8 @@ static inline struct nsproxy *task_nsproxy(struct task_struct *tsk) > return rcu_dereference(tsk->nsp…
creator: serue
date: 2008-01-15T21:32:04-00:00

Re: [PATCH] An attempt to have an unlimitedly extendable sys_clone
type: item
description: Quoting Cedric Le Goater (clg@fr.ibm.com): > Cedric Le Goater wrote: > > Pavel Emelyanov wrote: > >> We have one bit in the clone_flags left, so we won't be > >> able to create more namespaces after we make it busy. > >> Besides, for checkpoint/restart jobs we might want to > >> create tasks with pre-defined pids (virtual of course). > >> What else might be required from clone() - nobody knows. > >> > >> This is an attempt to create a extendable API for clone. > >> > >> I use the last bit in the clone_flags for CLONE_NEWCLONE. > >> When set it will denote that the child_tidptr is not a > >> pointer on the tid storage, but the pointer on the struct > >> long_clone_struct which currently looks like this: > >> > >> struct long_clone_arg { > >> int size; > >> }; > >> > >> When we want to add a new argument for clone we just put > >> it *at the end of this structure* and adjust the size. > >> The binary compatibility with older long_clone_arg-s is > >> facilitated with the clone_arg_has() macro. > > > > hmm, I wonder how lkml@ will react to this. do we have > > similar apis in the kernel ? > > > >> Sure, we lose the ability to clone tasks with extended > >> argument and the CLONE_CHILD_SETTID/CLEARTID, but do we > >> really need this? > > > > not in the extended clone flag version. I think. > > > >> The same thing is about to be done for unshare - we can > >> add the second argument for it and iff the CLONE_NEWCLONE > >> is specified - try to use it. Binary compatibility with > >> the old ushare will be kept. > >> > >> The new argument is pulled up to the create_new_namespaces > >> so that later we can easily use it w/o sending additional > >> patches. > >> > >> This is a final, but a pre-review patch for sys_clone() > >> that I plan to send to Andrew before we go on developing > >> new namespaces. > >> > >> Made against 2.6.24-rc5-mm1. > > > > The patch looks good and I compiled it and booted on x64 and > > x86_64. > > > > I think we should add the unshare support befor…
creator: serue
date: 2008-01-15T21:40:18-00:00

Re: [PATCH] An attempt to have an unlimitedly extendable sys_clone
type: item
description: Quoting Daniel Hokka Zakrisson (daniel@hozac.com): > Pavel Emelyanov wrote: > > We have one bit in the clone_flags left, so we won't be > > able to create more namespaces after we make it busy. > > Besides, for checkpoint/restart jobs we might want to > > create tasks with pre-defined pids (virtual of course). > > What else might be required from clone() - nobody knows. > > > > This is an attempt to create a extendable API for clone. > > > > I use the last bit in the clone_flags for CLONE_NEWCLONE. > > When set it will denote that the child_tidptr is not a > > pointer on the tid storage, but the pointer on the struct > > long_clone_struct which currently looks like this: > > > > struct long_clone_arg { > > int size; > > }; > > > > When we want to add a new argument for clone we just put > > it *at the end of this structure* and adjust the size. > > The binary compatibility with older long_clone_arg-s is > > facilitated with the clone_arg_has() macro. > > > > Sure, we lose the ability to clone tasks with extended > > argument and the CLONE_CHILD_SETTID/CLEARTID, but do we > > really need this? > > > > The same thing is about to be done for unshare - we can > > add the second argument for it and iff the CLONE_NEWCLONE > > is specified - try to use it. Binary compatibility with > > the old ushare will be kept. > > > > The new argument is pulled up to the create_new_namespaces > > so that later we can easily use it w/o sending additional > > patches. > > > > This is a final, but a pre-review patch for sys_clone() > > that I plan to send to Andrew before we go on developing > > new namespaces. > > > > Made against 2.6.24-rc5-mm1. > > > > Signed-off-by: Pavel Emelyanov <xemul@openzv.org> > > > > --- > > > > diff --git a/include/linux/nsproxy.h b/include/linux/nsproxy.h > > index 0e66b57..e01de56 100644 > > --- a/include/linux/nsproxy.h > > +++ b/include/linux/nsproxy.h > > @@ -62,7 +62,8 @@ static inline struct nsproxy *task_nsproxy(struct > > task_struct *tsk) > > return…
creator: serue
date: 2008-01-15T21:46:31-00:00

Re: [PATCH] An attempt to have an unlimitedly extendable sys_clone
type: item
description: [snip] >> +static struct long_clone_arg *get_long_clone_arg(int __user >> *child_tidptr) >> +{ >> + int size; >> + struct long_clone_arg *carg; >> + >> + if (get_user(size, child_tidptr)) >> + return ERR_PTR(-EFAULT); >> + >> + if (size > sizeof(struct long_clone_arg)) >> + return ERR_PTR(-EINVAL); > > This means that software built against a newer kernel won't work on an > older one. Surely that's not intended? It is intended. If I ask an old kernel to clone the mq namespace, but it doesn't support such, that I'd better like to get an -EINVAL error rather than be silently held in an old global namespace. [snip] _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Pavel Emelianov
date: 2008-01-16T07:26:36-00:00

Re: [PATCH] An attempt to have an unlimitedly extendable sys_clone
type: item
description: Oren Laadan wrote: > > Pavel Emelyanov wrote: >> We have one bit in the clone_flags left, so we won't be >> able to create more namespaces after we make it busy. >> Besides, for checkpoint/restart jobs we might want to >> create tasks with pre-defined pids (virtual of course). >> What else might be required from clone() - nobody knows. >> >> This is an attempt to create a extendable API for clone. >> >> I use the last bit in the clone_flags for CLONE_NEWCLONE. > > how about "CLONE_EXTEND" ? I don't mind. The names I selected are not perfect. Better ones are always welcome. >> When set it will denote that the child_tidptr is not a >> pointer on the tid storage, but the pointer on the struct >> long_clone_struct which currently looks like this: >> >> struct long_clone_arg { >> int size; >> }; > > how about "ext_clone_arg" ? > > (both suggestion make the use more explicit and are more > consistent with each other; but definitely a nit ...) "Extended" sounds good, thanks. >> When we want to add a new argument for clone we just put >> it *at the end of this structure* and adjust the size. >> The binary compatibility with older long_clone_arg-s is >> facilitated with the clone_arg_has() macro. > > Since the same kernel version (maj/min) can be compiled with > different config options, need to ensure that not only are > args added at the end, but also without #if/#ifdef around > them. Sure, but since this struct is declared outside the #ifdef __KERNEL__ then the internal ifdefs will look strange. But I will mention this in comment. > That also means, that if a feature isn't compile (but the > size argument remains larger because of fields required for > other arguments, a user program currently has no way to > figure out what's available and supported by the kernel. > (see also comment below about the code). > >> Sure, we lose the ability to clone tasks with extended >> argument and the CLONE_CHILD_SETTID/CLEARTID, but do we >> really need this? > > not necessarily. the re…
creator: Pavel Emelianov
date: 2008-01-16T07:37:16-00:00

Re: [PATCH] An attempt to have an unlimitedly extendable sys_clone
type: item
description: > Here's the unshare part if you want to fold that with your patch. Thanks, Cedric. I'll pick that up. > Thanks, OK, thanks everyone, I'll try to cook the final patch today and send it to Andrew as an RFC. > C. Thanks, Pavel _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Pavel Emelianov
date: 2008-01-16T07:39:56-00:00

Re: [PATCH] An attempt to have an unlimitedly extendable sys_clone
type: item
description: Pavel Emelyanov wrote: > [snip] > >>> +static struct long_clone_arg *get_long_clone_arg(int __user >>> *child_tidptr) >>> +{ >>> + int size; >>> + struct long_clone_arg *carg; >>> + >>> + if (get_user(size, child_tidptr)) >>> + return ERR_PTR(-EFAULT); >>> + >>> + if (size > sizeof(struct long_clone_arg)) >>> + return ERR_PTR(-EINVAL); >> >> This means that software built against a newer kernel won't work on an >> older one. Surely that's not intended? > > It is intended. If I ask an old kernel to clone the mq namespace, but > it doesn't support such, that I'd better like to get an -EINVAL error > rather than be silently held in an old global namespace. That rules out using the struct for things like child_tidptr, the desired pid for C/R, etc. I think the version Dave Hansen proposed would be better, or if it's really just for bits, use an array rather than a struct to make that obvious. > [snip] > -- Daniel Hokka Zakrisson _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Daniel Hokka Zakrisso
date: 2008-01-16T07:58:51-00:00

Re: [PATCH] An attempt to have an unlimitedly extendable sys_clone
type: item
description: I wrote: > Pavel Emelyanov wrote: >> [snip] >> >>>> +static struct long_clone_arg *get_long_clone_arg(int __user >>>> *child_tidptr) >>>> +{ >>>> + int size; >>>> + struct long_clone_arg *carg; >>>> + >>>> + if (get_user(size, child_tidptr)) >>>> + return ERR_PTR(-EFAULT); >>>> + >>>> + if (size > sizeof(struct long_clone_arg)) >>>> + return ERR_PTR(-EINVAL); >>> >>> This means that software built against a newer kernel won't work on an >>> older one. Surely that's not intended? >> >> It is intended. If I ask an old kernel to clone the mq namespace, but >> it doesn't support such, that I'd better like to get an -EINVAL error >> rather than be silently held in an old global namespace. > > That rules out using the struct for things like child_tidptr, the desired > pid for C/R, etc. I think the version Dave Hansen proposed would be > better, or if it's really just for bits, use an array rather than a struct > to make that obvious. s/Dave Hansen/Oren Laadan/ Sorry, too early for me. >> [snip] >> -- Daniel Hokka Zakrisson _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Daniel Hokka Zakrisso
date: 2008-01-16T08:07:21-00:00

Re: [PATCH] An attempt to have an unlimitedly extendable sys_clone
type: item
description: Daniel Hokka Zakrisson wrote: > Pavel Emelyanov wrote: >> [snip] >> >>>> +static struct long_clone_arg *get_long_clone_arg(int __user >>>> *child_tidptr) >>>> +{ >>>> + int size; >>>> + struct long_clone_arg *carg; >>>> + >>>> + if (get_user(size, child_tidptr)) >>>> + return ERR_PTR(-EFAULT); >>>> + >>>> + if (size > sizeof(struct long_clone_arg)) >>>> + return ERR_PTR(-EINVAL); >>> This means that software built against a newer kernel won't work on an >>> older one. Surely that's not intended? >> It is intended. If I ask an old kernel to clone the mq namespace, but >> it doesn't support such, that I'd better like to get an -EINVAL error >> rather than be silently held in an old global namespace. > > That rules out using the struct for things like child_tidptr, the desired > pid for C/R, etc. I think the version Dave Hansen proposed would be A new system call? Ok - what arguments should it take? > better, or if it's really just for bits, use an array rather than a struct > to make that obvious. It's for anything you'd like to tell to clone(). In the nearest future - for more bits to clone new namespaces. In the far future - for pid to create the task with (for c/r jobs). In the the far-far future - for anything you will need then. >> [snip] >> > _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Pavel Emelianov
date: 2008-01-16T08:09:06-00:00

Re: [PATCH] An attempt to have an unlimitedly extendable sys_clone
type: item
description: Pavel Emelyanov wrote: >> Here's the unshare part if you want to fold that with your patch. > > Thanks, Cedric. I'll pick that up. > >> Thanks, > > OK, thanks everyone, I'll try to cook the final patch today > and send it to Andrew as an RFC. yes. I'd changed the subject of the patch to something more explicit on what we're doing : 'use child_tidptr to extend clone() flags' we need feedback from the community on this clone extension ! :) C. _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Cedric Le Goater
date: 2008-01-16T08:15:13-00:00

Re: [PATCH] An attempt to have an unlimitedly extendable sys_clone
type: item
description: On Wed, 2008-01-16 at 10:26 +0300, Pavel Emelyanov wrote: > > This means that software built against a newer kernel won't work on an > > older one. Surely that's not intended? > > It is intended. If I ask an old kernel to clone the mq namespace, but > it doesn't support such, that I'd better like to get an -EINVAL error > rather than be silently held in an old global namespace. There's at least one thing that this doesn't handle. Let's say we have a clone64(long, long) but that the kernel we're running is old and don't know about any valid flags in the second long, yet. Somebody who knows about lots of new flags 10 years down the line passes clone(0x1234, 0x0) to the old kernel. That's OK, because the kernel knows about all of the flags that *ARE* set. Your proposal is basically this: clone(flags, {size, stuff}); and if the in-kernel size isn't exactly the same as the size passed in, it punts. It doesn't matter if anything in "stuff" was actually going to be used, or if the old kernel *could* have handled it, it just punts. Honestly, I don't think it's that big of a deal, I just wish you'd acknowledge it. ;) -- Dave _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Dave Hansen
date: 2008-01-16T17:52:57-00:00

Re: [PATCH] An attempt to have an unlimitedly extendable sys_clone
type: item
description: Hello Pavel ! Pavel Emelyanov wrote: >> Here's the unshare part if you want to fold that with your patch. > > Thanks, Cedric. I'll pick that up. > >> Thanks, > > OK, thanks everyone, I'll try to cook the final patch today > and send it to Andrew as an RFC. As Al said it, it's probably not the direction to follow to extend the clone flags :) But the problem being real, I think we need to insist. First, we could simplify the API by using child_tidptr to pass a 64bits array of flags. This would still be a brutal syscall argument hijack but we can't do better without a new syscall. Next, we could send the same code using a new syscall. I have time to work on that. C. _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Cedric Le Goater
date: 2008-01-21T10:36:38-00:00