* [PATCH 01/28] libxl: Further fix exit paths from libxl_device_events_handler
2015-03-31 19:00 [PATCH RFC v3 00/28] libxl: Cancelling asynchronous operations Ian Jackson
@ 2015-03-31 19:00 ` Ian Jackson
2015-03-31 19:00 ` [PATCH 02/28] libxl: Comment cleanups Ian Jackson
` (28 subsequent siblings)
29 siblings, 0 replies; 38+ messages in thread
From: Ian Jackson @ 2015-03-31 19:00 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
On the success path, do not call GC_FREE explicitly. Instead, call
AO_INPROGRESS.
GC_FREE will free the gc underlying the long-term ao, which is then
subsequently referenced in backend_watch_callback's call to
libxl__nested_ao_create. It is a miracle that this ever works at all.
Also, add an `if (rc) goto out;' after the xswatch registration.
After this, libxl_device_events_handler has the conventional and
correct ao initiation pattern.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v2: New in this version of the series.
---
tools/libxl/libxl.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 94b4d59..de0fc6b 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -4547,11 +4547,12 @@ int libxl_device_events_handler(libxl_ctx *ctx,
be_path = GCSPRINTF("/local/domain/%u/backend", domid);
rc = libxl__ev_xswatch_register(gc, &ddomain.watch, backend_watch_callback,
be_path);
+ if (rc) goto out;
-out:
- GC_FREE;
- if (rc) return AO_ABORT(rc);
return AO_INPROGRESS;
+
+out:
+ return AO_ABORT(rc);
}
/******************************************************************************/
--
1.7.10.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 02/28] libxl: Comment cleanups
2015-03-31 19:00 [PATCH RFC v3 00/28] libxl: Cancelling asynchronous operations Ian Jackson
2015-03-31 19:00 ` [PATCH 01/28] libxl: Further fix exit paths from libxl_device_events_handler Ian Jackson
@ 2015-03-31 19:00 ` Ian Jackson
2015-03-31 19:00 ` [PATCH 03/28] libxl: suspend: switch_logdirty_done takes rc Ian Jackson
` (27 subsequent siblings)
29 siblings, 0 replies; 38+ messages in thread
From: Ian Jackson @ 2015-03-31 19:00 UTC (permalink / raw)
To: xen-devel; +Cc: Yang Hongyang, Ian Jackson, Lai Jiangshan, Wen Congyang
* Add two comments in libxl_remus_disk_drbd documenting buggy handling
of the hotplug script exit status.
* Add a section heading for async exec in libxl_aoutils.c
* Mention the right function name (libxl__ev_child_fork, not
libxl__ev_fork) in libxl_internal.h
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Yang Hongyang <yanghy@cn.fujitsu.com>
CC: Wen Congyang <wency@cn.fujitsu.com>
CC: Lai Jiangshan <laijs@cn.fujitsu.com>
Acked-by: Yang Hongyang <yanghy@cn.fujitsu.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v2: New patch in this version of the series.
---
tools/libxl/libxl_aoutils.c | 2 ++
tools/libxl/libxl_internal.h | 2 +-
tools/libxl/libxl_remus_disk_drbd.c | 2 ++
3 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/tools/libxl/libxl_aoutils.c b/tools/libxl/libxl_aoutils.c
index b10d2e1..44dc222 100644
--- a/tools/libxl/libxl_aoutils.c
+++ b/tools/libxl/libxl_aoutils.c
@@ -451,6 +451,8 @@ int libxl__openptys(libxl__openpty_state *op,
return rc;
}
+/*----- async exec -----*/
+
static void async_exec_timeout(libxl__egc *egc,
libxl__ev_time *ev,
const struct timeval *requested_abs)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 934465a..99db92a 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1393,7 +1393,7 @@ _hidden int libxl__spawn_record_pid(libxl__gc*, libxl__spawn_state*,
* This is a NOT function for waiting for ordinary child processes.
* If you want to run (fork/exec/wait) subprocesses from libxl:
* - Make your libxl entrypoint use the ao machinery
- * - Use libxl__ev_fork, and use the callback programming style
+ * - Use libxl__ev_child_fork, and use the callback programming style
*
* This function is intended for interprocess communication with a
* service process. If the service process does not respond quickly,
diff --git a/tools/libxl/libxl_remus_disk_drbd.c b/tools/libxl/libxl_remus_disk_drbd.c
index 3215f93..afe9b61 100644
--- a/tools/libxl/libxl_remus_disk_drbd.c
+++ b/tools/libxl/libxl_remus_disk_drbd.c
@@ -145,6 +145,8 @@ static void match_async_exec_cb(libxl__egc *egc,
if (status) {
rc = ERROR_REMUS_DEVOPS_DOES_NOT_MATCH;
+ /* BUG: seems to assume that any exit status means `no match' */
+ /* BUG: exit status will have been logged as an error */
goto out;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 03/28] libxl: suspend: switch_logdirty_done takes rc
2015-03-31 19:00 [PATCH RFC v3 00/28] libxl: Cancelling asynchronous operations Ian Jackson
2015-03-31 19:00 ` [PATCH 01/28] libxl: Further fix exit paths from libxl_device_events_handler Ian Jackson
2015-03-31 19:00 ` [PATCH 02/28] libxl: Comment cleanups Ian Jackson
@ 2015-03-31 19:00 ` Ian Jackson
2015-04-15 15:09 ` Ian Campbell
2015-03-31 19:00 ` [PATCH 04/28] libxl: suspend: common suspend callbacks take rc Ian Jackson
` (26 subsequent siblings)
29 siblings, 1 reply; 38+ messages in thread
From: Ian Jackson @ 2015-03-31 19:00 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
switch_logdirty_done used to take the value to pass to
libxl__xc_domain_saverestore_async_callback_done (ie, the return value
from the callback). (This was mistakenly described as "ok" in the
prototype, but in the definition it is "broke" and all the call sites
passed 0 for success or -1 for error.)
Instead, make it take a libxl error code (rc). Convert this to the
suspend callback value at the end.
No functional change in this patch.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
tools/libxl/libxl_dom.c | 23 ++++++++++++++---------
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index a16d4a1..08a7600 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -947,7 +947,7 @@ static void switch_logdirty_timeout(libxl__egc *egc, libxl__ev_time *ev,
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,
- libxl__domain_suspend_state *dss, int ok);
+ libxl__domain_suspend_state *dss, int rc);
static void logdirty_init(libxl__logdirty_switch *lds)
{
@@ -1024,7 +1024,7 @@ static void domain_suspend_switch_qemu_xen_traditional_logdirty
out:
LOG(ERROR,"logdirty switch failed (rc=%d), aborting suspend",rc);
libxl__xs_transaction_abort(gc, &t);
- switch_logdirty_done(egc,dss,-1);
+ switch_logdirty_done(egc,dss,rc);
}
static void domain_suspend_switch_qemu_xen_logdirty
@@ -1072,7 +1072,7 @@ static void switch_logdirty_timeout(libxl__egc *egc, libxl__ev_time *ev,
libxl__domain_suspend_state *dss = CONTAINER_OF(ev, *dss, logdirty.timeout);
STATE_AO_GC(dss->ao);
LOG(ERROR,"logdirty switch: wait for device model timed out");
- switch_logdirty_done(egc,dss,-1);
+ switch_logdirty_done(egc,dss,ERROR_FAIL);
}
static void switch_logdirty_xswatch(libxl__egc *egc, libxl__ev_xswatch *watch,
@@ -1124,17 +1124,16 @@ static void switch_logdirty_xswatch(libxl__egc *egc, libxl__ev_xswatch *watch,
*/
libxl__xs_transaction_abort(gc, &t);
- if (!rc) {
- switch_logdirty_done(egc,dss,0);
- } else if (rc < 0) {
- LOG(ERROR,"logdirty switch: failed (rc=%d)",rc);
- switch_logdirty_done(egc,dss,-1);
+ if (rc <= 0) {
+ if (rc < 0)
+ LOG(ERROR,"logdirty switch: failed (rc=%d)",rc);
+ switch_logdirty_done(egc,dss,rc);
}
}
static void switch_logdirty_done(libxl__egc *egc,
libxl__domain_suspend_state *dss,
- int broke)
+ int rc)
{
STATE_AO_GC(dss->ao);
libxl__logdirty_switch *lds = &dss->logdirty;
@@ -1142,6 +1141,12 @@ static void switch_logdirty_done(libxl__egc *egc,
libxl__ev_xswatch_deregister(gc, &lds->watch);
libxl__ev_time_deregister(gc, &lds->timeout);
+ int broke;
+ if (rc) {
+ broke = -1;
+ } else {
+ broke = 0;
+ }
libxl__xc_domain_saverestore_async_callback_done(egc, &dss->shs, broke);
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH 03/28] libxl: suspend: switch_logdirty_done takes rc
2015-03-31 19:00 ` [PATCH 03/28] libxl: suspend: switch_logdirty_done takes rc Ian Jackson
@ 2015-04-15 15:09 ` Ian Campbell
0 siblings, 0 replies; 38+ messages in thread
From: Ian Campbell @ 2015-04-15 15:09 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel
On Tue, 2015-03-31 at 20:00 +0100, Ian Jackson wrote:
> switch_logdirty_done used to take the value to pass to
> libxl__xc_domain_saverestore_async_callback_done (ie, the return value
> from the callback). (This was mistakenly described as "ok" in the
> prototype, but in the definition it is "broke" and all the call sites
> passed 0 for success or -1 for error.)
>
> Instead, make it take a libxl error code (rc). Convert this to the
> suspend callback value at the end.
>
> No functional change in this patch.
>
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 04/28] libxl: suspend: common suspend callbacks take rc
2015-03-31 19:00 [PATCH RFC v3 00/28] libxl: Cancelling asynchronous operations Ian Jackson
` (2 preceding siblings ...)
2015-03-31 19:00 ` [PATCH 03/28] libxl: suspend: switch_logdirty_done takes rc Ian Jackson
@ 2015-03-31 19:00 ` Ian Jackson
2015-03-31 19:00 ` [PATCH 05/28] libxl: suspend: Return correct error from callbacks Ian Jackson
` (25 subsequent siblings)
29 siblings, 0 replies; 38+ messages in thread
From: Ian Jackson @ 2015-03-31 19:00 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
Change the following functions to take a libxl error code rather than
a boolean "ok" value, and translate that value to the boolean expected
by libxc at the last moment:
domain_suspend_callback_common_done } dss->callback_common_done
remus_domain_suspend_callback_common_done }
domain_suspend_common_done
Also, abolish domain_suspend_common_failed as
domain_suspend_common_done can easily do its job and the call sites
now have to supply the right rc value anyway.
In domain_suspend_common_guest_suspended, change "ret" to "rc"
as it contains a libxl error code.
There is no functional change in this patch: the proper rc value now
propagates further, but is still eventually smashed to a boolean.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v2: Fix a leftover comment referring to domain_suspend_common_failed
---
tools/libxl/libxl_dom.c | 54 +++++++++++++++++++++--------------------------
1 file changed, 24 insertions(+), 30 deletions(-)
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 08a7600..5eef7e3 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -930,7 +930,7 @@ int libxl__toolstack_restore(uint32_t domid, const uint8_t *buf,
static void domain_suspend_done(libxl__egc *egc,
libxl__domain_suspend_state *dss, int rc);
static void domain_suspend_callback_common_done(libxl__egc *egc,
- libxl__domain_suspend_state *dss, int ok);
+ libxl__domain_suspend_state *dss, int rc);
/*----- complicated callback, called by xc_domain_save -----*/
@@ -1217,11 +1217,9 @@ 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);
-static void domain_suspend_common_failed(libxl__egc *egc,
- libxl__domain_suspend_state *dss);
static void domain_suspend_common_done(libxl__egc *egc,
libxl__domain_suspend_state *dss,
- bool ok);
+ int rc);
static bool domain_suspend_pvcontrol_acked(const char *state) {
/* any value other than "suspend", including ENOENT (i.e. !state), is OK */
@@ -1251,6 +1249,7 @@ static void domain_suspend_callback_common(libxl__egc *egc,
ret = xc_evtchn_notify(CTX->xce, dss->guest_evtchn.port);
if (ret < 0) {
LOG(ERROR, "xc_evtchn_notify failed ret=%d", ret);
+ rc = ERROR_FAIL;
goto err;
}
@@ -1271,6 +1270,7 @@ static void domain_suspend_callback_common(libxl__egc *egc,
ret = xc_domain_shutdown(CTX->xch, domid, SHUTDOWN_suspend);
if (ret < 0) {
LOGE(ERROR, "xc_domain_shutdown failed");
+ rc = ERROR_FAIL;
goto err;
}
/* The guest does not (need to) respond to this sort of request. */
@@ -1285,7 +1285,7 @@ static void domain_suspend_callback_common(libxl__egc *egc,
libxl__domain_pvcontrol_write(gc, XBT_NULL, domid, "suspend");
dss->pvcontrol.path = libxl__domain_pvcontrol_xspath(gc, domid);
- if (!dss->pvcontrol.path) goto err;
+ if (!dss->pvcontrol.path) { rc = ERROR_FAIL; goto err; }
dss->pvcontrol.ao = ao;
dss->pvcontrol.what = "guest acknowledgement of suspend request";
@@ -1295,7 +1295,7 @@ static void domain_suspend_callback_common(libxl__egc *egc,
return;
err:
- domain_suspend_common_failed(egc, dss);
+ domain_suspend_common_done(egc, dss, rc);
}
static void domain_suspend_common_wait_guest_evtchn(libxl__egc *egc,
@@ -1305,8 +1305,8 @@ static void domain_suspend_common_wait_guest_evtchn(libxl__egc *egc,
STATE_AO_GC(dss->ao);
/* If we should be done waiting, suspend_common_wait_guest_check
* will end up calling domain_suspend_common_guest_suspended or
- * domain_suspend_common_failed, both of which cancel the evtchn
- * wait. So re-enable it now. */
+ * domain_suspend_common_done, both of which cancel the evtchn
+ * wait as needed. So re-enable it now. */
libxl__ev_evtchn_wait(gc, &dss->guest_evtchn);
suspend_common_wait_guest_check(egc, dss);
}
@@ -1371,7 +1371,7 @@ static void domain_suspend_common_pvcontrol_suspending(libxl__egc *egc,
err:
libxl__xs_transaction_abort(gc, &t);
- domain_suspend_common_failed(egc, dss);
+ domain_suspend_common_done(egc, dss, rc);
return;
}
@@ -1395,7 +1395,7 @@ static void domain_suspend_common_wait_guest(libxl__egc *egc,
return;
err:
- domain_suspend_common_failed(egc, dss);
+ domain_suspend_common_done(egc, dss, rc);
}
static void suspend_common_wait_guest_watch(libxl__egc *egc,
@@ -1445,7 +1445,7 @@ static void suspend_common_wait_guest_check(libxl__egc *egc,
return;
err:
- domain_suspend_common_failed(egc, dss);
+ domain_suspend_common_done(egc, dss, ERROR_FAIL);
}
static void suspend_common_wait_guest_timeout(libxl__egc *egc,
@@ -1454,46 +1454,40 @@ static void suspend_common_wait_guest_timeout(libxl__egc *egc,
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_failed(egc, dss);
+ domain_suspend_common_done(egc, dss, ERROR_GUEST_TIMEDOUT);
}
static void domain_suspend_common_guest_suspended(libxl__egc *egc,
libxl__domain_suspend_state *dss)
{
STATE_AO_GC(dss->ao);
- int ret;
+ int rc;
libxl__ev_evtchn_cancel(gc, &dss->guest_evtchn);
libxl__ev_xswatch_deregister(gc, &dss->guest_watch);
libxl__ev_time_deregister(gc, &dss->guest_timeout);
if (dss->hvm) {
- ret = libxl__domain_suspend_device_model(gc, dss);
- if (ret) {
- LOG(ERROR, "libxl__domain_suspend_device_model failed ret=%d", ret);
- domain_suspend_common_failed(egc, dss);
+ rc = libxl__domain_suspend_device_model(gc, dss);
+ if (rc) {
+ LOG(ERROR, "libxl__domain_suspend_device_model failed ret=%d", rc);
+ domain_suspend_common_done(egc, dss, rc);
return;
}
}
- domain_suspend_common_done(egc, dss, 1);
-}
-
-static void domain_suspend_common_failed(libxl__egc *egc,
- libxl__domain_suspend_state *dss)
-{
domain_suspend_common_done(egc, dss, 0);
}
static void domain_suspend_common_done(libxl__egc *egc,
libxl__domain_suspend_state *dss,
- bool ok)
+ int rc)
{
EGC_GC;
assert(!libxl__xswait_inuse(&dss->pvcontrol));
libxl__ev_evtchn_cancel(gc, &dss->guest_evtchn);
libxl__ev_xswatch_deregister(gc, &dss->guest_watch);
libxl__ev_time_deregister(gc, &dss->guest_timeout);
- dss->callback_common_done(egc, dss, ok);
+ dss->callback_common_done(egc, dss, rc);
}
static inline char *physmap_path(libxl__gc *gc, uint32_t domid,
@@ -1591,9 +1585,9 @@ static void libxl__domain_suspend_callback(void *data)
}
static void domain_suspend_callback_common_done(libxl__egc *egc,
- libxl__domain_suspend_state *dss, int ok)
+ libxl__domain_suspend_state *dss, int rc)
{
- libxl__xc_domain_saverestore_async_callback_done(egc, &dss->shs, ok);
+ libxl__xc_domain_saverestore_async_callback_done(egc, &dss->shs, !rc);
}
/*----- remus callbacks -----*/
@@ -1617,9 +1611,9 @@ static void libxl__remus_domain_suspend_callback(void *data)
}
static void remus_domain_suspend_callback_common_done(libxl__egc *egc,
- libxl__domain_suspend_state *dss, int ok)
+ libxl__domain_suspend_state *dss, int rc)
{
- if (!ok)
+ if (rc)
goto out;
libxl__remus_devices_state *const rds = &dss->rds;
@@ -1628,7 +1622,7 @@ static void remus_domain_suspend_callback_common_done(libxl__egc *egc,
return;
out:
- libxl__xc_domain_saverestore_async_callback_done(egc, &dss->shs, ok);
+ libxl__xc_domain_saverestore_async_callback_done(egc, &dss->shs, !rc);
}
static void remus_devices_postsuspend_cb(libxl__egc *egc,
--
1.7.10.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 05/28] libxl: suspend: Return correct error from callbacks
2015-03-31 19:00 [PATCH RFC v3 00/28] libxl: Cancelling asynchronous operations Ian Jackson
` (3 preceding siblings ...)
2015-03-31 19:00 ` [PATCH 04/28] libxl: suspend: common suspend callbacks take rc Ian Jackson
@ 2015-03-31 19:00 ` Ian Jackson
2015-03-31 19:00 ` [PATCH 06/28] libxl: Use libxl__xswait* in libxl__ao_device Ian Jackson
` (24 subsequent siblings)
29 siblings, 0 replies; 38+ messages in thread
From: Ian Jackson @ 2015-03-31 19:00 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
If a suspend callback fails, it has a libxl error code in its hand.
However we must return to libxc the values that libxc expects. So we
stash the libxl error code in dss->rc and fish it out again after
libxc returns from the suspend call.
While we're here, abolish the now-redundant `ok' variable in
remus_devices_postsuspend_cb.
The overall functional change is that libxl_domain_save now completes
with the correct error code as determined when the underlying failure
happened. (Usually this is, still, ERROR_FAIL.)
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v2: Add cleanup in remus_devices_postsuspend_cb.
---
tools/libxl/libxl_dom.c | 22 ++++++++++++++++------
tools/libxl/libxl_internal.h | 1 +
2 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 5eef7e3..291b803 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1041,6 +1041,7 @@ static void domain_suspend_switch_qemu_xen_logdirty
libxl__xc_domain_saverestore_async_callback_done(egc, shs, 0);
} else {
LOG(ERROR,"logdirty switch failed (rc=%d), aborting suspend",rc);
+ dss->rc = rc;
libxl__xc_domain_saverestore_async_callback_done(egc, shs, -1);
}
}
@@ -1063,6 +1064,7 @@ void libxl__domain_suspend_common_switch_qemu_logdirty
default:
LOG(ERROR,"logdirty switch failed"
", no valid device model version found, aborting suspend");
+ dss->rc = ERROR_FAIL;
libxl__xc_domain_saverestore_async_callback_done(egc, shs, -1);
}
}
@@ -1144,6 +1146,7 @@ static void switch_logdirty_done(libxl__egc *egc,
int broke;
if (rc) {
broke = -1;
+ dss->rc = rc;
} else {
broke = 0;
}
@@ -1587,6 +1590,7 @@ static void libxl__domain_suspend_callback(void *data)
static void domain_suspend_callback_common_done(libxl__egc *egc,
libxl__domain_suspend_state *dss, int rc)
{
+ dss->rc = rc;
libxl__xc_domain_saverestore_async_callback_done(egc, &dss->shs, !rc);
}
@@ -1622,6 +1626,7 @@ static void remus_domain_suspend_callback_common_done(libxl__egc *egc,
return;
out:
+ dss->rc = rc;
libxl__xc_domain_saverestore_async_callback_done(egc, &dss->shs, !rc);
}
@@ -1629,16 +1634,17 @@ static void remus_devices_postsuspend_cb(libxl__egc *egc,
libxl__remus_devices_state *rds,
int rc)
{
- int ok = 0;
libxl__domain_suspend_state *dss = CONTAINER_OF(rds, *dss, rds);
if (rc)
goto out;
- ok = 1;
+ rc = 0;
out:
- libxl__xc_domain_saverestore_async_callback_done(egc, &dss->shs, ok);
+ if (rc)
+ dss->rc = rc;
+ libxl__xc_domain_saverestore_async_callback_done(egc, &dss->shs, !rc);
}
static void libxl__remus_domain_resume_callback(void *data)
@@ -1657,7 +1663,6 @@ static void remus_devices_preresume_cb(libxl__egc *egc,
libxl__remus_devices_state *rds,
int rc)
{
- int ok = 0;
libxl__domain_suspend_state *dss = CONTAINER_OF(rds, *dss, rds);
STATE_AO_GC(dss->ao);
@@ -1669,10 +1674,12 @@ static void remus_devices_preresume_cb(libxl__egc *egc,
if (rc)
goto out;
- ok = 1;
+ rc = 0;
out:
- libxl__xc_domain_saverestore_async_callback_done(egc, &dss->shs, ok);
+ if (rc)
+ dss->rc = rc;
+ libxl__xc_domain_saverestore_async_callback_done(egc, &dss->shs, !rc);
}
/*----- remus asynchronous checkpoint callback -----*/
@@ -1790,6 +1797,7 @@ void libxl__domain_suspend(libxl__egc *egc, libxl__domain_suspend_state *dss)
libxl__srm_save_autogen_callbacks *const callbacks =
&dss->shs.callbacks.save.a;
+ dss->rc = 0;
logdirty_init(&dss->logdirty);
libxl__xswait_init(&dss->pvcontrol);
libxl__ev_evtchn_init(&dss->guest_evtchn);
@@ -1877,6 +1885,8 @@ void libxl__xc_domain_save_done(libxl__egc *egc, void *dss_void,
"domain did not respond to suspend request");
if ( !dss->guest_responded )
rc = ERROR_GUEST_TIMEDOUT;
+ else if (dss->rc)
+ rc = dss->rc;
else
rc = ERROR_FAIL;
goto out;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 99db92a..2862c69 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2800,6 +2800,7 @@ struct libxl__domain_suspend_state {
int debug;
const libxl_domain_remus_info *remus;
/* private */
+ int rc;
libxl__ev_evtchn guest_evtchn;
int guest_evtchn_lockfd;
int hvm;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 06/28] libxl: Use libxl__xswait* in libxl__ao_device
2015-03-31 19:00 [PATCH RFC v3 00/28] libxl: Cancelling asynchronous operations Ian Jackson
` (4 preceding siblings ...)
2015-03-31 19:00 ` [PATCH 05/28] libxl: suspend: Return correct error from callbacks Ian Jackson
@ 2015-03-31 19:00 ` Ian Jackson
2015-04-15 15:10 ` Ian Campbell
2015-03-31 19:00 ` [PATCH 07/28] libxl: xswait/devstate: Move xswait to before devstate Ian Jackson
` (23 subsequent siblings)
29 siblings, 1 reply; 38+ messages in thread
From: Ian Jackson @ 2015-03-31 19:00 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
Replace the separate timeout and xenstore watch with use of
libxl__xswait*.
Different control flow, but no ultimate functional change apart from
slight changes to the text of error messages.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
tools/libxl/libxl_device.c | 64 ++++++++++++------------------------------
tools/libxl/libxl_internal.h | 2 +-
2 files changed, 19 insertions(+), 47 deletions(-)
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 0f50d04..64ee541 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -450,7 +450,7 @@ void libxl__prepare_ao_device(libxl__ao *ao, libxl__ao_device *aodev)
* Initialize xs_watch, because it's not used on all possible
* execution paths, but it's unconditionally destroyed when finished.
*/
- libxl__ev_xswatch_init(&aodev->xs_watch);
+ libxl__xswait_init(&aodev->xswait);
aodev->active = 1;
/* We init this here because we might call device_hotplug_done
* without actually calling any hotplug script */
@@ -731,13 +731,9 @@ static void device_hotplug_child_death_cb(libxl__egc *egc,
libxl__async_exec_state *aes,
int status);
-static void device_destroy_be_timeout_cb(libxl__egc *egc, libxl__ev_time *ev,
- const struct timeval *requested_abs);
-
static void device_destroy_be_watch_cb(libxl__egc *egc,
- libxl__ev_xswatch *watch,
- const char *watch_path,
- const char *event_path);
+ libxl__xswait_state *xswait,
+ int rc, const char *data);
static void device_hotplug_done(libxl__egc *egc, libxl__ao_device *aodev);
@@ -988,22 +984,14 @@ static void device_hotplug(libxl__egc *egc, libxl__ao_device *aodev)
if (aodev->action != LIBXL__DEVICE_ACTION_REMOVE)
goto out;
- rc = libxl__ev_time_register_rel(gc, &aodev->timeout,
- device_destroy_be_timeout_cb,
- LIBXL_DESTROY_TIMEOUT * 1000);
- if (rc) {
- LOG(ERROR, "setup of xs watch timeout failed");
- goto out;
- }
-
- rc = libxl__ev_xswatch_register(gc, &aodev->xs_watch,
- device_destroy_be_watch_cb,
- be_path);
- if (rc) {
- LOG(ERROR, "setup of xs watch for %s failed", be_path);
- libxl__ev_time_deregister(gc, &aodev->timeout);
+ aodev->xswait.ao = ao;
+ aodev->xswait.what = "removal of backend path";
+ aodev->xswait.path = be_path;
+ aodev->xswait.timeout_ms = LIBXL_DESTROY_TIMEOUT * 1000;
+ aodev->xswait.callback = device_destroy_be_watch_cb;
+ rc = libxl__xswait_start(gc, &aodev->xswait);
+ if (rc)
goto out;
- }
return;
}
@@ -1101,37 +1089,21 @@ error:
device_hotplug_done(egc, aodev);
}
-static void device_destroy_be_timeout_cb(libxl__egc *egc, libxl__ev_time *ev,
- const struct timeval *requested_abs)
-{
- libxl__ao_device *aodev = CONTAINER_OF(ev, *aodev, timeout);
- STATE_AO_GC(aodev->ao);
-
- LOG(ERROR, "timed out while waiting for %s to be removed",
- libxl__device_backend_path(gc, aodev->dev));
-
- aodev->rc = ERROR_TIMEDOUT;
-
- device_hotplug_done(egc, aodev);
- return;
-}
-
static void device_destroy_be_watch_cb(libxl__egc *egc,
- libxl__ev_xswatch *watch,
- const char *watch_path,
- const char *event_path)
+ libxl__xswait_state *xswait,
+ int rc, const char *dir)
{
- libxl__ao_device *aodev = CONTAINER_OF(watch, *aodev, xs_watch);
+ libxl__ao_device *aodev = CONTAINER_OF(xswait, *aodev, xswait);
STATE_AO_GC(aodev->ao);
- const char *dir;
- int rc;
- rc = libxl__xs_read_checked(gc, XBT_NULL, watch_path, &dir);
if (rc) {
- LOG(ERROR, "unable to read backend path: %s", watch_path);
+ if (rc == ERROR_TIMEDOUT)
+ LOG(ERROR, "timed out while waiting for %s to be removed",
+ xswait->path);
aodev->rc = rc;
goto out;
}
+
if (dir) {
/* backend path still exists, wait a little longer... */
return;
@@ -1164,7 +1136,7 @@ static void device_hotplug_clean(libxl__gc *gc, libxl__ao_device *aodev)
{
/* Clean events and check reentrancy */
libxl__ev_time_deregister(gc, &aodev->timeout);
- libxl__ev_xswatch_deregister(gc, &aodev->xs_watch);
+ libxl__xswait_stop(gc, &aodev->xswait);
assert(!libxl__async_exec_inuse(&aodev->aes));
}
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 2862c69..5a76d51 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2152,7 +2152,7 @@ struct libxl__ao_device {
/* Bodge for Qemu devices */
libxl__ev_time timeout;
/* xenstore watch for backend path of driver domains */
- libxl__ev_xswatch xs_watch;
+ libxl__xswait_state xswait;
int num_exec;
/* for calling hotplug scripts */
libxl__async_exec_state aes;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 07/28] libxl: xswait/devstate: Move xswait to before devstate
2015-03-31 19:00 [PATCH RFC v3 00/28] libxl: Cancelling asynchronous operations Ian Jackson
` (5 preceding siblings ...)
2015-03-31 19:00 ` [PATCH 06/28] libxl: Use libxl__xswait* in libxl__ao_device Ian Jackson
@ 2015-03-31 19:00 ` Ian Jackson
2015-03-31 19:00 ` [PATCH 08/28] libxl: devstate: Use libxl__xswait* Ian Jackson
` (22 subsequent siblings)
29 siblings, 0 replies; 38+ messages in thread
From: Ian Jackson @ 2015-03-31 19:00 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
Pure code motion. We are going to make devstate use xswait.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
tools/libxl/libxl_internal.h | 109 +++++++++++++++++++++---------------------
1 file changed, 55 insertions(+), 54 deletions(-)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 5a76d51..edc33bb 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1090,6 +1090,61 @@ _hidden const char *libxl__device_nic_devname(libxl__gc *gc,
_hidden int libxl__get_domid(libxl__gc *gc, uint32_t *domid);
+/*----- xswait: wait for a xenstore node to be suitable -----*/
+
+typedef struct libxl__xswait_state libxl__xswait_state;
+
+/*
+ * rc describes the circumstances of this callback:
+ *
+ * rc==0
+ *
+ * The xenstore path (may have) changed. It has been read for
+ * you. The result is in data (allocated from the ao gc).
+ * data may be NULL, which means that the xenstore read gave
+ * ENOENT.
+ *
+ * If you are satisfied, you MUST call libxl__xswait_stop.
+ * Otherwise, xswait will continue waiting and watching and
+ * will call you back later.
+ *
+ * rc==ERROR_TIMEDOUT
+ *
+ * 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
+ *
+ * Some other error occurred.
+ * This HAS been logged.
+ * xswait will not continue (but calling libxl__xswait_stop is OK).
+ *
+ * xswait.path may start with with '@', in which case no read is done
+ * and the callback will always get data==0.
+ */
+typedef void libxl__xswait_callback(libxl__egc *egc,
+ libxl__xswait_state *xswa, int rc, const char *data);
+
+struct libxl__xswait_state {
+ /* caller must fill these in, and they must all remain valid */
+ libxl__ao *ao;
+ const char *what; /* for error msgs: noun phrase, what we're waiting for */
+ const char *path;
+ int timeout_ms; /* as for poll(2) */
+ libxl__xswait_callback *callback;
+ /* remaining fields are private to xswait */
+ libxl__ev_time time_ev;
+ libxl__ev_xswatch watch_ev;
+};
+
+void libxl__xswait_init(libxl__xswait_state*);
+void libxl__xswait_stop(libxl__gc*, libxl__xswait_state*); /*idempotent*/
+bool libxl__xswait_inuse(const libxl__xswait_state *ss);
+
+int libxl__xswait_start(libxl__gc*, libxl__xswait_state*);
+
+
/*
* libxl__ev_devstate - waits a given time for a device to
* reach a given state. Follows the libxl_ev_* conventions.
@@ -1177,60 +1232,6 @@ _hidden int libxl__create_pci_backend(libxl__gc *gc, uint32_t domid,
libxl_device_pci *pcidev, int num);
_hidden int libxl__device_pci_destroy_all(libxl__gc *gc, uint32_t domid);
-/*----- xswait: wait for a xenstore node to be suitable -----*/
-
-typedef struct libxl__xswait_state libxl__xswait_state;
-
-/*
- * rc describes the circumstances of this callback:
- *
- * rc==0
- *
- * The xenstore path (may have) changed. It has been read for
- * you. The result is in data (allocated from the ao gc).
- * data may be NULL, which means that the xenstore read gave
- * ENOENT.
- *
- * If you are satisfied, you MUST call libxl__xswait_stop.
- * Otherwise, xswait will continue waiting and watching and
- * will call you back later.
- *
- * rc==ERROR_TIMEDOUT
- *
- * 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
- *
- * Some other error occurred.
- * This HAS been logged.
- * xswait will not continue (but calling libxl__xswait_stop is OK).
- *
- * xswait.path may start with with '@', in which case no read is done
- * and the callback will always get data==0.
- */
-typedef void libxl__xswait_callback(libxl__egc *egc,
- libxl__xswait_state *xswa, int rc, const char *data);
-
-struct libxl__xswait_state {
- /* caller must fill these in, and they must all remain valid */
- libxl__ao *ao;
- const char *what; /* for error msgs: noun phrase, what we're waiting for */
- const char *path;
- int timeout_ms; /* as for poll(2) */
- libxl__xswait_callback *callback;
- /* remaining fields are private to xswait */
- libxl__ev_time time_ev;
- libxl__ev_xswatch watch_ev;
-};
-
-void libxl__xswait_init(libxl__xswait_state*);
-void libxl__xswait_stop(libxl__gc*, libxl__xswait_state*); /*idempotent*/
-bool libxl__xswait_inuse(const libxl__xswait_state *ss);
-
-int libxl__xswait_start(libxl__gc*, libxl__xswait_state*);
-
/*
*----- spawn -----
*
--
1.7.10.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 08/28] libxl: devstate: Use libxl__xswait*
2015-03-31 19:00 [PATCH RFC v3 00/28] libxl: Cancelling asynchronous operations Ian Jackson
` (6 preceding siblings ...)
2015-03-31 19:00 ` [PATCH 07/28] libxl: xswait/devstate: Move xswait to before devstate Ian Jackson
@ 2015-03-31 19:00 ` Ian Jackson
2015-03-31 19:00 ` [PATCH 09/28] libxl: New error codes CANCELLED etc Ian Jackson
` (21 subsequent siblings)
29 siblings, 0 replies; 38+ messages in thread
From: Ian Jackson @ 2015-03-31 19:00 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v3: Initialise ds->w.ao
---
tools/libxl/libxl_device.c | 4 +--
tools/libxl/libxl_event.c | 79 +++++++++++++++++++-----------------------
tools/libxl/libxl_internal.h | 11 +++---
3 files changed, 41 insertions(+), 53 deletions(-)
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 64ee541..0455134 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -758,7 +758,7 @@ void libxl__wait_device_connection(libxl__egc *egc, libxl__ao_device *aodev)
return;
}
- rc = libxl__ev_devstate_wait(gc, &aodev->backend_ds,
+ rc = libxl__ev_devstate_wait(ao, &aodev->backend_ds,
device_backend_callback,
state_path, XenbusStateInitWait,
LIBXL_INIT_TIMEOUT * 1000);
@@ -859,7 +859,7 @@ void libxl__initiate_device_remove(libxl__egc *egc,
if (rc < 0) goto out;
}
- rc = libxl__ev_devstate_wait(gc, &aodev->backend_ds,
+ rc = libxl__ev_devstate_wait(ao, &aodev->backend_ds,
device_backend_callback,
state_path, XenbusStateClosed,
LIBXL_DESTROY_TIMEOUT * 1000);
diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c
index 595da2b..d4d62a8 100644
--- a/tools/libxl/libxl_event.c
+++ b/tools/libxl/libxl_event.c
@@ -805,68 +805,59 @@ void libxl__ev_evtchn_cancel(libxl__gc *gc, libxl__ev_evtchn *evev)
* waiting for device state
*/
-static void devstate_watch_callback(libxl__egc *egc, libxl__ev_xswatch *watch,
- const char *watch_path, const char *event_path)
+static void devstate_callback(libxl__egc *egc, libxl__xswait_state *xsw,
+ int rc, const char *sstate)
{
EGC_GC;
- libxl__ev_devstate *ds = CONTAINER_OF(watch, *ds, watch);
- int rc;
+ libxl__ev_devstate *ds = CONTAINER_OF(xsw, *ds, w);
- char *sstate = libxl__xs_read(gc, XBT_NULL, watch_path);
+ if (rc) {
+ if (rc == ERROR_TIMEDOUT)
+ LIBXL__LOG(CTX, LIBXL__LOG_DEBUG, "backend %s wanted state %d "
+ " timed out", ds->w.path, ds->wanted);
+ goto out;
+ }
if (!sstate) {
- if (errno == ENOENT) {
- LIBXL__LOG(CTX, LIBXL__LOG_DEBUG, "backend %s wanted state %d"
- " but it was removed", watch_path, ds->wanted);
- rc = ERROR_INVAL;
- } else {
- LIBXL__LOG_ERRNO(CTX, LIBXL__LOG_ERROR, "backend %s wanted state"
- " %d but read failed", watch_path, ds->wanted);
- rc = ERROR_FAIL;
- }
+ LIBXL__LOG(CTX, LIBXL__LOG_DEBUG, "backend %s wanted state %d"
+ " but it was removed", ds->w.path, ds->wanted);
+ rc = ERROR_INVAL;
+ goto out;
+ }
+
+ int got = atoi(sstate);
+ if (got == ds->wanted) {
+ LIBXL__LOG(CTX, LIBXL__LOG_DEBUG, "backend %s wanted state %d ok",
+ ds->w.path, ds->wanted);
+ rc = 0;
} else {
- int got = atoi(sstate);
- if (got == ds->wanted) {
- LIBXL__LOG(CTX, LIBXL__LOG_DEBUG, "backend %s wanted state %d ok",
- watch_path, ds->wanted);
- rc = 0;
- } else {
- LIBXL__LOG(CTX, LIBXL__LOG_DEBUG, "backend %s wanted state %d"
- " still waiting state %d", watch_path, ds->wanted, got);
- return;
- }
+ LIBXL__LOG(CTX, LIBXL__LOG_DEBUG, "backend %s wanted state %d"
+ " still waiting state %d", ds->w.path, ds->wanted, got);
+ return;
}
- libxl__ev_devstate_cancel(gc, ds);
- ds->callback(egc, ds, rc);
-}
-static void devstate_timeout(libxl__egc *egc, libxl__ev_time *ev,
- const struct timeval *requested_abs)
-{
- EGC_GC;
- libxl__ev_devstate *ds = CONTAINER_OF(ev, *ds, timeout);
- LIBXL__LOG(CTX, LIBXL__LOG_DEBUG, "backend %s wanted state %d "
- " timed out", ds->watch.path, ds->wanted);
+ out:
libxl__ev_devstate_cancel(gc, ds);
- ds->callback(egc, ds, ERROR_TIMEDOUT);
+ ds->callback(egc, ds, rc);
}
-int libxl__ev_devstate_wait(libxl__gc *gc, libxl__ev_devstate *ds,
+int libxl__ev_devstate_wait(libxl__ao *ao, libxl__ev_devstate *ds,
libxl__ev_devstate_callback cb,
const char *state_path, int state, int milliseconds)
{
+ AO_GC;
int rc;
- libxl__ev_time_init(&ds->timeout);
- libxl__ev_xswatch_init(&ds->watch);
+ libxl__xswait_init(&ds->w);
ds->wanted = state;
ds->callback = cb;
- rc = libxl__ev_time_register_rel(gc, &ds->timeout, devstate_timeout,
- milliseconds);
- if (rc) goto out;
-
- rc = libxl__ev_xswatch_register(gc, &ds->watch, devstate_watch_callback,
- state_path);
+ ds->w.ao = ao;
+ ds->w.what = GCSPRINTF("backend %s (hoping for state change to %d)",
+ state_path, state);
+ ds->w.path = state_path;
+ ds->w.timeout_ms = milliseconds;
+ ds->w.callback = devstate_callback;
+ rc = libxl__xswait_start(gc, &ds->w);
if (rc) goto out;
return 0;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index edc33bb..6bb208c 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1163,24 +1163,21 @@ struct libxl__ev_devstate {
libxl__ev_devstate_callback *callback;
/* as for the remainder, read-only public parts may also be
* read by the caller (notably, watch.path), but only when waiting: */
- libxl__ev_xswatch watch;
- libxl__ev_time timeout;
+ libxl__xswait_state w;
};
static inline void libxl__ev_devstate_init(libxl__ev_devstate *ds)
{
- libxl__ev_time_init(&ds->timeout);
- libxl__ev_xswatch_init(&ds->watch);
+ libxl__xswait_init(&ds->w);
}
static inline void libxl__ev_devstate_cancel(libxl__gc *gc,
libxl__ev_devstate *ds)
{
- libxl__ev_time_deregister(gc,&ds->timeout);
- libxl__ev_xswatch_deregister(gc,&ds->watch);
+ libxl__xswait_stop(gc,&ds->w);
}
-_hidden int libxl__ev_devstate_wait(libxl__gc *gc, libxl__ev_devstate *ds,
+_hidden int libxl__ev_devstate_wait(libxl__ao *ao, libxl__ev_devstate *ds,
libxl__ev_devstate_callback cb,
const char *state_path,
int state, int milliseconds);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 09/28] libxl: New error codes CANCELLED etc.
2015-03-31 19:00 [PATCH RFC v3 00/28] libxl: Cancelling asynchronous operations Ian Jackson
` (7 preceding siblings ...)
2015-03-31 19:00 ` [PATCH 08/28] libxl: devstate: Use libxl__xswait* Ian Jackson
@ 2015-03-31 19:00 ` Ian Jackson
2015-03-31 19:00 ` [PATCH 10/28] libxl: events: Make timeout and async exec setup take an ao, not a gc Ian Jackson
` (20 subsequent siblings)
29 siblings, 0 replies; 38+ messages in thread
From: Ian Jackson @ 2015-03-31 19:00 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
We introduce ERROR_CANCELLED now, so that we can write code to handle
it, and decreee that functions might return it, even though currently
there is nowhere where this error is generated.
While we're here, provide ERROR_NOTFOUND and ERROR_NOTIMPLEMENTED,
which will also be used later, but only as part of the public API.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v2: Rebase means new errors have bigger (more negative) numbers.
---
tools/libxl/libxl_types.idl | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 47af340..478c561 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -63,6 +63,9 @@ libxl_error = Enumeration("error", [
(-17, "DEVICE_EXISTS"),
(-18, "REMUS_DEVOPS_DOES_NOT_MATCH"),
(-19, "REMUS_DEVICE_NOT_SUPPORTED"),
+ (-20, "CANCELLED"),
+ (-21, "NOTFOUND"),
+ (-22, "NOTIMPLEMENTED"),
], value_namespace = "")
libxl_domain_type = Enumeration("domain_type", [
--
1.7.10.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 10/28] libxl: events: Make timeout and async exec setup take an ao, not a gc
2015-03-31 19:00 [PATCH RFC v3 00/28] libxl: Cancelling asynchronous operations Ian Jackson
` (8 preceding siblings ...)
2015-03-31 19:00 ` [PATCH 09/28] libxl: New error codes CANCELLED etc Ian Jackson
@ 2015-03-31 19:00 ` Ian Jackson
2015-03-31 19:00 ` [PATCH 11/28] libxl: events: Make libxl__async_exec_* pass caller an rc Ian Jackson
` (19 subsequent siblings)
29 siblings, 0 replies; 38+ messages in thread
From: Ian Jackson @ 2015-03-31 19:00 UTC (permalink / raw)
To: xen-devel; +Cc: Yang Hongyang, Ian Jackson, Lai Jiangshan, Wen Congyang
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 <ian.jackson@eu.citrix.com>
CC: Yang Hongyang <yanghy@cn.fujitsu.com>
CC: Wen Congyang <wency@cn.fujitsu.com>
CC: Lai Jiangshan <laijs@cn.fujitsu.com>
Acked-by: Wen Congyang <wency@cn.fujitsu.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
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 291b803..dcce394 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 d4d62a8..8641ae1 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<NTIMES; 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;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 11/28] libxl: events: Make libxl__async_exec_* pass caller an rc
2015-03-31 19:00 [PATCH RFC v3 00/28] libxl: Cancelling asynchronous operations Ian Jackson
` (9 preceding siblings ...)
2015-03-31 19:00 ` [PATCH 10/28] libxl: events: Make timeout and async exec setup take an ao, not a gc Ian Jackson
@ 2015-03-31 19:00 ` Ian Jackson
2015-04-15 15:12 ` Ian Campbell
2015-03-31 19:00 ` [PATCH 12/28] libxl: events: Permit timeouts to signal cancellation Ian Jackson
` (18 subsequent siblings)
29 siblings, 1 reply; 38+ messages in thread
From: Ian Jackson @ 2015-03-31 19:00 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
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 <Ian.Jackson@eu.citrix.com>
---
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)
+ libxl_report_child_exitstatus(CTX, LIBXL__LOG_ERROR,
+ aes->what, pid, status);
}
- aes->callback(egc, aes, status);
+ aes->callback(egc, aes, aes->rc, status);
}
void libxl__async_exec_init(libxl__async_exec_state *aes)
@@ -506,6 +507,8 @@ int libxl__async_exec_start(libxl__async_exec_state *aes)
libxl__ev_child *const child = &aes->child;
char ** const args = aes->args;
+ aes->rc = 0;
+
/* Set execution timeout */
if (libxl__ev_time_register_rel(ao, &aes->time,
async_exec_timeout,
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index c80749f..84114ff 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -729,7 +729,7 @@ static void device_hotplug(libxl__egc *egc, libxl__ao_device *aodev);
static void device_hotplug_child_death_cb(libxl__egc *egc,
libxl__async_exec_state *aes,
- int status);
+ int rc, int status);
static void device_destroy_be_watch_cb(libxl__egc *egc,
libxl__xswait_state *xswait,
@@ -1052,7 +1052,7 @@ out:
static void device_hotplug_child_death_cb(libxl__egc *egc,
libxl__async_exec_state *aes,
- int status)
+ int rc, int status)
{
libxl__ao_device *aodev = CONTAINER_OF(aes, *aodev, aes);
STATE_AO_GC(aodev->ao);
@@ -1061,12 +1061,17 @@ static void device_hotplug_child_death_cb(libxl__egc *egc,
device_hotplug_clean(gc, aodev);
- if (status) {
+ if (status && !rc) {
hotplug_error = libxl__xs_read(gc, XBT_NULL,
GCSPRINTF("%s/hotplug-error", be_path));
if (hotplug_error)
LOG(ERROR, "script: %s", hotplug_error);
- aodev->rc = ERROR_FAIL;
+ rc = ERROR_FAIL;
+ }
+
+ if (rc) {
+ if (!aodev->rc)
+ aodev->rc = rc;
if (aodev->action == LIBXL__DEVICE_ACTION_ADD)
/*
* Only fail on device connection, on disconnection
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index b615fc5..02cac7b 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2089,7 +2089,15 @@ _hidden const char *libxl__run_dir_path(void);
typedef struct libxl__async_exec_state libxl__async_exec_state;
typedef void libxl__async_exec_callback(libxl__egc *egc,
- libxl__async_exec_state *aes, int status);
+ 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.
+ */
struct libxl__async_exec_state {
/* caller must fill these in */
@@ -2105,6 +2113,7 @@ struct libxl__async_exec_state {
/* private */
libxl__ev_time time;
libxl__ev_child child;
+ int rc;
};
void libxl__async_exec_init(libxl__async_exec_state *aes);
diff --git a/tools/libxl/libxl_netbuffer.c b/tools/libxl/libxl_netbuffer.c
index edc6843..ff2d6c7 100644
--- a/tools/libxl/libxl_netbuffer.c
+++ b/tools/libxl/libxl_netbuffer.c
@@ -219,10 +219,10 @@ out:
static void netbuf_setup_script_cb(libxl__egc *egc,
libxl__async_exec_state *aes,
- int status);
+ int rc, int status);
static void netbuf_teardown_script_cb(libxl__egc *egc,
libxl__async_exec_state *aes,
- int status);
+ int rc, int status);
/*
* the script needs the following env & args
@@ -327,14 +327,13 @@ out:
*/
static void netbuf_setup_script_cb(libxl__egc *egc,
libxl__async_exec_state *aes,
- int status)
+ int rc, int status)
{
libxl__ao_device *aodev = CONTAINER_OF(aes, *aodev, aes);
libxl__remus_device *dev = CONTAINER_OF(aodev, *dev, aodev);
libxl__remus_device_nic *remus_nic = dev->concrete_data;
libxl__remus_devices_state *rds = dev->rds;
const char *out_path_base, *hotplug_error = NULL;
- int rc;
STATE_AO_GC(rds->ao);
@@ -344,6 +343,11 @@ static void netbuf_setup_script_cb(libxl__egc *egc,
const char *const vif = remus_nic->vif;
const char **const ifb = &remus_nic->ifb;
+ if (status && !rc)
+ rc = ERROR_FAIL;
+ if (rc)
+ goto out;
+
/*
* we need to get ifb first because it's needed for teardown
*/
@@ -411,17 +415,14 @@ out:
static void netbuf_teardown_script_cb(libxl__egc *egc,
libxl__async_exec_state *aes,
- int status)
+ int rc, int status)
{
- int rc;
libxl__ao_device *aodev = CONTAINER_OF(aes, *aodev, aes);
libxl__remus_device *dev = CONTAINER_OF(aodev, *dev, aodev);
libxl__remus_device_nic *remus_nic = dev->concrete_data;
- if (status)
+ if (status && !rc)
rc = ERROR_FAIL;
- else
- rc = 0;
free_qdisc(remus_nic);
diff --git a/tools/libxl/libxl_remus_disk_drbd.c b/tools/libxl/libxl_remus_disk_drbd.c
index 5e0c9a6..fc76b89 100644
--- a/tools/libxl/libxl_remus_disk_drbd.c
+++ b/tools/libxl/libxl_remus_disk_drbd.c
@@ -78,7 +78,7 @@ out:
/* callbacks */
static void match_async_exec_cb(libxl__egc *egc,
libxl__async_exec_state *aes,
- int status);
+ int rc, int status);
/* implementations */
@@ -133,9 +133,8 @@ out:
static void match_async_exec_cb(libxl__egc *egc,
libxl__async_exec_state *aes,
- int status)
+ int rc, int status)
{
- int rc;
libxl__ao_device *aodev = CONTAINER_OF(aes, *aodev, aes);
libxl__remus_device *dev = CONTAINER_OF(aodev, *dev, aodev);
libxl__remus_drbd_disk *drbd_disk;
@@ -143,6 +142,9 @@ static void match_async_exec_cb(libxl__egc *egc,
STATE_AO_GC(aodev->ao);
+ if (rc)
+ goto out;
+
if (status) {
rc = ERROR_REMUS_DEVOPS_DOES_NOT_MATCH;
/* BUG: seems to assume that any exit status means `no match' */
--
1.7.10.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 12/28] libxl: events: Permit timeouts to signal cancellation
2015-03-31 19:00 [PATCH RFC v3 00/28] libxl: Cancelling asynchronous operations Ian Jackson
` (10 preceding siblings ...)
2015-03-31 19:00 ` [PATCH 11/28] libxl: events: Make libxl__async_exec_* pass caller an rc Ian Jackson
@ 2015-03-31 19:00 ` Ian Jackson
2015-03-31 19:00 ` [PATCH 13/28] libxl: domain create: Do not destroy on cancellation Ian Jackson
` (17 subsequent siblings)
29 siblings, 0 replies; 38+ messages in thread
From: Ian Jackson @ 2015-03-31 19:00 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
The callback functions provided by users must take an rc value. This
rc value can be ERROR_TIMEDOUT or ERROR_CANCELLED.
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@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
tools/libxl/libxl_aoutils.c | 11 ++++++++---
tools/libxl/libxl_device.c | 8 +++++---
tools/libxl/libxl_dom.c | 29 ++++++++++++++++++++---------
tools/libxl/libxl_event.c | 8 ++++----
tools/libxl/libxl_internal.h | 12 +++++++-----
tools/libxl/libxl_test_timedereg.c | 8 +++++---
6 files changed, 49 insertions(+), 27 deletions(-)
diff --git a/tools/libxl/libxl_aoutils.c b/tools/libxl/libxl_aoutils.c
index 891cdb8..0b6d750 100644
--- a/tools/libxl/libxl_aoutils.c
+++ b/tools/libxl/libxl_aoutils.c
@@ -80,12 +80,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,
@@ -455,11 +456,15 @@ int libxl__openptys(libxl__openpty_state *op,
static void async_exec_timeout(libxl__egc *egc,
libxl__ev_time *ev,
- const struct timeval *requested_abs)
+ const struct timeval *requested_abs,
+ int rc)
{
libxl__async_exec_state *aes = CONTAINER_OF(ev, *aes, time);
STATE_AO_GC(aes->ao);
+ if (!aes->rc)
+ aes->rc = rc;
+
libxl__ev_time_deregister(gc, &aes->time);
assert(libxl__ev_child_inuse(&aes->child));
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 84114ff..3b1c3b2 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -717,7 +717,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);
@@ -880,7 +880,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);
@@ -888,7 +888,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);
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index dcce394..379ac07 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -943,7 +943,8 @@ static void 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,
@@ -1069,7 +1070,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);
@@ -1218,7 +1220,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,
@@ -1452,12 +1454,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,
@@ -1690,7 +1695,8 @@ static void remus_devices_commit_cb(libxl__egc *egc,
libxl__remus_devices_state *rds,
int rc);
static void remus_next_checkpoint(libxl__egc *egc, libxl__ev_time *ev,
- const struct timeval *requested_abs);
+ const struct timeval *requested_abs,
+ int rc);
static void libxl__remus_domain_checkpoint_callback(void *data)
{
@@ -1765,7 +1771,8 @@ out:
}
static void remus_next_checkpoint(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, checkpoint_timeout);
@@ -1777,7 +1784,11 @@ static void remus_next_checkpoint(libxl__egc *egc, libxl__ev_time *ev,
* (xc_domain_save.c). in order to continue executing the infinite loop
* (suspend, checkpoint, resume) in xc_domain_save().
*/
- libxl__xc_domain_saverestore_async_callback_done(egc, &dss->shs, 1);
+
+ if (rc)
+ dss->rc = rc;
+
+ libxl__xc_domain_saverestore_async_callback_done(egc, &dss->shs, !rc);
}
/*----- main code for suspending, in order of execution -----*/
diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c
index 8641ae1..8604610 100644
--- a/tools/libxl/libxl_event.c
+++ b/tools/libxl/libxl_event.c
@@ -383,7 +383,7 @@ void libxl__ev_time_deregister(libxl__gc *gc, libxl__ev_time *ev)
return;
}
-static void time_occurs(libxl__egc *egc, libxl__ev_time *etime)
+static void time_occurs(libxl__egc *egc, libxl__ev_time *etime, int rc)
{
DBG("ev_time=%p occurs abs=%lu.%06lu",
etime, (unsigned long)etime->abs.tv_sec,
@@ -391,7 +391,7 @@ static void time_occurs(libxl__egc *egc, libxl__ev_time *etime)
libxl__ev_time_callback *func = etime->func;
etime->func = 0;
- func(egc, etime, &etime->abs);
+ func(egc, etime, &etime->abs, rc);
}
@@ -1188,7 +1188,7 @@ static void afterpoll_internal(libxl__egc *egc, libxl__poller *poller,
time_deregister(gc, etime);
- time_occurs(egc, etime);
+ time_occurs(egc, etime, ERROR_TIMEDOUT);
}
}
@@ -1272,7 +1272,7 @@ void libxl_osevent_occurred_timeout(libxl_ctx *ctx, void *for_libxl)
LIBXL_TAILQ_REMOVE(&CTX->etimes, ev, entry);
- time_occurs(egc, ev);
+ time_occurs(egc, ev, ERROR_TIMEDOUT);
out:
CTX_UNLOCK;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 02cac7b..accbab8 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -175,7 +175,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: */
@@ -1108,13 +1109,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.
@@ -1154,8 +1155,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: */
diff --git a/tools/libxl/libxl_test_timedereg.c b/tools/libxl/libxl_test_timedereg.c
index e2cc27d..c464663 100644
--- a/tools/libxl/libxl_test_timedereg.c
+++ b/tools/libxl/libxl_test_timedereg.c
@@ -28,7 +28,7 @@ static libxl__ao *tao;
static int seq;
static void occurs(libxl__egc *egc, libxl__ev_time *ev,
- const struct timeval *requested_abs);
+ const struct timeval *requested_abs, int rc);
static void regs(libxl__ao *ao, int j)
{
@@ -59,13 +59,15 @@ int libxl_test_timedereg(libxl_ctx *ctx, libxl_asyncop_how *ao_how)
}
static void occurs(libxl__egc *egc, libxl__ev_time *ev,
- const struct timeval *requested_abs)
+ const struct timeval *requested_abs, int rc)
{
EGC_GC;
int i;
int off = ev - &et[0][0];
- LOG(DEBUG,"occurs[%d][%d] seq=%d", off/NTIMES, off%NTIMES, seq);
+ LOG(DEBUG,"occurs[%d][%d] seq=%d rc=%d", off/NTIMES, off%NTIMES, seq, rc);
+
+ assert(!rc);
switch (seq) {
case 0:
--
1.7.10.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 13/28] libxl: domain create: Do not destroy on cancellation
2015-03-31 19:00 [PATCH RFC v3 00/28] libxl: Cancelling asynchronous operations Ian Jackson
` (11 preceding siblings ...)
2015-03-31 19:00 ` [PATCH 12/28] libxl: events: Permit timeouts to signal cancellation Ian Jackson
@ 2015-03-31 19:00 ` Ian Jackson
2015-03-31 19:00 ` [PATCH 14/28] libxl: ao: Record ultimate parent of a nested ao Ian Jackson
` (16 subsequent siblings)
29 siblings, 0 replies; 38+ messages in thread
From: Ian Jackson @ 2015-03-31 19:00 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
If we cancelled the domain creation, do not try to tear it down again
Document this.
This is a backwards-compatible API change since old libxl users will
never cancel any operations.
In the current code, there is no functional change, because
ERROR_CANCELLED is never generated anywhere yet.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
tools/libxl/libxl.h | 4 ++++
tools/libxl/libxl_create.c | 6 ++++--
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 5eec092..dc05e02 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -920,6 +920,10 @@ int libxl_ctx_free(libxl_ctx *ctx /* 0 is OK */);
/* domain related functions */
+/* If the result is ERROR_CANCELLED, the domain may or may not exist
+ * (in a half-created state). *domid will be valid and will be the
+ * domain id, or -1, as appropriate */
+
int libxl_domain_create_new(libxl_ctx *ctx, libxl_domain_config *d_config,
uint32_t *domid,
const libxl_asyncop_how *ao_how,
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 98687bd..f12ed72 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1446,7 +1446,9 @@ static void domcreate_complete(libxl__egc *egc,
if (!rc && d_config->b_info.exec_ssidref)
rc = xc_flask_relabel_domain(CTX->xch, dcs->guest_domid, d_config->b_info.exec_ssidref);
- if (!rc) {
+ bool retain_domain = !rc || rc == ERROR_CANCELLED;
+
+ if (retain_domain) {
libxl__domain_userdata_lock *lock;
/* Note that we hold CTX lock at this point so only need to
@@ -1465,7 +1467,7 @@ static void domcreate_complete(libxl__egc *egc,
libxl_domain_config_dispose(d_config_saved);
- if (rc) {
+ if (!retain_domain) {
if (dcs->guest_domid) {
dcs->dds.ao = ao;
dcs->dds.domid = dcs->guest_domid;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 14/28] libxl: ao: Record ultimate parent of a nested ao
2015-03-31 19:00 [PATCH RFC v3 00/28] libxl: Cancelling asynchronous operations Ian Jackson
` (12 preceding siblings ...)
2015-03-31 19:00 ` [PATCH 13/28] libxl: domain create: Do not destroy on cancellation Ian Jackson
@ 2015-03-31 19:00 ` Ian Jackson
2015-03-31 19:00 ` [PATCH 15/28] libxl: ao: Count the nested progeny of an ao Ian Jackson
` (15 subsequent siblings)
29 siblings, 0 replies; 38+ messages in thread
From: Ian Jackson @ 2015-03-31 19:00 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
This will be used by the cancellation machinery.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
tools/libxl/libxl_event.c | 25 +++++++++++++++----------
tools/libxl/libxl_internal.h | 3 ++-
2 files changed, 17 insertions(+), 11 deletions(-)
diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c
index 8604610..c9ec3c4 100644
--- a/tools/libxl/libxl_event.c
+++ b/tools/libxl/libxl_event.c
@@ -31,6 +31,9 @@
#define DBG(args, ...) LIBXL__DBG_LOG(CTX, args, __VA_ARGS__)
+static libxl__ao *ao_nested_root(libxl__ao *ao);
+
+
/*
* The counter osevent_in_hook is used to ensure that the application
* honours the reentrancy restriction documented in libxl_event.h.
@@ -1759,7 +1762,7 @@ void libxl__ao_complete(libxl__egc *egc, libxl__ao *ao, int rc)
LOG(DEBUG,"ao %p: complete, rc=%d",ao,rc);
assert(ao->magic == LIBXL__AO_MAGIC);
assert(!ao->complete);
- assert(!ao->nested);
+ assert(!ao->nested_root);
ao->complete = 1;
ao->rc = rc;
@@ -1930,7 +1933,7 @@ void libxl__ao_progress_report(libxl__egc *egc, libxl__ao *ao,
const libxl_asyncprogress_how *how, libxl_event *ev)
{
AO_GC;
- assert(!ao->nested);
+ assert(!ao->nested_root);
if (how->callback == dummy_asyncprogress_callback_ignore) {
LOG(DEBUG,"ao %p: progress report: ignored",ao);
libxl_event_free(CTX,ev);
@@ -1953,21 +1956,23 @@ void libxl__ao_progress_report(libxl__egc *egc, libxl__ao *ao,
/* nested ao */
+static libxl__ao *ao_nested_root(libxl__ao *ao) {
+ libxl__ao *root = ao->nested_root ? : ao;
+ assert(!root->nested_root);
+ return root;
+}
+
_hidden libxl__ao *libxl__nested_ao_create(libxl__ao *parent)
{
- /* We only use the parent to get the ctx. However, we require the
- * caller to provide us with an ao, not just a ctx, to prove that
- * they are already in an asynchronous operation. That will avoid
- * people using this to (for example) make an ao in a non-ao_how
- * function somewhere in the middle of libxl. */
- libxl__ao *child = NULL;
+ libxl__ao *child = NULL, *root;
libxl_ctx *ctx = libxl__gc_owner(&parent->gc);
assert(parent->magic == LIBXL__AO_MAGIC);
+ root = ao_nested_root(parent);
child = libxl__zalloc(&ctx->nogc_gc, sizeof(*child));
child->magic = LIBXL__AO_MAGIC;
- child->nested = 1;
+ child->nested_root = root;
LIBXL_INIT_GC(child->gc, ctx);
libxl__gc *gc = &child->gc;
@@ -1978,7 +1983,7 @@ _hidden libxl__ao *libxl__nested_ao_create(libxl__ao *parent)
_hidden void libxl__nested_ao_free(libxl__ao *child)
{
assert(child->magic == LIBXL__AO_MAGIC);
- assert(child->nested);
+ assert(child->nested_root);
libxl_ctx *ctx = libxl__gc_owner(&child->gc);
libxl__ao__destroy(ctx, child);
}
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index accbab8..fe5c94f 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -448,7 +448,8 @@ struct libxl__ao {
* only in libxl__ao_complete.)
*/
uint32_t magic;
- unsigned constructing:1, in_initiator:1, complete:1, notified:1, nested:1;
+ unsigned constructing:1, in_initiator:1, complete:1, notified:1;
+ libxl__ao *nested_root;
int progress_reports_outstanding;
int rc;
libxl__gc gc;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 15/28] libxl: ao: Count the nested progeny of an ao
2015-03-31 19:00 [PATCH RFC v3 00/28] libxl: Cancelling asynchronous operations Ian Jackson
` (13 preceding siblings ...)
2015-03-31 19:00 ` [PATCH 14/28] libxl: ao: Record ultimate parent of a nested ao Ian Jackson
@ 2015-03-31 19:00 ` Ian Jackson
2015-03-31 19:00 ` [PATCH 16/28] libxl: ao: Provide manip_refcnt Ian Jackson
` (14 subsequent siblings)
29 siblings, 0 replies; 38+ messages in thread
From: Ian Jackson @ 2015-03-31 19:00 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
This will detect any "escaped" nested aos.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
tools/libxl/libxl_event.c | 8 +++++++-
tools/libxl/libxl_internal.h | 1 +
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c
index c9ec3c4..c95db5b 100644
--- a/tools/libxl/libxl_event.c
+++ b/tools/libxl/libxl_event.c
@@ -1763,6 +1763,7 @@ void libxl__ao_complete(libxl__egc *egc, libxl__ao *ao, int rc)
assert(ao->magic == LIBXL__AO_MAGIC);
assert(!ao->complete);
assert(!ao->nested_root);
+ assert(!ao->nested_progeny);
ao->complete = 1;
ao->rc = rc;
@@ -1973,6 +1974,8 @@ _hidden libxl__ao *libxl__nested_ao_create(libxl__ao *parent)
child = libxl__zalloc(&ctx->nogc_gc, sizeof(*child));
child->magic = LIBXL__AO_MAGIC;
child->nested_root = root;
+ assert(root->nested_progeny < INT_MAX);
+ root->nested_progeny++;
LIBXL_INIT_GC(child->gc, ctx);
libxl__gc *gc = &child->gc;
@@ -1983,7 +1986,10 @@ _hidden libxl__ao *libxl__nested_ao_create(libxl__ao *parent)
_hidden void libxl__nested_ao_free(libxl__ao *child)
{
assert(child->magic == LIBXL__AO_MAGIC);
- assert(child->nested_root);
+ libxl__ao *root = child->nested_root;
+ assert(root);
+ assert(root->nested_progeny > 0);
+ root->nested_progeny--;
libxl_ctx *ctx = libxl__gc_owner(&child->gc);
libxl__ao__destroy(ctx, child);
}
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index fe5c94f..e29db43 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -450,6 +450,7 @@ struct libxl__ao {
uint32_t magic;
unsigned constructing:1, in_initiator:1, complete:1, notified:1;
libxl__ao *nested_root;
+ int nested_progeny;
int progress_reports_outstanding;
int rc;
libxl__gc gc;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 16/28] libxl: ao: Provide manip_refcnt
2015-03-31 19:00 [PATCH RFC v3 00/28] libxl: Cancelling asynchronous operations Ian Jackson
` (14 preceding siblings ...)
2015-03-31 19:00 ` [PATCH 15/28] libxl: ao: Count the nested progeny of an ao Ian Jackson
@ 2015-03-31 19:00 ` Ian Jackson
2015-04-15 15:13 ` Ian Campbell
2015-03-31 19:00 ` [PATCH 17/28] libxl: cancellation: Provide public ao cancellation API Ian Jackson
` (13 subsequent siblings)
29 siblings, 1 reply; 38+ messages in thread
From: Ian Jackson @ 2015-03-31 19:00 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
Previously we used in_initiator to stop the ao being freed while we
were still in the initiator function (which would result in the
initiator's call to lixl__ao_inprogress accessing the ao after it had
been freed).
We are going to introduce a new libxl entrypoint which finds, and
operates on, ongoing aos. This function needs the same protection,
and might even end up running on the same ao multiple times
concurrently.
So do this with reference counting instead, with a new variable
ao->manip_refcnt.
We keep ao->in_initiator because that allows us to keep some useful
asserts about the sequencing of libxl__ao_inprogress, etc.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
v3: Add a missing space.
Mention locking in the comment.
---
tools/libxl/libxl_event.c | 43 +++++++++++++++++++++++++++++++++---------
tools/libxl/libxl_internal.h | 1 +
2 files changed, 35 insertions(+), 9 deletions(-)
diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c
index c95db5b..fffadf3 100644
--- a/tools/libxl/libxl_event.c
+++ b/tools/libxl/libxl_event.c
@@ -33,6 +33,8 @@
static libxl__ao *ao_nested_root(libxl__ao *ao);
+static void ao__check_destroy(libxl_ctx *ctx, libxl__ao *ao);
+
/*
* The counter osevent_in_hook is used to ensure that the application
@@ -1345,8 +1347,7 @@ static void egc_run_callbacks(libxl__egc *egc)
ao->how.callback(CTX, ao->rc, ao->how.u.for_callback);
CTX_LOCK;
ao->notified = 1;
- if (!ao->in_initiator)
- libxl__ao__destroy(CTX, ao);
+ ao__check_destroy(CTX, ao);
CTX_UNLOCK;
}
}
@@ -1727,6 +1728,33 @@ int libxl_event_wait(libxl_ctx *ctx, libxl_event **event_r,
* - destroy the ao
*/
+
+/*
+ * A "manip" is a libxl public function manipulating this ao, which
+ * has a pointer to it. We have to not destroy it while that's the
+ * case, obviously. Callers must have the ctx locked, obviously.
+ */
+static void ao__manip_enter(libxl__ao *ao)
+{
+ assert(ao->manip_refcnt < INT_MAX);
+ ao->manip_refcnt++;
+}
+
+static void ao__manip_leave(libxl_ctx *ctx, libxl__ao *ao)
+{
+ assert(ao->manip_refcnt > 0);
+ ao->manip_refcnt--;
+ ao__check_destroy(ctx, ao);
+}
+
+static void ao__check_destroy(libxl_ctx *ctx, libxl__ao *ao)
+{
+ if (!ao->manip_refcnt && ao->notified) {
+ assert(ao->complete);
+ libxl__ao__destroy(ctx, ao);
+ }
+}
+
void libxl__ao__destroy(libxl_ctx *ctx, libxl__ao *ao)
{
AO_GC;
@@ -1808,8 +1836,8 @@ void libxl__ao_complete_check_progress_reports(libxl__egc *egc, libxl__ao *ao)
}
ao->notified = 1;
}
- if (!ao->in_initiator && ao->notified)
- libxl__ao__destroy(ctx, ao);
+
+ ao__check_destroy(ctx, ao);
}
libxl__ao *libxl__ao_create(libxl_ctx *ctx, uint32_t domid,
@@ -1824,6 +1852,7 @@ libxl__ao *libxl__ao_create(libxl_ctx *ctx, uint32_t domid,
ao->magic = LIBXL__AO_MAGIC;
ao->constructing = 1;
ao->in_initiator = 1;
+ ao__manip_enter(ao);
ao->poller = 0;
ao->domid = domid;
LIBXL_INIT_GC(ao->gc, ctx);
@@ -1904,11 +1933,7 @@ int libxl__ao_inprogress(libxl__ao *ao,
}
ao->in_initiator = 0;
-
- if (ao->notified) {
- assert(ao->complete);
- libxl__ao__destroy(CTX,ao);
- }
+ ao__manip_leave(CTX, ao);
return rc;
}
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index e29db43..d2c2637 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -449,6 +449,7 @@ struct libxl__ao {
*/
uint32_t magic;
unsigned constructing:1, in_initiator:1, complete:1, notified:1;
+ int manip_refcnt;
libxl__ao *nested_root;
int nested_progeny;
int progress_reports_outstanding;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH 16/28] libxl: ao: Provide manip_refcnt
2015-03-31 19:00 ` [PATCH 16/28] libxl: ao: Provide manip_refcnt Ian Jackson
@ 2015-04-15 15:13 ` Ian Campbell
0 siblings, 0 replies; 38+ messages in thread
From: Ian Campbell @ 2015-04-15 15:13 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel
On Tue, 2015-03-31 at 20:00 +0100, Ian Jackson wrote:
> Previously we used in_initiator to stop the ao being freed while we
> were still in the initiator function (which would result in the
> initiator's call to lixl__ao_inprogress accessing the ao after it had
> been freed).
>
> We are going to introduce a new libxl entrypoint which finds, and
> operates on, ongoing aos. This function needs the same protection,
> and might even end up running on the same ao multiple times
> concurrently.
>
> So do this with reference counting instead, with a new variable
> ao->manip_refcnt.
>
> We keep ao->in_initiator because that allows us to keep some useful
> asserts about the sequencing of libxl__ao_inprogress, etc.
>
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 17/28] libxl: cancellation: Provide public ao cancellation API
2015-03-31 19:00 [PATCH RFC v3 00/28] libxl: Cancelling asynchronous operations Ian Jackson
` (15 preceding siblings ...)
2015-03-31 19:00 ` [PATCH 16/28] libxl: ao: Provide manip_refcnt Ian Jackson
@ 2015-03-31 19:00 ` Ian Jackson
2015-03-31 19:00 ` [PATCH 18/28] libxl: cancellation: Provide explicit internal cancel check API Ian Jackson
` (12 subsequent siblings)
29 siblings, 0 replies; 38+ messages in thread
From: Ian Jackson @ 2015-03-31 19:00 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
Provide libxl_ao_cancel.
There is machinery to allow an ao to register an interest in its
cancellation, using a libxl__ao_cancellable.
This API is not currently very functional: attempting cancellation it
will always return NOTIMPLEMENTED and have no effect.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v2: Minor comment improvements
---
tools/libxl/libxl.c | 3 ++
tools/libxl/libxl.h | 64 ++++++++++++++++++++++
tools/libxl/libxl_event.c | 123 ++++++++++++++++++++++++++++++++++++++++++
tools/libxl/libxl_internal.h | 42 ++++++++++++++-
4 files changed, 231 insertions(+), 1 deletion(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index de0fc6b..19e36f1 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -73,6 +73,8 @@ int libxl_ctx_alloc(libxl_ctx **pctx, int version,
LIBXL_LIST_INIT(&ctx->evtchns_waiting);
libxl__ev_fd_init(&ctx->evtchn_efd);
+ LIBXL_LIST_INIT(&ctx->aos_inprogress);
+
LIBXL_TAILQ_INIT(&ctx->death_list);
libxl__ev_xswatch_init(&ctx->death_watch);
@@ -174,6 +176,7 @@ int libxl_ctx_free(libxl_ctx *ctx)
assert(LIBXL_LIST_EMPTY(&ctx->efds));
assert(LIBXL_TAILQ_EMPTY(&ctx->etimes));
assert(LIBXL_LIST_EMPTY(&ctx->evtchns_waiting));
+ assert(LIBXL_LIST_EMPTY(&ctx->aos_inprogress));
if (ctx->xch) xc_interface_close(ctx->xch);
libxl_version_info_dispose(&ctx->version_info);
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index dc05e02..a688070 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -641,6 +641,11 @@ typedef struct libxl__ctx libxl_ctx;
*/
#define LIBXL_HAVE_DEVICE_CHANNEL 1
+/*
+ * LIBXL_HAVE_AO_CANCEL indicates the availability of libxl_ao_cancel
+ */
+#define LIBXL_HAVE_AO_CANCEL 1
+
/* Functions annotated with LIBXL_EXTERNAL_CALLERS_ONLY may not be
* called from within libxl itself. Callers outside libxl, who
* do not #include libxl_internal.h, are fine. */
@@ -910,6 +915,65 @@ typedef struct {
void *for_callback; /* passed to callback */
} libxl_asyncprogress_how;
+/*
+ * It is sometimes possible to cancel an asynchronous operation.
+ *
+ * libxl_ao_cancel searches for an ongoing asynchronous operation whose
+ * ao_how is identical to *how, and tries to cancel it. The return
+ * values from libxl_ao_cancel are as follows:
+ *
+ * 0
+ *
+ * The operation in question has (at least some) support for
+ * cancellation. It will be cut short. However, it may still
+ * take some time to cancel.
+ *
+ * ERROR_NOTFOUND
+ *
+ * No matching ongoing operation was found. This might happen
+ * for an actual operation if the operation has already completed
+ * (perhaps on another thread). The call to libxl_ao_cancel has
+ * had no effect.
+ *
+ * ERROR_NOTIMPLEMENTED
+ *
+ * As far as could be determined, the operation in question does
+ * not support cancellation. The operation may subsequently
+ * complete normally, as if it had never been cancelled; however,
+ * the cancellation attempt will still have been noted and it is
+ * possible that the operation will be successfully cancelled.
+ *
+ * ERROR_CANCELLED
+ *
+ * The operation has already been the subject of at least one
+ * call to libxl_ao_cancel.
+ *
+ * If the operation was indeed cut short due to the cancellation, it
+ * will complete, at some point in the future, with ERROR_CANCELLED.
+ * In that case, depending on the operation it have performed some of
+ * the work in question and left the operation half-done. Consult the
+ * documentation for individual operations.
+ *
+ * Note that a cancelled operation might still fail for other reasons
+ * even after it has been cancelled.
+ *
+ * If your application is multithreaded you must not reuse an
+ * ao_how->for_event or ao_how->for_callback value (with a particular
+ * ao_how->callback) unless you are sure that none of your other
+ * threads are going to cancel the previous operation using that
+ * value; otherwise you risk cancelling the wrong operation if the
+ * intended target of the cancellation completes in the meantime.
+ *
+ * It is possible to cancel even an operation which is being performed
+ * synchronously, but since in that case how==NULL you had better only
+ * have one such operation, because it is not possible to tell them
+ * apart. (And, if you want to do this, obviously the cancellation
+ * would have to be requested on a different thread.)
+ */
+int libxl_ao_cancel(libxl_ctx *ctx, const libxl_asyncop_how *how)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
+
+
#define LIBXL_VERSION 0
/* context functions */
diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c
index fffadf3..c224715 100644
--- a/tools/libxl/libxl_event.c
+++ b/tools/libxl/libxl_event.c
@@ -1774,6 +1774,7 @@ void libxl__ao_abort(libxl__ao *ao)
assert(ao->in_initiator);
assert(!ao->complete);
assert(!ao->progress_reports_outstanding);
+ assert(!ao->cancelling);
libxl__ao__destroy(CTX, ao);
}
@@ -1939,6 +1940,128 @@ int libxl__ao_inprogress(libxl__ao *ao,
}
+/* cancellation */
+
+static int ao__cancel(libxl_ctx *ctx, libxl__ao *parent)
+/* Temporarily unlocks ctx, which must be locked exactly once on entry. */
+{
+ int rc;
+ ao__manip_enter(parent);
+
+ if (parent->cancelling) {
+ rc = ERROR_CANCELLED;
+ goto out;
+ }
+
+ parent->cancelling = 1;
+
+ if (LIBXL_LIST_EMPTY(&parent->cancellables)) {
+ LIBXL__LOG(ctx, XTL_DEBUG,
+ "ao %p: cancellation requested, but not not implemented",
+ parent);
+ rc = ERROR_NOTIMPLEMENTED;
+ goto out;
+ }
+
+ /* We keep calling cancellation hooks until there are none left */
+ while (!LIBXL_LIST_EMPTY(&parent->cancellables)) {
+ libxl__egc egc;
+ LIBXL_INIT_EGC(egc,ctx);
+
+ assert(!parent->complete);
+
+ libxl__ao_cancellable *canc = LIBXL_LIST_FIRST(&parent->cancellables);
+ assert(parent == ao_nested_root(canc->ao));
+
+ LIBXL_LIST_REMOVE(canc, entry);
+ canc->registered = 0;
+
+ LIBXL__LOG(ctx, XTL_DEBUG, "ao %p: canc=%p: cancelling",
+ parent, canc->ao);
+ canc->callback(&egc, canc, ERROR_CANCELLED);
+
+ libxl__ctx_unlock(ctx);
+ libxl__egc_cleanup(&egc);
+ libxl__ctx_lock(ctx);
+ }
+
+ rc = 0;
+
+ out:
+ ao__manip_leave(ctx, parent);
+ return rc;
+}
+
+_hidden int libxl_ao_cancel(libxl_ctx *ctx, const libxl_asyncop_how *how)
+{
+ libxl__ao *search;
+ libxl__ctx_lock(ctx);
+ int rc;
+
+ LIBXL_LIST_FOREACH(search, &ctx->aos_inprogress, inprogress_entry) {
+ if (how) {
+ /* looking for ao to be reported by callback or event */
+ if (search->poller)
+ /* sync */
+ continue;
+ if (how->callback != search->how.callback)
+ continue;
+ if (how->callback
+ ? (how->u.for_callback != search->how.u.for_callback)
+ : (how->u.for_event != search->how.u.for_event))
+ continue;
+ } else {
+ /* looking for synchronous call */
+ if (!search->poller)
+ /* async */
+ continue;
+ }
+ goto found;
+ }
+ rc = ERROR_NOTFOUND;
+ goto out;
+
+ found:
+ rc = ao__cancel(ctx, search);
+ out:
+ libxl__ctx_unlock(ctx);
+ return rc;
+}
+
+int libxl__ao_cancellable_register(libxl__ao_cancellable *canc)
+{
+ libxl__ao *ao = canc->ao;
+ libxl__ao *root = ao_nested_root(ao);
+ AO_GC;
+
+ if (root->cancelling) {
+ DBG("ao=%p: preemptively cancelling cancellable registration %p (root=%p)",
+ ao, canc, root);
+ return ERROR_CANCELLED;
+ }
+
+ DBG("ao=%p, canc=%p: registering (root=%p)", ao, canc, root);
+ LIBXL_LIST_INSERT_HEAD(&root->cancellables, canc, entry);
+ canc->registered = 1;
+
+ return 0;
+}
+
+_hidden void libxl__ao_cancellable_deregister(libxl__ao_cancellable *canc)
+{
+ if (!canc->registered)
+ return;
+
+ libxl__ao *ao = canc->ao;
+ libxl__ao *root __attribute__((unused)) = ao_nested_root(ao);
+ AO_GC;
+
+ DBG("ao=%p, canc=%p: deregistering (root=%p)", ao, canc, root);
+ LIBXL_LIST_REMOVE(canc, entry);
+ canc->registered = 0;
+}
+
+
/* progress reporting */
/* The application indicates a desire to ignore events by passing NULL
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index d2c2637..46383c4 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -173,6 +173,41 @@ struct libxl__ev_fd {
};
+typedef struct libxl__ao_cancellable libxl__ao_cancellable;
+typedef void libxl__ao_cancellable_callback(libxl__egc *egc,
+ libxl__ao_cancellable *cancellable, int rc /* CANCELLED */);
+
+struct libxl__ao_cancellable {
+ /* caller must fill this in and it must remain valid */
+ libxl__ao *ao;
+ libxl__ao_cancellable_callback *callback;
+ /* remainder is private for cancellation machinery */
+ bool registered;
+ LIBXL_LIST_ENTRY(libxl__ao_cancellable) entry;
+ /*
+ * For nested aos:
+ * Semantically, cancellation affects the whole tree of aos,
+ * not just the parent.
+ * libxl__ao_cancellable.ao refers to the child, so
+ * that the child callback sees the right ao. (After all,
+ * it was code dealing with the child that set .ao.)
+ * But, the cancellable is recorded on the "cancellables" list
+ * for the ultimate root ao, so that every possible child
+ * cancellation occurs as a result of the cancellation of the
+ * parent.
+ * We set ao->cancelling only in the root.
+ */
+};
+
+_hidden int libxl__ao_cancellable_register(libxl__ao_cancellable*);
+_hidden void libxl__ao_cancellable_deregister(libxl__ao_cancellable*);
+
+static inline void libxl__ao_cancellable_init
+ (libxl__ao_cancellable *c) { c->registered = 0; }
+static inline bool libxl__ao_cancellable_isregistered
+ (const libxl__ao_cancellable *c) { return c->registered; }
+
+
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,
@@ -362,6 +397,8 @@ struct libxl__ctx {
LIBXL_LIST_HEAD(, libxl__ev_evtchn) evtchns_waiting;
libxl__ev_fd evtchn_efd;
+ LIBXL_LIST_HEAD(, libxl__ao) aos_inprogress;
+
LIBXL_TAILQ_HEAD(libxl__evgen_domain_death_list, libxl_evgen_domain_death)
death_list /* sorted by domid */,
death_reported;
@@ -448,12 +485,15 @@ struct libxl__ao {
* only in libxl__ao_complete.)
*/
uint32_t magic;
- unsigned constructing:1, in_initiator:1, complete:1, notified:1;
+ unsigned constructing:1, in_initiator:1, complete:1, notified:1,
+ cancelling:1;
int manip_refcnt;
libxl__ao *nested_root;
int nested_progeny;
int progress_reports_outstanding;
int rc;
+ LIBXL_LIST_HEAD(, libxl__ao_cancellable) cancellables;
+ LIBXL_LIST_ENTRY(libxl__ao) inprogress_entry;
libxl__gc gc;
libxl_asyncop_how how;
libxl__poller *poller;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 18/28] libxl: cancellation: Provide explicit internal cancel check API
2015-03-31 19:00 [PATCH RFC v3 00/28] libxl: Cancelling asynchronous operations Ian Jackson
` (16 preceding siblings ...)
2015-03-31 19:00 ` [PATCH 17/28] libxl: cancellation: Provide public ao cancellation API Ian Jackson
@ 2015-03-31 19:00 ` Ian Jackson
2015-03-31 19:00 ` [PATCH 19/28] libxl: cancellation: Make timeouts cancellable Ian Jackson
` (11 subsequent siblings)
29 siblings, 0 replies; 38+ messages in thread
From: Ian Jackson @ 2015-03-31 19:00 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
Some places in libxl which can't handle cancellation via a
libxl__ao_cancellable callback might nevertheless benefit from being
able to explicitly check for cancellation.
Provide the (fairly trivial) internal API function to do this.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v2: New in this version of the series.
---
tools/libxl/libxl_event.c | 11 +++++++++++
tools/libxl/libxl_internal.h | 2 ++
2 files changed, 13 insertions(+)
diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c
index c224715..d7c478c 100644
--- a/tools/libxl/libxl_event.c
+++ b/tools/libxl/libxl_event.c
@@ -2028,6 +2028,17 @@ _hidden int libxl_ao_cancel(libxl_ctx *ctx, const libxl_asyncop_how *how)
return rc;
}
+int libxl__ao_cancelling(libxl__ao *ao)
+{
+ libxl__ao *root = ao_nested_root(ao);
+ if (root->cancelling) {
+ DBG("ao=%p: cancelling at explicit check (root=%p)", ao, root);
+ return ERROR_CANCELLED;
+ }
+
+ return 0;
+}
+
int libxl__ao_cancellable_register(libxl__ao_cancellable *canc)
{
libxl__ao *ao = canc->ao;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 46383c4..6caf042 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -207,6 +207,8 @@ static inline void libxl__ao_cancellable_init
static inline bool libxl__ao_cancellable_isregistered
(const libxl__ao_cancellable *c) { return c->registered; }
+int libxl__ao_cancelling(libxl__ao *ao); /* -> 0 or ERROR_CANCELLED */
+
typedef struct libxl__ev_time libxl__ev_time;
typedef void libxl__ev_time_callback(libxl__egc *egc, libxl__ev_time *ev,
--
1.7.10.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 19/28] libxl: cancellation: Make timeouts cancellable
2015-03-31 19:00 [PATCH RFC v3 00/28] libxl: Cancelling asynchronous operations Ian Jackson
` (17 preceding siblings ...)
2015-03-31 19:00 ` [PATCH 18/28] libxl: cancellation: Provide explicit internal cancel check API Ian Jackson
@ 2015-03-31 19:00 ` Ian Jackson
2015-03-31 19:00 ` [PATCH 20/28] libxl: cancellation: Note that driver domain task cannot be usefully cancelled Ian Jackson
` (10 subsequent siblings)
29 siblings, 0 replies; 38+ messages in thread
From: Ian Jackson @ 2015-03-31 19:00 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
Make libxl__ev_time* register with the cancellation machinery, so that
libxl_ao_cancel can cancel any operation which has a timeout.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
tools/libxl/libxl_event.c | 27 +++++++++++++++++++++++++++
tools/libxl/libxl_internal.h | 3 ++-
2 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c
index d7c478c..db3d419 100644
--- a/tools/libxl/libxl_event.c
+++ b/tools/libxl/libxl_event.c
@@ -292,6 +292,8 @@ static int time_register_finite(libxl__gc *gc, libxl__ev_time *ev,
static void time_deregister(libxl__gc *gc, libxl__ev_time *ev)
{
+ libxl__ao_cancellable_deregister(&ev->cancel);
+
if (!ev->infinite) {
struct timeval right_away = { 0, 0 };
if (ev->nexus) /* only set if app provided hooks */
@@ -314,6 +316,23 @@ static void time_done_debug(libxl__gc *gc, const char *func,
#endif
}
+static void time_cancelled(libxl__egc *egc, libxl__ao_cancellable *canc, int rc)
+{
+ libxl__ev_time *ev = CONTAINER_OF(canc, *ev, cancel);
+ EGC_GC;
+
+ time_deregister(gc, ev);
+ DBG("ev_time=%p cancelled", ev);
+ ev->func(egc, ev, &ev->abs, rc);
+}
+
+static int time_register_cancel(libxl__ao *ao, libxl__ev_time *ev)
+{
+ ev->cancel.ao = ao;
+ ev->cancel.callback = time_cancelled;
+ return libxl__ao_cancellable_register(&ev->cancel);
+}
+
int libxl__ev_time_register_abs(libxl__ao *ao, libxl__ev_time *ev,
libxl__ev_time_callback *func,
struct timeval absolute)
@@ -326,6 +345,9 @@ int libxl__ev_time_register_abs(libxl__ao *ao, libxl__ev_time *ev,
DBG("ev_time=%p register abs=%lu.%06lu",
ev, (unsigned long)absolute.tv_sec, (unsigned long)absolute.tv_usec);
+ rc = time_register_cancel(ao, ev);
+ if (rc) goto out;
+
rc = time_register_finite(gc, ev, absolute);
if (rc) goto out;
@@ -333,6 +355,7 @@ int libxl__ev_time_register_abs(libxl__ao *ao, libxl__ev_time *ev,
rc = 0;
out:
+ libxl__ao_cancellable_deregister(&ev->cancel);
time_done_debug(gc,__func__,ev,rc);
CTX_UNLOCK;
return rc;
@@ -351,6 +374,9 @@ int libxl__ev_time_register_rel(libxl__ao *ao, libxl__ev_time *ev,
DBG("ev_time=%p register ms=%d", ev, milliseconds);
+ rc = time_register_cancel(ao, ev);
+ if (rc) goto out;
+
if (milliseconds < 0) {
ev->infinite = 1;
} else {
@@ -365,6 +391,7 @@ int libxl__ev_time_register_rel(libxl__ao *ao, libxl__ev_time *ev,
rc = 0;
out:
+ libxl__ao_cancellable_deregister(&ev->cancel);
time_done_debug(gc,__func__,ev,rc);
CTX_UNLOCK;
return rc;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 6caf042..790a489 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -223,6 +223,7 @@ struct libxl__ev_time {
LIBXL_TAILQ_ENTRY(libxl__ev_time) entry;
struct timeval abs;
libxl__osevent_hook_nexus *nexus;
+ libxl__ao_cancellable cancel;
};
typedef struct libxl__ev_xswatch libxl__ev_xswatch;
@@ -828,7 +829,7 @@ _hidden int libxl__ev_time_modify_abs(libxl__gc*, libxl__ev_time *ev,
struct timeval);
_hidden void libxl__ev_time_deregister(libxl__gc*, libxl__ev_time *ev);
static inline void libxl__ev_time_init(libxl__ev_time *ev)
- { ev->func = 0; }
+ { ev->func = 0; libxl__ao_cancellable_init(&ev->cancel); }
static inline int libxl__ev_time_isregistered(const libxl__ev_time *ev)
{ return !!ev->func; }
--
1.7.10.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 20/28] libxl: cancellation: Note that driver domain task cannot be usefully cancelled
2015-03-31 19:00 [PATCH RFC v3 00/28] libxl: Cancelling asynchronous operations Ian Jackson
` (18 preceding siblings ...)
2015-03-31 19:00 ` [PATCH 19/28] libxl: cancellation: Make timeouts cancellable Ian Jackson
@ 2015-03-31 19:00 ` Ian Jackson
2015-03-31 19:00 ` [PATCH 21/28] libxl: Introduce DOMAIN_DESTROYED error code Ian Jackson
` (9 subsequent siblings)
29 siblings, 0 replies; 38+ messages in thread
From: Ian Jackson @ 2015-03-31 19:00 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Roger Pau Monné
In practice, cancelling this task will cause all subsequent actual
backend operations to fail, but will not actually cause the
libxl_device_events_handler operation to complete.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v2: New in this version of the series.
---
tools/libxl/libxl.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index a688070..04c399f 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -1371,6 +1371,9 @@ libxl_device_pci *libxl_device_pci_list(libxl_ctx *ctx, uint32_t domid,
* From a libxl API point of view, this starts a long-running
* operation. That operation consists of "being a driver domain"
* and never completes.
+ *
+ * Attempting to cancel this operation is not advisable; proper
+ * shutdown of the driver domain task is not supported.
*/
int libxl_device_events_handler(libxl_ctx *ctx,
const libxl_asyncop_how *ao_how)
--
1.7.10.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 21/28] libxl: Introduce DOMAIN_DESTROYED error code
2015-03-31 19:00 [PATCH RFC v3 00/28] libxl: Cancelling asynchronous operations Ian Jackson
` (19 preceding siblings ...)
2015-03-31 19:00 ` [PATCH 20/28] libxl: cancellation: Note that driver domain task cannot be usefully cancelled Ian Jackson
@ 2015-03-31 19:00 ` Ian Jackson
2015-03-31 19:00 ` [PATCH 22/28] libxl: cancellation: Support cancellation where we spot domain death Ian Jackson
` (8 subsequent siblings)
29 siblings, 0 replies; 38+ messages in thread
From: Ian Jackson @ 2015-03-31 19:00 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
This is currently reported only by the bootloader code, if the domain
is destroyed while the bootloader is running.
In the future it would be nice to return it for other circumstances
where the domain existed when the operation started but subsequently
vanished.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v2: New in this version of the series.
---
tools/libxl/libxl_bootloader.c | 2 +-
tools/libxl/libxl_types.idl | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/libxl/libxl_bootloader.c b/tools/libxl/libxl_bootloader.c
index 79947d4..c3f3a1f 100644
--- a/tools/libxl/libxl_bootloader.c
+++ b/tools/libxl/libxl_bootloader.c
@@ -611,7 +611,7 @@ static void bootloader_display_copyfail(libxl__egc *egc,
static void bootloader_domaindeath(libxl__egc *egc, libxl__domaindeathcheck *dc)
{
libxl__bootloader_state *bl = CONTAINER_OF(dc, *bl, deathcheck);
- bootloader_stop(egc, bl, ERROR_FAIL);
+ bootloader_stop(egc, bl, ERROR_DOMAIN_DESTROYED);
}
static void bootloader_finished(libxl__egc *egc, libxl__ev_child *child,
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 478c561..2ddaef1 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -66,6 +66,7 @@ libxl_error = Enumeration("error", [
(-20, "CANCELLED"),
(-21, "NOTFOUND"),
(-22, "NOTIMPLEMENTED"),
+ (-23, "DOMAIN_DESTROYED"),
], value_namespace = "")
libxl_domain_type = Enumeration("domain_type", [
--
1.7.10.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 22/28] libxl: cancellation: Support cancellation where we spot domain death
2015-03-31 19:00 [PATCH RFC v3 00/28] libxl: Cancelling asynchronous operations Ian Jackson
` (20 preceding siblings ...)
2015-03-31 19:00 ` [PATCH 21/28] libxl: Introduce DOMAIN_DESTROYED error code Ian Jackson
@ 2015-03-31 19:00 ` Ian Jackson
2015-03-31 19:01 ` [PATCH 23/28] libxl: Introduce FILLZERO Ian Jackson
` (7 subsequent siblings)
29 siblings, 0 replies; 38+ messages in thread
From: Ian Jackson @ 2015-03-31 19:00 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
Make an active libxl__domaindeathcheck contain an active
libxl__ao_cancellable.
Consequential changes are:
* domaindeath callbacks now take an rc value.
* libxl__domaindeathcheck_start takes an ao, not a gc.
* bootloader_domaindeath plumbs the rc through to its caller.
* libxl__domaindeathcheck_init and _stop are not quite trivial any
more so are moved from (inline functions) in libxl_internal.h, to
ordinary functions defined in libxl_event.c.
* libxl__domaindeathcheck_start is not trivial any more, and now has
the standard error-handling pattern.
The only current user of libxl__domaindeathcheck is the bootloader.
So the result is that now it is possible to effectively cancel domain
creation while the bootloader is running.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v2: New in this version of the series.
---
tools/libxl/libxl_bootloader.c | 11 +++++----
tools/libxl/libxl_event.c | 50 ++++++++++++++++++++++++++++++++++++----
tools/libxl/libxl_internal.h | 12 +++++-----
3 files changed, 59 insertions(+), 14 deletions(-)
diff --git a/tools/libxl/libxl_bootloader.c b/tools/libxl/libxl_bootloader.c
index c3f3a1f..21f92dc 100644
--- a/tools/libxl/libxl_bootloader.c
+++ b/tools/libxl/libxl_bootloader.c
@@ -33,7 +33,8 @@ static void bootloader_keystrokes_copyfail(libxl__egc *egc,
libxl__datacopier_state *dc, int onwrite, int errnoval);
static void bootloader_display_copyfail(libxl__egc *egc,
libxl__datacopier_state *dc, int onwrite, int errnoval);
-static void bootloader_domaindeath(libxl__egc*, libxl__domaindeathcheck *dc);
+static void bootloader_domaindeath(libxl__egc*, libxl__domaindeathcheck *dc,
+ int rc);
static void bootloader_finished(libxl__egc *egc, libxl__ev_child *child,
pid_t pid, int status);
@@ -496,7 +497,7 @@ static void bootloader_gotptys(libxl__egc *egc, libxl__openpty_state *op)
bl->deathcheck.what = "stopping bootloader";
bl->deathcheck.domid = bl->domid;
bl->deathcheck.callback = bootloader_domaindeath;
- rc = libxl__domaindeathcheck_start(gc, &bl->deathcheck);
+ rc = libxl__domaindeathcheck_start(ao, &bl->deathcheck);
if (rc) goto out;
if (bl->console_available)
@@ -608,10 +609,12 @@ static void bootloader_display_copyfail(libxl__egc *egc,
bootloader_copyfail(egc, "bootloader output", bl, 1, onwrite, errnoval);
}
-static void bootloader_domaindeath(libxl__egc *egc, libxl__domaindeathcheck *dc)
+static void bootloader_domaindeath(libxl__egc *egc,
+ libxl__domaindeathcheck *dc,
+ int rc)
{
libxl__bootloader_state *bl = CONTAINER_OF(dc, *bl, deathcheck);
- bootloader_stop(egc, bl, ERROR_DOMAIN_DESTROYED);
+ bootloader_stop(egc, bl, rc);
}
static void bootloader_finished(libxl__egc *egc, libxl__ev_child *child,
diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c
index db3d419..e28c465 100644
--- a/tools/libxl/libxl_event.c
+++ b/tools/libxl/libxl_event.c
@@ -921,6 +921,18 @@ int libxl__ev_devstate_wait(libxl__ao *ao, libxl__ev_devstate *ds,
* futile.
*/
+void libxl__domaindeathcheck_init(libxl__domaindeathcheck *dc)
+{
+ libxl__ao_cancellable_init(&dc->cancel);
+ libxl__ev_xswatch_init(&dc->watch);
+}
+
+void libxl__domaindeathcheck_stop(libxl__gc *gc, libxl__domaindeathcheck *dc)
+{
+ libxl__ao_cancellable_deregister(&dc->cancel);
+ libxl__ev_xswatch_deregister(gc,&dc->watch);
+}
+
static void domaindeathcheck_callback(libxl__egc *egc, libxl__ev_xswatch *w,
const char *watch_path, const char *event_path)
{
@@ -929,6 +941,8 @@ static void domaindeathcheck_callback(libxl__egc *egc, libxl__ev_xswatch *w,
const char *p = libxl__xs_read(gc, XBT_NULL, watch_path);
if (p) return;
+ libxl__domaindeathcheck_stop(gc,dc);
+
if (errno!=ENOENT) {
LIBXL__EVENT_DISASTER(egc,"failed to read xenstore"
" for domain detach check", errno, 0);
@@ -937,15 +951,43 @@ static void domaindeathcheck_callback(libxl__egc *egc, libxl__ev_xswatch *w,
LOG(ERROR,"%s: domain %"PRIu32" removed (%s no longer in xenstore)",
dc->what, dc->domid, watch_path);
- dc->callback(egc, dc);
+ dc->callback(egc, dc, ERROR_DOMAIN_DESTROYED);
+}
+
+static void domaindeathcheck_cancel(libxl__egc *egc,
+ libxl__ao_cancellable *cancel,
+ int rc)
+{
+ libxl__domaindeathcheck *dc = CONTAINER_OF(cancel, *dc, cancel);
+ EGC_GC;
+
+ libxl__domaindeathcheck_stop(gc,dc);
+ dc->callback(egc, dc, rc);
}
-int libxl__domaindeathcheck_start(libxl__gc *gc,
+int libxl__domaindeathcheck_start(libxl__ao *ao,
libxl__domaindeathcheck *dc)
{
+ AO_GC;
+ int rc;
const char *path = GCSPRINTF("/local/domain/%"PRIu32, dc->domid);
- return libxl__ev_xswatch_register(gc, &dc->watch,
- domaindeathcheck_callback, path);
+
+ libxl__domaindeathcheck_init(dc);
+
+ dc->cancel.ao = ao;
+ dc->cancel.callback = domaindeathcheck_cancel;
+ rc = libxl__ao_cancellable_register(&dc->cancel);
+ if (rc) goto out;
+
+ rc = libxl__ev_xswatch_register(gc, &dc->watch,
+ domaindeathcheck_callback, path);
+ if (rc) goto out;
+
+ return 0;
+
+ out:
+ libxl__domaindeathcheck_stop(gc,dc);
+ return rc;
}
/*
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 790a489..36a13ea 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1238,7 +1238,8 @@ _hidden int libxl__ev_devstate_wait(libxl__ao *ao, libxl__ev_devstate *ds,
typedef struct libxl__domaindeathcheck libxl__domaindeathcheck;
typedef void libxl___domaindeathcheck_callback(libxl__egc *egc,
- libxl__domaindeathcheck*);
+ libxl__domaindeathcheck*,
+ int rc /* DESTROYED or CANCELLED */);
struct libxl__domaindeathcheck {
/* must be filled in by caller, and remain valid: */
@@ -1246,16 +1247,15 @@ struct libxl__domaindeathcheck {
uint32_t domid;
libxl___domaindeathcheck_callback *callback;
/* private */
+ libxl__ao_cancellable cancel;
libxl__ev_xswatch watch;
};
-_hidden int libxl__domaindeathcheck_start(libxl__gc *gc,
+_hidden int libxl__domaindeathcheck_start(libxl__ao *ao,
libxl__domaindeathcheck *dc);
-static inline void libxl__domaindeathcheck_init
- (libxl__domaindeathcheck *dc) { libxl__ev_xswatch_init(&dc->watch); }
-static inline void libxl__domaindeathcheck_stop(libxl__gc *gc,
- libxl__domaindeathcheck *dc) { libxl__ev_xswatch_deregister(gc,&dc->watch); }
+void libxl__domaindeathcheck_init(libxl__domaindeathcheck *dc);
+void libxl__domaindeathcheck_stop(libxl__gc *gc, libxl__domaindeathcheck *dc);
/*
--
1.7.10.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 23/28] libxl: Introduce FILLZERO
2015-03-31 19:00 [PATCH RFC v3 00/28] libxl: Cancelling asynchronous operations Ian Jackson
` (21 preceding siblings ...)
2015-03-31 19:00 ` [PATCH 22/28] libxl: cancellation: Support cancellation where we spot domain death Ian Jackson
@ 2015-03-31 19:01 ` Ian Jackson
2015-04-15 15:14 ` Ian Campbell
2015-03-31 19:01 ` [PATCH 24/28] libxl: cancellation: Preparations for save/restore cancellation Ian Jackson
` (6 subsequent siblings)
29 siblings, 1 reply; 38+ messages in thread
From: Ian Jackson @ 2015-03-31 19:01 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
FILLZERO is a macro for memset(&foo,0,sizeof(foo)). It eliminates the
possiblity to make the error memset(&foo,0,sizeof(&foo)).
No callers yet, but document it in CODING_STYLE. (In accordance with
existing libxl policy, I haven't gone through all existing possible
call sites.)
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
v2: New in this version of the series.
---
tools/libxl/CODING_STYLE | 1 +
tools/libxl/libxl_internal.h | 3 +++
tools/libxl/libxl_utils.h | 3 +++
3 files changed, 7 insertions(+)
diff --git a/tools/libxl/CODING_STYLE b/tools/libxl/CODING_STYLE
index f5b5890..a65efb3 100644
--- a/tools/libxl/CODING_STYLE
+++ b/tools/libxl/CODING_STYLE
@@ -62,6 +62,7 @@ whenever they are applicable. For example:
libxl__ctx_[un]lock CTX_LOCK, CTX_UNLOCK
gc=...; ao=...; EGC_GC, AO_GC, STATE_AO_GC
explicit gc creation GC_INIT, GC_FREE
+ memset(..,0,sizeof..) FILLZERO
ERROR HANDLING
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 36a13ea..465cdda 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -3225,6 +3225,9 @@ _hidden const char *libxl__device_model_savefile(libxl__gc *gc, uint32_t domid);
})
+#define FILLZERO LIBXL_FILLZERO
+
+
/*
* All of these assume (or define)
* libxl__gc *gc;
diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
index acacdd9..51eac68 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -154,6 +154,9 @@ int libxl_cpumap_to_nodemap(libxl_ctx *ctx,
void libxl_string_copy(libxl_ctx *ctx, char **dst, char **src);
+
+#define LIBXL_FILLZERO(object) (memset(&(object), 0, sizeof((object))))
+
#endif
/*
--
1.7.10.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH 23/28] libxl: Introduce FILLZERO
2015-03-31 19:01 ` [PATCH 23/28] libxl: Introduce FILLZERO Ian Jackson
@ 2015-04-15 15:14 ` Ian Campbell
0 siblings, 0 replies; 38+ messages in thread
From: Ian Campbell @ 2015-04-15 15:14 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel
On Tue, 2015-03-31 at 20:01 +0100, Ian Jackson wrote:
> FILLZERO is a macro for memset(&foo,0,sizeof(foo)). It eliminates the
> possiblity to make the error memset(&foo,0,sizeof(&foo)).
>
> No callers yet, but document it in CODING_STYLE. (In accordance with
> existing libxl policy, I haven't gone through all existing possible
> call sites.)
>
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 24/28] libxl: cancellation: Preparations for save/restore cancellation
2015-03-31 19:00 [PATCH RFC v3 00/28] libxl: Cancelling asynchronous operations Ian Jackson
` (22 preceding siblings ...)
2015-03-31 19:01 ` [PATCH 23/28] libxl: Introduce FILLZERO Ian Jackson
@ 2015-03-31 19:01 ` Ian Jackson
2015-03-31 19:01 ` [PATCH 25/28] libxl: cancellation: Handle SIGTERM in save/restore helper Ian Jackson
` (5 subsequent siblings)
29 siblings, 0 replies; 38+ messages in thread
From: Ian Jackson @ 2015-03-31 19:01 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
Two unrelated non-functional changes, broken out into a pre-patch for
easier review:
Break out a function sendsig() in libxl_save_callout.c.
Move io_fd to be a global variable in libxl_save_helper.c.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v2: New in this version of the series.
---
tools/libxl/libxl_save_callout.c | 10 +++++++---
tools/libxl/libxl_save_helper.c | 5 +++--
2 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/tools/libxl/libxl_save_callout.c b/tools/libxl/libxl_save_callout.c
index 40b25e4..1d584f1 100644
--- a/tools/libxl/libxl_save_callout.c
+++ b/tools/libxl/libxl_save_callout.c
@@ -237,6 +237,12 @@ static void run_helper(libxl__egc *egc, libxl__save_helper_state *shs,
libxl__carefd_close(childs_pipes[1]);
helper_failed(egc, shs, rc);;
}
+static void sendsig(libxl__gc *gc, libxl__save_helper_state *shs, int sig)
+{
+ int r = kill(shs->child.pid, sig);
+ if (r) LOGE(WARN, "failed to kill save/restore helper [%lu] (signal %d)",
+ (unsigned long)shs->child.pid, sig);
+}
static void helper_failed(libxl__egc *egc, libxl__save_helper_state *shs,
int rc)
@@ -253,9 +259,7 @@ static void helper_failed(libxl__egc *egc, libxl__save_helper_state *shs,
return;
}
- int r = kill(shs->child.pid, SIGKILL);
- if (r) LOGE(WARN, "failed to kill save/restore helper [%lu]",
- (unsigned long)shs->child.pid);
+ sendsig(gc, shs, SIGKILL);
}
static void helper_stdout_readable(libxl__egc *egc, libxl__ev_fd *ev,
diff --git a/tools/libxl/libxl_save_helper.c b/tools/libxl/libxl_save_helper.c
index 74826a1..7514b2e 100644
--- a/tools/libxl/libxl_save_helper.c
+++ b/tools/libxl/libxl_save_helper.c
@@ -85,6 +85,7 @@ static xentoollog_logger logger = {
tellparent_destroy,
};
static xc_interface *xch;
+static int io_fd;
/*----- error handling -----*/
@@ -211,7 +212,7 @@ int main(int argc, char **argv)
if (!strcmp(mode,"--save-domain")) {
- int io_fd = atoi(NEXTARG);
+ io_fd = atoi(NEXTARG);
uint32_t dom = strtoul(NEXTARG,0,10);
uint32_t max_iters = strtoul(NEXTARG,0,10);
uint32_t max_factor = strtoul(NEXTARG,0,10);
@@ -234,7 +235,7 @@ int main(int argc, char **argv)
} else if (!strcmp(mode,"--restore-domain")) {
- int io_fd = atoi(NEXTARG);
+ io_fd = atoi(NEXTARG);
uint32_t dom = strtoul(NEXTARG,0,10);
unsigned store_evtchn = strtoul(NEXTARG,0,10);
domid_t store_domid = strtoul(NEXTARG,0,10);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 25/28] libxl: cancellation: Handle SIGTERM in save/restore helper
2015-03-31 19:00 [PATCH RFC v3 00/28] libxl: Cancelling asynchronous operations Ian Jackson
` (23 preceding siblings ...)
2015-03-31 19:01 ` [PATCH 24/28] libxl: cancellation: Preparations for save/restore cancellation Ian Jackson
@ 2015-03-31 19:01 ` Ian Jackson
2015-03-31 19:01 ` [PATCH 26/28] libxl: cancellation: Cancel libxc save/restore Ian Jackson
` (4 subsequent siblings)
29 siblings, 0 replies; 38+ messages in thread
From: Ian Jackson @ 2015-03-31 19:01 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
During startup of the save/restore helper, set the disposition of
SIGTERM appropriately.
For restore, we can simply die immediately - there is no point trying
to do any kind of cleanup on what is now going to be a trashed domain.
For save, we want to arrange that libxc's cleanup code (eg turning off
logdirty) takes place. So our signal handler replaces the fd with one
on which writes will fail, causing libxc's own loop to fail next time
it actually tries to do a write.
Currently this has only a minor beneficial effect: we don't send the
helper a SIGTERM ourselves, and if someone else contrives to send our
helper a SIGTERM they have probably sent one to libxl too in which
case things are going to be a bit messy anyway.
But in the next patch libxl is going to use SIGTERM itself on ao
cancellation.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v2: New in this version of the series.
---
tools/libxl/libxl_save_helper.c | 58 +++++++++++++++++++++++++++++++++++++++
1 file changed, 58 insertions(+)
diff --git a/tools/libxl/libxl_save_helper.c b/tools/libxl/libxl_save_helper.c
index 7514b2e..0be77c9 100644
--- a/tools/libxl/libxl_save_helper.c
+++ b/tools/libxl/libxl_save_helper.c
@@ -40,8 +40,10 @@
#include <unistd.h>
#include <assert.h>
#include <inttypes.h>
+#include <fcntl.h>
#include "libxl.h"
+#include "libxl_utils.h"
#include "xenctrl.h"
#include "xenguest.h"
@@ -120,6 +122,58 @@ static void *xmalloc(size_t sz)
return r;
}
+/*----- signal handling -----*/
+
+static int unwriteable_fd;
+
+static void save_signal_handler(int num)
+{
+ /*
+ * We want to be able to interrupt save. But the code in libxc
+ * which does the actual saving is straight-through, and we need
+ * to execute its error path to put the guest back to sanity.
+ *
+ * So what we do is this: when we get the signal, we dup2
+ * the result of open("/dev/null",O_RDONLY) onto the output fd.
+ *
+ * This is guaranteed to 1. interrupt libxc's write (causing it to
+ * return short, or maybe EINTR); 2. make the next write give
+ * EBADF, so that: 3. at latest, libxc will notice when it next
+ * tries to write data and will then go into its cleanup path.
+ *
+ * We make no effort here to sanitise the resulting errors.
+ * That's libxl's job.
+ */
+ int esave = errno;
+
+ int r = dup2(unwriteable_fd, io_fd);
+ assert(r == io_fd); /* if not we can't write an xtl message because we
+ * might end up interleaving on our control stream */
+
+ errno = esave;
+}
+
+static void setup_signals(void (*handler)(int))
+{
+ struct sigaction sa;
+ sigset_t spmask;
+ int r;
+
+ unwriteable_fd = open("/dev/null",O_RDONLY);
+ if (unwriteable_fd < 0) fail(errno,"open /dev/null for reading");
+
+ LIBXL_FILLZERO(sa);
+ sa.sa_handler = handler;
+ sigemptyset(&sa.sa_mask);
+ r = sigaction(SIGTERM, &sa, 0);
+ if (r) fail(errno,"sigaction SIGTERM failed");
+
+ sigemptyset(&spmask);
+ sigaddset(&spmask,SIGTERM);
+ r = sigprocmask(SIG_UNBLOCK,&spmask,0);
+ if (r) fail(errno,"sigprocmask unblock SIGTERM failed");
+}
+
/*----- helper functions called by autogenerated stubs -----*/
unsigned char * helper_allocbuf(int len, void *user)
@@ -229,6 +283,8 @@ int main(int argc, char **argv)
helper_setcallbacks_save(&helper_save_callbacks, cbflags);
startup("save");
+ setup_signals(save_signal_handler);
+
r = xc_domain_save(xch, io_fd, dom, max_iters, max_factor, flags,
&helper_save_callbacks, hvm);
complete(r);
@@ -254,6 +310,8 @@ int main(int argc, char **argv)
unsigned long console_mfn = 0;
startup("restore");
+ setup_signals(SIG_DFL);
+
r = xc_domain_restore(xch, io_fd, dom, store_evtchn, &store_mfn,
store_domid, console_evtchn, &console_mfn,
console_domid, hvm, pae, superpages,
--
1.7.10.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 26/28] libxl: cancellation: Cancel libxc save/restore
2015-03-31 19:00 [PATCH RFC v3 00/28] libxl: Cancelling asynchronous operations Ian Jackson
` (24 preceding siblings ...)
2015-03-31 19:01 ` [PATCH 25/28] libxl: cancellation: Handle SIGTERM in save/restore helper Ian Jackson
@ 2015-03-31 19:01 ` Ian Jackson
2015-03-31 19:01 ` [PATCH 27/28] libxl: ao: datacopier callback gets an rc Ian Jackson
` (3 subsequent siblings)
29 siblings, 0 replies; 38+ messages in thread
From: Ian Jackson @ 2015-03-31 19:01 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
Register the the save/restore helper interface with the cancellation
machinery. When we are informed that save/restore should be
cancelled, we make a note of the that in our rc variable, and send the
helper a SIGTERM. It will die in due course.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v2: New in this version of the series.
---
tools/libxl/libxl_internal.h | 1 +
tools/libxl/libxl_save_callout.c | 25 +++++++++++++++++++++++++
2 files changed, 26 insertions(+)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 465cdda..883daae 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2638,6 +2638,7 @@ typedef struct libxl__save_helper_state {
int rc;
int completed; /* retval/errnoval valid iff completed */
int retval, errnoval; /* from xc_domain_save / xc_domain_restore */
+ libxl__ao_cancellable cancel;
libxl__carefd *pipes[2]; /* 0 = helper's stdin, 1 = helper's stdout */
libxl__ev_fd readable;
libxl__ev_child child;
diff --git a/tools/libxl/libxl_save_callout.c b/tools/libxl/libxl_save_callout.c
index 1d584f1..d9fa0d2 100644
--- a/tools/libxl/libxl_save_callout.c
+++ b/tools/libxl/libxl_save_callout.c
@@ -32,6 +32,7 @@ static void run_helper(libxl__egc *egc, libxl__save_helper_state *shs,
const unsigned long *argnums, int num_argnums);
static void helper_failed(libxl__egc*, libxl__save_helper_state *shs, int rc);
+static void helper_cancel(libxl__egc *egc, libxl__ao_cancellable*, int rc);
static void helper_stdout_readable(libxl__egc *egc, libxl__ev_fd *ev,
int fd, short events, short revents);
static void helper_exited(libxl__egc *egc, libxl__ev_child *ch,
@@ -166,9 +167,15 @@ static void run_helper(libxl__egc *egc, libxl__save_helper_state *shs,
shs->rc = 0;
shs->completed = 0;
shs->pipes[0] = shs->pipes[1] = 0;
+ libxl__ao_cancellable_init(&shs->cancel);
libxl__ev_fd_init(&shs->readable);
libxl__ev_child_init(&shs->child);
+ shs->cancel.ao = shs->ao;
+ shs->cancel.callback = helper_cancel;
+ rc = libxl__ao_cancellable_register(&shs->cancel);
+ if (rc) goto out;
+
shs->stdin_what = GCSPRINTF("domain %"PRIu32" save/restore helper"
" stdin pipe", domid);
shs->stdout_what = GCSPRINTF("domain %"PRIu32" save/restore helper"
@@ -262,6 +269,23 @@ static void helper_failed(libxl__egc *egc, libxl__save_helper_state *shs,
sendsig(gc, shs, SIGKILL);
}
+static void helper_cancel(libxl__egc *egc, libxl__ao_cancellable *cancel,
+ int rc)
+{
+ libxl__save_helper_state *shs = CONTAINER_OF(cancel, *shs, cancel);
+ STATE_AO_GC(shs->ao);
+
+ if (!libxl__ev_child_inuse(&shs->child)) {
+ helper_failed(egc, shs, rc);
+ return;
+ }
+
+ if (!shs->rc)
+ shs->rc = rc;
+
+ sendsig(gc, shs, SIGTERM);
+}
+
static void helper_stdout_readable(libxl__egc *egc, libxl__ev_fd *ev,
int fd, short events, short revents)
{
@@ -332,6 +356,7 @@ static void helper_done(libxl__egc *egc, libxl__save_helper_state *shs)
{
STATE_AO_GC(shs->ao);
+ libxl__ao_cancellable_deregister(&shs->cancel);
libxl__ev_fd_deregister(gc, &shs->readable);
libxl__carefd_close(shs->pipes[0]); shs->pipes[0] = 0;
libxl__carefd_close(shs->pipes[1]); shs->pipes[1] = 0;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 27/28] libxl: ao: datacopier callback gets an rc
2015-03-31 19:00 [PATCH RFC v3 00/28] libxl: Cancelling asynchronous operations Ian Jackson
` (25 preceding siblings ...)
2015-03-31 19:01 ` [PATCH 26/28] libxl: cancellation: Cancel libxc save/restore Ian Jackson
@ 2015-03-31 19:01 ` Ian Jackson
2015-03-31 19:01 ` [PATCH 28/28] libxl: cancellation: Make datacopiers cancellable Ian Jackson
` (2 subsequent siblings)
29 siblings, 0 replies; 38+ messages in thread
From: Ian Jackson @ 2015-03-31 19:01 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Ian Jackson, David Vrabel
libxl__datacopier_* now provides its caller's callback function with
an rc value. This relieves the caller of the need to figure out an
appropriate rc value.
Arrange that the `other internal failure' cases now get a valid
positive errno value (EIO).
In a few places, assert that errno is nonzero before passing it to our
caller.
Extend the datacopier callback API to permit the dc to signal
CANCELLED. (It doesn't actually do this yet, though.)
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: David Vrabel <david.vrabel@citrix.com>
---
v2: New in this version of the series.
---
tools/libxl/libxl_aoutils.c | 22 ++++++++++++----------
tools/libxl/libxl_bootloader.c | 20 +++++++++++---------
tools/libxl/libxl_dom.c | 10 +++-------
tools/libxl/libxl_internal.h | 18 +++++++++++-------
4 files changed, 37 insertions(+), 33 deletions(-)
diff --git a/tools/libxl/libxl_aoutils.c b/tools/libxl/libxl_aoutils.c
index 0b6d750..ece7981 100644
--- a/tools/libxl/libxl_aoutils.c
+++ b/tools/libxl/libxl_aoutils.c
@@ -121,10 +121,10 @@ void libxl__datacopier_kill(libxl__datacopier_state *dc)
}
static void datacopier_callback(libxl__egc *egc, libxl__datacopier_state *dc,
- int onwrite, int errnoval)
+ int rc, int onwrite, int errnoval)
{
libxl__datacopier_kill(dc);
- dc->callback(egc, dc, onwrite, errnoval);
+ dc->callback(egc, dc, rc, onwrite, errnoval);
}
static void datacopier_writable(libxl__egc *egc, libxl__ev_fd *ev,
@@ -142,13 +142,13 @@ static void datacopier_check_state(libxl__egc *egc, libxl__datacopier_state *dc)
if (rc) {
LOG(ERROR, "unable to establish write event on %s"
" during copy of %s", dc->writewhat, dc->copywhat);
- datacopier_callback(egc, dc, -1, 0);
+ datacopier_callback(egc, dc, ERROR_FAIL, -1, EIO);
return;
}
}
} else if (!libxl__ev_fd_isregistered(&dc->toread)) {
/* we have had eof */
- datacopier_callback(egc, dc, 0, 0);
+ datacopier_callback(egc, dc, 0, 0, 0);
return;
} else {
/* nothing buffered, but still reading */
@@ -190,7 +190,7 @@ static int datacopier_pollhup_handled(libxl__egc *egc,
onwrite ? dc->writewhat : dc->readwhat,
dc->copywhat);
libxl__datacopier_kill(dc);
- dc->callback_pollhup(egc, dc, onwrite, -1);
+ dc->callback_pollhup(egc, dc, ERROR_FAIL, onwrite, -1);
return 1;
}
return 0;
@@ -207,7 +207,7 @@ static void datacopier_readable(libxl__egc *egc, libxl__ev_fd *ev,
if (revents & ~POLLIN) {
LOG(ERROR, "unexpected poll event 0x%x (should be POLLIN)"
" on %s during copy of %s", revents, dc->readwhat, dc->copywhat);
- datacopier_callback(egc, dc, -1, 0);
+ datacopier_callback(egc, dc, ERROR_FAIL, -1, EIO);
return;
}
assert(revents & POLLIN);
@@ -234,9 +234,10 @@ static void datacopier_readable(libxl__egc *egc, libxl__ev_fd *ev,
if (r < 0) {
if (errno == EINTR) continue;
if (errno == EWOULDBLOCK) break;
+ assert(errno);
LOGE(ERROR, "error reading %s during copy of %s",
dc->readwhat, dc->copywhat);
- datacopier_callback(egc, dc, 0, errno);
+ datacopier_callback(egc, dc, ERROR_FAIL, 0, errno);
return;
}
if (r == 0) {
@@ -249,7 +250,7 @@ static void datacopier_readable(libxl__egc *egc, libxl__ev_fd *ev,
assert(ferror(dc->log));
assert(errno);
LOGE(ERROR, "error logging %s", dc->copywhat);
- datacopier_callback(egc, dc, 0, errno);
+ datacopier_callback(egc, dc, ERROR_FAIL, 0, errno);
return;
}
}
@@ -271,7 +272,7 @@ static void datacopier_writable(libxl__egc *egc, libxl__ev_fd *ev,
if (revents & ~POLLOUT) {
LOG(ERROR, "unexpected poll event 0x%x (should be POLLOUT)"
" on %s during copy of %s", revents, dc->writewhat, dc->copywhat);
- datacopier_callback(egc, dc, -1, 0);
+ datacopier_callback(egc, dc, ERROR_FAIL, -1, EIO);
return;
}
assert(revents & POLLOUT);
@@ -288,9 +289,10 @@ static void datacopier_writable(libxl__egc *egc, libxl__ev_fd *ev,
if (r < 0) {
if (errno == EINTR) continue;
if (errno == EWOULDBLOCK) break;
+ assert(errno);
LOGE(ERROR, "error writing to %s during copy of %s",
dc->writewhat, dc->copywhat);
- datacopier_callback(egc, dc, 1, errno);
+ datacopier_callback(egc, dc, ERROR_FAIL, 1, errno);
return;
}
assert(r > 0);
diff --git a/tools/libxl/libxl_bootloader.c b/tools/libxl/libxl_bootloader.c
index 21f92dc..c26f1d6 100644
--- a/tools/libxl/libxl_bootloader.c
+++ b/tools/libxl/libxl_bootloader.c
@@ -30,9 +30,9 @@
static void bootloader_gotptys(libxl__egc *egc, libxl__openpty_state *op);
static void bootloader_keystrokes_copyfail(libxl__egc *egc,
- libxl__datacopier_state *dc, int onwrite, int errnoval);
+ libxl__datacopier_state *dc, int rc, int onwrite, int errnoval);
static void bootloader_display_copyfail(libxl__egc *egc,
- libxl__datacopier_state *dc, int onwrite, int errnoval);
+ libxl__datacopier_state *dc, int rc, int onwrite, int errnoval);
static void bootloader_domaindeath(libxl__egc*, libxl__domaindeathcheck *dc,
int rc);
static void bootloader_finished(libxl__egc *egc, libxl__ev_child *child,
@@ -577,10 +577,10 @@ static void bootloader_gotptys(libxl__egc *egc, libxl__openpty_state *op)
/* perhaps one of these will be called, but perhaps not */
static void bootloader_copyfail(libxl__egc *egc, const char *which,
- libxl__bootloader_state *bl, int ondisplay, int onwrite, int errnoval)
+ libxl__bootloader_state *bl, int ondisplay,
+ int rc, int onwrite, int errnoval)
{
STATE_AO_GC(bl->ao);
- int rc = ERROR_FAIL;
if (errnoval==-1) {
/* POLLHUP */
@@ -591,22 +591,24 @@ static void bootloader_copyfail(libxl__egc *egc, const char *which,
LOG(ERROR, "unexpected POLLHUP on %s", which);
}
}
- if (!onwrite && !errnoval)
+ if (!rc) {
LOG(ERROR, "unexpected eof copying %s", which);
+ rc = ERROR_FAIL;
+ }
bootloader_stop(egc, bl, rc);
}
static void bootloader_keystrokes_copyfail(libxl__egc *egc,
- libxl__datacopier_state *dc, int onwrite, int errnoval)
+ libxl__datacopier_state *dc, int rc, int onwrite, int errnoval)
{
libxl__bootloader_state *bl = CONTAINER_OF(dc, *bl, keystrokes);
- bootloader_copyfail(egc, "bootloader input", bl, 0, onwrite, errnoval);
+ bootloader_copyfail(egc, "bootloader input", bl, 0, rc,onwrite,errnoval);
}
static void bootloader_display_copyfail(libxl__egc *egc,
- libxl__datacopier_state *dc, int onwrite, int errnoval)
+ libxl__datacopier_state *dc, int rc, int onwrite, int errnoval)
{
libxl__bootloader_state *bl = CONTAINER_OF(dc, *bl, display);
- bootloader_copyfail(egc, "bootloader output", bl, 1, onwrite, errnoval);
+ bootloader_copyfail(egc, "bootloader output", bl, 1, rc,onwrite,errnoval);
}
static void bootloader_domaindeath(libxl__egc *egc,
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 379ac07..2209d8c 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1918,7 +1918,7 @@ out:
}
static void save_device_model_datacopier_done(libxl__egc *egc,
- libxl__datacopier_state *dc, int onwrite, int errnoval);
+ libxl__datacopier_state *dc, int rc, int onwrite, int errnoval);
void libxl__domain_save_device_model(libxl__egc *egc,
libxl__domain_suspend_state *dss,
@@ -1977,11 +1977,11 @@ void libxl__domain_save_device_model(libxl__egc *egc,
return;
out:
- save_device_model_datacopier_done(egc, dc, -1, 0);
+ save_device_model_datacopier_done(egc, dc, rc, -1, EIO);
}
static void save_device_model_datacopier_done(libxl__egc *egc,
- libxl__datacopier_state *dc, int onwrite, int errnoval)
+ libxl__datacopier_state *dc, int our_rc, int onwrite, int errnoval)
{
libxl__domain_suspend_state *dss =
CONTAINER_OF(dc, *dss, save_dm_datacopier);
@@ -1989,14 +1989,10 @@ static void save_device_model_datacopier_done(libxl__egc *egc,
/* Convenience aliases */
const char *const filename = dss->dm_savefile;
- int our_rc = 0;
int rc;
libxl__datacopier_kill(dc);
- if (onwrite || errnoval)
- our_rc = ERROR_FAIL;
-
if (dc->readfd >= 0) {
close(dc->readfd);
dc->readfd = -1;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 883daae..be5bece 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2561,16 +2561,20 @@ _hidden void libxl__device_disk_local_initiate_detach(libxl__egc *egc,
typedef struct libxl__datacopier_state libxl__datacopier_state;
typedef struct libxl__datacopier_buf libxl__datacopier_buf;
-/* onwrite==1 means failure happened when writing, logged, errnoval is valid
- * onwrite==0 means failure happened when reading
- * errnoval==0 means we got eof and all data was written
- * errnoval!=0 means we had a read error, logged
- * onwrite==-1 means some other internal failure, errnoval not valid, logged
- * If we get POLLHUP, we call callback_pollhup(..., onwrite, -1);
+/* onwrite==1 means problem happened when writing
+ * rc==FAIL errnoval >0 we had a write error, logged
+ * onwrite==0 means problem happened when reading
+ * rc==0 errnoval==0 we got eof and all data was written
+ * rc==FAIL errnoval >0 we had a read error, logged
+ * onwrite==-1 means some other internal problem
+ * rc==FAIL errnoval==EIO some other internal failure, logged
+ * rc==CANCEL errnoval==0 cancellation requested, not logged
+ * If we get POLLHUP, we call callback_pollhup with
+ * rc==FAIL errnoval==-1 POLLHUP signalled
* or if callback_pollhup==0 this is an internal failure, as above.
* In all cases copier is killed before calling this callback */
typedef void libxl__datacopier_callback(libxl__egc *egc,
- libxl__datacopier_state *dc, int onwrite, int errnoval);
+ libxl__datacopier_state *dc, int rc, int onwrite, int errnoval);
struct libxl__datacopier_buf {
/* private to datacopier */
--
1.7.10.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 28/28] libxl: cancellation: Make datacopiers cancellable
2015-03-31 19:00 [PATCH RFC v3 00/28] libxl: Cancelling asynchronous operations Ian Jackson
` (26 preceding siblings ...)
2015-03-31 19:01 ` [PATCH 27/28] libxl: ao: datacopier callback gets an rc Ian Jackson
@ 2015-03-31 19:01 ` Ian Jackson
2015-04-07 10:27 ` [PATCH RFC v3 00/28] libxl: Cancelling asynchronous operations Ian Jackson
2015-04-08 11:37 ` [PATCH 29/28] libxl: ao internal API docs: Mention synchronous ao completion Ian Jackson
29 siblings, 0 replies; 38+ messages in thread
From: Ian Jackson @ 2015-03-31 19:01 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
libxl__datacopier_* can now actually generate a callback with
rc==CANCELLED.
This provides cancellation during some corner cases, including (at
least) copying the device model data during the end of domain save.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v2: New in this version of the series.
---
tools/libxl/libxl_aoutils.c | 16 ++++++++++++++++
tools/libxl/libxl_internal.h | 1 +
2 files changed, 17 insertions(+)
diff --git a/tools/libxl/libxl_aoutils.c b/tools/libxl/libxl_aoutils.c
index ece7981..919bf12 100644
--- a/tools/libxl/libxl_aoutils.c
+++ b/tools/libxl/libxl_aoutils.c
@@ -103,6 +103,7 @@ static void xswait_report_error(libxl__egc *egc, libxl__xswait_state *xswa,
void libxl__datacopier_init(libxl__datacopier_state *dc)
{
assert(dc->ao);
+ libxl__ao_cancellable_init(&dc->cancel);
libxl__ev_fd_init(&dc->toread);
libxl__ev_fd_init(&dc->towrite);
LIBXL_TAILQ_INIT(&dc->bufs);
@@ -113,6 +114,7 @@ void libxl__datacopier_kill(libxl__datacopier_state *dc)
STATE_AO_GC(dc->ao);
libxl__datacopier_buf *buf, *tbuf;
+ libxl__ao_cancellable_deregister(&dc->cancel);
libxl__ev_fd_deregister(gc, &dc->toread);
libxl__ev_fd_deregister(gc, &dc->towrite);
LIBXL_TAILQ_FOREACH_SAFE(buf, &dc->bufs, entry, tbuf)
@@ -196,6 +198,15 @@ static int datacopier_pollhup_handled(libxl__egc *egc,
return 0;
}
+static void datacopier_cancel(libxl__egc *egc, libxl__ao_cancellable *cancel,
+ int rc)
+{
+ libxl__datacopier_state *dc = CONTAINER_OF(cancel, *dc, cancel);
+ STATE_AO_GC(dc->ao);
+
+ datacopier_callback(egc, dc, rc, -1, 0);
+}
+
static void datacopier_readable(libxl__egc *egc, libxl__ev_fd *ev,
int fd, short events, short revents) {
libxl__datacopier_state *dc = CONTAINER_OF(ev, *dc, toread);
@@ -312,6 +323,11 @@ int libxl__datacopier_start(libxl__datacopier_state *dc)
libxl__datacopier_init(dc);
+ dc->cancel.ao = ao;
+ dc->cancel.callback = datacopier_cancel;
+ rc = libxl__ao_cancellable_register(&dc->cancel);
+ if (rc) goto out;
+
rc = libxl__ev_fd_register(gc, &dc->toread, datacopier_readable,
dc->readfd, POLLIN);
if (rc) goto out;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index be5bece..35e6643 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2593,6 +2593,7 @@ struct libxl__datacopier_state {
libxl__datacopier_callback *callback;
libxl__datacopier_callback *callback_pollhup;
/* remaining fields are private to datacopier */
+ libxl__ao_cancellable cancel;
libxl__ev_fd toread, towrite;
ssize_t used;
LIBXL_TAILQ_HEAD(libxl__datacopier_bufs, libxl__datacopier_buf) bufs;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH RFC v3 00/28] libxl: Cancelling asynchronous operations
2015-03-31 19:00 [PATCH RFC v3 00/28] libxl: Cancelling asynchronous operations Ian Jackson
` (27 preceding siblings ...)
2015-03-31 19:01 ` [PATCH 28/28] libxl: cancellation: Make datacopiers cancellable Ian Jackson
@ 2015-04-07 10:27 ` Ian Jackson
2015-04-08 11:37 ` [PATCH 29/28] libxl: ao internal API docs: Mention synchronous ao completion Ian Jackson
29 siblings, 0 replies; 38+ messages in thread
From: Ian Jackson @ 2015-04-07 10:27 UTC (permalink / raw)
To: xen-devel; +Cc: Euan Harris
Ian Jackson writes ("[PATCH RFC v3 00/28] libxl: Cancelling asynchronous operations"):
> This is v3 of my work-in-progress series to support cancellation of
> long-running libxl operations.
These are also here
http://xenbits.xen.org/gitweb/?p=people/iwj/xen.git;a=summary
git://xenbits.xen.org/people/iwj/xen.git
base.ao-cancel.v3..wip.ao-cancel.v3..
Ian.
^ permalink raw reply [flat|nested] 38+ messages in thread* [PATCH 29/28] libxl: ao internal API docs: Mention synchronous ao completion
2015-03-31 19:00 [PATCH RFC v3 00/28] libxl: Cancelling asynchronous operations Ian Jackson
` (28 preceding siblings ...)
2015-04-07 10:27 ` [PATCH RFC v3 00/28] libxl: Cancelling asynchronous operations Ian Jackson
@ 2015-04-08 11:37 ` Ian Jackson
2015-04-08 11:37 ` [PATCH RFC UNTESTED 30/28] squash! libxl: cancellation: Provide public ao cancellation API Ian Jackson
2015-04-08 15:09 ` [PATCH 29/28] libxl: ao internal API docs: Mention synchronous ao completion Wei Liu
29 siblings, 2 replies; 38+ messages in thread
From: Ian Jackson @ 2015-04-08 11:37 UTC (permalink / raw)
To: xen-devel
Cc: Ian Jackson, Wei Liu, Euan Harris, Ian Campbell, Koushik Chakravarty
This doc comment about ao lifecycle failed to mention the option of
completing the ao during the initiator function. (Indeed, the most
obvious reading would forbid it.)
Restructure the comment, describe this situation, and generally
improve the wording.
Also, fix a grammar problem (missing word `a').
CC: Koushik Chakravarty <koushik.chakravarty@citrix.com>
Reported-by: Koushik Chakravarty <koushik.chakravarty@citrix.com>
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
tools/libxl/libxl_internal.h | 33 +++++++++++++++++++++++----------
1 file changed, 23 insertions(+), 10 deletions(-)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 35e6643..49c6779 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1955,28 +1955,41 @@ _hidden void libxl__egc_cleanup(libxl__egc *egc);
* must be copied into the per-operation structure using
* libxl__ao_progress_gethow.
*
- * - If initiation is successful, the initiating function needs
- * to run libxl__ao_inprogress right before unlocking and
- * returning, and return whatever it returns (AO_INPROGRESS macro).
- *
* - If the initiation is unsuccessful, the initiating function must
* call libxl__ao_abort before unlocking and returning whatever
* error code is appropriate (AO_ABORT macro).
*
+ * If initiation is successful:
+ *
+ * - The initiating function must run libxl__ao_inprogress right
+ * before unlocking and returning, and return whatever it returns.
+ * This is best achieved with the AO_INPROGRESS macro.
+ *
* - If the operation supports progress reports, it may generate
* suitable events with NEW_EVENT and report them with
* libxl__ao_progress_report (with the ctx locked).
*
- * - Later, some callback function, whose callback has been requested
- * directly or indirectly, should call libxl__ao_complete (with the
- * ctx locked, as it will generally already be in any event callback
- * function). This must happen exactly once for each ao (and not if
- * the ao has been destroyed, obviously).
+ * - Eventually, some callback function, whose callback has been
+ * requested directly or indirectly, should call libxl__ao_complete
+ * (with the ctx locked, as it will generally already be in any
+ * event callback function). This must happen exactly once for each
+ * ao, as the last that happens with that ao.
+ *
+ * - However, it is permissible for the initiating function to call
+ * libxl__ao_inprogress and/or libxl__ao_complete (directly or
+ * indirectly), before it uses AO_INPROGRESS to return. (The ao
+ * infrastructure will arrange to defer destruction of the ao, etc.,
+ * until the proper time.) An initiating function should do this
+ * if it takes a codepath which completes synchronously.
+ *
+ * - Conversely it is forbidden to call libxl__ao_complete in the
+ * initiating function _after_ AO_INPROGRESS, because
+ * libxl__ao_complete requires the ctx to be locked.
*
* - Note that during callback functions, two gcs are available:
* - The one in egc, whose lifetime is only this callback
* - The one in ao, whose lifetime is the asynchronous operation
- * Usually callback function should use CONTAINER_OF to obtain its
+ * Usually a callback function should use CONTAINER_OF to obtain its
* own state structure, containing a pointer to the ao. It should
* then obtain the ao and use the ao's gc; this is most easily done
* using the convenience macro STATE_AO_GC.
--
1.7.10.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH RFC UNTESTED 30/28] squash! libxl: cancellation: Provide public ao cancellation API
2015-04-08 11:37 ` [PATCH 29/28] libxl: ao internal API docs: Mention synchronous ao completion Ian Jackson
@ 2015-04-08 11:37 ` Ian Jackson
2015-04-08 15:09 ` [PATCH 29/28] libxl: ao internal API docs: Mention synchronous ao completion Wei Liu
1 sibling, 0 replies; 38+ messages in thread
From: Ian Jackson @ 2015-04-08 11:37 UTC (permalink / raw)
To: xen-devel
Cc: Ian Jackson, Wei Liu, Euan Harris, Ian Campbell, Koushik Chakravarty
[ In v4 of the series I will combine this fixup with the named patch
using git-rebase -i --autosquash -iwj. ]
v4: Actually record aos on aos_inprogress.
(Report from Koushik Chakravarty at Citrix.)
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
tools/libxl/libxl_event.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c
index e28c465..ed2dbfc 100644
--- a/tools/libxl/libxl_event.c
+++ b/tools/libxl/libxl_event.c
@@ -1844,6 +1844,7 @@ void libxl__ao_abort(libxl__ao *ao)
assert(!ao->complete);
assert(!ao->progress_reports_outstanding);
assert(!ao->cancelling);
+ LIBXL_LIST_REMOVE(ao, inprogress_entry);
libxl__ao__destroy(CTX, ao);
}
@@ -1864,7 +1865,7 @@ void libxl__ao_complete(libxl__egc *egc, libxl__ao *ao, int rc)
assert(!ao->nested_progeny);
ao->complete = 1;
ao->rc = rc;
-
+ LIBXL_LIST_REMOVE(ao, inprogress_entry);
libxl__ao_complete_check_progress_reports(egc, ao);
}
@@ -1937,6 +1938,8 @@ libxl__ao *libxl__ao_create(libxl_ctx *ctx, uint32_t domid,
"ao %p: create: how=%p callback=%p poller=%p",
ao, how, ao->how.callback, ao->poller);
+ LIBXL_LIST_INSERT_HEAD(&ctx->aos_inprogress, ao, inprogress_entry);
+
return ao;
out:
--
1.7.10.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH 29/28] libxl: ao internal API docs: Mention synchronous ao completion
2015-04-08 11:37 ` [PATCH 29/28] libxl: ao internal API docs: Mention synchronous ao completion Ian Jackson
2015-04-08 11:37 ` [PATCH RFC UNTESTED 30/28] squash! libxl: cancellation: Provide public ao cancellation API Ian Jackson
@ 2015-04-08 15:09 ` Wei Liu
1 sibling, 0 replies; 38+ messages in thread
From: Wei Liu @ 2015-04-08 15:09 UTC (permalink / raw)
To: Ian Jackson
Cc: Wei Liu, xen-devel, Euan Harris, Ian Campbell, Koushik Chakravarty
On Wed, Apr 08, 2015 at 12:37:58PM +0100, Ian Jackson wrote:
> This doc comment about ao lifecycle failed to mention the option of
> completing the ao during the initiator function. (Indeed, the most
> obvious reading would forbid it.)
>
> Restructure the comment, describe this situation, and generally
> improve the wording.
>
> Also, fix a grammar problem (missing word `a').
>
> CC: Koushik Chakravarty <koushik.chakravarty@citrix.com>
> Reported-by: Koushik Chakravarty <koushik.chakravarty@citrix.com>
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
> ---
> tools/libxl/libxl_internal.h | 33 +++++++++++++++++++++++----------
> 1 file changed, 23 insertions(+), 10 deletions(-)
>
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 35e6643..49c6779 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -1955,28 +1955,41 @@ _hidden void libxl__egc_cleanup(libxl__egc *egc);
> * must be copied into the per-operation structure using
> * libxl__ao_progress_gethow.
> *
> - * - If initiation is successful, the initiating function needs
> - * to run libxl__ao_inprogress right before unlocking and
> - * returning, and return whatever it returns (AO_INPROGRESS macro).
> - *
> * - If the initiation is unsuccessful, the initiating function must
> * call libxl__ao_abort before unlocking and returning whatever
> * error code is appropriate (AO_ABORT macro).
> *
> + * If initiation is successful:
> + *
> + * - The initiating function must run libxl__ao_inprogress right
> + * before unlocking and returning, and return whatever it returns.
> + * This is best achieved with the AO_INPROGRESS macro.
> + *
> * - If the operation supports progress reports, it may generate
> * suitable events with NEW_EVENT and report them with
> * libxl__ao_progress_report (with the ctx locked).
> *
> - * - Later, some callback function, whose callback has been requested
> - * directly or indirectly, should call libxl__ao_complete (with the
> - * ctx locked, as it will generally already be in any event callback
> - * function). This must happen exactly once for each ao (and not if
> - * the ao has been destroyed, obviously).
> + * - Eventually, some callback function, whose callback has been
> + * requested directly or indirectly, should call libxl__ao_complete
> + * (with the ctx locked, as it will generally already be in any
> + * event callback function). This must happen exactly once for each
> + * ao, as the last that happens with that ao.
> + *
> + * - However, it is permissible for the initiating function to call
> + * libxl__ao_inprogress and/or libxl__ao_complete (directly or
> + * indirectly), before it uses AO_INPROGRESS to return. (The ao
> + * infrastructure will arrange to defer destruction of the ao, etc.,
> + * until the proper time.) An initiating function should do this
> + * if it takes a codepath which completes synchronously.
> + *
> + * - Conversely it is forbidden to call libxl__ao_complete in the
> + * initiating function _after_ AO_INPROGRESS, because
> + * libxl__ao_complete requires the ctx to be locked.
> *
> * - Note that during callback functions, two gcs are available:
> * - The one in egc, whose lifetime is only this callback
> * - The one in ao, whose lifetime is the asynchronous operation
> - * Usually callback function should use CONTAINER_OF to obtain its
> + * Usually a callback function should use CONTAINER_OF to obtain its
> * own state structure, containing a pointer to the ao. It should
> * then obtain the ao and use the ao's gc; this is most easily done
> * using the convenience macro STATE_AO_GC.
> --
> 1.7.10.4
^ permalink raw reply [flat|nested] 38+ messages in thread