[lxc-devel] [PATCH] fix checking hook script exit code

[Serge Hallyn]

Quoting Dwight Engen (dwight.engen at oracle.com):
> On Fri, 12 Apr 2013 15:36:03 -0500
> Serge Hallyn <serge.hallyn at ubuntu.com> wrote:
> 
> > Quoting Dwight Engen (dwight.engen at 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.engen at 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.

-serge