On Fri, 12 Apr 2013 17:32:49 -0500
Serge Hallyn <serge.hal...@ubuntu.com> wrote:
> Quoting Dwight Engen (dwight.en...@oracle.com):
> > On Fri, 12 Apr 2013 15:36:03 -0500
> > Serge Hallyn <serge.hal...@ubuntu.com> wrote:
> >
> > > Quoting Dwight Engen (dwight.en...@oracle.com):
> > > > pclose returns the exit status from wait, we need to check that
> > > > to see if the script itself failed or not. Tested a script that
> > > > returned 0, 1, and also one that did a sleep and then was
> > > > killed by a signal (abnormal termination).
> > > >
> > > > Signed-off-by: Dwight Engen <dwight.en...@oracle.com>
> > > > ---
> > > > src/lxc/conf.c | 7 +++++--
> > > > 1 file changed, 5 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/src/lxc/conf.c b/src/lxc/conf.c
> > > > index 6b3f318..07529da 100644
> > > > --- a/src/lxc/conf.c
> > > > +++ b/src/lxc/conf.c
> > > > @@ -299,6 +299,7 @@ static int run_buffer(char *buffer)
> > > > {
> > > > FILE *f;
> > > > char *output;
> > > > + int ret;
> > > >
> > > > f = popen(buffer, "r");
> > > > if (!f) {
> > > > @@ -317,8 +318,10 @@ static int run_buffer(char *buffer)
> > > >
> > > > free(output);
> > > >
> > > > - if (pclose(f) == -1) {
> > > > - SYSERROR("Script exited on error");
> > > > + ret = pclose(f);
> > > > + if (ret < 0 || !WIFEXITED(ret) || WEXITSTATUS(ret) !=
> > > > 0) {
> > > > + SYSERROR("Script exited:%snormally with:%d",
> > > > + WIFEXITED(ret) ? "" : "ab",
> > > > WEXITSTATUS(ret));
> > >
> > > I don't know pclose all that well... but the manpage says
> > > The pclose() function returns -1 if wait4(2) returns an
> > > error, or some other error is detected.
> > >
> > > You're treating it as though it contains the status filled in
> > > by wait4 et al. Is that valid?
> > >
> > > not nack'ing, just not clear on what we can do with ret :)
> >
> > Yes RETURN VALUE section of the man page confused me as well upon
> > first reading :) Then I found this
> >
> > https://groups.google.com/forum/?fromgroups=#!topic/utah-c/g4MpZVnJ7es
> >
> > which was helpful, but more importantly I did some testing with it
> > to make sure it actually was the wait status. I did this by writing
> > a pre-start hook script that returned different values and verified
> > what I saw in the lxc log (which is why I added to that SYSERROR).
> >
> > I then read the full pclose man page and at the end of the
> > DESCRIPTION section it says "The pclose() function waits for
> > the associated process to terminate and returns the exit status of
> > the command as returned by wait4(2)." so I'm pretty sure it does
> > intend to return the exit status as opposed to code.
>
> Thanks, I get it :)
>
> My other question then is about (a) other wai4 errors or (b) any other
> error that might happen. pclose will return -1 then, so should your
> error message say 'there was some error' on -1, and only say "the
> scrip exited with WEXITSTATUS(ret)" if ret != -1? Else it'll just
> report 255, but it could mislead an admin trying to debug his
> hooks.
Yeah that is a good idea, I think the -1 case is not the norm but I
agree that we shouldn't just log 255 if it does happen. I'll fix
it up and resubmit.
> -serge
------------------------------------------------------------------------------
Precog is a next-generation analytics platform capable of advanced
analytics on semi-structured data. The platform includes APIs for building
apps and a phenomenal toolset for data science. Developers can use
our toolset for easy data analysis & visualization. Get a free account!
http://www2.precog.com/precogplatform/slashdotnewsletter
_______________________________________________
Lxc-devel mailing list
Lxc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/lxc-devel
