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

[Kir Kolyshkin]

On 05/14/2013 03:52 AM, Glauber Costa wrote:
> We would like to know early on in the container lifetime (before creation) if
> the container should use user namespaces. At this point, we not yet have a ct

s/not/do not/
> configuration file, just the global one. It may very well be that the user have
> overriden the global configuration file information through the command line.
> If that is the case, this value should take precedence. To achieve this, we
> should also pass the cmd_p information to the open functions.

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(), but maybe this non-zero checks can be done later in the 
code, so
instead of using just h->can_join_userns you can call a function, 
something like

int is_joining_userns(vps_handler *h, vps_param *param) {
     return h->can_join_userns && param->misc.localuid && 
param->misc.localgid;
}

(maybe you need to also check the values for not being 0, I don't know)

In any place where you want to call this function you do have vps_param 
and h available,
and more to say, localuid and localgid is already merged (so cmdline 
value takes precedence).

Also you can distinguish between "unsupported" and "disabled" cases
and there's no need to patch a lot of code and pass all these structures 
around.

Makes sense?

>
> Signed-off-by: Glauber Costa <glommer at openvz.org>
> ---
>   include/env.h       | 8 +++++---
>   src/lib/env.c       | 7 ++++---
>   src/lib/hooks_ct.c  | 2 +-
>   src/lib/hooks_vz.c  | 2 +-
>   src/vzctl-actions.c | 2 +-
>   5 files changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/include/env.h b/include/env.h
> index 4fef438..35d427f 100644
> --- a/include/env.h
> +++ b/include/env.h
> @@ -49,8 +49,10 @@ enum {
>    *
>    * @param veid		CT ID.
>    * @return		handler or NULL on error.
> + * @param		global ct parameters.
> + * @cmd			command line ct parameters.

You probably wanted to say

@param param        global ct parameters
@param cmd            command line ct parameters


and please move it to before
@return

>    */
> -vps_handler *vz_open(envid_t veid, vps_param *param);
> +vps_handler *vz_open(envid_t veid, vps_param *param, vps_param *cmd);
>   
>   /** Close CT handler.
>    *
> @@ -126,6 +128,6 @@ int exec_container_init(struct arg_start *arg,
>   			struct env_create_param3 *create_param);
>   
>   int set_personality32();
> -int vz_do_open(vps_handler *h, vps_param *param);
> -int ct_do_open(vps_handler *h, vps_param *param);
> +int vz_do_open(vps_handler *h, vps_param *param, vps_param *cmd);
> +int ct_do_open(vps_handler *h, vps_param *param, vps_param *cmd);
>   #endif
> diff --git a/src/lib/env.c b/src/lib/env.c
> index 96400c0..166c83b 100644
> --- a/src/lib/env.c
> +++ b/src/lib/env.c
> @@ -74,9 +74,10 @@ static int reset_std()
>    *
>    * @param veid		CT ID.
>    * @param param		CT parameters.
> + * @param cmd		CT command line parameters.
>    * @return		handler or NULL on error.
>    */
> -vps_handler *vz_open(envid_t veid, vps_param *param)
> +vps_handler *vz_open(envid_t veid, vps_param *param, vps_param *cmd)
>   {
>   	vps_handler *h = NULL;
>   	int ret = -1;
> @@ -90,7 +91,7 @@ vps_handler *vz_open(envid_t veid, vps_param *param)
>   #ifdef VZ_KERNEL_SUPPORTED
>   	/* Find out if we are under OpenVZ or upstream kernel */
>   	if (stat_file("/proc/vz"))
> -		ret = vz_do_open(h, param);
> +		ret = vz_do_open(h, param, cmd);
>   	else
>   #endif
>   	{
> @@ -98,7 +99,7 @@ vps_handler *vz_open(envid_t veid, vps_param *param)
>   				"non-OpenVZ kernel");
>   		h->vzfd = -1;
>   #ifdef HAVE_UPSTREAM
> -		ret = ct_do_open(h, param);
> +		ret = ct_do_open(h, param, cmd);
>   #else
>   		logger(-1, 0, "No suitable kernel support compiled in");
>   #endif
> diff --git a/src/lib/hooks_ct.c b/src/lib/hooks_ct.c
> index 8738eeb..f4d8f48 100644
> --- a/src/lib/hooks_ct.c
> +++ b/src/lib/hooks_ct.c
> @@ -791,7 +791,7 @@ static int ct_setcontext(envid_t veid)
>   	return 0;
>   }
>   
> -int ct_do_open(vps_handler *h, vps_param *param)
> +int ct_do_open(vps_handler *h, vps_param *param, vps_param *cmd)
>   {
>   	int ret;
>   	char path[STR_SIZE];
> diff --git a/src/lib/hooks_vz.c b/src/lib/hooks_vz.c
> index 50770c0..c373b13 100644
> --- a/src/lib/hooks_vz.c
> +++ b/src/lib/hooks_vz.c
> @@ -466,7 +466,7 @@ static int vz_veth_ctl(vps_handler *h, envid_t veid, int op, veth_dev *dev)
>   	return ret;
>   }
>   
> -int vz_do_open(vps_handler *h, vps_param *param)
> +int vz_do_open(vps_handler *h, vps_param *param, vps_param *cmd)
>   {
>   	if ((h->vzfd = open(VZCTLDEV, O_RDWR)) < 0) {
>   		logger(-1, errno, "Unable to open %s", VZCTLDEV);
> diff --git a/src/vzctl-actions.c b/src/vzctl-actions.c
> index 0dd2ae7..7bf15c9 100644
> --- a/src/vzctl-actions.c
> +++ b/src/vzctl-actions.c
> @@ -1470,7 +1470,7 @@ int run_action(envid_t veid, act_t action, vps_param *g_p, vps_param *vps_p,
>   	int ret = 0, lock_id = -1;
>   	struct sigaction act;
>   
> -	if ((h = vz_open(veid, g_p)) == NULL) {
> +	if ((h = vz_open(veid, g_p, cmd_p)) == NULL) {
>   		/* Accept to run "set --save --force" on any kernel,
>   		 * otherwise error out if initialization failed
>   		 */