From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH 3/3] libxl: Domain destroy: fork Date: Wed, 18 Mar 2015 12:27:20 +0000 Message-ID: <1426681640.18247.359.camel@citrix.com> References: <1426606259-9692-1-git-send-email-jfehlig@suse.com> <1426606259-9692-4-git-send-email-jfehlig@suse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1426606259-9692-4-git-send-email-jfehlig@suse.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: Jim Fehlig Cc: wei.liu2@citrix.com, Ian.Jackson@eu.citrix.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On Tue, 2015-03-17 at 09:30 -0600, Jim Fehlig wrote: > From: Ian Jackson > > Call xc_domain_destroy in a subprocess. That allows us to do so > asynchronously, rather than blocking the whole process calling libxl. > > The changes in detail: > > * Provide an libxl__ev_child in libxl__domain_destroy_state, and > initialise it in libxl__domain_destroy. There is no possibility > to `clean up' a libxl__ev_child, but there need to clean it up, as ^is no ? > the control flow ensures that we only continue after the child has > exited. > > * Call libxl__ev_child_fork at the right point and put the call to > xc_domain_destroy and associated logging in the child. (The child > opens a new xenctrl handle because we mustn't use the parent's.) Do I read it correctly that this is arranging to use the same logger as the parent was, IOW the log messages will still go somewhere the application considers useful? > > * Consequently, the success return path from domain_destroy_domid_cb > no longer calls dis->callback. Instead it simply returns. > > * We plumb the errorno value through the child's exit status, if it > fits. This means we normally do the logging only in the parent. (This makes my question above moot I think). > * Incidentally, we fix the bug that we were treating the return value > from xc_domain_destroy as an errno value when in fact it is a > return value from do_domctl (in this case, 0 or -1 setting errno). > > Signed-off-by: Ian Jackson > Reviewed-by: Jim Fehlig > Tested-by: Jim Fehlig > --- > tools/libxl/libxl.c | 57 ++++++++++++++++++++++++++++++++++++++++---- > tools/libxl/libxl_internal.h | 1 + > 2 files changed, 53 insertions(+), 5 deletions(-) > > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c > index b6541d4..b43db1a 100644 > --- a/tools/libxl/libxl.c > +++ b/tools/libxl/libxl.c > @@ -1481,6 +1481,10 @@ static void domain_destroy_callback(libxl__egc *egc, > static void destroy_finish_check(libxl__egc *egc, > libxl__domain_destroy_state *dds); > > +static void domain_destroy_domid_cb(libxl__egc *egc, > + libxl__ev_child *destroyer, > + pid_t pid, int status); This seems misplaced, doesn't it want to go before libxl__destroy_domid just after the prototype of devices_destroy_cb, which is where it lives in the sequence? Ian.