Re: [Devel] [PATCH v5 6/6] allow for distro-specific fix ups at creation time.

2013-05-17 Thread Kir Kolyshkin
PLease see inline. On 05/17/2013 09:26 AM, Glauber Costa wrote: From: Glauber Costa We will need that infrastucture when running with Linux upstream, since some support is very unlikely to ever land in the Kernel. We need to do things like account for the fact that udev may kick in and destro

[Devel] [PATCH v5 6/6] allow for distro-specific fix ups at creation time.

2013-05-17 Thread Glauber Costa
From: Glauber Costa We will need that infrastucture when running with Linux upstream, since some support is very unlikely to ever land in the Kernel. We need to do things like account for the fact that udev may kick in and destroy all the setup we have done for /dev. Since preemptively preventi

[Devel] [PATCH v5 5/6] automatically add bridge venet0 when needed

2013-05-17 Thread Glauber Costa
From: Glauber Costa The chosen architecture to deal with --ipadd with upstream containers is to create a veth pair and add the host side information to a bridge called venet0. This way, all the code that expects venet0 to exist can still work without modifications, (or with just a few). Our inte

[Devel] [PATCH v5 4/6] modify tar extraction to account for user namespace

2013-05-17 Thread Glauber Costa
From: Glauber Costa If we are running upstream with user namespaces, we need to create the container filesystem not with the ownership preserved, but reflecting the mapping we need to apply. Note that according to our documentation, we should ignore this if the user explicitly requested an uid ma

[Devel] [PATCH v5 3/6] allow local uid and gid to be specified at container creation

2013-05-17 Thread Glauber Costa
From: Glauber Costa It is a valid use case to run a container with host uid and gid different than the default. In particular, already deployed versions of vzctl are expected to have this value unset, effectively meaning they are not expecting user namespaces to be present. We also deem as a vali

[Devel] [PATCH v5 2/6] add user mismatch test

2013-05-17 Thread Glauber Costa
From: Glauber Costa In theory, we won't be able to run if our private area is not owned by ourselves. We could, if it have very wide open security permissions, but we should never set up a container like that. Aside from a basic sanity check, this is intended to catch problems for the few peopl

[Devel] [PATCH v5 1/6] user namespace support for upstream containers

2013-05-17 Thread Glauber Costa
From: Glauber Costa This patch allows the execution of unprivileged containers running ontop of an upstream Linux Kernel. We will run at whatever UID is found in the configuration file (so far empty, thus disabled). Signed-off-by: Glauber Costa --- include/env.h | 1 + include/types.h

[Devel] [PATCH v5 0/6] User namespace support for upstream containers

2013-05-17 Thread Glauber Costa
Kir, In this patchset, I hope to be addressing all your concerns. I don't use cmd_p any longer, as you requested. I have also tried to merge most of your comments at the main userns patch. It is a bit massive code, so if is there still anything that you would me to change, don't shy away. I will

Re: [Devel] [PATCH v4 3/7] Also pass cmd_p pointer to container open

2013-05-17 Thread Glauber Costa
On 05/17/2013 05:35 AM, Kir Kolyshkin wrote: > > This is kinda becoming over-engineered (and now I realized I should have > said it earlier, > when reviewing the patch that added the first param). > > I understand well why you need local_uid and local_gid from config and > cmdline > in ct_open(),

Re: [Devel] [PATCH v4 1/7] user namespace support for upstream containers

2013-05-17 Thread Glauber Costa
On 05/17/2013 04:11 AM, Kir Kolyshkin wrote: >> +if ((arg->userns_p != -1) && (read(arg->userns_p, &ret, >> sizeof(ret)) != sizeof(ret))) { >> +logger(-1, errno, "Cannot read from user namespace pipe"); > > We don't close arg->userns_p in case of error here. > And it seems we do not cl

Re: [Devel] [PATCH v4 2/7] add user mismatch test

2013-05-17 Thread Glauber Costa
On 05/17/2013 04:18 AM, Kir Kolyshkin wrote: >> +stat(res->fs.private, &private_stat); >> +if ((local_uid && (private_stat.st_uid != *local_uid)) || >> +(local_gid && (private_stat.st_gid != *local_gid))) { > > As I just commented at the very end of a previous patch, >

Re: [Devel] [PATCH 1/6] vzctl: split ct_env_create

2013-05-17 Thread Kir Kolyshkin
On 05/16/2013 09:47 AM, Andrey Wagin wrote: 2013/5/16 Glauber Costa : On 05/16/2013 04:14 PM, Andrey Vagin wrote: + ret = ct_env_create_real(arg); + if (ret < 0) return VZ_RESOURCE_ERROR; - } Isn't it better to just keep the return values intact in create_real, and t

Re: [Devel] [PATCH 2/6] vzctl: save PID of init in a state file

2013-05-17 Thread Andrew Vagin
On Thu, May 16, 2013 at 08:25:57PM +0400, Glauber Costa wrote: > On 05/16/2013 04:14 PM, Andrey Vagin wrote: > > CRIU requires a pid of the init. > > > > Signed-off-by: Andrey Vagin > > The way you coded it, it seems to me that we will always overwrite the > pid file, which is fine: this way we