[systemd-devel] [PATCH] nspawn: When exiting with an error, make the error code meaningful.

[Luke Shumaker]

At Sun, 29 Jun 2014 12:31:13 +0100,
Djalal Harouni wrote:
> On Sat, Jun 28, 2014 at 12:09:56PM -0400, Luke Shumaker wrote:
> > This is accomplished by having wait_for_container() return a positive error
> > code when we would like that error code to make its way to the user.  This
> > is at odds with the CODING_STYLE rule that error codes should be returned
> > as negative values.
> This is not odd, we already do that!
> 
> When I did convert to wait_for_container(), I remember I checked all
> calls to wait_for_terminate() to see what others do, and there was the:
> src/shared/util.c:wait_for_terminate_and_warn()
> 
> Which also returns the positive 'status.si_status' code to caller, and it
> is used in all places...

I missed that wait_for_terminate_and_warn() also did that!

With that in consideration, shouldn't the check of the return value
from wait_for_terminate_and_warn() at src/quotacheck/quotacheck.c:110
be '== 0' instead of '>= 0'?

> So if you are able to send a v2 of this patch, here are some comments:

I'm putting one together now; thanks for the feedback!

> > + * Return values:
> > + * < 0 : The container was terminated by a signal, or failed for an
> > + *       unknown reason.  No change is made to the container argument.
> wait_for_container() failed to get the state of the container, the
> container was terminated by a signal or failed for an unknown reason.

You mean wait_for_terminate()?

> A minor thing:
> 
> in wait_for_container() could you add please a log_warning() if
> wait_for_terminate() fails, as it is done in
> wait_for_terminate_and_warn().

Yeah, I'll add it as a separate commit.

> Thank you for the report and the patch!

Thank you for taking the time to review it!

Happy hacking,
~ Luke Shumaker