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

Dwight Engen dwight.engen at oracle.com
Sat Apr 13 00:06:24 UTC 2013


On Fri, 12 Apr 2013 17:32:49 -0500
Serge Hallyn <serge.hallyn at ubuntu.com> wrote:

> 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.

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




More information about the lxc-devel mailing list