From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH 11/29] libxl: events: Make libxl__async_exec_* pass caller an rc Date: Tue, 24 Mar 2015 11:20:13 +0000 Message-ID: <1427196013.21742.346.camel@citrix.com> References: <1423599016-32639-1-git-send-email-ian.jackson@eu.citrix.com> <1423599016-32639-12-git-send-email-ian.jackson@eu.citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1423599016-32639-12-git-send-email-ian.jackson@eu.citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Jackson Cc: xen-devel@lists.xensource.com, Euan Harris List-Id: xen-devel@lists.xenproject.org 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 > --- > 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.