Quoting Eric W. Biederman (ebied...@xmission.com): > 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.
IMO performance would be a poor reason to do this. I would feel better with it because the case of "I've unshared everything, now setns to my own namespace" seems too easy to get to a point where you put the last ref to your ns before you get the new ns. Yes at least the mntns_install seems to prevent this, and yes it would be a bug, but I simply consider this good defensive coding. > 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. Yes so currently setns to your own ns behaves differently for different namespace types. That also seems like a reason to fix this. > 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. I'm not going to push too hard on this, I simply disagree. -serge -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
