On 05/19/2013 11:59 AM, Andrew Vagin wrote:
On Fri, May 17, 2013 at 10:24:55AM -0700, Kir Kolyshkin wrote:
On 05/16/2013 09:47 AM, Andrey Wagin wrote:
2013/5/16 Glauber Costa <glom...@parallels.com>:
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 then return them as is if ret != 0 ?
It returns PID of the init process. VZ_RESOURCE_ERROR is positive too

It does not (maybe it's a bug in your patch).

+       /*
+        * Belong in the setup phase
+        */
+       clone_flags = SIGCHLD;
+       /* FIXME: USERNS is still work in progress */
+       clone_flags |= CLONE_NEWUTS|CLONE_NEWPID|CLONE_NEWIPC;
+       clone_flags |= CLONE_NEWNET|CLONE_NEWNS;
+
+       ret = clone(_env_create, child_stack, clone_flags, arg);
+       if (ret  < 0) {
+               logger(-1, errno, "Unable to clone");
+               /* FIXME: remove ourselves from container first */
+               destroy_container(arg->veid);
+               return -1;
+       }
+
+       return 0;
+}

Did you mean "return ret" here?
Yes, it is fixed in "vzctl: save PID of init in a state file". Sorry for
that.
Also, to not change all those return statements, I suggest to pass pid_t * as a 
second argument.
I don't like that. If you want, I can do that.


Let's do this. If there is no need to return different error codes
from ct_env_create_real() then I'm fine with your approach.
Just make sure to make the patch logical (i.e. return ret, otherwise
it's not clear from the patch why you change VZ_RESOURCE_ERROR to -1)
and briefly document its purpose and return code in a comment
just before the function.
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Reply via email to