On Fri, Nov 29, 2013 at 02:40:35PM -0500, S.Çağlar Onur wrote:
> On Fri, Nov 29, 2013 at 2:34 PM, Serge Hallyn <serge.hal...@ubuntu.com> wrote:
> > Quoting Stéphane Graber (stgra...@ubuntu.com):
> >> As mentioned in a previous commit, this does two changes:
> >> - Make want_daemonize return a bool (false on failure, true on success)
> >> - Make both want_daemonize and want_close_all_fds take a "state"
> >> argument so the user can choose to unset those flags.
> >>
> >> This commit also updates all occurences of those two functions.
> >>
> >> Signed-off-by: Stéphane Graber <stgra...@ubuntu.com>
> >
> > Two comments below. With that and James' comments addressed,
> >
> > Acked-by: Serge E. Hallyn <serge.hal...@ubuntu.com>
> >
> >> ---
> >> src/lua-lxc/core.c | 2 +-
> >> src/lxc/lxc_start.c | 4 ++--
> >> src/lxc/lxccontainer.c | 20 +++++++++++++-------
> >> src/lxc/lxccontainer.h | 4 ++--
> >> src/python-lxc/lxc.c | 10 ++++++++--
> >> src/tests/attach.c | 2 +-
> >> src/tests/cgpath.c | 2 +-
> >> src/tests/concurrent.c | 2 +-
> >> src/tests/console.c | 2 +-
> >> src/tests/containertests.c | 2 +-
> >> src/tests/createtest.c | 2 +-
> >> src/tests/shutdowntest.c | 2 +-
> >> 12 files changed, 33 insertions(+), 21 deletions(-)
> >>
> >> diff --git a/src/lua-lxc/core.c b/src/lua-lxc/core.c
> >> index 9492c07..04f2f1d 100644
> >> --- a/src/lua-lxc/core.c
> >> +++ b/src/lua-lxc/core.c
> >> @@ -156,7 +156,7 @@ static int container_start(lua_State *L)
> >> argv[j] = NULL;
> >> }
> >>
> >> - c->want_daemonize(c);
> >> + c->want_daemonize(c, 1);
> >> lua_pushboolean(L, !!c->start(c, useinit, argv));
> >> return 1;
> >> }
> >> diff --git a/src/lxc/lxc_start.c b/src/lxc/lxc_start.c
> >> index e537846..2a833a6 100644
> >> --- a/src/lxc/lxc_start.c
> >> +++ b/src/lxc/lxc_start.c
> >> @@ -325,7 +325,7 @@ int main(int argc, char *argv[])
> >> }
> >>
> >> if (my_args.daemonize) {
> >> - c->want_daemonize(c);
> >> + c->want_daemonize(c, 1);
> >> }
> >>
> >> if (pid_fp != NULL) {
> >> @@ -337,7 +337,7 @@ int main(int argc, char *argv[])
> >> }
> >>
> >> if (my_args.close_all_fds)
> >> - c->want_close_all_fds(c);
> >> + c->want_close_all_fds(c, 1);
> >>
> >> err = c->start(c, 0, args) ? 0 : -1;
> >>
> >> diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> >> index 283fbb5..4234760 100644
> >> --- a/src/lxc/lxccontainer.c
> >> +++ b/src/lxc/lxccontainer.c
> >> @@ -455,29 +455,35 @@ static bool lxcapi_load_config(struct lxc_container
> >> *c, const char *alt_file)
> >> return ret;
> >> }
> >>
> >> -static void lxcapi_want_daemonize(struct lxc_container *c)
> >> +static bool lxcapi_want_daemonize(struct lxc_container *c, int state)
> >> {
> >> + if (state > 1)
> >
> > What about < 0?
>
> Why we are not passing a bool instead of int?
I based this on similar parameters of other API functions (useinit being
one). Though looking back now it seems we have a couple of cases where
we're also passing bool in such case...
I guess another reason would be if we ever wanted to add finer grained
options for those two commands as unlikely as it may be :)
Serge: Any preference?
> >> + return false;
> >> if (!c || !c->lxc_conf)
> >> - return;
> >> + return false;
> >> if (container_mem_lock(c)) {
> >> ERROR("Error getting mem lock");
> >> - return;
> >> + return false;
> >> }
> >> - c->daemonize = 1;
> >> + c->daemonize = state;
> >> /* daemonize implies close_all_fds so set it */
> >> - c->lxc_conf->close_all_fds = 1;
> >> + if (state == 1)
> >> + c->lxc_conf->close_all_fds = 1;
> >> container_mem_unlock(c);
> >> + return true;
> >> }
> >>
> >> -static bool lxcapi_want_close_all_fds(struct lxc_container *c)
> >> +static bool lxcapi_want_close_all_fds(struct lxc_container *c, int state)
> >> {
> >> + if (state > 1)
> >
> > Same.
> >
> >> + return false;
> >> if (!c || !c->lxc_conf)
> >> return false;
> >> if (container_mem_lock(c)) {
> >> ERROR("Error getting mem lock");
> >> return false;
> >> }
> >> - c->lxc_conf->close_all_fds = 1;
> >> + c->lxc_conf->close_all_fds = state;
> >> container_mem_unlock(c);
> >> return true;
> >> }
> >> diff --git a/src/lxc/lxccontainer.h b/src/lxc/lxccontainer.h
> >> index 6044f4d..8333610 100644
> >> --- a/src/lxc/lxccontainer.h
> >> +++ b/src/lxc/lxccontainer.h
> >> @@ -209,7 +209,7 @@ struct lxc_container {
> >> *
> >> * \return \c true if container wants to be daemonised, else \c
> >> false.
> >> */
> >> - void (*want_daemonize)(struct lxc_container *c);
> >> + bool (*want_daemonize)(struct lxc_container *c, int state);
> >>
> >> /*!
> >> * \brief Determine whether container wishes all file descriptors
> >> @@ -220,7 +220,7 @@ struct lxc_container {
> >> * \return \c true if container wants all file descriptors closed,
> >> * else \c false.
> >> */
> >> - bool (*want_close_all_fds)(struct lxc_container *c);
> >> + bool (*want_close_all_fds)(struct lxc_container *c, int state);
> >>
> >> /*!
> >> * \brief Return current config file name.
> >> diff --git a/src/python-lxc/lxc.c b/src/python-lxc/lxc.c
> >> index b4f1da3..92d79f9 100644
> >> --- a/src/python-lxc/lxc.c
> >> +++ b/src/python-lxc/lxc.c
> >> @@ -1301,11 +1301,17 @@ Container_start(Container *self, PyObject *args,
> >> PyObject *kwds)
> >> }
> >>
> >> if (close_fds && close_fds == Py_True) {
> >> - self->container->want_close_all_fds(self->container);
> >> + self->container->want_close_all_fds(self->container, 1);
> >> + }
> >> + else {
> >> + self->container->want_close_all_fds(self->container, 0);
> >> }
> >>
> >> if (!daemonize || daemonize == Py_True) {
> >> - self->container->want_daemonize(self->container);
> >> + self->container->want_daemonize(self->container, 1);
> >> + }
> >> + else {
> >> + self->container->want_daemonize(self->container, 0);
> >> }
> >>
> >> if (self->container->start(self->container, init_useinit, init_args))
> >> diff --git a/src/tests/attach.c b/src/tests/attach.c
> >> index 57a4bdd..52bb9e9 100644
> >> --- a/src/tests/attach.c
> >> +++ b/src/tests/attach.c
> >> @@ -315,7 +315,7 @@ static struct lxc_container *test_ct_create(const char
> >> *lxcpath,
> >> if (lsm_enabled())
> >> test_attach_lsm_set_config(ct);
> >>
> >> - ct->want_daemonize(ct);
> >> + ct->want_daemonize(ct, 1);
> >> if (!ct->startl(ct, 0, NULL)) {
> >> TSTERR("starting container %s", name);
> >> goto out2;
> >> diff --git a/src/tests/cgpath.c b/src/tests/cgpath.c
> >> index 493b07a..cd4842b 100644
> >> --- a/src/tests/cgpath.c
> >> +++ b/src/tests/cgpath.c
> >> @@ -172,7 +172,7 @@ static int test_container(const char *lxcpath,
> >> goto out2;
> >> }
> >> c->load_config(c, NULL);
> >> - c->want_daemonize(c);
> >> + c->want_daemonize(c, 1);
> >> if (!c->startl(c, 0, NULL)) {
> >> TSTERR("starting container %s", name);
> >> goto out3;
> >> diff --git a/src/tests/concurrent.c b/src/tests/concurrent.c
> >> index 76fae87..d46e216 100644
> >> --- a/src/tests/concurrent.c
> >> +++ b/src/tests/concurrent.c
> >> @@ -88,7 +88,7 @@ static void do_function(void *arguments)
> >> }
> >> } else if(strcmp(args->mode, "start") == 0) {
> >> if (c->is_defined(c) && !c->is_running(c)) {
> >> - c->want_daemonize(c);
> >> + c->want_daemonize(c, 1);
> >> if (!c->start(c, false, NULL)) {
> >> fprintf(stderr, "Starting the container (%s)
> >> failed...\n", name);
> >> goto out;
> >> diff --git a/src/tests/console.c b/src/tests/console.c
> >> index c4cb3b2..e1045b9 100644
> >> --- a/src/tests/console.c
> >> +++ b/src/tests/console.c
> >> @@ -145,7 +145,7 @@ static int test_console(const char *lxcpath,
> >> c->load_config(c, NULL);
> >> c->set_config_item(c, "lxc.tty", TTYCNT_STR);
> >> c->save_config(c, NULL);
> >> - c->want_daemonize(c);
> >> + c->want_daemonize(c, 1);
> >> if (!c->startl(c, 0, NULL)) {
> >> TSTERR("starting container %s", name);
> >> goto out3;
> >> diff --git a/src/tests/containertests.c b/src/tests/containertests.c
> >> index e463e35..041526d 100644
> >> --- a/src/tests/containertests.c
> >> +++ b/src/tests/containertests.c
> >> @@ -225,7 +225,7 @@ int main(int argc, char *argv[])
> >> goto out;
> >>
> >> /* non-daemonized is tested in 'startone' */
> >> - c->want_daemonize(c);
> >> + c->want_daemonize(c, 1);
> >> if (!c->startl(c, 0, NULL, NULL)) {
> >> fprintf(stderr, "%d: %s failed to start daemonized\n",
> >> __LINE__, c->name);
> >> goto out;
> >> diff --git a/src/tests/createtest.c b/src/tests/createtest.c
> >> index c018458..473add1 100644
> >> --- a/src/tests/createtest.c
> >> +++ b/src/tests/createtest.c
> >> @@ -61,7 +61,7 @@ int main(int argc, char *argv[])
> >> }
> >>
> >> c->load_config(c, NULL);
> >> - c->want_daemonize(c);
> >> + c->want_daemonize(c, 1);
> >> if (!c->startl(c, 0, NULL)) {
> >> fprintf(stderr, "%d: failed to start %s\n", __LINE__,
> >> MYNAME);
> >> goto out;
> >> diff --git a/src/tests/shutdowntest.c b/src/tests/shutdowntest.c
> >> index c7070c1..5ce328b 100644
> >> --- a/src/tests/shutdowntest.c
> >> +++ b/src/tests/shutdowntest.c
> >> @@ -62,7 +62,7 @@ int main(int argc, char *argv[])
> >> }
> >>
> >> c->load_config(c, NULL);
> >> - c->want_daemonize(c);
> >> + c->want_daemonize(c, 1);
> >> if (!c->startl(c, 0, NULL)) {
> >> fprintf(stderr, "%d: failed to start %s\n", __LINE__,
> >> MYNAME);
> >> goto out;
> >> --
> >> 1.8.4.4
> >>
> >>
> >> ------------------------------------------------------------------------------
> >> Rapidly troubleshoot problems before they affect your business. Most IT
> >> organizations don't have a clear picture of how application performance
> >> affects their revenue. With AppDynamics, you get 100% visibility into your
> >> Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics
> >> Pro!
> >> http://pubads.g.doubleclick.net/gampad/clk?id=84349351&iu=/4140/ostg.clktrk
> >> _______________________________________________
> >> Lxc-devel mailing list
> >> Lxc-devel@lists.sourceforge.net
> >> https://lists.sourceforge.net/lists/listinfo/lxc-devel
> >
> > ------------------------------------------------------------------------------
> > Rapidly troubleshoot problems before they affect your business. Most IT
> > organizations don't have a clear picture of how application performance
> > affects their revenue. With AppDynamics, you get 100% visibility into your
> > Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics
> > Pro!
> > http://pubads.g.doubleclick.net/gampad/clk?id=84349351&iu=/4140/ostg.clktrk
> > _______________________________________________
> > Lxc-devel mailing list
> > Lxc-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/lxc-devel
>
>
>
> --
> S.Çağlar Onur <cag...@10ur.org>
--
Stéphane Graber
Ubuntu developer
http://www.ubuntu.com
signature.asc
Description: Digital signature
------------------------------------------------------------------------------ Rapidly troubleshoot problems before they affect your business. Most IT organizations don't have a clear picture of how application performance affects their revenue. With AppDynamics, you get 100% visibility into your Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro! http://pubads.g.doubleclick.net/gampad/clk?id=84349351&iu=/4140/ostg.clktrk
_______________________________________________ Lxc-devel mailing list Lxc-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/lxc-devel
