[lxc-devel] API errors

[Serge Hallyn]

Quoting S.Çağlar Onur (caglar at 10ur.org):
> Hi Serge,
> 
> 
> On Sat, Oct 5, 2013 at 12:27 AM, Serge Hallyn <serge.hallyn at ubuntu.com>wrote:
> 
> > Quoting Serge Hallyn (serge.hallyn at ubuntu.com):
> > > Quoting S.Çağlar Onur (caglar at 10ur.org):
> > > > Hi,
> > > >
> > > > src/lxc/lxccontainer.h contains two public field named error_string and
> > > > error_num. If I'm not missing anything no one seems to be using them.
> > > >
> > > > What was the intention? Should API calls set those and stop using
> > macros
> > > > like ERROR and SYSERROR?
> > >
> > > That is the idea.  It's not yet clear to me how to best track that.
> > > Maybe it should become an array of strings, so that each stack layer
> > > moving up and seeing an error happened can give its own input on what
> > > it was doing.  Then when lxc-start fails, we can see something like
> > >
> > >       [0] failed to create container
> > >       [1] failed to setup network
> > >       [2] failed to create veth
> > >       [3] failed to pass veth into network namespce
> > >
> > > And then yes, ERROR and SYSERROR should be reserved for the actual
> > > programs, and should spit out error stacks as well.
> >
> > So I'm thinking an API like the below.  Much in bdev.c for instance can
> > be converted to PUSH_ERR.  Although all the places where we have
> > fork()ed obviouslly cannot.  There they'll have to keep doing ERROR(),
> > and the parent can point to the logfile for further info.
> >
> 
> I'm imagining API methods themselves are going to call the clear_errors
> (i.e  something like below) and the only reason to export that is to
> provide an alternative way to manage the stack? If that's the case,
> wouldn't be helpful if there will be a get_error call as well, which
> removes the last error from the stack and returns to the caller?
> 
> diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> index 02126b2..7390cac 100644
> --- a/src/lxc/lxccontainer.c
> +++ b/src/lxc/lxccontainer.c
> @@ -510,6 +510,8 @@ static bool lxcapi_start(struct lxc_container *c, int
> useinit, char * const argv
>         if (!c->lxc_conf)
>                 return false;
> 
> +       c->clear_errors();
> +
>         if ((ret = ongoing_create(c)) < 0) {
>                 ERROR("Error checking for incomplete creation");
>                 return false;

Thinking about it more, I'm less sure about sticking the
errors in the lxc_container struct.  That's fine for
things like start and freeze, but not for things like
freeze or even a failed lxc_container_new().

I thought maybe it should go in thread-local storage.  But
then if we end up with a crash before the caller has a
chance to write the info out to disk, we lose the info.

We could use ipc shared memory, so then we could actually
go in after the fact with a lxc-diagnose tool and get the
latest error messages for a particular (lxcpath,lxcname)
tuple.  That actually seems to have the fewest downsides,
and may address all our requirements.

-serge