[Devel] Re: [PATCH 1/4] userns: let clone_uts_ns() handle setting uts-user_ns
On 02/21/2011 05:01 AM, Serge E. Hallyn wrote:
To do so we need to pass in the task_struct who'll get the utsname,
so we can get its user_ns.
Signed-off-by: Serge E. Hallynserge.hal...@canonical.com
---
include/linux/utsname.h | 10 ++
kernel/nsproxy.c|7 +--
kernel/utsname.c| 12 +++-
3 files changed, 14 insertions(+), 15 deletions(-)
diff --git a/include/linux/utsname.h b/include/linux/utsname.h
index 85171be..165b17b 100644
--- a/include/linux/utsname.h
+++ b/include/linux/utsname.h
@@ -52,8 +52,9 @@ static inline void get_uts_ns(struct uts_namespace *ns)
kref_get(ns-kref);
}
-extern struct uts_namespace *copy_utsname(unsigned long flags,
- struct uts_namespace *ns);
+extern struct uts_namespace *copy_utsname(struct task_struct *tsk,
+ unsigned long flags,
+ struct uts_namespace *ns);
Why don't we pass 'user_ns' instead of 'tsk' ? that will look
semantically clearer for the caller no ?
(example below).
extern void free_uts_ns(struct kref *kref);
static inline void put_uts_ns(struct uts_namespace *ns)
@@ -69,8 +70,9 @@ static inline void put_uts_ns(struct uts_namespace *ns)
{
}
-static inline struct uts_namespace *copy_utsname(unsigned long flags,
- struct uts_namespace *ns)
+static inline struct uts_namespace *copy_utsname(struct task_struct *tsk,
+ unsigned long flags,
+ struct uts_namespace *ns)
{
if (flags CLONE_NEWUTS)
return ERR_PTR(-EINVAL);
diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c
index b6dbff2..ffa6b67 100644
--- a/kernel/nsproxy.c
+++ b/kernel/nsproxy.c
@@ -69,16 +69,11 @@ static struct nsproxy *create_new_namespaces(unsigned
long flags,
goto out_ns;
}
- new_nsp-uts_ns = copy_utsname(flags, tsk-nsproxy-uts_ns);
+ new_nsp-uts_ns = copy_utsname(tsk, flags, tsk-nsproxy-uts_ns);
if (IS_ERR(new_nsp-uts_ns)) {
err = PTR_ERR(new_nsp-uts_ns);
goto out_uts;
}
...
new_nsp-uts_ns = copy_utsname(flags, tsk-nsproxy-uts_ns, task_cred_xxx(tsk,
user)-user_ns);
...
- if (new_nsp-uts_ns != tsk-nsproxy-uts_ns) {
- put_user_ns(new_nsp-uts_ns-user_ns);
- new_nsp-uts_ns-user_ns = task_cred_xxx(tsk, user)-user_ns;
- get_user_ns(new_nsp-uts_ns-user_ns);
- }
new_nsp-ipc_ns = copy_ipcs(flags, tsk-nsproxy-ipc_ns);
if (IS_ERR(new_nsp-ipc_ns)) {
diff --git a/kernel/utsname.c b/kernel/utsname.c
index a7b3a8d..9462580 100644
--- a/kernel/utsname.c
+++ b/kernel/utsname.c
@@ -31,7 +31,8 @@ static struct uts_namespace *create_uts_ns(void)
* @old_ns: namespace to clone
* Return NULL on error (failure to kmalloc), new ns otherwise
*/
-static struct uts_namespace *clone_uts_ns(struct uts_namespace *old_ns)
+static struct uts_namespace *clone_uts_ns(struct task_struct *tsk,
+ struct uts_namespace *old_ns)
{
struct uts_namespace *ns;
@@ -41,8 +42,7 @@ static struct uts_namespace *clone_uts_ns(struct
uts_namespace *old_ns)
down_read(uts_sem);
memcpy(ns-name,old_ns-name, sizeof(ns-name));
- ns-user_ns = old_ns-user_ns;
- get_user_ns(ns-user_ns);
+ ns-user_ns = get_user_ns(task_cred_xxx(tsk, user)-user_ns);
up_read(uts_sem);
return ns;
}
@@ -53,7 +53,9 @@ static struct uts_namespace *clone_uts_ns(struct
uts_namespace *old_ns)
* utsname of this process won't be seen by parent, and vice
* versa.
*/
-struct uts_namespace *copy_utsname(unsigned long flags, struct uts_namespace
*old_ns)
+struct uts_namespace *copy_utsname(struct task_struct *tsk,
+unsigned long flags,
+struct uts_namespace *old_ns)
{
struct uts_namespace *new_ns;
@@ -63,7 +65,7 @@ struct uts_namespace *copy_utsname(unsigned long flags,
struct uts_namespace *ol
if (!(flags CLONE_NEWUTS))
return old_ns;
- new_ns = clone_uts_ns(old_ns);
+ new_ns = clone_uts_ns(tsk, old_ns);
put_uts_ns(old_ns);
return new_ns;
___
Containers mailing list
contain...@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers
___
Devel mailing list
Devel@openvz.org
https://openvz.org/mailman/listinfo/devel
[Devel] Re: [PATCH 2/4] userns: let copy_ipcs handle setting ipc_ns-user_ns
On 02/21/2011 05:02 AM, Serge E. Hallyn wrote:
To do that, we have to pass in the task_struct of the task which
will own the ipc_ns, so we can assign its user_ns.
Signed-off-by: Serge E. Hallynserge.hal...@canonical.com
---
include/linux/ipc_namespace.h |8 +---
ipc/namespace.c | 12 +++-
kernel/nsproxy.c |7 +--
3 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/include/linux/ipc_namespace.h b/include/linux/ipc_namespace.h
index 46d2eb4..9974429 100644
--- a/include/linux/ipc_namespace.h
+++ b/include/linux/ipc_namespace.h
@@ -92,7 +92,8 @@ static inline int mq_init_ns(struct ipc_namespace *ns) {
return 0; }
#endif
#if defined(CONFIG_IPC_NS)
-extern struct ipc_namespace *copy_ipcs(unsigned long flags,
+extern struct ipc_namespace *copy_ipcs(struct task_struct *tsk,
+unsigned long flags,
struct ipc_namespace *ns);
Same comment than patch 1/4
___
Containers mailing list
contain...@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers
___
Devel mailing list
Devel@openvz.org
https://openvz.org/mailman/listinfo/devel
[Devel] Re: [PATCH 1/4] userns: let clone_uts_ns() handle setting uts-user_ns
On 02/21, Daniel Lezcano wrote: On 02/21/2011 05:01 AM, Serge E. Hallyn wrote: To do so we need to pass in the task_struct who'll get the utsname, so we can get its user_ns. -extern struct uts_namespace *copy_utsname(unsigned long flags, -struct uts_namespace *ns); +extern struct uts_namespace *copy_utsname(struct task_struct *tsk, + unsigned long flags, + struct uts_namespace *ns); Why don't we pass 'user_ns' instead of 'tsk' ? that will look semantically clearer for the caller no ? (example below). ... new_nsp-uts_ns = copy_utsname(flags, tsk-nsproxy-uts_ns, task_cred_xxx(tsk, user)-user_ns); To me tsk looks more readable, I mean new_nsp-uts_ns = copy_utsname(flags, tsk); copy_utsname() can find both uts_ns and user_ns looking at task_strcut. But this is cosmetic and up to you and Serge. But. I think it makes sense to pass tsk argument to copy_pid_ns() as well. This way we can remove some CLONE_PIDNS code in copy_process(), and this looks like a nice cleanaup (even if minor) to me. Oleg. ___ Containers mailing list contain...@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers ___ Devel mailing list Devel@openvz.org https://openvz.org/mailman/listinfo/devel
[Devel] Re: [PATCH 1/4] userns: let clone_uts_ns() handle setting uts-user_ns
Quoting Oleg Nesterov (o...@redhat.com): On 02/21, Daniel Lezcano wrote: On 02/21/2011 05:01 AM, Serge E. Hallyn wrote: To do so we need to pass in the task_struct who'll get the utsname, so we can get its user_ns. -extern struct uts_namespace *copy_utsname(unsigned long flags, - struct uts_namespace *ns); +extern struct uts_namespace *copy_utsname(struct task_struct *tsk, +unsigned long flags, +struct uts_namespace *ns); Why don't we pass 'user_ns' instead of 'tsk' ? that will look semantically clearer for the caller no ? (example below). ... new_nsp-uts_ns = copy_utsname(flags, tsk-nsproxy-uts_ns, task_cred_xxx(tsk, user)-user_ns); To me tsk looks more readable, I mean new_nsp-uts_ns = copy_utsname(flags, tsk); copy_utsname() can find both uts_ns and user_ns looking at task_strcut. Uh, yeah. I should remove the 'ns' argument there shouldn't I. Daniel, does that sway your opinion then? thanks, -serge ___ Containers mailing list contain...@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers ___ Devel mailing list Devel@openvz.org https://openvz.org/mailman/listinfo/devel
[Devel] Re: [PATCH 1/4] userns: let clone_uts_ns() handle setting uts-user_ns
On 02/21/2011 02:58 PM, Serge E. Hallyn wrote: Quoting Oleg Nesterov (o...@redhat.com): On 02/21, Daniel Lezcano wrote: On 02/21/2011 05:01 AM, Serge E. Hallyn wrote: To do so we need to pass in the task_struct who'll get the utsname, so we can get its user_ns. -extern struct uts_namespace *copy_utsname(unsigned long flags, - struct uts_namespace *ns); +extern struct uts_namespace *copy_utsname(struct task_struct *tsk, +unsigned long flags, +struct uts_namespace *ns); Why don't we pass 'user_ns' instead of 'tsk' ? that will look semantically clearer for the caller no ? (example below). ... new_nsp-uts_ns = copy_utsname(flags, tsk-nsproxy-uts_ns, task_cred_xxx(tsk, user)-user_ns); To me tsk looks more readable, I mean new_nsp-uts_ns = copy_utsname(flags, tsk); copy_utsname() can find both uts_ns and user_ns looking at task_strcut. Uh, yeah. I should remove the 'ns' argument there shouldn't I. Daniel, does that sway your opinion then? Well, I prefer to pass the needed parameters to a function. AFAICS, 'tsk' is not really needed but 'user_ns'. But it is a detail, so if passing the tsk parameter in the other copy_* functions helps to cleanup, that will be consistent. So I am fine with that. ___ Containers mailing list contain...@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers ___ Devel mailing list Devel@openvz.org https://openvz.org/mailman/listinfo/devel
[Devel] Re: [PATCH][usercr]: Ghost tasks must be detached
Louis Rilling [louis.rill...@kerlabs.com] wrote: | But in 2.6.32 i.e RHEL5, tsk-signal is set to NULL in __exit_signal(). | So, I am trying to rule out the following scenario: | | Child (may not be a ghost) Parent | - -- | - exit_notify(): is EXIT_DEAD | - release_task(): | - drops task_list_lock | - itself proceeds to exit. | - enters release_task() | - sets own-signal = NULL |(in 2.6.32, __exit_signal()) | | - enters exit_checkpoint() | - __wake_up_parent() | access parents-signal NULL ptr | | Not sure if holding task_list_lock here is needed or will help. | | Giving my 2 cents since I've been Cc'ed. Thanks, appreciate the input :-) | | AFAICS, holding tasklist_lock prevents __exit_signal() from setting | parent-signal to NULL in your back. So something like this should be safe: | | read_lock(tasklist_lock); | if (current-parent-signal) | __wake_up_parent(...); | read_unlock(tasklist_lock); Yes, checking the parent-signal with task_list_lock would work. | | I haven't looked at the context, but of course this also requires that some | get_task_struct() on current-parent has been done somewhere else before current | has passed __exit_signal(). | | By the way, instead of checking current-parent-signal, | current-parent-exit_state would look cleaner to me. current-parent is not | supposed to wait on -wait_chldexit after calling do_exit(), right? ^^^ Hmm, do you mean exit_notify() here ? If so, yes checking the exit_state is cleaner. If the parent's exit_state is set, then it can't be waiting for the ghost, so no need to wake_up_parent(). If exit state is not set, then it is safe to wake_up_parent() (parent-signal would not yet have been cleared for instance). The one case where a parent in do_exit() could still wait for the child is the container-init which waits on wait_chldexit in do_exit() - zap_pid_ns_processes() - but even in that case the __wake_up_parent() call would be safe. Sukadev ___ Containers mailing list contain...@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers ___ Devel mailing list Devel@openvz.org https://openvz.org/mailman/listinfo/devel
[Devel] c/r:read pipe error when restart
hi all: I have a very simple script test for c/r,but I'm unable to get a restart to complete. I get both linux-cr and user-cr code from the git a few weeks ago, and i think they are the newest. I'm using RHEL5.4, x86-64. Here are my steps: # mkdir -p /cgroup # mount -t cgroup -o freezer cgroup /cgroup # mkdir /cgroup/1 # cat foo.sh EOF #!/bin/sh echo $$ /cgroup/1/tasks while true; do echo this is a test. sleep 1s done EOF # nsexec -tgcmpiUP pid foo.sh about to clone with 3802 this is a test. this is a test. ... /* the following steps go on in another terminal */ # cat pid 29750 # echo FROZEN /cgroup/1/freezer.state # cat /cgroup/1/tasks 29750 29772 # checkpoint 29750 -l clog -o image.out # kill -9 29750 # echo THAWED /cgroup/1/freezer.state (It seems ok for now, but when I try restart, the problem comes.) # restart -l rlog -i image.out read pipe: Bad file descriptor Killed (I have tried lots of times, and the rlog is usually empty, but I also see msg once in it) # cat rlog [err -512][pos 0][E @ ckpt_read_obj_type:426]Expecting to read type 1 (when I add -d -v, it comes like this) # restart -d -v -i image.out 31720number of tasks: 2 31720number of vpids: 0 31720total tasks (including ghosts): 3 31720pid 99: inherit sid 0 31720pid 99: creator set to 1 31720pid 301: orphan session 1 31720pid 301: creator set to 302 31720== TASKS 31720 [0] pid 1 ppid 0 sid 0 creator 0 placeholder 302 31720 [1] pid 99 ppid 1 sid 0 creator 1 prev 302 31720 [2] pid 301 ppid 1 sid 1 creator 302 G 31720 [3] pid 302 ppid 1 sid 1 creator 1 next 99 D 31720 31720task[0].vidx = 1 (depth 0, rpid 1) 31720task[1].vidx = 2 (depth 0, rpid 99) 31720subtree (existing pidns) 31720fork child vpid 1 flags 0x1 31721== PIDS ARRAY 31721[0] pid 1 ppid 301 sid 301 pgid 301 31721[1] pid 99 ppid 301 sid 301 pgid 301 31720task 1 forking with flags 11 numpids 1 31721 31720task 1 pid[0]=0 read pipe: Bad file descriptor Killed (I found a similar discussion from http://openvz.org/pipermail/devel/2010-July/025274.html , but it didn't work for me. Because the code I use has fixed them all.) Look forward and very appreciate for your helps! Thank you! Kadyz ___ Devel mailing list Devel@openvz.org https://openvz.org/mailman/listinfo/devel
