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: Wed, 1 Apr 2015 10:29:52 +0100 Message-ID: <1427880592.2115.249.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> <1427196013.21742.346.camel@citrix.com> <21786.58224.721010.592110@mariner.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <21786.58224.721010.592110@mariner.uk.xensource.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-03-31 at 19:12 +0100, Ian Jackson wrote: > Ian Campbell writes ("Re: [Xen-devel] [PATCH 11/29] libxl: events: Make libxl__async_exec_* pass caller an rc"): > > On Tue, 2015-02-10 at 20:09 +0000, Ian Jackson wrote: > > > 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? > > No, there is no more to come. I find it clearer this way but I don't > mind changing it. No it's fine if you don't want to. > > > + 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? > > status is definitely valid but maybe uninteresting, as stated in the > comment. I think I initially parsed it as "real exit status, maybe", which isn't really what it says... > Would it help to add something about status to the third row of the > little table bit at the left ? Perhaps, or perhaps: s/ maybe//; s/child)/child, and therefore likely to be uninteresting)/ ? In any case now that I've read it correctly: Acked-by: Ian Campbell But if you want to clarify any further please go ahead and retain the ack. Ian.