On Sun, May 19, 2013 at 12:12:47PM -0700, Kir Kolyshkin wrote: > 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.
Eh. Actually it's not one error code, we need to return VZ_RESTORE_ERROR on restore. I prefer to return -VZ_*ERROR, but it's out of the common style for this project. If we add pid_t *pid, we will need to add it in env_create_FN too... _______________________________________________ Devel mailing list Devel@openvz.org https://lists.openvz.org/mailman/listinfo/devel