Re: [Xen-devel] [PATCH 11/29] libxl: events: Make libxl__async_exec_* pass caller an rc

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Tue, 2015-02-10 at 20:09 +0000, Ian Jackson wrote:
> The internal user of libxl__async_exec_start et al now gets an rc as
> well as the process's exit status.
> 
> For now this is always either 0 or ERROR_FAIL, but with ao
> cancellation this will possibly be CANCELLED or TIMEDOUT too.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
> ---
> v2: New patch due to rebause; v1 had changes to device_hotplug_*
>      scripts instead.
>     Callback now gets unambiguous information about error situation:
>      previously, if only thing that went wrong was that child died
>      badly, rc would be FAILED, which was unambigously; now rc=0.
>     Add a comment document the meaning of the rc and status parameters
>      to the callback.
> ---
>  tools/libxl/libxl_aoutils.c         |    9 ++++++---
>  tools/libxl/libxl_device.c          |   13 +++++++++----
>  tools/libxl/libxl_internal.h        |   11 ++++++++++-
>  tools/libxl/libxl_netbuffer.c       |   19 ++++++++++---------
>  tools/libxl/libxl_remus_disk_drbd.c |    8 +++++---
>  5 files changed, 40 insertions(+), 20 deletions(-)
> 
> diff --git a/tools/libxl/libxl_aoutils.c b/tools/libxl/libxl_aoutils.c
> index 754e2d1..891cdb8 100644
> --- a/tools/libxl/libxl_aoutils.c
> +++ b/tools/libxl/libxl_aoutils.c
> @@ -483,11 +483,12 @@ static void async_exec_done(libxl__egc *egc,
>      libxl__ev_time_deregister(gc, &aes->time);
>  
>      if (status) {
> -        libxl_report_child_exitstatus(CTX, LIBXL__LOG_ERROR,
> -                                      aes->what, pid, status);
> +        if (!aes->rc)

Could be one "if (status && !aes->rc)", unless perhaps there is more
code to come in this block?

> +                        libxl__async_exec_state *aes, int rc, int status);
> +/*
> + * Meaning of status and rc:
> + *  rc==0, status==0    all went well
> + *  rc==0, status!=0    everything OK except child exited nonzero (logged)
> + *  rc!=0               something else went wrong (status is real
> + *                       exit status, maybe reflecting SIGKILL if aes
> + *                       code killed the child).  Logged unless CANCELLED.

I'm unclear on whether status is valid in this third case or not. I
think you are saying that it is (probably?) valid but if rc!=0 the
caller likely doesn't actually care what it is?

Everything else looks fine to me.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel