[lxc-devel] [PATCH] python3: Allow setting daemonize and close_fds

[Stéphane Graber]

On Fri, Nov 29, 2013 at 09:27:15AM -0600, Serge Hallyn wrote:
> Quoting Stéphane Graber (stgraber at ubuntu.com):
> > This extends the list of arguments of start() allowing the user to
> > request the container be started in the foreground and have control on
> > whether fds will be closed or not (daemonize=True implies that too).
> > 
> > One problem at the moment however is that while we have functions to set
> > close_fds and daemonize in the API, we don't have functions to unset
> > those flags, so those new parameters will only work on the initial call
> > to start() any further call will use the values of the previous one.
> > 
> > I think it'd make sense to change lxcapi slightly to have daemonize and
> > close_fds offer a similar interface, both returning booleans and both
> > accepting a value as a parameter so API users can set the value they
> > want.
> 
> What would be the point in checking the value as opposed to simply
> setting the one you want?
> 
> If unsetting is all we need, we could just add a boolean argument to
> want_damonize and want_close_all_fds.  If there is a good reason to
> be able to check the values, then we can either add a get_daemonize,
> or make the second argument to want_daemonize an int, where -1 means
> unset, 1 means set, and 0 means just give me the return value.
> 
> Or maybe we want to just add new api fns so as not to change the
> existing api?  I'm feeling indecisive.

I don't want to check the values but I want to get error reporting.

Currently want_daemonize doesn't return anything so I don't know whether
the setting was save or not. want_close_all_fds solves that issue by
returning a bool with true meaning that the value was saved and false
meaning that something went wrong.


Considering that we haven't commited to a stable API yet, I'd think that
just adding a second argument to both functions to pass the state we
want would be perfectly fine and it'll be trivial to update any code
using that.

If you're happy with that, I'll send a patch later today doing just that.

> 
> > Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
> 
> Acked-by: Serge E. Hallyn <serge.hallyn at ubuntu.com>
> 
> > ---
> >  src/python-lxc/lxc.c | 33 +++++++++++++++++++++++++--------
> >  1 file changed, 25 insertions(+), 8 deletions(-)
> > 
> > diff --git a/src/python-lxc/lxc.c b/src/python-lxc/lxc.c
> > index f850a3d..5a20ff4 100644
> > --- a/src/python-lxc/lxc.c
> > +++ b/src/python-lxc/lxc.c
> > @@ -1221,13 +1221,21 @@ Container_snapshot_restore(Container *self, PyObject *args, PyObject *kwds)
> >  static PyObject *
> >  Container_start(Container *self, PyObject *args, PyObject *kwds)
> >  {
> > +    PyObject *useinit = NULL;
> > +    PyObject *daemonize = NULL;
> > +    PyObject *close_fds = NULL;
> > +
> > +    PyObject *vargs = NULL;
> >      char** init_args = {NULL};
> > -    PyObject *useinit = NULL, *retval = NULL, *vargs = NULL;
> > +
> > +    PyObject *retval = NULL;
> >      int init_useinit = 0, i = 0;
> > -    static char *kwlist[] = {"useinit", "cmd", NULL};
> > +    static char *kwlist[] = {"useinit", "daemonize", "close_fds",
> > +                             "cmd", NULL};
> >  
> > -    if (! PyArg_ParseTupleAndKeywords(args, kwds, "|OO", kwlist,
> > -                                      &useinit, &vargs))
> > +    if (! PyArg_ParseTupleAndKeywords(args, kwds, "|OOOO", kwlist,
> > +                                      &useinit, &daemonize, &close_fds,
> > +                                      &vargs))
> >          return NULL;
> >  
> >      if (useinit && useinit == Py_True) {
> > @@ -1241,7 +1249,13 @@ Container_start(Container *self, PyObject *args, PyObject *kwds)
> >          }
> >      }
> >  
> > -    self->container->want_daemonize(self->container);
> > +    if (close_fds && close_fds == Py_True) {
> > +        self->container->want_close_all_fds(self->container);
> > +    }
> > +
> > +    if (!daemonize || daemonize == Py_True) {
> > +        self->container->want_daemonize(self->container);
> > +    }
> >  
> >      if (self->container->start(self->container, init_useinit, init_args))
> >          retval = Py_True;
> > @@ -1519,10 +1533,13 @@ static PyMethodDef Container_methods[] = {
> >      },
> >      {"start", (PyCFunction)Container_start,
> >       METH_VARARGS|METH_KEYWORDS,
> > -     "start(useinit = False, cmd = (,)) -> boolean\n"
> > +     "start(useinit = False, daemonize=True, close_fds=False, "
> > +     "cmd = (,)) -> boolean\n"
> >       "\n"
> > -     "Start the container, optionally using lxc-init and "
> > -     "an alternate init command, then returns its return code."
> > +     "Start the container, return True on success.\n"
> > +     "When set useinit will make LXC use lxc-init to start the container.\n"
> > +     "The container can be started in the foreground with daemonize=False.\n"
> > +     "All fds may also be closed by passing close_fds=True."
> >      },
> >      {"stop", (PyCFunction)Container_stop,
> >       METH_NOARGS,
> > -- 
> > 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 at lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/lxc-devel

-- 
Stéphane Graber
Ubuntu developer
http://www.ubuntu.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20131129/8e7f0f64/attachment.pgp>