> -----Original Message-----
> From: Eric W. Biederman [mailto:ebied...@xmission.com]
> Sent: Thursday, October 09, 2014 1:56 AM
>
> Serge Hallyn <serge.hal...@ubuntu.com> writes:
>
> > Quoting Chen Hanxiao (chenhanx...@cn.fujitsu.com):
> >> We could use setns to join the current ns,
> >> which did a lot of unnecessary work.
> >> This patch will check this senario and
> >> return 0 directly.
> >>
> >> Signed-off-by: Chen Hanxiao <chenhanx...@cn.fujitsu.com>
> >
> > Plus it's just asking for trouble.
> >
> > I would ack this, except you need to fclose(file) on the
> > return paths. So just set err = 0 and goto out.
>
> I completely disagree.
>
> Nacked-by: "Eric W. Biederman" <ebied...@xmission.com>
>
> This patch adds a new code path to test, and gets that new code path
> wrong. So unless there is a performance advantage for some real world
> case I don't see the point. Is there real software that is rejoining
> the a current namespace.
>
> This patch changes the behavior of CLONE_NEWNS (which always does a
> chdir and chroot) when you change into the current namespace.
>
> This patch changes the behavior of CLONE_NEWUSER which current errors
> out.
>
As reentering the same namespace looks meaningless,
and handling reentering same ns we behaved differently,
How about just *reject* the behaviour of setns to current namespace?
+ switch (ops->type) {
+ case CLONE_NEWIPC:
+ if (ei->ns == tsk->nsproxy->ipc_ns) {
+ err = -EINVAL;
+ goto out;
+ }
...
And things became easy, 6 simply cases could cover the whole scenario
and will not bring troubles to users.
Thanks,
- Chen
> This code adds a big switch statement to code that is otherwise table
> driven. With the result that two pieces of code must be looked at
> and modified whenever we want to tweak the behavior of setns for a
> namespace.
>
> So in general I think this piece of code is a maintenance disaster,
> with no apparent redeem virtues.
>
> Eric
