<div dir="ltr">Hi Serge,<div class="gmail_extra"><br><br><div class="gmail_quote">On Sat, Oct 5, 2013 at 12:27 AM, Serge Hallyn <span dir="ltr"><<a href="mailto:serge.hallyn@ubuntu.com" target="_blank">serge.hallyn@ubuntu.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class=""><div class="h5">Quoting Serge Hallyn (<a href="mailto:serge.hallyn@ubuntu.com">serge.hallyn@ubuntu.com</a>):<br>
> Quoting S.Çağlar Onur (<a href="mailto:caglar@10ur.org">caglar@10ur.org</a>):<br>
> > Hi,<br>
> ><br>
> > src/lxc/lxccontainer.h contains two public field named error_string and<br>
> > error_num. If I'm not missing anything no one seems to be using them.<br>
> ><br>
> > What was the intention? Should API calls set those and stop using macros<br>
> > like ERROR and SYSERROR?<br>
><br>
> That is the idea. It's not yet clear to me how to best track that.<br>
> Maybe it should become an array of strings, so that each stack layer<br>
> moving up and seeing an error happened can give its own input on what<br>
> it was doing. Then when lxc-start fails, we can see something like<br>
><br>
> [0] failed to create container<br>
> [1] failed to setup network<br>
> [2] failed to create veth<br>
> [3] failed to pass veth into network namespce<br>
><br>
> And then yes, ERROR and SYSERROR should be reserved for the actual<br>
> programs, and should spit out error stacks as well.<br>
<br>
</div></div>So I'm thinking an API like the below. Much in bdev.c for instance can<br>
be converted to PUSH_ERR. Although all the places where we have<br>
fork()ed obviouslly cannot. There they'll have to keep doing ERROR(),<br>
and the parent can point to the logfile for further info.<br></blockquote><div><br></div><div>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 <span style="color:rgb(0,0,0)">get_error call as well, which removes the last error from the stack and returns to the caller?</span></div>
<div><br></div><div><div>diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c<br></div><div>index 02126b2..7390cac 100644</div><div>--- a/src/lxc/lxccontainer.c</div><div>+++ b/src/lxc/lxccontainer.c</div><div>@@ -510,6 +510,8 @@ static bool lxcapi_start(struct lxc_container *c, int useinit, char * const argv</div>
<div> if (!c->lxc_conf)</div><div> return false;</div><div><br></div><div>+ c->clear_errors();</div><div>+</div><div> if ((ret = ongoing_create(c)) < 0) {</div><div> ERROR("Error checking for incomplete creation");</div>
<div> return false;</div></div><div> </div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
diff --git a/src/lxc/lxccontainer.h b/src/lxc/lxccontainer.h<br>
index 20ab8e8..b3c9641 100644<br>
--- a/src/lxc/lxccontainer.h<br>
+++ b/src/lxc/lxccontainer.h<br>
@@ -49,9 +49,16 @@ struct lxc_container {<br>
int numthreads; /* protected by privlock. */<br>
struct lxc_conf *lxc_conf; // maybe we'll just want the whole lxc_handler?<br>
<br>
- // public fields<br>
- char *error_string;<br>
+ // error stack - protected by privlock<br>
+ // These can be accessed using get_errors() and clear_errors()<br>
+ // below.<br>
+ char **error_string;<br>
int error_num;<br>
+ bool oom; // true if there were more errors but no free memory;<br>
+ char oom_error[MAXPATHLEN]; // if we oomed, last error goes in here<br>
+<br>
+ // public fields<br>
+ bool<br>
int daemonize;<br>
<br>
char *config_path;<br>
@@ -229,6 +236,18 @@ struct lxc_container {<br>
* and the caller may not access it. Return true otherwise.<br>
*/<br>
bool (*may_control)(struct lxc_container *c);<br>
+<br>
+ /*<br>
+ * Get stack of errors - one per line<br>
+ * @output: preallocated string into which to print the errors<br>
+ * @inlen: the length of passed-in buffer<br>
+ * @oomed: will be set to 1 if we also oomed, in which case the oom msg<br>
+ * is also a part of @output.<br>
+ *<br>
+ * Returns the needed length (including terminating \0)<br>
+ */<br>
+ int (*get_errors)(struct lxc_container *c, char *output, int inlen, int *oomed);<br>
+ void (*clear_errors)();<br>
};<br>
<br>
struct lxc_snapshot {<br>
@@ -248,6 +267,18 @@ const char *lxc_get_default_lvm_vg(void);<br>
const char *lxc_get_default_zfs_root(void);<br>
const char *lxc_get_version(void);<br>
<br>
+/*<br>
+ * Add another error to c->error_string, bump error_num. If we run<br>
+ * out of memory and c->oom is not yet set, then set c->oom to 1 and<br>
+ * put the message into c->oom_error.<br>
+ */<br>
+int DO_PUSH_ERR(struct lxc_container *c, char *fmt, ...);<br>
+<br>
+/*<br>
+ * DO_PUSH_ERR_locked is same as DO_PUSH_ERR but will not call process_lock().<br>
+ */<br>
+int DO_PUSH_ERR_locked(struct lxc_container *c, char *fmt, ...);<br>
+<br>
#if 0<br>
char ** lxc_get_valid_keys();<br>
char ** lxc_get_valid_values(char *key);<br>
</blockquote></div><br><br clear="all"><div><br></div>-- <br>S.Çağlar Onur <<a href="mailto:caglar@10ur.org">caglar@10ur.org</a>>
</div></div>