From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wen Congyang Subject: Re: [PATCH 10/29] libxl: events: Make timeout and async exec setup take an ao, not a gc Date: Wed, 11 Feb 2015 09:04:43 +0800 Message-ID: <54DAAAAB.5040109@cn.fujitsu.com> References: <1423599016-32639-1-git-send-email-ian.jackson@eu.citrix.com> <1423599016-32639-11-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-11-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 , xen-devel@lists.xensource.com Cc: Yang Hongyang , Euan Harris , Lai Jiangshan List-Id: xen-devel@lists.xenproject.org On 02/11/2015 04:09 AM, Ian Jackson wrote: > Change the timeout setup functions to take a libxl__ao, not a > libxl__gc. This is going to be needed for ao cancellation, because > timeouts are going to be a main hook for ao cancellation - so the > timeouts need to be associated with an ao. > > This means that timeouts can only occur as part of a long-running > libxl function (but this is of course correct, as libxl shouldn't have > any global timeouts, and indeed all the call sites have an ao). > > Also remove the gc parameter from libxl__async_exec_start. It can > just use the gc from the ao supplied in the aes. > > All the callers follow the obvious patterns and therefore supply the > ao's gc to libxl__async_exec_start and the timeout setup functions. > There is therefore no functional change in this patch. > > Signed-off-by: Ian Jackson > CC: Yang Hongyang > CC: Wen Congyang > CC: Lai Jiangshan libxl__async_exec_start() related modifications look fine to me. > --- > v2: This patch split off from "Permit timeouts to signal cancellation". > Rebased; consequently, deal with libxl__async_exec_start. > CC'd authors of the libxl__async_exec_* functions. > --- > tools/libxl/libxl_aoutils.c | 8 +++++--- > tools/libxl/libxl_device.c | 4 ++-- > tools/libxl/libxl_dom.c | 8 ++++---- > tools/libxl/libxl_event.c | 6 ++++-- > tools/libxl/libxl_internal.h | 6 +++--- > tools/libxl/libxl_remus_disk_drbd.c | 2 +- > tools/libxl/libxl_test_timedereg.c | 9 +++++---- > 7 files changed, 24 insertions(+), 19 deletions(-) > > diff --git a/tools/libxl/libxl_aoutils.c b/tools/libxl/libxl_aoutils.c > index 44dc222..754e2d1 100644 > --- a/tools/libxl/libxl_aoutils.c > +++ b/tools/libxl/libxl_aoutils.c > @@ -46,7 +46,7 @@ int libxl__xswait_start(libxl__gc *gc, libxl__xswait_state *xswa) > { > int rc; > > - rc = libxl__ev_time_register_rel(gc, &xswa->time_ev, > + rc = libxl__ev_time_register_rel(xswa->ao, &xswa->time_ev, > xswait_timeout_callback, xswa->timeout_ms); > if (rc) goto err; > > @@ -496,16 +496,18 @@ void libxl__async_exec_init(libxl__async_exec_state *aes) > libxl__ev_child_init(&aes->child); > } > > -int libxl__async_exec_start(libxl__gc *gc, libxl__async_exec_state *aes) > +int libxl__async_exec_start(libxl__async_exec_state *aes) > { > pid_t pid; > > /* Convenience aliases */ > + libxl__ao *ao = aes->ao; > + AO_GC; > libxl__ev_child *const child = &aes->child; > char ** const args = aes->args; > > /* Set execution timeout */ > - if (libxl__ev_time_register_rel(gc, &aes->time, > + if (libxl__ev_time_register_rel(ao, &aes->time, > async_exec_timeout, > aes->timeout_ms)) { > LOG(ERROR, "unable to register timeout for executing: %s", aes->what); > diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c > index 0455134..c80749f 100644 > --- a/tools/libxl/libxl_device.c > +++ b/tools/libxl/libxl_device.c > @@ -808,7 +808,7 @@ void libxl__initiate_device_remove(libxl__egc *egc, > * TODO: 4.2 Bodge due to QEMU, see comment on top of > * libxl__initiate_device_remove in libxl_internal.h > */ > - rc = libxl__ev_time_register_rel(gc, &aodev->timeout, > + rc = libxl__ev_time_register_rel(ao, &aodev->timeout, > device_qemu_timeout, > LIBXL_QEMU_BODGE_TIMEOUT * 1000); > if (rc) { > @@ -1034,7 +1034,7 @@ static void device_hotplug(libxl__egc *egc, libxl__ao_device *aodev) > aes->stdfds[1] = 2; > aes->stdfds[2] = -1; > > - rc = libxl__async_exec_start(gc, aes); > + rc = libxl__async_exec_start(aes); > if (rc) > goto out; > > diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c > index 90877d6..e292cb3 100644 > --- a/tools/libxl/libxl_dom.c > +++ b/tools/libxl/libxl_dom.c > @@ -980,7 +980,7 @@ static void domain_suspend_switch_qemu_xen_traditional_logdirty > switch_logdirty_xswatch, lds->ret_path); > if (rc) goto out; > > - rc = libxl__ev_time_register_rel(gc, &lds->timeout, > + rc = libxl__ev_time_register_rel(ao, &lds->timeout, > switch_logdirty_timeout, 10*1000); > if (rc) goto out; > > @@ -1260,7 +1260,7 @@ static void domain_suspend_callback_common(libxl__egc *egc, > rc = libxl__ev_evtchn_wait(gc, &dss->guest_evtchn); > if (rc) goto err; > > - rc = libxl__ev_time_register_rel(gc, &dss->guest_timeout, > + rc = libxl__ev_time_register_rel(ao, &dss->guest_timeout, > suspend_common_wait_guest_timeout, > 60*1000); > if (rc) goto err; > @@ -1391,7 +1391,7 @@ static void domain_suspend_common_wait_guest(libxl__egc *egc, > "@releaseDomain"); > if (rc) goto err; > > - rc = libxl__ev_time_register_rel(gc, &dss->guest_timeout, > + rc = libxl__ev_time_register_rel(ao, &dss->guest_timeout, > suspend_common_wait_guest_timeout, > 60*1000); > if (rc) goto err; > @@ -1751,7 +1751,7 @@ static void remus_devices_commit_cb(libxl__egc *egc, > */ > > /* Set checkpoint interval timeout */ > - rc = libxl__ev_time_register_rel(gc, &dss->checkpoint_timeout, > + rc = libxl__ev_time_register_rel(ao, &dss->checkpoint_timeout, > remus_next_checkpoint, > dss->interval); > > diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c > index fb6daeb..1a97cf8 100644 > --- a/tools/libxl/libxl_event.c > +++ b/tools/libxl/libxl_event.c > @@ -309,10 +309,11 @@ static void time_done_debug(libxl__gc *gc, const char *func, > #endif > } > > -int libxl__ev_time_register_abs(libxl__gc *gc, libxl__ev_time *ev, > +int libxl__ev_time_register_abs(libxl__ao *ao, libxl__ev_time *ev, > libxl__ev_time_callback *func, > struct timeval absolute) > { > + AO_GC; > int rc; > > CTX_LOCK; > @@ -333,10 +334,11 @@ int libxl__ev_time_register_abs(libxl__gc *gc, libxl__ev_time *ev, > } > > > -int libxl__ev_time_register_rel(libxl__gc *gc, libxl__ev_time *ev, > +int libxl__ev_time_register_rel(libxl__ao *ao, libxl__ev_time *ev, > libxl__ev_time_callback *func, > int milliseconds /* as for poll(2) */) > { > + AO_GC; > struct timeval absolute; > int rc; > > diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h > index 6bb208c..b615fc5 100644 > --- a/tools/libxl/libxl_internal.h > +++ b/tools/libxl/libxl_internal.h > @@ -770,10 +770,10 @@ static inline void libxl__ev_fd_init(libxl__ev_fd *efd) > static inline int libxl__ev_fd_isregistered(const libxl__ev_fd *efd) > { return efd->fd >= 0; } > > -_hidden int libxl__ev_time_register_rel(libxl__gc*, libxl__ev_time *ev_out, > +_hidden int libxl__ev_time_register_rel(libxl__ao*, libxl__ev_time *ev_out, > libxl__ev_time_callback*, > int milliseconds /* as for poll(2) */); > -_hidden int libxl__ev_time_register_abs(libxl__gc*, libxl__ev_time *ev_out, > +_hidden int libxl__ev_time_register_abs(libxl__ao*, libxl__ev_time *ev_out, > libxl__ev_time_callback*, > struct timeval); > _hidden int libxl__ev_time_modify_rel(libxl__gc*, libxl__ev_time *ev, > @@ -2108,7 +2108,7 @@ struct libxl__async_exec_state { > }; > > void libxl__async_exec_init(libxl__async_exec_state *aes); > -int libxl__async_exec_start(libxl__gc *gc, libxl__async_exec_state *aes); > +int libxl__async_exec_start(libxl__async_exec_state *aes); > bool libxl__async_exec_inuse(const libxl__async_exec_state *aes); > > /*----- device addition/removal -----*/ > diff --git a/tools/libxl/libxl_remus_disk_drbd.c b/tools/libxl/libxl_remus_disk_drbd.c > index afe9b61..5e0c9a6 100644 > --- a/tools/libxl/libxl_remus_disk_drbd.c > +++ b/tools/libxl/libxl_remus_disk_drbd.c > @@ -120,7 +120,7 @@ static void match_async_exec(libxl__egc *egc, libxl__remus_device *dev) > aes->stdfds[1] = -1; > aes->stdfds[2] = -1; > > - rc = libxl__async_exec_start(gc, aes); > + rc = libxl__async_exec_start(aes); > if (rc) > goto out; > > diff --git a/tools/libxl/libxl_test_timedereg.c b/tools/libxl/libxl_test_timedereg.c > index a44639f..e2cc27d 100644 > --- a/tools/libxl/libxl_test_timedereg.c > +++ b/tools/libxl/libxl_test_timedereg.c > @@ -30,12 +30,13 @@ static int seq; > static void occurs(libxl__egc *egc, libxl__ev_time *ev, > const struct timeval *requested_abs); > > -static void regs(libxl__gc *gc, int j) > +static void regs(libxl__ao *ao, int j) > { > + AO_GC; > int rc, i; > LOG(DEBUG,"regs(%d)", j); > for (i=0; i - rc = libxl__ev_time_register_rel(gc, &et[j][i], occurs, ms[j][i]); > + rc = libxl__ev_time_register_rel(ao, &et[j][i], occurs, ms[j][i]); > assert(!rc); > } > } > @@ -52,7 +53,7 @@ int libxl_test_timedereg(libxl_ctx *ctx, libxl_asyncop_how *ao_how) > libxl__ev_time_init(&et[1][i]); > } > > - regs(gc, 0); > + regs(ao, 0); > > return AO_INPROGRESS; > } > @@ -71,7 +72,7 @@ static void occurs(libxl__egc *egc, libxl__ev_time *ev, > assert(ev == &et[0][1]); > libxl__ev_time_deregister(gc, &et[0][0]); > libxl__ev_time_deregister(gc, &et[0][2]); > - regs(gc, 1); > + regs(tao, 1); > libxl__ev_time_deregister(gc, &et[0][1]); > break; > >