[lxc-devel] API errors

[S.Çağlar Onur]

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;


diff --git a/src/lxc/lxccontainer.h b/src/lxc/lxccontainer.h
> index 20ab8e8..b3c9641 100644
> --- a/src/lxc/lxccontainer.h
> +++ b/src/lxc/lxccontainer.h
> @@ -49,9 +49,16 @@ struct lxc_container {
>         int numthreads; /* protected by privlock. */
>         struct lxc_conf *lxc_conf; // maybe we'll just want the whole
> lxc_handler?
>
> -       // public fields
> -       char *error_string;
> +       // error stack - protected by privlock
> +       // These can be accessed using get_errors() and clear_errors()
> +       // below.
> +       char **error_string;
>         int error_num;
> +       bool oom;  // true if there were more errors but no free memory;
> +       char oom_error[MAXPATHLEN];  // if we oomed, last error goes in
> here
> +
> +       // public fields
> +       bool
>         int daemonize;
>
>         char *config_path;
> @@ -229,6 +236,18 @@ struct lxc_container {
>          * and the caller may not access it.  Return true otherwise.
>          */
>         bool (*may_control)(struct lxc_container *c);
> +
> +       /*
> +        * Get stack of errors - one per line
> +        * @output: preallocated string into which to print the errors
> +        * @inlen: the length of passed-in buffer
> +        * @oomed: will be set to 1 if we also oomed, in which case the
> oom msg
> +        *         is also a part of @output.
> +        *
> +        * Returns the needed length (including terminating \0)
> +        */
> +       int (*get_errors)(struct lxc_container *c, char *output, int
> inlen, int *oomed);
> +       void (*clear_errors)();
>  };
>
>  struct lxc_snapshot {
> @@ -248,6 +267,18 @@ const char *lxc_get_default_lvm_vg(void);
>  const char *lxc_get_default_zfs_root(void);
>  const char *lxc_get_version(void);
>
> +/*
> + * Add another error to c->error_string, bump error_num.  If we run
> + * out of memory and c->oom is not yet set, then set c->oom to 1 and
> + * put the message into c->oom_error.
> + */
> +int DO_PUSH_ERR(struct lxc_container *c, char *fmt, ...);
> +
> +/*
> + * DO_PUSH_ERR_locked is same as DO_PUSH_ERR but will not call
> process_lock().
> + */
> +int DO_PUSH_ERR_locked(struct lxc_container *c, char *fmt, ...);
> +
>  #if 0
>  char ** lxc_get_valid_keys();
>  char ** lxc_get_valid_values(char *key);
>



-- 
S.Çağlar Onur <caglar at 10ur.org>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20131005/6e016b9e/attachment.html>