|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH 08/14] libxl: events: Permit timeouts to signal cancellation
We make these semantic changes to the interface to libxl__ev_time:
* The callback functions provided by users must take an rc value.
This rc value can be ERROR_TIMEDOUT or ERROR_CANCELLED.
* The setup functions take a libxl__ao, not a libxl__gc. 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).
The consequential changes at the points where timeouts are used are
generally obvious. However:
* device_hotplug_timeout_cb/device_hotplug_child_death_cb need to
take a bit more care to preserve the error code passed to the
timeout.
* Users of xswait are now expected to deal correctly with
ERROR_CANCELLED. If they experience this, it hasn't been logged.
And the caller won't log it either since it's not TIMEDOUT.
Luckily this is correct, so we can just change the doc comment.
Currently nothing generates ERROR_CANCELLED; in particular the
timeouts cannot in fact signal cancellation.
There should be no publicly visible change except that some error
returns from libxl will change from ERROR_FAIL to ERROR_TIMEDOUT, and
some changes to debugging messages.
Signed-off-by: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
---
tools/libxl/libxl_aoutils.c | 7 ++++---
tools/libxl/libxl_device.c | 24 ++++++++++++++++--------
tools/libxl/libxl_dom.c | 23 ++++++++++++++---------
tools/libxl/libxl_event.c | 10 ++++++----
tools/libxl/libxl_internal.h | 16 +++++++++-------
5 files changed, 49 insertions(+), 31 deletions(-)
diff --git a/tools/libxl/libxl_aoutils.c b/tools/libxl/libxl_aoutils.c
index 477717b..34a3305 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;
@@ -76,12 +76,13 @@ void xswait_xswatch_callback(libxl__egc *egc,
libxl__ev_xswatch *xsw,
}
void xswait_timeout_callback(libxl__egc *egc, libxl__ev_time *ev,
- const struct timeval *requested_abs)
+ const struct timeval *requested_abs,
+ int rc)
{
EGC_GC;
libxl__xswait_state *xswa = CONTAINER_OF(ev, *xswa, time_ev);
LOG(DEBUG, "%s: xswait timeout (path=%s)", xswa->what, xswa->path);
- xswait_report_error(egc, xswa, ERROR_TIMEDOUT);
+ xswait_report_error(egc, xswa, rc);
}
static void xswait_report_error(libxl__egc *egc, libxl__xswait_state *xswa,
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 2ea95d6..a795671 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -697,7 +697,7 @@ out:
/* This callback is part of the Qemu devices Badge */
static void device_qemu_timeout(libxl__egc *egc, libxl__ev_time *ev,
- const struct timeval *requested_abs);
+ const struct timeval *requested_abs, int rc);
static void device_backend_callback(libxl__egc *egc, libxl__ev_devstate *ds,
int rc);
@@ -708,7 +708,8 @@ static void device_backend_cleanup(libxl__gc *gc,
static void device_hotplug(libxl__egc *egc, libxl__ao_device *aodev);
static void device_hotplug_timeout_cb(libxl__egc *egc, libxl__ev_time *ev,
- const struct timeval *requested_abs);
+ const struct timeval *requested_abs,
+ int rc);
static void device_hotplug_child_death_cb(libxl__egc *egc,
libxl__ev_child *child,
@@ -790,7 +791,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) {
@@ -862,7 +863,7 @@ out:
}
static void device_qemu_timeout(libxl__egc *egc, libxl__ev_time *ev,
- const struct timeval *requested_abs)
+ const struct timeval *requested_abs, int rc)
{
libxl__ao_device *aodev = CONTAINER_OF(ev, *aodev, timeout);
STATE_AO_GC(aodev->ao);
@@ -870,7 +871,9 @@ static void device_qemu_timeout(libxl__egc *egc,
libxl__ev_time *ev,
char *state_path = GCSPRINTF("%s/state", be_path);
const char *xs_state;
xs_transaction_t t = 0;
- int rc = 0;
+
+ if (rc != ERROR_TIMEDOUT)
+ goto out;
libxl__ev_time_deregister(gc, &aodev->timeout);
@@ -998,7 +1001,7 @@ static void device_hotplug(libxl__egc *egc,
libxl__ao_device *aodev)
}
/* Set hotplug timeout */
- rc = libxl__ev_time_register_rel(gc, &aodev->timeout,
+ rc = libxl__ev_time_register_rel(ao, &aodev->timeout,
device_hotplug_timeout_cb,
LIBXL_HOTPLUG_TIMEOUT * 1000);
if (rc) {
@@ -1035,11 +1038,15 @@ out:
}
static void device_hotplug_timeout_cb(libxl__egc *egc, libxl__ev_time *ev,
- const struct timeval *requested_abs)
+ const struct timeval *requested_abs,
+ int rc)
{
libxl__ao_device *aodev = CONTAINER_OF(ev, *aodev, timeout);
STATE_AO_GC(aodev->ao);
+ if (!aodev->rc)
+ aodev->rc = rc;
+
libxl__ev_time_deregister(gc, &aodev->timeout);
assert(libxl__ev_child_inuse(&aodev->child));
@@ -1070,7 +1077,8 @@ static void device_hotplug_child_death_cb(libxl__egc *egc,
GCSPRINTF("%s/hotplug-error", be_path));
if (hotplug_error)
LOG(ERROR, "script: %s", hotplug_error);
- aodev->rc = ERROR_FAIL;
+ if (!aodev->rc)
+ aodev->rc = ERROR_FAIL;
if (aodev->action == LIBXL__DEVICE_ACTION_ADD)
/*
* Only fail on device connection, on disconnection
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 0fda9a4..1b1d06f 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -771,7 +771,8 @@ static void
remus_domain_suspend_callback_common_done(libxl__egc *egc,
*/
static void switch_logdirty_timeout(libxl__egc *egc, libxl__ev_time *ev,
- const struct timeval *requested_abs);
+ const struct timeval *requested_abs,
+ int rc);
static void switch_logdirty_xswatch(libxl__egc *egc, libxl__ev_xswatch*,
const char *watch_path, const char *event_path);
static void switch_logdirty_done(libxl__egc *egc,
@@ -808,7 +809,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;
@@ -897,7 +898,8 @@ void libxl__domain_suspend_common_switch_qemu_logdirty
}
}
static void switch_logdirty_timeout(libxl__egc *egc, libxl__ev_time *ev,
- const struct timeval *requested_abs)
+ const struct timeval *requested_abs,
+ int rc)
{
libxl__domain_suspend_state *dss = CONTAINER_OF(ev, *dss,
logdirty.timeout);
STATE_AO_GC(dss->ao);
@@ -1046,7 +1048,7 @@ static void suspend_common_wait_guest_watch(libxl__egc
*egc,
static void suspend_common_wait_guest_check(libxl__egc *egc,
libxl__domain_suspend_state *dss);
static void suspend_common_wait_guest_timeout(libxl__egc *egc,
- libxl__ev_time *ev, const struct timeval *requested_abs);
+ libxl__ev_time *ev, const struct timeval *requested_abs, int rc);
static void domain_suspend_common_done(libxl__egc *egc,
libxl__domain_suspend_state *dss,
@@ -1088,7 +1090,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;
@@ -1218,7 +1220,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;
@@ -1279,12 +1281,15 @@ static void suspend_common_wait_guest_check(libxl__egc
*egc,
}
static void suspend_common_wait_guest_timeout(libxl__egc *egc,
- libxl__ev_time *ev, const struct timeval *requested_abs)
+ libxl__ev_time *ev, const struct timeval *requested_abs, int rc)
{
libxl__domain_suspend_state *dss = CONTAINER_OF(ev, *dss, guest_timeout);
STATE_AO_GC(dss->ao);
- LOG(ERROR, "guest did not suspend, timed out");
- domain_suspend_common_done(egc, dss, ERROR_GUEST_TIMEDOUT);
+ if (rc == ERROR_TIMEDOUT) {
+ LOG(ERROR, "guest did not suspend, timed out");
+ rc = ERROR_GUEST_TIMEDOUT;
+ }
+ domain_suspend_common_done(egc, dss, rc);
}
static void domain_suspend_common_guest_suspended(libxl__egc *egc,
diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c
index 696c744..28b134a 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;
@@ -1154,7 +1156,7 @@ static void afterpoll_internal(libxl__egc *egc,
libxl__poller *poller,
etime, (unsigned long)etime->abs.tv_sec,
(unsigned long)etime->abs.tv_usec);
- etime->func(egc, etime, &etime->abs);
+ etime->func(egc, etime, &etime->abs, ERROR_TIMEDOUT);
}
}
@@ -1235,7 +1237,7 @@ void libxl_osevent_occurred_timeout(libxl_ctx *ctx, void
*for_libxl)
assert(!ev->infinite);
LIBXL_TAILQ_REMOVE(&CTX->etimes, ev, entry);
- ev->func(egc, ev, &ev->abs);
+ ev->func(egc, ev, &ev->abs, ERROR_TIMEDOUT);
out:
CTX_UNLOCK;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index af41200..b2109ea 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -172,7 +172,8 @@ struct libxl__ev_fd {
typedef struct libxl__ev_time libxl__ev_time;
typedef void libxl__ev_time_callback(libxl__egc *egc, libxl__ev_time *ev,
- const struct timeval *requested_abs);
+ const struct timeval *requested_abs,
+ int rc); /* TIMEDOUT or CANCELLED */
struct libxl__ev_time {
/* caller should include this in their own struct */
/* read-only for caller, who may read only when registered: */
@@ -734,10 +735,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,
@@ -1048,13 +1049,13 @@ typedef struct libxl__xswait_state libxl__xswait_state;
* Otherwise, xswait will continue waiting and watching and
* will call you back later.
*
- * rc==ERROR_TIMEDOUT
+ * rc==ERROR_TIMEDOUT, rc==ERROR_CANCELLED
*
* The specified timeout was reached.
* This has NOT been logged (except to the debug log).
* xswait will not continue (but calling libxl__xswait_stop is OK).
*
- * rc!=0, !=ERROR_TIMEDOUT
+ * rc!=0, !=ERROR_TIMEDOUT, !=ERROR_CANCELLED
*
* Some other error occurred.
* This HAS been logged.
@@ -1092,8 +1093,9 @@ int libxl__xswait_start(libxl__gc*, libxl__xswait_state*);
typedef struct libxl__ev_devstate libxl__ev_devstate;
typedef void libxl__ev_devstate_callback(libxl__egc *egc, libxl__ev_devstate*,
int rc);
- /* rc will be 0, ERROR_TIMEDOUT, ERROR_INVAL (meaning path was removed),
- * or ERROR_FAIL if other stuff went wrong (in which latter case, logged) */
+ /* rc will be 0, ERROR_TIMEDOUT, ERROR_CANCELLED, ERROR_INVAL
+ * (meaning path was removed), or ERROR_FAIL if other stuff went
+ * wrong (in which latter case, logged) */
struct libxl__ev_devstate {
/* read-only for caller, who may read only when waiting: */
--
1.7.10.4
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |