* [PATCH 01/22] libxl: init: Provide a gc later in libxl_ctx_alloc
2014-03-17 13:29 [PATCH v3 RESEND 00/22] libxl: asynchronous suspend Ian Jackson
@ 2014-03-17 13:29 ` Ian Jackson
2014-03-17 13:29 ` [PATCH 02/22] libxl: init: libxl__poller_init and _get take gc Ian Jackson
` (21 subsequent siblings)
22 siblings, 0 replies; 29+ messages in thread
From: Ian Jackson @ 2014-03-17 13:29 UTC (permalink / raw)
To: xen-devel
Cc: Shriram Rajagopalan, Stefano Stabellini, Ian Jackson,
Ian Campbell, Lai Jiangshan
Provide libxl__gc *gc for the second half of libxl_ctx_alloc.
(For the first half of the function, gc is in scope but set to NULL.)
This makes it possible to make gc-requiring calls. For example, it
makes error logging more convenient.
Make use of this by changing the logging calls to use the LOG*
convenience macros.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
tools/libxl/libxl.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 2d29ad2..224697a 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -25,6 +25,7 @@ int libxl_ctx_alloc(libxl_ctx **pctx, int version,
unsigned flags, xentoollog_logger * lg)
{
libxl_ctx *ctx = NULL;
+ libxl__gc gc_buf, *gc = NULL;
int rc;
if (version != LIBXL_VERSION) { rc = ERROR_VERSION; goto out; }
@@ -79,6 +80,9 @@ int libxl_ctx_alloc(libxl_ctx **pctx, int version,
}
/* Now ctx is safe for ctx_free; failures simply set rc and "goto out" */
+ LIBXL_INIT_GC(gc_buf,ctx);
+ gc = &gc_buf;
+ /* Now gc is useable */
rc = libxl__atfork_init(ctx);
if (rc) goto out;
@@ -88,8 +92,7 @@ int libxl_ctx_alloc(libxl_ctx **pctx, int version,
ctx->xch = xc_interface_open(lg,lg,0);
if (!ctx->xch) {
- LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, errno,
- "cannot open libxc handle");
+ LOGEV(ERROR, errno, "cannot open libxc handle");
rc = ERROR_FAIL; goto out;
}
@@ -97,8 +100,7 @@ int libxl_ctx_alloc(libxl_ctx **pctx, int version,
if (!ctx->xsh)
ctx->xsh = xs_domain_open();
if (!ctx->xsh) {
- LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, errno,
- "cannot connect to xenstore");
+ LOGEV(ERROR, errno, "cannot connect to xenstore");
rc = ERROR_FAIL; goto out;
}
@@ -106,6 +108,7 @@ int libxl_ctx_alloc(libxl_ctx **pctx, int version,
return 0;
out:
+ if (gc) libxl__free_all(gc);
libxl_ctx_free(ctx);
*pctx = NULL;
return rc;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 02/22] libxl: init: libxl__poller_init and _get take gc
2014-03-17 13:29 [PATCH v3 RESEND 00/22] libxl: asynchronous suspend Ian Jackson
2014-03-17 13:29 ` [PATCH 01/22] libxl: init: Provide a gc later in libxl_ctx_alloc Ian Jackson
@ 2014-03-17 13:29 ` Ian Jackson
2014-03-17 13:29 ` [PATCH 03/22] libxl: events: const-correct *_inuse, *_isregistered Ian Jackson
` (20 subsequent siblings)
22 siblings, 0 replies; 29+ messages in thread
From: Ian Jackson @ 2014-03-17 13:29 UTC (permalink / raw)
To: xen-devel
Cc: Shriram Rajagopalan, Stefano Stabellini, Ian Jackson,
Ian Campbell, Lai Jiangshan
Change libxl__poller_init and libxl__poller__get to take a libxl__gc*
rather than a libxl_ctx*. The gc is not used for memory allocation
but simply to provide the standard local variable "gc" expected by the
convenience macros. Doing this makes the error logging more
convenient.
Hence, convert the logging calls to use the LOG* convenience macros.
And consequently, change the call sites, and the function bodies to
use CTX rather than ctx.
Also convert a call to malloc() (with error check) in
libxl__poller_get, to libxl__zalloc (no error check needed).
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
tools/libxl/libxl.c | 2 +-
tools/libxl/libxl_event.c | 21 ++++++++-------------
tools/libxl/libxl_internal.h | 4 ++--
3 files changed, 11 insertions(+), 16 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 224697a..0a825df 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -87,7 +87,7 @@ int libxl_ctx_alloc(libxl_ctx **pctx, int version,
rc = libxl__atfork_init(ctx);
if (rc) goto out;
- rc = libxl__poller_init(ctx, &ctx->poller_app);
+ rc = libxl__poller_init(gc, &ctx->poller_app);
if (rc) goto out;
ctx->xch = xc_interface_open(lg,lg,0);
diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c
index ea8c744..3e465af 100644
--- a/tools/libxl/libxl_event.c
+++ b/tools/libxl/libxl_event.c
@@ -1347,13 +1347,13 @@ int libxl__self_pipe_eatall(int fd)
* Manipulation of pollers
*/
-int libxl__poller_init(libxl_ctx *ctx, libxl__poller *p)
+int libxl__poller_init(libxl__gc *gc, libxl__poller *p)
{
int rc;
p->fd_polls = 0;
p->fd_rindices = 0;
- rc = libxl__pipe_nonblock(ctx, p->wakeup_pipe);
+ rc = libxl__pipe_nonblock(CTX, p->wakeup_pipe);
if (rc) goto out;
return 0;
@@ -1370,25 +1370,20 @@ void libxl__poller_dispose(libxl__poller *p)
free(p->fd_rindices);
}
-libxl__poller *libxl__poller_get(libxl_ctx *ctx)
+libxl__poller *libxl__poller_get(libxl__gc *gc)
{
/* must be called with ctx locked */
int rc;
- libxl__poller *p = LIBXL_LIST_FIRST(&ctx->pollers_idle);
+ libxl__poller *p = LIBXL_LIST_FIRST(&CTX->pollers_idle);
if (p) {
LIBXL_LIST_REMOVE(p, entry);
return p;
}
- p = malloc(sizeof(*p));
- if (!p) {
- LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "cannot allocate poller");
- return 0;
- }
- memset(p, 0, sizeof(*p));
+ p = libxl__zalloc(NOGC, sizeof(*p));
- rc = libxl__poller_init(ctx, p);
+ rc = libxl__poller_init(gc, p);
if (rc) {
free(p);
return NULL;
@@ -1477,7 +1472,7 @@ int libxl_event_wait(libxl_ctx *ctx, libxl_event **event_r,
EGC_INIT(ctx);
CTX_LOCK;
- poller = libxl__poller_get(ctx);
+ poller = libxl__poller_get(gc);
if (!poller) { rc = ERROR_FAIL; goto out; }
for (;;) {
@@ -1653,7 +1648,7 @@ libxl__ao *libxl__ao_create(libxl_ctx *ctx, uint32_t domid,
if (how) {
ao->how = *how;
} else {
- ao->poller = libxl__poller_get(ctx);
+ ao->poller = libxl__poller_get(&ao->gc);
if (!ao->poller) goto out;
}
libxl__log(ctx,XTL_DEBUG,-1,file,line,func,
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 9d17586..b67ed79 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -834,13 +834,13 @@ _hidden void libxl__event_disaster(libxl__egc*, const char *msg, int errnoval,
/* Fills in, or disposes of, the resources held by, a poller whose
* space the caller has allocated. ctx must be locked. */
-_hidden int libxl__poller_init(libxl_ctx *ctx, libxl__poller *p);
+_hidden int libxl__poller_init(libxl__gc *gc, libxl__poller *p);
_hidden void libxl__poller_dispose(libxl__poller *p);
/* Obtain a fresh poller from malloc or the idle list, and put it
* away again afterwards. _get can fail, returning NULL.
* ctx must be locked. */
-_hidden libxl__poller *libxl__poller_get(libxl_ctx *ctx);
+_hidden libxl__poller *libxl__poller_get(libxl__gc *gc);
_hidden void libxl__poller_put(libxl_ctx*, libxl__poller *p /* may be NULL */);
/* Notifies whoever is polling using p that they should wake up.
--
1.7.10.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 03/22] libxl: events: const-correct *_inuse, *_isregistered
2014-03-17 13:29 [PATCH v3 RESEND 00/22] libxl: asynchronous suspend Ian Jackson
2014-03-17 13:29 ` [PATCH 01/22] libxl: init: Provide a gc later in libxl_ctx_alloc Ian Jackson
2014-03-17 13:29 ` [PATCH 02/22] libxl: init: libxl__poller_init and _get take gc Ian Jackson
@ 2014-03-17 13:29 ` Ian Jackson
2014-03-17 13:29 ` [PATCH 04/22] libxl: Formally document libxl__xs_transaction Ian Jackson
` (19 subsequent siblings)
22 siblings, 0 replies; 29+ messages in thread
From: Ian Jackson @ 2014-03-17 13:29 UTC (permalink / raw)
To: xen-devel
Cc: Shriram Rajagopalan, Stefano Stabellini, Ian Jackson,
Ian Campbell, Lai Jiangshan
The comments for libxl__ev_time_isregistered and the corresponding
watch function even say that these should be const. Make it so.
Also fix libxl__ev_child_inuse and libxl__ev_spawn_inuse.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
tools/libxl/libxl_internal.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index b67ed79..9e02861 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -727,7 +727,7 @@ _hidden int libxl__ev_fd_modify(libxl__gc*, libxl__ev_fd *ev,
_hidden void libxl__ev_fd_deregister(libxl__gc*, libxl__ev_fd *ev);
static inline void libxl__ev_fd_init(libxl__ev_fd *efd)
{ efd->fd = -1; }
-static inline int libxl__ev_fd_isregistered(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,
@@ -743,7 +743,7 @@ _hidden int libxl__ev_time_modify_abs(libxl__gc*, libxl__ev_time *ev,
_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; }
-static inline int libxl__ev_time_isregistered(libxl__ev_time *ev)
+static inline int libxl__ev_time_isregistered(const libxl__ev_time *ev)
{ return !!ev->func; }
@@ -783,7 +783,7 @@ _hidden pid_t libxl__ev_child_fork(libxl__gc *gc, libxl__ev_child *childw_out,
libxl__ev_child_callback *death);
static inline void libxl__ev_child_init(libxl__ev_child *childw_out)
{ childw_out->pid = -1; }
-static inline int libxl__ev_child_inuse(libxl__ev_child *childw_out)
+static inline int libxl__ev_child_inuse(const libxl__ev_child *childw_out)
{ return childw_out->pid >= 0; }
/* Useable (only) in the child to once more make the ctx useable for
@@ -1225,7 +1225,7 @@ struct libxl__spawn_state {
libxl__ev_xswatch xswatch;
};
-static inline int libxl__spawn_inuse(libxl__spawn_state *ss)
+static inline int libxl__spawn_inuse(const libxl__spawn_state *ss)
{ return libxl__ev_child_inuse(&ss->mid); }
/*
--
1.7.10.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 04/22] libxl: Formally document libxl__xs_transaction
2014-03-17 13:29 [PATCH v3 RESEND 00/22] libxl: asynchronous suspend Ian Jackson
` (2 preceding siblings ...)
2014-03-17 13:29 ` [PATCH 03/22] libxl: events: const-correct *_inuse, *_isregistered Ian Jackson
@ 2014-03-17 13:29 ` Ian Jackson
2014-03-17 13:38 ` Ian Campbell
2014-03-17 13:29 ` [PATCH 05/22] libxl: events: Provide libxl__xswait_* Ian Jackson
` (18 subsequent siblings)
22 siblings, 1 reply; 29+ messages in thread
From: Ian Jackson @ 2014-03-17 13:29 UTC (permalink / raw)
To: xen-devel; +Cc: Shriram Rajagopalan, Ian Jackson, Ian Campbell, Lai Jiangshan
Provide a more formal description of the semantics of
libxl__xs_transaction_start, _commit and _abort, in addition to the
usage pattern example.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
tools/libxl/libxl_internal.h | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 9e02861..60a39ed 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -626,6 +626,26 @@ int libxl__xs_rm_checked(libxl__gc *gc, xs_transaction_t t, const char *path);
* // other cleanups
* return rc;
* }
+ *
+ * Formally the states of *t are:
+ *
+ * name value of *t description
+ * Idle 0 no transaction exists
+ * Ready non-0 ready for work, nothing done yet
+ * Busy non-0 writes have been made but we are not finished
+ * Uncommitted non-0 writes have been made and should be committed
+ *
+ * libxl__xs_transaction_start: Idle -> Ready (on error: Idle)
+ *
+ * The transaction goes from Ready to Busy, and from Busy to
+ * Uncommitted, by the use of xenstore read and write operations
+ * (libxl__xs_..., xs_...) made by libxl__xs_transaction's caller.
+ *
+ * libxl__xs_transaction_commit: Ready/Uncommitted -> Idle
+ * on success (returns 0): xenstore has been updated
+ * on error (<0) or conflict (+1): updates discarded
+ *
+ * libxl__xs_transaction_abort: Any -> Idle (any updates discarded)
*/
int libxl__xs_transaction_start(libxl__gc *gc, xs_transaction_t *t);
int libxl__xs_transaction_commit(libxl__gc *gc, xs_transaction_t *t);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 04/22] libxl: Formally document libxl__xs_transaction
2014-03-17 13:29 ` [PATCH 04/22] libxl: Formally document libxl__xs_transaction Ian Jackson
@ 2014-03-17 13:38 ` Ian Campbell
0 siblings, 0 replies; 29+ messages in thread
From: Ian Campbell @ 2014-03-17 13:38 UTC (permalink / raw)
To: Ian Jackson; +Cc: Shriram Rajagopalan, xen-devel, Lai Jiangshan
On Mon, 2014-03-17 at 13:29 +0000, Ian Jackson wrote:
> Provide a more formal description of the semantics of
> libxl__xs_transaction_start, _commit and _abort, in addition to the
> usage pattern example.
>
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
> ---
> tools/libxl/libxl_internal.h | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 9e02861..60a39ed 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -626,6 +626,26 @@ int libxl__xs_rm_checked(libxl__gc *gc, xs_transaction_t t, const char *path);
> * // other cleanups
> * return rc;
> * }
> + *
> + * Formally the states of *t are:
> + *
> + * name value of *t description
> + * Idle 0 no transaction exists
> + * Ready non-0 ready for work, nothing done yet
> + * Busy non-0 writes have been made but we are not finished
> + * Uncommitted non-0 writes have been made and should be committed
> + *
> + * libxl__xs_transaction_start: Idle -> Ready (on error: Idle)
> + *
> + * The transaction goes from Ready to Busy, and from Busy to
> + * Uncommitted, by the use of xenstore read and write operations
> + * (libxl__xs_..., xs_...) made by libxl__xs_transaction's caller.
> + *
> + * libxl__xs_transaction_commit: Ready/Uncommitted -> Idle
> + * on success (returns 0): xenstore has been updated
> + * on error (<0) or conflict (+1): updates discarded
> + *
> + * libxl__xs_transaction_abort: Any -> Idle (any updates discarded)
> */
> int libxl__xs_transaction_start(libxl__gc *gc, xs_transaction_t *t);
> int libxl__xs_transaction_commit(libxl__gc *gc, xs_transaction_t *t);
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 05/22] libxl: events: Provide libxl__xswait_*
2014-03-17 13:29 [PATCH v3 RESEND 00/22] libxl: asynchronous suspend Ian Jackson
` (3 preceding siblings ...)
2014-03-17 13:29 ` [PATCH 04/22] libxl: Formally document libxl__xs_transaction Ian Jackson
@ 2014-03-17 13:29 ` Ian Jackson
2014-03-17 13:29 ` [PATCH 06/22] libxl: events: libxl__xswait* support @paths Ian Jackson
` (17 subsequent siblings)
22 siblings, 0 replies; 29+ messages in thread
From: Ian Jackson @ 2014-03-17 13:29 UTC (permalink / raw)
To: xen-devel
Cc: Shriram Rajagopalan, Stefano Stabellini, Ian Jackson,
Ian Campbell, Lai Jiangshan
This is an ao utility for for conveniently doing a timed wait on
xenstore. It handles setting up and cancelling the timeout, and also
conveniently reads the key for you.
No callers yet in this patch.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v2: Fix doc comments to refer to correct rc values
---
tools/libxl/libxl_aoutils.c | 77 ++++++++++++++++++++++++++++++++++++++++++
tools/libxl/libxl_internal.h | 52 ++++++++++++++++++++++++++++
2 files changed, 129 insertions(+)
diff --git a/tools/libxl/libxl_aoutils.c b/tools/libxl/libxl_aoutils.c
index b4eb6e5..477717b 100644
--- a/tools/libxl/libxl_aoutils.c
+++ b/tools/libxl/libxl_aoutils.c
@@ -16,6 +16,83 @@
#include "libxl_internal.h"
+/*----- xswait -----*/
+
+static libxl__ev_xswatch_callback xswait_xswatch_callback;
+static libxl__ev_time_callback xswait_timeout_callback;
+static void xswait_report_error(libxl__egc*, libxl__xswait_state*, int rc);
+
+void libxl__xswait_init(libxl__xswait_state *xswa)
+{
+ libxl__ev_time_init(&xswa->time_ev);
+ libxl__ev_xswatch_init(&xswa->watch_ev);
+}
+
+void libxl__xswait_stop(libxl__gc *gc, libxl__xswait_state *xswa)
+{
+ libxl__ev_time_deregister(gc, &xswa->time_ev);
+ libxl__ev_xswatch_deregister(gc, &xswa->watch_ev);
+}
+
+bool libxl__xswait_inuse(const libxl__xswait_state *xswa)
+{
+ bool time_inuse = libxl__ev_time_isregistered(&xswa->time_ev);
+ bool watch_inuse = libxl__ev_xswatch_isregistered(&xswa->watch_ev);
+ assert(time_inuse == watch_inuse);
+ return time_inuse;
+}
+
+int libxl__xswait_start(libxl__gc *gc, libxl__xswait_state *xswa)
+{
+ int rc;
+
+ rc = libxl__ev_time_register_rel(gc, &xswa->time_ev,
+ xswait_timeout_callback, xswa->timeout_ms);
+ if (rc) goto err;
+
+ rc = libxl__ev_xswatch_register(gc, &xswa->watch_ev,
+ xswait_xswatch_callback, xswa->path);
+ if (rc) goto err;
+
+ return 0;
+
+ err:
+ libxl__xswait_stop(gc, xswa);
+ return rc;
+}
+
+void xswait_xswatch_callback(libxl__egc *egc, libxl__ev_xswatch *xsw,
+ const char *watch_path, const char *event_path)
+{
+ EGC_GC;
+ libxl__xswait_state *xswa = CONTAINER_OF(xsw, *xswa, watch_ev);
+ int rc;
+ const char *data;
+
+ rc = libxl__xs_read_checked(gc, XBT_NULL, xswa->path, &data);
+ if (rc) { xswait_report_error(egc, xswa, rc); return; }
+
+ xswa->callback(egc, xswa, 0, data);
+}
+
+void xswait_timeout_callback(libxl__egc *egc, libxl__ev_time *ev,
+ const struct timeval *requested_abs)
+{
+ 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);
+}
+
+static void xswait_report_error(libxl__egc *egc, libxl__xswait_state *xswa,
+ int rc)
+{
+ EGC_GC;
+ libxl__xswait_stop(gc, xswa);
+ xswa->callback(egc, xswa, rc, 0);
+}
+
+
/*----- data copier -----*/
void libxl__datacopier_init(libxl__datacopier_state *dc)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 60a39ed..a208be7 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1100,6 +1100,58 @@ _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).
+ *
+ */
+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] 29+ messages in thread* [PATCH 06/22] libxl: events: libxl__xswait* support @paths
2014-03-17 13:29 [PATCH v3 RESEND 00/22] libxl: asynchronous suspend Ian Jackson
` (4 preceding siblings ...)
2014-03-17 13:29 ` [PATCH 05/22] libxl: events: Provide libxl__xswait_* Ian Jackson
@ 2014-03-17 13:29 ` Ian Jackson
2014-03-17 13:29 ` [PATCH 07/22] libxl: events: Use libxl__xswait_* in spawn code Ian Jackson
` (16 subsequent siblings)
22 siblings, 0 replies; 29+ messages in thread
From: Ian Jackson @ 2014-03-17 13:29 UTC (permalink / raw)
To: xen-devel; +Cc: Shriram Rajagopalan, Ian Jackson, Ian Campbell, Lai Jiangshan
Special-case paths starting with '@' in libxl__xswait. Attempting to
read these from xenstore gives EINVAL. Callers waiting for (say)
@releaseDomain will be checking for some condition which can be
observed other than by looking at xenstore.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v3: New patch in this version of the series.
---
tools/libxl/libxl_aoutils.c | 8 ++++++--
tools/libxl/libxl_internal.h | 2 ++
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/tools/libxl/libxl_aoutils.c b/tools/libxl/libxl_aoutils.c
index 477717b..1c9eb9e 100644
--- a/tools/libxl/libxl_aoutils.c
+++ b/tools/libxl/libxl_aoutils.c
@@ -69,8 +69,12 @@ void xswait_xswatch_callback(libxl__egc *egc, libxl__ev_xswatch *xsw,
int rc;
const char *data;
- rc = libxl__xs_read_checked(gc, XBT_NULL, xswa->path, &data);
- if (rc) { xswait_report_error(egc, xswa, rc); return; }
+ if (xswa->path[0] == '@') {
+ data = 0;
+ } else {
+ rc = libxl__xs_read_checked(gc, XBT_NULL, xswa->path, &data);
+ if (rc) { xswait_report_error(egc, xswa, rc); return; }
+ }
xswa->callback(egc, xswa, 0, data);
}
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index a208be7..a82a43d 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1130,6 +1130,8 @@ typedef struct libxl__xswait_state libxl__xswait_state;
* 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);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 07/22] libxl: events: Use libxl__xswait_* in spawn code
2014-03-17 13:29 [PATCH v3 RESEND 00/22] libxl: asynchronous suspend Ian Jackson
` (5 preceding siblings ...)
2014-03-17 13:29 ` [PATCH 06/22] libxl: events: libxl__xswait* support @paths Ian Jackson
@ 2014-03-17 13:29 ` Ian Jackson
2014-03-17 13:29 ` [PATCH 08/22] libxl: events: Provide libxl__ev_evtchn* Ian Jackson
` (15 subsequent siblings)
22 siblings, 0 replies; 29+ messages in thread
From: Ian Jackson @ 2014-03-17 13:29 UTC (permalink / raw)
To: xen-devel
Cc: Shriram Rajagopalan, Stefano Stabellini, Ian Jackson,
Ian Campbell, Lai Jiangshan
Replace open-coded use of ev_time and ev_xswatch with xswait.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
tools/libxl/libxl_exec.c | 49 +++++++++++++++---------------------------
tools/libxl/libxl_internal.h | 3 +--
2 files changed, 18 insertions(+), 34 deletions(-)
diff --git a/tools/libxl/libxl_exec.c b/tools/libxl/libxl_exec.c
index b6efa0f..4b012dc 100644
--- a/tools/libxl/libxl_exec.c
+++ b/tools/libxl/libxl_exec.c
@@ -257,10 +257,8 @@ err:
*/
/* Event callbacks. */
-static void spawn_watch_event(libxl__egc *egc, libxl__ev_xswatch *xsw,
- const char *watch_path, const char *event_path);
-static void spawn_timeout(libxl__egc *egc, libxl__ev_time *ev,
- const struct timeval *requested_abs);
+static void spawn_watch_event(libxl__egc *egc, libxl__xswait_state *xswa,
+ int rc, const char *xsdata);
static void spawn_middle_death(libxl__egc *egc, libxl__ev_child *childw,
pid_t pid, int status);
@@ -274,8 +272,7 @@ static void spawn_fail(libxl__egc *egc, libxl__spawn_state *ss);
void libxl__spawn_init(libxl__spawn_state *ss)
{
libxl__ev_child_init(&ss->mid);
- libxl__ev_time_init(&ss->timeout);
- libxl__ev_xswatch_init(&ss->xswatch);
+ libxl__xswait_init(&ss->xswait);
}
int libxl__spawn_spawn(libxl__egc *egc, libxl__spawn_state *ss)
@@ -288,12 +285,12 @@ int libxl__spawn_spawn(libxl__egc *egc, libxl__spawn_state *ss)
libxl__spawn_init(ss);
ss->failed = ss->detaching = 0;
- rc = libxl__ev_time_register_rel(gc, &ss->timeout,
- spawn_timeout, ss->timeout_ms);
- if (rc) goto out_err;
-
- rc = libxl__ev_xswatch_register(gc, &ss->xswatch,
- spawn_watch_event, ss->xspath);
+ ss->xswait.ao = ao;
+ ss->xswait.what = GCSPRINTF("%s startup", ss->what);
+ ss->xswait.path = ss->xspath;
+ ss->xswait.timeout_ms = ss->timeout_ms;
+ ss->xswait.callback = spawn_watch_event;
+ rc = libxl__xswait_start(gc, &ss->xswait);
if (rc) goto out_err;
pid_t middle = libxl__ev_child_fork(gc, &ss->mid, spawn_middle_death);
@@ -350,8 +347,7 @@ int libxl__spawn_spawn(libxl__egc *egc, libxl__spawn_state *ss)
static void spawn_cleanup(libxl__gc *gc, libxl__spawn_state *ss)
{
assert(!libxl__ev_child_inuse(&ss->mid));
- libxl__ev_time_deregister(gc, &ss->timeout);
- libxl__ev_xswatch_deregister(gc, &ss->xswatch);
+ libxl__xswait_stop(gc, &ss->xswait);
}
static void spawn_detach(libxl__gc *gc, libxl__spawn_state *ss)
@@ -362,8 +358,7 @@ static void spawn_detach(libxl__gc *gc, libxl__spawn_state *ss)
int r;
assert(libxl__ev_child_inuse(&ss->mid));
- libxl__ev_time_deregister(gc, &ss->timeout);
- libxl__ev_xswatch_deregister(gc, &ss->xswatch);
+ libxl__xswait_stop(gc, &ss->xswait);
pid_t child = ss->mid.pid;
r = kill(child, SIGKILL);
@@ -387,25 +382,15 @@ static void spawn_fail(libxl__egc *egc, libxl__spawn_state *ss)
spawn_detach(gc, ss);
}
-static void spawn_timeout(libxl__egc *egc, libxl__ev_time *ev,
- const struct timeval *requested_abs)
-{
- /* Before event, was Attached. */
- EGC_GC;
- libxl__spawn_state *ss = CONTAINER_OF(ev, *ss, timeout);
- LOG(ERROR, "%s: startup timed out", ss->what);
- spawn_fail(egc, ss); /* must be last */
-}
-
-static void spawn_watch_event(libxl__egc *egc, libxl__ev_xswatch *xsw,
- const char *watch_path, const char *event_path)
+static void spawn_watch_event(libxl__egc *egc, libxl__xswait_state *xswa,
+ int rc, const char *p)
{
/* On entry, is Attached. */
EGC_GC;
- libxl__spawn_state *ss = CONTAINER_OF(xsw, *ss, xswatch);
- char *p = libxl__xs_read(gc, 0, ss->xspath);
- if (!p && errno != ENOENT) {
- LOG(ERROR, "%s: xenstore read of %s failed", ss->what, ss->xspath);
+ libxl__spawn_state *ss = CONTAINER_OF(xswa, *ss, xswait);
+ if (rc) {
+ if (rc == ERROR_TIMEDOUT)
+ LOG(ERROR, "%s: startup timed out", ss->what);
spawn_fail(egc, ss); /* must be last */
return;
}
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index a82a43d..0e4859b 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1295,8 +1295,7 @@ struct libxl__spawn_state {
int detaching; /* we are in Detaching */
int failed; /* might be true whenever we are not Idle */
libxl__ev_child mid; /* always in use whenever we are not Idle */
- libxl__ev_time timeout;
- libxl__ev_xswatch xswatch;
+ libxl__xswait_state xswait;
};
static inline int libxl__spawn_inuse(const libxl__spawn_state *ss)
--
1.7.10.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 08/22] libxl: events: Provide libxl__ev_evtchn*
2014-03-17 13:29 [PATCH v3 RESEND 00/22] libxl: asynchronous suspend Ian Jackson
` (6 preceding siblings ...)
2014-03-17 13:29 ` [PATCH 07/22] libxl: events: Use libxl__xswait_* in spawn code Ian Jackson
@ 2014-03-17 13:29 ` Ian Jackson
2014-03-17 13:29 ` [PATCH 09/22] libxc: suspend: Rename, improve xc_suspend_evtchn_init Ian Jackson
` (14 subsequent siblings)
22 siblings, 0 replies; 29+ messages in thread
From: Ian Jackson @ 2014-03-17 13:29 UTC (permalink / raw)
To: xen-devel
Cc: Shriram Rajagopalan, Stefano Stabellini, Ian Jackson,
Ian Campbell, Lai Jiangshan
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v2: Fix commit message not to refer to libxl_ctx_alloc's gc, done earlier
Change type of port in evtchn_fd_callback to evtchn_port_or_error_t
Clarify comment about use of ctx->xce.
Fix typo in comment.
---
tools/libxl/libxl.c | 9 +++
tools/libxl/libxl_event.c | 153 ++++++++++++++++++++++++++++++++++++++++++
tools/libxl/libxl_internal.h | 48 +++++++++++++
3 files changed, 210 insertions(+)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 0a825df..7b7ffd3 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -60,6 +60,10 @@ int libxl_ctx_alloc(libxl_ctx **pctx, int version,
LIBXL_SLIST_INIT(&ctx->watch_freeslots);
libxl__ev_fd_init(&ctx->watch_efd);
+ ctx->xce = 0;
+ LIBXL_LIST_INIT(&ctx->evtchns_waiting);
+ libxl__ev_fd_init(&ctx->evtchn_efd);
+
LIBXL_TAILQ_INIT(&ctx->death_list);
libxl__ev_xswatch_init(&ctx->death_watch);
@@ -104,6 +108,8 @@ int libxl_ctx_alloc(libxl_ctx **pctx, int version,
rc = ERROR_FAIL; goto out;
}
+ rc = libxl__ctx_evtchn_init(gc);
+
*pctx = ctx;
return 0;
@@ -147,16 +153,19 @@ int libxl_ctx_free(libxl_ctx *ctx)
for (i = 0; i < ctx->watch_nslots; i++)
assert(!libxl__watch_slot_contents(gc, i));
libxl__ev_fd_deregister(gc, &ctx->watch_efd);
+ libxl__ev_fd_deregister(gc, &ctx->evtchn_efd);
libxl__ev_fd_deregister(gc, &ctx->sigchld_selfpipe_efd);
/* Now there should be no more events requested from the application: */
assert(LIBXL_LIST_EMPTY(&ctx->efds));
assert(LIBXL_TAILQ_EMPTY(&ctx->etimes));
+ assert(LIBXL_LIST_EMPTY(&ctx->evtchns_waiting));
if (ctx->xch) xc_interface_close(ctx->xch);
libxl_version_info_dispose(&ctx->version_info);
if (ctx->xsh) xs_daemon_close(ctx->xsh);
+ if (ctx->xce) xc_evtchn_close(ctx->xce);
libxl__poller_dispose(&ctx->poller_app);
assert(LIBXL_LIST_EMPTY(&ctx->pollers_event));
diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c
index 3e465af..22b1227 100644
--- a/tools/libxl/libxl_event.c
+++ b/tools/libxl/libxl_event.c
@@ -625,6 +625,159 @@ void libxl__ev_xswatch_deregister(libxl__gc *gc, libxl__ev_xswatch *w)
}
/*
+ * evtchn
+ */
+
+static int evtchn_revents_check(libxl__egc *egc, int revents)
+{
+ EGC_GC;
+
+ if (revents & ~POLLIN) {
+ LOG(ERROR, "unexpected poll event on event channel fd: %x", revents);
+ LIBXL__EVENT_DISASTER(egc,
+ "unexpected poll event on event channel fd", 0, 0);
+ libxl__ev_fd_deregister(gc, &CTX->evtchn_efd);
+ return ERROR_FAIL;
+ }
+
+ assert(revents & POLLIN);
+
+ return 0;
+}
+
+static void evtchn_fd_callback(libxl__egc *egc, libxl__ev_fd *ev,
+ int fd, short events, short revents)
+{
+ EGC_GC;
+ libxl__ev_evtchn *evev;
+ int r, rc;
+ evtchn_port_or_error_t port;
+ struct pollfd recheck;
+
+ rc = evtchn_revents_check(egc, revents);
+ if (rc) return;
+
+ for (;;) {
+ /* Check the fd again. The incoming revent may no longer be
+ * true, because the libxl ctx lock has not necessarily been
+ * held continuously since someone noticed the fd. Normally
+ * this wouldn't be a problem but evtchn devices don't always
+ * honour O_NONBLOCK (see xenctrl.h). */
+
+ recheck.fd = fd;
+ recheck.events = POLLIN;
+ recheck.revents = 0;
+ r = poll(&recheck, 1, 0);
+ DBG("ev_evtchn recheck r=%d revents=%#x", r, recheck.revents);
+ if (r < 0) {
+ LIBXL__EVENT_DISASTER(egc,
+ "unexpected failure polling event channel fd for recheck",
+ errno, 0);
+ return;
+ }
+ if (r == 0)
+ break;
+ rc = evtchn_revents_check(egc, recheck.revents);
+ if (rc) return;
+
+ /* OK, that's that workaround done. We can actually check for
+ * work for us to do: */
+
+ port = xc_evtchn_pending(CTX->xce);
+ if (port < 0) {
+ if (errno == EAGAIN)
+ break;
+ LIBXL__EVENT_DISASTER(egc,
+ "unexpected failure fetching occurring event port number from evtchn",
+ errno, 0);
+ return;
+ }
+
+ LIBXL_LIST_FOREACH(evev, &CTX->evtchns_waiting, entry)
+ if (port == evev->port)
+ goto found;
+ /* not found */
+ DBG("ev_evtchn port=%d no-one cared", port);
+ continue;
+
+ found:
+ DBG("ev_evtchn=%p port=%d signaled", evev, port);
+ evev->waiting = 0;
+ LIBXL_LIST_REMOVE(evev, entry);
+ evev->callback(egc, evev);
+ }
+}
+
+int libxl__ctx_evtchn_init(libxl__gc *gc) {
+ xc_evtchn *xce;
+ int rc, fd;
+
+ if (CTX->xce)
+ return 0;
+
+ xce = xc_evtchn_open(CTX->lg, 0);
+ if (!xce) {
+ LOGE(ERROR,"cannot open libxc evtchn handle");
+ rc = ERROR_FAIL;
+ goto out;
+ }
+
+ fd = xc_evtchn_fd(xce);
+ assert(fd >= 0);
+
+ rc = libxl_fd_set_nonblock(CTX, fd, 1);
+ if (rc) goto out;
+
+ rc = libxl__ev_fd_register(gc, &CTX->evtchn_efd,
+ evtchn_fd_callback, fd, POLLIN);
+ if (rc) goto out;
+
+ CTX->xce = xce;
+ return 0;
+
+ out:
+ xc_evtchn_close(xce);
+ return rc;
+}
+
+int libxl__ev_evtchn_wait(libxl__gc *gc, libxl__ev_evtchn *evev)
+{
+ int r, rc;
+
+ DBG("ev_evtchn=%p port=%d wait (was waiting=%d)",
+ evev, evev->port, evev->waiting);
+
+ if (evev->waiting)
+ return 0;
+
+ r = xc_evtchn_unmask(CTX->xce, evev->port);
+ if (r) {
+ LOGE(ERROR,"cannot unmask event channel %d",evev->port);
+ rc = ERROR_FAIL;
+ goto out;
+ }
+
+ evev->waiting = 1;
+ LIBXL_LIST_INSERT_HEAD(&CTX->evtchns_waiting, evev, entry);
+ return 0;
+
+ out:
+ return rc;
+}
+
+void libxl__ev_evtchn_cancel(libxl__gc *gc, libxl__ev_evtchn *evev)
+{
+ DBG("ev_evtchn=%p port=%d cancel (was waiting=%d)",
+ evev, evev->port, evev->waiting);
+
+ if (!evev->waiting)
+ return;
+
+ evev->waiting = 0;
+ LIBXL_LIST_REMOVE(evev, entry);
+}
+
+/*
* waiting for device state
*/
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 0e4859b..1914d1b 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -197,6 +197,17 @@ struct libxl__ev_xswatch {
uint32_t counterval;
};
+typedef struct libxl__ev_evtchn libxl__ev_evtchn;
+typedef void libxl__ev_evtchn_callback(libxl__egc *egc, libxl__ev_evtchn*);
+struct libxl__ev_evtchn {
+ /* caller must fill these in, and they must all remain valid */
+ libxl__ev_evtchn_callback *callback;
+ int port;
+ /* remainder is private for libxl__ev_evtchn_... */
+ bool waiting;
+ LIBXL_LIST_ENTRY(libxl__ev_evtchn) entry;
+};
+
/*
* An entry in the watch_slots table is either:
* 1. an entry in the free list, ie NULL or pointer to next free list entry
@@ -343,6 +354,10 @@ struct libxl__ctx {
uint32_t watch_counter; /* helps disambiguate slot reuse */
libxl__ev_fd watch_efd;
+ xc_evtchn *xce; /* waiting must be done only with libxl__ev_evtchn* */
+ LIBXL_LIST_HEAD(, libxl__ev_evtchn) evtchns_waiting;
+ libxl__ev_fd evtchn_efd;
+
LIBXL_TAILQ_HEAD(libxl__evgen_domain_death_list, libxl_evgen_domain_death)
death_list /* sorted by domid */,
death_reported;
@@ -779,6 +794,39 @@ static inline int libxl__ev_xswatch_isregistered(const libxl__ev_xswatch *xw)
/*
+ * The evtchn facility is one-shot per call to libxl__ev_evtchn_wait.
+ * You should call some suitable xc bind function on (or to obtain)
+ * the port, then libxl__ev_evtchn_wait.
+ *
+ * When the event is signaled then the callback will be made, once.
+ * Then you must call libxl__ev_evtchn_wait again, if desired.
+ *
+ * You must NOT call xc_evtchn_unmask. wait will do that for you.
+ *
+ * Calling libxl__ev_evtchn_cancel will arrange for libxl to disregard
+ * future occurrences of event. Both libxl__ev_evtchn_wait and
+ * libxl__ev_evtchn_cancel are idempotent.
+ *
+ * (Note of course that an event channel becomes signaled when it is
+ * first bound, so you will get one call to libxl__ev_evtchn_wait
+ * "right away"; unless you have won a very fast race, the condition
+ * you were waiting for won't exist yet so when you check for it
+ * you'll find you need to call wait again.)
+ *
+ * You must not wait on the same port twice at once (that is, with
+ * two separate libxl__ev_evtchn's).
+ */
+_hidden int libxl__ev_evtchn_wait(libxl__gc*, libxl__ev_evtchn *evev);
+_hidden void libxl__ev_evtchn_cancel(libxl__gc *gc, libxl__ev_evtchn *evev);
+
+static inline void libxl__ev_evtchn_init(libxl__ev_evtchn *evev)
+ { evev->waiting = 0; }
+static inline bool libxl__ev_evtchn_iswaiting(const libxl__ev_evtchn *evev)
+ { return evev->waiting; }
+
+_hidden int libxl__ctx_evtchn_init(libxl__gc *gc); /* for libxl_ctx_alloc */
+
+/*
* For making subprocesses. This is the only permitted mechanism for
* code in libxl to do so.
*
--
1.7.10.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 09/22] libxc: suspend: Rename, improve xc_suspend_evtchn_init
2014-03-17 13:29 [PATCH v3 RESEND 00/22] libxl: asynchronous suspend Ian Jackson
` (7 preceding siblings ...)
2014-03-17 13:29 ` [PATCH 08/22] libxl: events: Provide libxl__ev_evtchn* Ian Jackson
@ 2014-03-17 13:29 ` Ian Jackson
2014-03-17 13:29 ` [PATCH 10/22] libxc: suspend: Fix suspend event channel locking Ian Jackson
` (13 subsequent siblings)
22 siblings, 0 replies; 29+ messages in thread
From: Ian Jackson @ 2014-03-17 13:29 UTC (permalink / raw)
To: xen-devel
Cc: Shriram Rajagopalan, Stefano Stabellini, Ian Jackson,
Ian Campbell, Lai Jiangshan
xc_suspend_evtchn_init expects to eat the first event on the xce. If
the xce is used for any other purpose then this can break. Document
this fact and rename the function to xc_suspend_evtchn_init_exclusive.
(I haven't checked the call sites for improper shared use of the xce.)
Provide a corresponding xc_suspend_evtchn_init_sane which doesn't try
to eat an event, and instead leaves the caller the ability to
demultiplex.
Also document that xc_await_suspend needs exclusive use of the xce.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
CC: Shriram Rajagopalan <rshriram@cs.ubc.ca>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v3: Drop spurious addition of #include <assert.h>
---
tools/libxc/xc_suspend.c | 19 +++++++++++++++----
tools/libxc/xenguest.h | 13 ++++++++++++-
tools/libxl/libxl_dom.c | 2 +-
tools/misc/xen-hptool.c | 2 +-
.../python/xen/lowlevel/checkpoint/libcheckpoint.c | 2 +-
tools/xcutils/xc_save.c | 2 +-
6 files changed, 31 insertions(+), 9 deletions(-)
diff --git a/tools/libxc/xc_suspend.c b/tools/libxc/xc_suspend.c
index 1ace411..eed3be2 100644
--- a/tools/libxc/xc_suspend.c
+++ b/tools/libxc/xc_suspend.c
@@ -102,7 +102,7 @@ int xc_suspend_evtchn_release(xc_interface *xch, xc_evtchn *xce, int domid, int
return unlock_suspend_event(xch, domid);
}
-int xc_suspend_evtchn_init(xc_interface *xch, xc_evtchn *xce, int domid, int port)
+int xc_suspend_evtchn_init_sane(xc_interface *xch, xc_evtchn *xce, int domid, int port)
{
int rc, suspend_evtchn = -1;
@@ -121,9 +121,6 @@ int xc_suspend_evtchn_init(xc_interface *xch, xc_evtchn *xce, int domid, int por
goto cleanup;
}
- /* event channel is pending immediately after binding */
- xc_await_suspend(xch, xce, suspend_evtchn);
-
return suspend_evtchn;
cleanup:
@@ -132,3 +129,17 @@ cleanup:
return -1;
}
+
+int xc_suspend_evtchn_init_exclusive(xc_interface *xch, xc_evtchn *xce, int domid, int port)
+{
+ int suspend_evtchn;
+
+ suspend_evtchn = xc_suspend_evtchn_init_sane(xch, xce, domid, port);
+ if (suspend_evtchn < 0)
+ return suspend_evtchn;
+
+ /* event channel is pending immediately after binding */
+ xc_await_suspend(xch, xce, suspend_evtchn);
+
+ return suspend_evtchn;
+}
diff --git a/tools/libxc/xenguest.h b/tools/libxc/xenguest.h
index a0e30e1..ce5456c 100644
--- a/tools/libxc/xenguest.h
+++ b/tools/libxc/xenguest.h
@@ -256,10 +256,21 @@ int xc_hvm_build_target_mem(xc_interface *xch,
int xc_suspend_evtchn_release(xc_interface *xch, xc_evtchn *xce, int domid, int suspend_evtchn);
-int xc_suspend_evtchn_init(xc_interface *xch, xc_evtchn *xce, int domid, int port);
+/**
+ * This function eats the initial notification.
+ * xce must not be used for anything else
+ */
+int xc_suspend_evtchn_init_exclusive(xc_interface *xch, xc_evtchn *xce, int domid, int port);
+/* xce must not be used for anything else */
int xc_await_suspend(xc_interface *xch, xc_evtchn *xce, int suspend_evtchn);
+/**
+ * The port will be signaled immediately after this call
+ * The caller should check the domain status and look for the next event
+ */
+int xc_suspend_evtchn_init_sane(xc_interface *xch, xc_evtchn *xce, int domid, int port);
+
int xc_get_bit_size(xc_interface *xch,
const char *image_name, const char *cmdline,
const char *features, int *type);
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 55f74b2..4b42856 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1358,7 +1358,7 @@ void libxl__domain_suspend(libxl__egc *egc, libxl__domain_suspend_state *dss)
if (port >= 0) {
dss->suspend_eventchn =
- xc_suspend_evtchn_init(CTX->xch, dss->xce, dss->domid, port);
+ xc_suspend_evtchn_init_exclusive(CTX->xch, dss->xce, dss->domid, port);
if (dss->suspend_eventchn < 0)
LOG(WARN, "Suspend event channel initialization failed");
diff --git a/tools/misc/xen-hptool.c b/tools/misc/xen-hptool.c
index f8570f2..1923be9 100644
--- a/tools/misc/xen-hptool.c
+++ b/tools/misc/xen-hptool.c
@@ -111,7 +111,7 @@ static int suspend_guest(xc_interface *xch, xc_evtchn *xce, int domid, int *evtc
fprintf(stderr, "DOM%d: No suspend port, try live migration\n", domid);
goto failed;
}
- suspend_evtchn = xc_suspend_evtchn_init(xch, xce, domid, port);
+ suspend_evtchn = xc_suspend_evtchn_init_exclusive(xch, xce, domid, port);
if (suspend_evtchn < 0)
{
fprintf(stderr, "Suspend evtchn initialization failed\n");
diff --git a/tools/python/xen/lowlevel/checkpoint/libcheckpoint.c b/tools/python/xen/lowlevel/checkpoint/libcheckpoint.c
index 01c0d47..817d272 100644
--- a/tools/python/xen/lowlevel/checkpoint/libcheckpoint.c
+++ b/tools/python/xen/lowlevel/checkpoint/libcheckpoint.c
@@ -360,7 +360,7 @@ static int setup_suspend_evtchn(checkpoint_state* s)
return -1;
}
- s->suspend_evtchn = xc_suspend_evtchn_init(s->xch, s->xce, s->domid, port);
+ s->suspend_evtchn = xc_suspend_evtchn_init_exclusive(s->xch, s->xce, s->domid, port);
if (s->suspend_evtchn < 0) {
s->errstr = "failed to bind suspend event channel";
return -1;
diff --git a/tools/xcutils/xc_save.c b/tools/xcutils/xc_save.c
index 654c9c2..aaa09b0 100644
--- a/tools/xcutils/xc_save.c
+++ b/tools/xcutils/xc_save.c
@@ -199,7 +199,7 @@ main(int argc, char **argv)
else
{
si.suspend_evtchn =
- xc_suspend_evtchn_init(si.xch, si.xce, si.domid, port);
+ xc_suspend_evtchn_init_exclusive(si.xch, si.xce, si.domid, port);
if (si.suspend_evtchn < 0)
warnx("suspend event channel initialization failed, "
--
1.7.10.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 10/22] libxc: suspend: Fix suspend event channel locking
2014-03-17 13:29 [PATCH v3 RESEND 00/22] libxl: asynchronous suspend Ian Jackson
` (8 preceding siblings ...)
2014-03-17 13:29 ` [PATCH 09/22] libxc: suspend: Rename, improve xc_suspend_evtchn_init Ian Jackson
@ 2014-03-17 13:29 ` Ian Jackson
2014-03-18 8:44 ` Olaf Hering
2014-03-17 13:29 ` [PATCH 11/22] libxl: suspend: trailing ws in libxl_save_msgs_gen.pl Ian Jackson
` (12 subsequent siblings)
22 siblings, 1 reply; 29+ messages in thread
From: Ian Jackson @ 2014-03-17 13:29 UTC (permalink / raw)
To: xen-devel
Cc: Shriram Rajagopalan, Stefano Stabellini, Ian Jackson,
Ian Campbell, Lai Jiangshan
Use fcntl F_SETLK, rather than writing our pid into a "lock" file.
That way if we crash we don't leave the lockfile lying about. Callers
now need to keep the fd for our lockfile. (We don't use flock because
we don't want anyone who inherits this fd across fork to end up with a
handle onto the lock.)
While we are here:
* Move the lockfile to /var/run/xen
* De-duplicate the calculation of the pathname
* Compute the buffer size for the pathname so that it will definitely
not overrun (and use the computed value everywhere)
* Fix various error handling bugs
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>
---
tools/libxc/xc_suspend.c | 160 +++++++++++++-------
tools/libxc/xenguest.h | 16 +-
tools/libxl/libxl_dom.c | 6 +-
tools/libxl/libxl_internal.h | 1 +
tools/misc/xen-hptool.c | 19 ++-
tools/python/xen/lowlevel/checkpoint/checkpoint.h | 2 +-
.../python/xen/lowlevel/checkpoint/libcheckpoint.c | 7 +-
tools/xcutils/xc_save.c | 8 +-
8 files changed, 150 insertions(+), 69 deletions(-)
diff --git a/tools/libxc/xc_suspend.c b/tools/libxc/xc_suspend.c
index eed3be2..84ee139 100644
--- a/tools/libxc/xc_suspend.c
+++ b/tools/libxc/xc_suspend.c
@@ -14,65 +14,115 @@
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
*/
+#include <unistd.h>
+#include <fcntl.h>
+
#include "xc_private.h"
#include "xenguest.h"
-#define SUSPEND_LOCK_FILE "/var/lib/xen/suspend_evtchn"
-static int lock_suspend_event(xc_interface *xch, int domid)
+#define SUSPEND_LOCK_FILE "/var/run/xen/suspend-evtchn-%d.lock"
+
+/*
+ * locking
+ */
+
+#define ERR(x) do{ \
+ ERROR("Can't " #x " lock file for suspend event channel %s: %s\n", \
+ suspend_file, strerror(errno)); \
+ goto err; \
+}while(0)
+
+#define SUSPEND_FILE_BUFLEN (sizeof(SUSPEND_LOCK_FILE) + 10)
+
+static void get_suspend_file(char buf[SUSPEND_FILE_BUFLEN], int domid)
{
- int fd, rc;
- mode_t mask;
- char buf[128];
- char suspend_file[256];
-
- snprintf(suspend_file, sizeof(suspend_file), "%s_%d_lock.d",
- SUSPEND_LOCK_FILE, domid);
- mask = umask(022);
- fd = open(suspend_file, O_CREAT | O_EXCL | O_RDWR, 0666);
- if (fd < 0)
- {
- ERROR("Can't create lock file for suspend event channel %s\n",
- suspend_file);
- return -EINVAL;
+ snprintf(buf, sizeof(buf), SUSPEND_LOCK_FILE, domid);
+}
+
+static int lock_suspend_event(xc_interface *xch, int domid, int *lockfd)
+{
+ int fd = -1, r;
+ char suspend_file[SUSPEND_FILE_BUFLEN];
+ struct stat ours, theirs;
+ struct flock fl;
+
+ get_suspend_file(suspend_file, domid);
+
+ *lockfd = -1;
+
+ for (;;) {
+ if (fd >= 0)
+ close (fd);
+
+ fd = open(suspend_file, O_CREAT | O_RDWR, 0600);
+ if (fd < 0)
+ ERR("create");
+
+ r = fcntl(fd, F_SETFD, FD_CLOEXEC);
+ if (r)
+ ERR("fcntl F_SETFD FD_CLOEXEC");
+
+ memset(&fl, 0, sizeof(fl));
+ fl.l_type = F_WRLCK;
+ fl.l_whence = SEEK_SET;
+ fl.l_len = 1;
+ r = fcntl(fd, F_SETLK, &fl);
+ if (r)
+ ERR("fcntl F_SETLK");
+
+ r = fstat(fd, &ours);
+ if (r)
+ ERR("fstat");
+
+ r = stat(suspend_file, &theirs);
+ if (r) {
+ if (errno == ENOENT)
+ /* try again */
+ continue;
+ ERR("stat");
+ }
+
+ if (ours.st_ino != theirs.st_ino)
+ /* someone else must have removed it while we were locking it */
+ continue;
+
+ break;
}
- umask(mask);
- snprintf(buf, sizeof(buf), "%10ld", (long)getpid());
- rc = write_exact(fd, buf, strlen(buf));
- close(fd);
+ *lockfd = fd;
+ return 0;
- return rc;
+ err:
+ if (fd >= 0)
+ close(fd);
+
+ return -1;
}
-static int unlock_suspend_event(xc_interface *xch, int domid)
+static int unlock_suspend_event(xc_interface *xch, int domid, int *lockfd)
{
- int fd, pid, n;
- char buf[128];
- char suspend_file[256];
+ int r;
+ char suspend_file[SUSPEND_FILE_BUFLEN];
- snprintf(suspend_file, sizeof(suspend_file), "%s_%d_lock.d",
- SUSPEND_LOCK_FILE, domid);
- fd = open(suspend_file, O_RDWR);
+ if (*lockfd < 0)
+ return 0;
- if (fd < 0)
- return -EINVAL;
+ get_suspend_file(suspend_file, domid);
- n = read(fd, buf, 127);
+ r = unlink(suspend_file);
+ if (r)
+ ERR("unlink");
- close(fd);
+ r = close(*lockfd);
+ *lockfd = -1;
+ if (r)
+ ERR("close");
- if (n > 0)
- {
- sscanf(buf, "%d", &pid);
- /* We are the owner, so we can simply delete the file */
- if (pid == getpid())
- {
- unlink(suspend_file);
- return 0;
- }
- }
+ err:
+ if (*lockfd >= 0)
+ close(*lockfd);
- return -EPERM;
+ return -1;
}
int xc_await_suspend(xc_interface *xch, xc_evtchn *xce, int suspend_evtchn)
@@ -94,20 +144,26 @@ int xc_await_suspend(xc_interface *xch, xc_evtchn *xce, int suspend_evtchn)
return 0;
}
-int xc_suspend_evtchn_release(xc_interface *xch, xc_evtchn *xce, int domid, int suspend_evtchn)
+/* Internal callers are allowed to call this with suspend_evtchn<0
+ * but *lockfd>0. */
+int xc_suspend_evtchn_release(xc_interface *xch, xc_evtchn *xce,
+ int domid, int suspend_evtchn, int *lockfd)
{
if (suspend_evtchn >= 0)
xc_evtchn_unbind(xce, suspend_evtchn);
- return unlock_suspend_event(xch, domid);
+ return unlock_suspend_event(xch, domid, lockfd);
}
-int xc_suspend_evtchn_init_sane(xc_interface *xch, xc_evtchn *xce, int domid, int port)
+int xc_suspend_evtchn_init_sane(xc_interface *xch, xc_evtchn *xce,
+ int domid, int port, int *lockfd)
{
int rc, suspend_evtchn = -1;
- if (lock_suspend_event(xch, domid))
- return -EINVAL;
+ if (lock_suspend_event(xch, domid, lockfd)) {
+ errno = EINVAL;
+ goto cleanup;
+ }
suspend_evtchn = xc_evtchn_bind_interdomain(xce, domid, port);
if (suspend_evtchn < 0) {
@@ -124,17 +180,17 @@ int xc_suspend_evtchn_init_sane(xc_interface *xch, xc_evtchn *xce, int domid, in
return suspend_evtchn;
cleanup:
- if (suspend_evtchn != -1)
- xc_suspend_evtchn_release(xch, xce, domid, suspend_evtchn);
+ xc_suspend_evtchn_release(xch, xce, domid, suspend_evtchn, lockfd);
return -1;
}
-int xc_suspend_evtchn_init_exclusive(xc_interface *xch, xc_evtchn *xce, int domid, int port)
+int xc_suspend_evtchn_init_exclusive(xc_interface *xch, xc_evtchn *xce,
+ int domid, int port, int *lockfd)
{
int suspend_evtchn;
- suspend_evtchn = xc_suspend_evtchn_init_sane(xch, xce, domid, port);
+ suspend_evtchn = xc_suspend_evtchn_init_sane(xch, xce, domid, port, lockfd);
if (suspend_evtchn < 0)
return suspend_evtchn;
diff --git a/tools/libxc/xenguest.h b/tools/libxc/xenguest.h
index ce5456c..1f216cd 100644
--- a/tools/libxc/xenguest.h
+++ b/tools/libxc/xenguest.h
@@ -254,13 +254,19 @@ int xc_hvm_build_target_mem(xc_interface *xch,
int target,
const char *image_name);
-int xc_suspend_evtchn_release(xc_interface *xch, xc_evtchn *xce, int domid, int suspend_evtchn);
+/*
+ * Sets *lockfd to -1.
+ * Has deallocated everything even on error.
+ */
+int xc_suspend_evtchn_release(xc_interface *xch, xc_evtchn *xce, int domid, int suspend_evtchn, int *lockfd);
/**
* This function eats the initial notification.
* xce must not be used for anything else
+ * See xc_suspend_evtchn_init_sane re lockfd.
*/
-int xc_suspend_evtchn_init_exclusive(xc_interface *xch, xc_evtchn *xce, int domid, int port);
+int xc_suspend_evtchn_init_exclusive(xc_interface *xch, xc_evtchn *xce,
+ int domid, int port, int *lockfd);
/* xce must not be used for anything else */
int xc_await_suspend(xc_interface *xch, xc_evtchn *xce, int suspend_evtchn);
@@ -268,8 +274,12 @@ int xc_await_suspend(xc_interface *xch, xc_evtchn *xce, int suspend_evtchn);
/**
* The port will be signaled immediately after this call
* The caller should check the domain status and look for the next event
+ * On success, *lockfd will be set to >=0 and *lockfd must be preserved
+ * and fed to xc_suspend_evtchn_release. (On error *lockfd is
+ * undefined and xc_suspend_evtchn_release is not allowed.)
*/
-int xc_suspend_evtchn_init_sane(xc_interface *xch, xc_evtchn *xce, int domid, int port);
+int xc_suspend_evtchn_init_sane(xc_interface *xch, xc_evtchn *xce,
+ int domid, int port, int *lockfd);
int xc_get_bit_size(xc_interface *xch,
const char *image_name, const char *cmdline,
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 4b42856..48a4b8e 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1340,6 +1340,7 @@ void libxl__domain_suspend(libxl__egc *egc, libxl__domain_suspend_state *dss)
| (dss->hvm ? XCFLAGS_HVM : 0);
dss->suspend_eventchn = -1;
+ dss->guest_evtchn_lockfd = -1;
dss->guest_responded = 0;
dss->dm_savefile = libxl__device_model_savefile(gc, domid);
@@ -1358,7 +1359,8 @@ void libxl__domain_suspend(libxl__egc *egc, libxl__domain_suspend_state *dss)
if (port >= 0) {
dss->suspend_eventchn =
- xc_suspend_evtchn_init_exclusive(CTX->xch, dss->xce, dss->domid, port);
+ xc_suspend_evtchn_init_exclusive(CTX->xch, dss->xce,
+ dss->domid, port, &dss->guest_evtchn_lockfd);
if (dss->suspend_eventchn < 0)
LOG(WARN, "Suspend event channel initialization failed");
@@ -1522,7 +1524,7 @@ static void domain_suspend_done(libxl__egc *egc,
if (dss->suspend_eventchn > 0)
xc_suspend_evtchn_release(CTX->xch, dss->xce, domid,
- dss->suspend_eventchn);
+ dss->suspend_eventchn, &dss->guest_evtchn_lockfd);
if (dss->xce != NULL)
xc_evtchn_close(dss->xce);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 1914d1b..d15a4b2 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2439,6 +2439,7 @@ struct libxl__domain_suspend_state {
/* private */
xc_evtchn *xce; /* event channel handle */
int suspend_eventchn;
+ int guest_evtchn_lockfd;
int hvm;
int xcflags;
int guest_responded;
diff --git a/tools/misc/xen-hptool.c b/tools/misc/xen-hptool.c
index 1923be9..8aac51c 100644
--- a/tools/misc/xen-hptool.c
+++ b/tools/misc/xen-hptool.c
@@ -98,10 +98,13 @@ static int hp_mem_query_func(int argc, char *argv[])
extern int xs_suspend_evtchn_port(int domid);
-static int suspend_guest(xc_interface *xch, xc_evtchn *xce, int domid, int *evtchn)
+static int suspend_guest(xc_interface *xch, xc_evtchn *xce, int domid,
+ int *evtchn, int *lockfd)
{
int port, rc, suspend_evtchn = -1;
+ *lockfd = -1;
+
if (!evtchn)
return -1;
@@ -111,7 +114,8 @@ static int suspend_guest(xc_interface *xch, xc_evtchn *xce, int domid, int *evtc
fprintf(stderr, "DOM%d: No suspend port, try live migration\n", domid);
goto failed;
}
- suspend_evtchn = xc_suspend_evtchn_init_exclusive(xch, xce, domid, port);
+ suspend_evtchn = xc_suspend_evtchn_init_exclusive(xch, xce, domid,
+ port, lockfd);
if (suspend_evtchn < 0)
{
fprintf(stderr, "Suspend evtchn initialization failed\n");
@@ -134,7 +138,8 @@ static int suspend_guest(xc_interface *xch, xc_evtchn *xce, int domid, int *evtc
failed:
if (suspend_evtchn != -1)
- xc_suspend_evtchn_release(xch, xce, domid, suspend_evtchn);
+ xc_suspend_evtchn_release(xch, xce, domid,
+ suspend_evtchn, lockfd);
return -1;
}
@@ -192,7 +197,7 @@ static int hp_mem_offline_func(int argc, char *argv[])
}
else if (status & PG_OFFLINE_OWNED)
{
- int result, suspend_evtchn = -1;
+ int result, suspend_evtchn = -1, suspend_lockfd = -1;
xc_evtchn *xce;
xce = xc_evtchn_open(NULL, 0);
@@ -204,7 +209,8 @@ static int hp_mem_offline_func(int argc, char *argv[])
}
domid = status >> PG_OFFLINE_OWNER_SHIFT;
- if (suspend_guest(xch, xce, domid, &suspend_evtchn))
+ if (suspend_guest(xch, xce, domid,
+ &suspend_evtchn, &suspend_lockfd))
{
fprintf(stderr, "Failed to suspend guest %d for"
" mfn %lx\n", domid, mfn);
@@ -230,7 +236,8 @@ static int hp_mem_offline_func(int argc, char *argv[])
mfn, domid);
}
xc_domain_resume(xch, domid, 1);
- xc_suspend_evtchn_release(xch, xce, domid, suspend_evtchn);
+ xc_suspend_evtchn_release(xch, xce, domid,
+ suspend_evtchn, &suspend_lockfd);
xc_evtchn_close(xce);
}
break;
diff --git a/tools/python/xen/lowlevel/checkpoint/checkpoint.h b/tools/python/xen/lowlevel/checkpoint/checkpoint.h
index 187d9d7..2414956 100644
--- a/tools/python/xen/lowlevel/checkpoint/checkpoint.h
+++ b/tools/python/xen/lowlevel/checkpoint/checkpoint.h
@@ -27,7 +27,7 @@ typedef struct {
checkpoint_domtype domtype;
int fd;
- int suspend_evtchn;
+ int suspend_evtchn, suspend_lockfd;
char* errstr;
diff --git a/tools/python/xen/lowlevel/checkpoint/libcheckpoint.c b/tools/python/xen/lowlevel/checkpoint/libcheckpoint.c
index 817d272..74ca062 100644
--- a/tools/python/xen/lowlevel/checkpoint/libcheckpoint.c
+++ b/tools/python/xen/lowlevel/checkpoint/libcheckpoint.c
@@ -57,6 +57,7 @@ void checkpoint_init(checkpoint_state* s)
s->fd = -1;
s->suspend_evtchn = -1;
+ s->suspend_lockfd = -1;
s->errstr = NULL;
@@ -360,7 +361,8 @@ static int setup_suspend_evtchn(checkpoint_state* s)
return -1;
}
- s->suspend_evtchn = xc_suspend_evtchn_init_exclusive(s->xch, s->xce, s->domid, port);
+ s->suspend_evtchn = xc_suspend_evtchn_init_exclusive(s->xch, s->xce,
+ s->domid, port, &s->suspend_lockfd);
if (s->suspend_evtchn < 0) {
s->errstr = "failed to bind suspend event channel";
return -1;
@@ -377,7 +379,8 @@ static void release_suspend_evtchn(checkpoint_state *s)
{
/* TODO: teach xen to clean up if port is unbound */
if (s->xce != NULL && s->suspend_evtchn >= 0) {
- xc_suspend_evtchn_release(s->xch, s->xce, s->domid, s->suspend_evtchn);
+ xc_suspend_evtchn_release(s->xch, s->xce, s->domid,
+ s->suspend_evtchn, &s->suspend_lockfd);
s->suspend_evtchn = -1;
}
}
diff --git a/tools/xcutils/xc_save.c b/tools/xcutils/xc_save.c
index aaa09b0..01adc56 100644
--- a/tools/xcutils/xc_save.c
+++ b/tools/xcutils/xc_save.c
@@ -164,7 +164,7 @@ int
main(int argc, char **argv)
{
unsigned int maxit, max_f, lflags;
- int io_fd, ret, port;
+ int io_fd, ret, port, suspend_lockfd = -1;
struct save_callbacks callbacks;
xentoollog_level lvl;
xentoollog_logger *l;
@@ -199,7 +199,8 @@ main(int argc, char **argv)
else
{
si.suspend_evtchn =
- xc_suspend_evtchn_init_exclusive(si.xch, si.xce, si.domid, port);
+ xc_suspend_evtchn_init_exclusive(si.xch, si.xce, si.domid,
+ port, &suspend_lockfd);
if (si.suspend_evtchn < 0)
warnx("suspend event channel initialization failed, "
@@ -213,7 +214,8 @@ main(int argc, char **argv)
&callbacks, !!(si.flags & XCFLAGS_HVM), 0);
if (si.suspend_evtchn > 0)
- xc_suspend_evtchn_release(si.xch, si.xce, si.domid, si.suspend_evtchn);
+ xc_suspend_evtchn_release(si.xch, si.xce, si.domid,
+ si.suspend_evtchn, &suspend_lockfd);
if (si.xce > 0)
xc_evtchn_close(si.xce);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 10/22] libxc: suspend: Fix suspend event channel locking
2014-03-17 13:29 ` [PATCH 10/22] libxc: suspend: Fix suspend event channel locking Ian Jackson
@ 2014-03-18 8:44 ` Olaf Hering
0 siblings, 0 replies; 29+ messages in thread
From: Olaf Hering @ 2014-03-18 8:44 UTC (permalink / raw)
To: Ian Jackson
Cc: Shriram Rajagopalan, Lai Jiangshan, xen-devel, Ian Campbell,
Stefano Stabellini
On Mon, Mar 17, Ian Jackson wrote:
> +#define SUSPEND_FILE_BUFLEN (sizeof(SUSPEND_LOCK_FILE) + 10)
> +
> +static void get_suspend_file(char buf[SUSPEND_FILE_BUFLEN], int domid)
> {
> - int fd, rc;
> - mode_t mask;
> - char buf[128];
> - char suspend_file[256];
> -
> - snprintf(suspend_file, sizeof(suspend_file), "%s_%d_lock.d",
> - SUSPEND_LOCK_FILE, domid);
> - mask = umask(022);
> - fd = open(suspend_file, O_CREAT | O_EXCL | O_RDWR, 0666);
> - if (fd < 0)
> - {
> - ERROR("Can't create lock file for suspend event channel %s\n",
> - suspend_file);
> - return -EINVAL;
> + snprintf(buf, sizeof(buf), SUSPEND_LOCK_FILE, domid);
> +}
This fails with newer gcc, such as gcc-4.8:
xc_suspend.c: In function 'get_suspend_file':
xc_suspend.c:39:25: error: argument to 'sizeof' in 'snprintf' call is the same expression as the destination; did you mean to provide an explicit length? [-Werror=sizeof-pointer-memaccess]
snprintf(buf, sizeof(buf), SUSPEND_LOCK_FILE, domid);
^
cc1: all warnings being treated as errors
Olaf
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 11/22] libxl: suspend: trailing ws in libxl_save_msgs_gen.pl
2014-03-17 13:29 [PATCH v3 RESEND 00/22] libxl: asynchronous suspend Ian Jackson
` (9 preceding siblings ...)
2014-03-17 13:29 ` [PATCH 10/22] libxc: suspend: Fix suspend event channel locking Ian Jackson
@ 2014-03-17 13:29 ` Ian Jackson
2014-03-17 13:38 ` Ian Campbell
2014-03-17 13:29 ` [PATCH 12/22] libxl: suspend: Async libxl__domain_suspend_callback Ian Jackson
` (11 subsequent siblings)
22 siblings, 1 reply; 29+ messages in thread
From: Ian Jackson @ 2014-03-17 13:29 UTC (permalink / raw)
To: xen-devel; +Cc: Shriram Rajagopalan, Ian Jackson, Ian Campbell, Lai Jiangshan
libxl_save_msgs_gen.pl has some trailing whitespace. This can make
git complain annoyingly. Remove the trailing whitespace.
(The file also contains tabs. Leave these as they are to avoid a big
and intrusive diff.)
No functional change.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
v3: New patch in v3 of the series.
---
tools/libxl/libxl_save_msgs_gen.pl | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/tools/libxl/libxl_save_msgs_gen.pl b/tools/libxl/libxl_save_msgs_gen.pl
index ee126c7..0dbafca 100755
--- a/tools/libxl/libxl_save_msgs_gen.pl
+++ b/tools/libxl/libxl_save_msgs_gen.pl
@@ -23,9 +23,9 @@ our @msgs = (
STRING doing_what),
'unsigned long', 'done',
'unsigned long', 'total'] ],
- [ 3, 'scxW', "suspend", [] ],
- [ 4, 'scxW', "postcopy", [] ],
- [ 5, 'scxA', "checkpoint", [] ],
+ [ 3, 'scxW', "suspend", [] ],
+ [ 4, 'scxW', "postcopy", [] ],
+ [ 5, 'scxA', "checkpoint", [] ],
[ 6, 'scxA', "switch_qemu_logdirty", [qw(int domid
unsigned enable)] ],
# toolstack_save done entirely `by hand'
@@ -199,7 +199,7 @@ static void BLOCK_put(unsigned char *const buf,
uint32_t_put(buf, len, size);
bytes_put(buf, len, bytes, size);
}
-
+
static void STRING_put(unsigned char *const buf,
int *len,
const char *string)
@@ -209,7 +209,7 @@ static void STRING_put(unsigned char *const buf,
assert(slen < (uint32_t)0x40000000);
BLOCK_put(buf, len, (const void*)string, slen+1);
}
-
+
END
foreach my $sr (qw(save restore)) {
--
1.7.10.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 12/22] libxl: suspend: Async libxl__domain_suspend_callback
2014-03-17 13:29 [PATCH v3 RESEND 00/22] libxl: asynchronous suspend Ian Jackson
` (10 preceding siblings ...)
2014-03-17 13:29 ` [PATCH 11/22] libxl: suspend: trailing ws in libxl_save_msgs_gen.pl Ian Jackson
@ 2014-03-17 13:29 ` Ian Jackson
2014-03-17 13:29 ` [PATCH 13/22] libxl: suspend: Async domain_suspend_callback_common Ian Jackson
` (10 subsequent siblings)
22 siblings, 0 replies; 29+ messages in thread
From: Ian Jackson @ 2014-03-17 13:29 UTC (permalink / raw)
To: xen-devel
Cc: Shriram Rajagopalan, Stefano Stabellini, Ian Jackson,
Ian Campbell, Lai Jiangshan
Mark the suspend callback libxl__domain_suspend_callback as
asynchronous in the helper stub generator (libxl_save_msgs_gen.pl).
We are going to want to provide an asynchronous version of this
function to get rid of the usleeps and waiting loops in the suspend
code.
libxl__domain_suspend_common_callback, the common synchronous core,
which used to be provided directly as the callback function for the
helper machinery, becomes libxl__domain_suspend_callback_common. It
can now take a typesafe parameter.
For now, provide two very similar asynchronous wrappers for it
(normal, and remus). Each is a simple function which contains only
boilerplate, calls the common synchronous core, and returns the
asynchronous response.
Essentially, we have just moved (in the case of suspend callbacks) the
call site of libxl__srm_callout_sendreply. It was in the switch
statement in the autogenerated _libxl_save_msgs_callout.c, and is now
in the handwritten libxl_dom.c.
No functional change.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: Shriram Rajagopalan <rshriram@cs.ubc.ca>
CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v3: Clarify commit message.
Fix a misformatted CC in the commit message
Do not introduce a whitespace error in libxl_save_msgs_gen.pl
v2: Commit message mentions usleeps, not Remus, as motivation.
---
tools/libxl/libxl_dom.c | 25 +++++++++++++++++++------
tools/libxl/libxl_internal.h | 2 +-
tools/libxl/libxl_save_msgs_gen.pl | 2 +-
3 files changed, 21 insertions(+), 8 deletions(-)
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 48a4b8e..1c0f04f 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1020,10 +1020,8 @@ int libxl__domain_resume_device_model(libxl__gc *gc, uint32_t domid)
return 0;
}
-int libxl__domain_suspend_common_callback(void *user)
+int libxl__domain_suspend_callback_common(libxl__domain_suspend_state *dss)
{
- libxl__save_helper_state *shs = user;
- libxl__domain_suspend_state *dss = CONTAINER_OF(shs, *dss, shs);
STATE_AO_GC(dss->ao);
unsigned long hvm_s_state = 0, hvm_pvdrv = 0;
int ret;
@@ -1242,12 +1240,27 @@ int libxl__toolstack_save(uint32_t domid, uint8_t **buf,
return 0;
}
+static void libxl__domain_suspend_callback(void *data)
+{
+ libxl__save_helper_state *shs = data;
+ libxl__egc *egc = shs->egc;
+ libxl__domain_suspend_state *dss = CONTAINER_OF(shs, *dss, shs);
+
+ int ok = libxl__domain_suspend_callback_common(dss);
+ libxl__xc_domain_saverestore_async_callback_done(egc, shs, ok);
+}
+
/*----- remus callbacks -----*/
-static int libxl__remus_domain_suspend_callback(void *data)
+static void libxl__remus_domain_suspend_callback(void *data)
{
+ libxl__save_helper_state *shs = data;
+ libxl__egc *egc = shs->egc;
+ libxl__domain_suspend_state *dss = CONTAINER_OF(shs, *dss, shs);
+
/* REMUS TODO: Issue disk and network checkpoint reqs. */
- return libxl__domain_suspend_common_callback(data);
+ int ok = libxl__domain_suspend_callback_common(dss);
+ libxl__xc_domain_saverestore_async_callback_done(egc, shs, ok);
}
static int libxl__remus_domain_resume_callback(void *data)
@@ -1373,7 +1386,7 @@ void libxl__domain_suspend(libxl__egc *egc, libxl__domain_suspend_state *dss)
callbacks->postcopy = libxl__remus_domain_resume_callback;
callbacks->checkpoint = libxl__remus_domain_checkpoint_callback;
} else
- callbacks->suspend = libxl__domain_suspend_common_callback;
+ callbacks->suspend = libxl__domain_suspend_callback;
callbacks->switch_qemu_logdirty = libxl__domain_suspend_common_switch_qemu_logdirty;
dss->shs.callbacks.save.toolstack_save = libxl__toolstack_save;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index d15a4b2..e531dca 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2739,7 +2739,7 @@ _hidden void libxl__xc_domain_save_done(libxl__egc*, void *dss_void,
void libxl__xc_domain_saverestore_async_callback_done(libxl__egc *egc,
libxl__save_helper_state *shs, int return_value);
-_hidden int libxl__domain_suspend_common_callback(void *data);
+_hidden int libxl__domain_suspend_callback_common(libxl__domain_suspend_state*);
_hidden void libxl__domain_suspend_common_switch_qemu_logdirty
(int domid, unsigned int enable, void *data);
_hidden int libxl__toolstack_save(uint32_t domid, uint8_t **buf,
diff --git a/tools/libxl/libxl_save_msgs_gen.pl b/tools/libxl/libxl_save_msgs_gen.pl
index 0dbafca..745e2ac 100755
--- a/tools/libxl/libxl_save_msgs_gen.pl
+++ b/tools/libxl/libxl_save_msgs_gen.pl
@@ -23,7 +23,7 @@ our @msgs = (
STRING doing_what),
'unsigned long', 'done',
'unsigned long', 'total'] ],
- [ 3, 'scxW', "suspend", [] ],
+ [ 3, 'scxA', "suspend", [] ],
[ 4, 'scxW', "postcopy", [] ],
[ 5, 'scxA', "checkpoint", [] ],
[ 6, 'scxA', "switch_qemu_logdirty", [qw(int domid
--
1.7.10.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 13/22] libxl: suspend: Async domain_suspend_callback_common
2014-03-17 13:29 [PATCH v3 RESEND 00/22] libxl: asynchronous suspend Ian Jackson
` (11 preceding siblings ...)
2014-03-17 13:29 ` [PATCH 12/22] libxl: suspend: Async libxl__domain_suspend_callback Ian Jackson
@ 2014-03-17 13:29 ` Ian Jackson
2014-03-17 13:29 ` [PATCH 14/22] libxl: suspend: Reorg domain_suspend_callback_common Ian Jackson
` (9 subsequent siblings)
22 siblings, 0 replies; 29+ messages in thread
From: Ian Jackson @ 2014-03-17 13:29 UTC (permalink / raw)
To: xen-devel
Cc: Shriram Rajagopalan, Stefano Stabellini, Ian Jackson,
Ian Campbell, Lai Jiangshan
Make domain_suspend_callback_common do its work and then call
dss->callback_common_done, rather than simply returning its answer.
This is preparatory to abolishing the usleeps in this function and
replacing them with use of the event machinery.
No functional change.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v3: Remove some trailing whitespace
---
tools/libxl/libxl_dom.c | 44 ++++++++++++++++++++++++++++++------------
tools/libxl/libxl_internal.h | 4 +++-
2 files changed, 35 insertions(+), 13 deletions(-)
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 1c0f04f..bfdb372 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -755,6 +755,10 @@ 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);
+static void remus_domain_suspend_callback_common_done(libxl__egc *egc,
+ libxl__domain_suspend_state *dss, int ok);
/*----- complicated callback, called by xc_domain_save -----*/
@@ -1020,7 +1024,9 @@ int libxl__domain_resume_device_model(libxl__gc *gc, uint32_t domid)
return 0;
}
-int libxl__domain_suspend_callback_common(libxl__domain_suspend_state *dss)
+/* calls dss->callback_common_done when done */
+static void domain_suspend_callback_common(libxl__egc *egc,
+ libxl__domain_suspend_state *dss)
{
STATE_AO_GC(dss->ao);
unsigned long hvm_s_state = 0, hvm_pvdrv = 0;
@@ -1043,12 +1049,12 @@ int libxl__domain_suspend_callback_common(libxl__domain_suspend_state *dss)
ret = xc_evtchn_notify(dss->xce, dss->suspend_eventchn);
if (ret < 0) {
LOG(ERROR, "xc_evtchn_notify failed ret=%d", ret);
- return 0;
+ goto err;
}
ret = xc_await_suspend(CTX->xch, dss->xce, dss->suspend_eventchn);
if (ret < 0) {
LOG(ERROR, "xc_await_suspend failed ret=%d", ret);
- return 0;
+ goto err;
}
dss->guest_responded = 1;
goto guest_suspended;
@@ -1059,7 +1065,7 @@ int libxl__domain_suspend_callback_common(libxl__domain_suspend_state *dss)
ret = xc_domain_shutdown(CTX->xch, domid, SHUTDOWN_suspend);
if (ret < 0) {
LOGE(ERROR, "xc_domain_shutdown failed");
- return 0;
+ goto err;
}
/* The guest does not (need to) respond to this sort of request. */
dss->guest_responded = 1;
@@ -1113,7 +1119,7 @@ int libxl__domain_suspend_callback_common(libxl__domain_suspend_state *dss)
*/
if (!strcmp(state, "suspend")) {
LOG(ERROR, "guest didn't acknowledge suspend, request cancelled");
- return 0;
+ goto err;
}
LOG(DEBUG, "guest acknowledged suspend request");
@@ -1143,17 +1149,19 @@ int libxl__domain_suspend_callback_common(libxl__domain_suspend_state *dss)
}
LOG(ERROR, "guest did not suspend");
- return 0;
+ err:
+ dss->callback_common_done(egc, dss, 0);
+ return;
guest_suspended:
if (dss->hvm) {
ret = libxl__domain_suspend_device_model(gc, dss);
if (ret) {
LOG(ERROR, "libxl__domain_suspend_device_model failed ret=%d", ret);
- return 0;
+ goto err;
}
}
- return 1;
+ dss->callback_common_done(egc, dss, 1);
}
static inline char *physmap_path(libxl__gc *gc, uint32_t domid,
@@ -1246,8 +1254,14 @@ static void libxl__domain_suspend_callback(void *data)
libxl__egc *egc = shs->egc;
libxl__domain_suspend_state *dss = CONTAINER_OF(shs, *dss, shs);
- int ok = libxl__domain_suspend_callback_common(dss);
- libxl__xc_domain_saverestore_async_callback_done(egc, shs, ok);
+ dss->callback_common_done = domain_suspend_callback_common_done;
+ domain_suspend_callback_common(egc, dss);
+}
+
+static void domain_suspend_callback_common_done(libxl__egc *egc,
+ libxl__domain_suspend_state *dss, int ok)
+{
+ libxl__xc_domain_saverestore_async_callback_done(egc, &dss->shs, ok);
}
/*----- remus callbacks -----*/
@@ -1258,9 +1272,15 @@ static void libxl__remus_domain_suspend_callback(void *data)
libxl__egc *egc = shs->egc;
libxl__domain_suspend_state *dss = CONTAINER_OF(shs, *dss, shs);
+ dss->callback_common_done = remus_domain_suspend_callback_common_done;
+ domain_suspend_callback_common(egc, dss);
+}
+
+static void remus_domain_suspend_callback_common_done(libxl__egc *egc,
+ libxl__domain_suspend_state *dss, int ok)
+{
/* REMUS TODO: Issue disk and network checkpoint reqs. */
- int ok = libxl__domain_suspend_callback_common(dss);
- libxl__xc_domain_saverestore_async_callback_done(egc, shs, ok);
+ libxl__xc_domain_saverestore_async_callback_done(egc, &dss->shs, ok);
}
static int libxl__remus_domain_resume_callback(void *data)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index e531dca..923ad27 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2447,6 +2447,8 @@ struct libxl__domain_suspend_state {
int interval; /* checkpoint interval (for Remus) */
libxl__save_helper_state shs;
libxl__logdirty_switch logdirty;
+ void (*callback_common_done)(libxl__egc*,
+ struct libxl__domain_suspend_state*, int ok);
/* private for libxl__domain_save_device_model */
libxl__save_device_model_cb *save_dm_callback;
libxl__datacopier_state save_dm_datacopier;
@@ -2739,7 +2741,7 @@ _hidden void libxl__xc_domain_save_done(libxl__egc*, void *dss_void,
void libxl__xc_domain_saverestore_async_callback_done(libxl__egc *egc,
libxl__save_helper_state *shs, int return_value);
-_hidden int libxl__domain_suspend_callback_common(libxl__domain_suspend_state*);
+
_hidden void libxl__domain_suspend_common_switch_qemu_logdirty
(int domid, unsigned int enable, void *data);
_hidden int libxl__toolstack_save(uint32_t domid, uint8_t **buf,
--
1.7.10.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 14/22] libxl: suspend: Reorg domain_suspend_callback_common
2014-03-17 13:29 [PATCH v3 RESEND 00/22] libxl: asynchronous suspend Ian Jackson
` (12 preceding siblings ...)
2014-03-17 13:29 ` [PATCH 13/22] libxl: suspend: Async domain_suspend_callback_common Ian Jackson
@ 2014-03-17 13:29 ` Ian Jackson
2014-03-17 13:29 ` [PATCH 15/22] libxl: suspend: New libxl__domain_pvcontrol_xspath Ian Jackson
` (8 subsequent siblings)
22 siblings, 0 replies; 29+ messages in thread
From: Ian Jackson @ 2014-03-17 13:29 UTC (permalink / raw)
To: xen-devel
Cc: Shriram Rajagopalan, Stefano Stabellini, Ian Jackson,
Ian Campbell, Lai Jiangshan
Make domain_suspend_callback_common more callback-oriented:
* Turn the functionality behind the goto labels "err" and
"guest_suspended" into functions which can be called just before
"return".
* Deindent the "issuing %s suspend request via XenBus control node"
branch; it is going to be split up into various functions as the
xenstore work becomes callback-based.
No functional change.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
tools/libxl/libxl_dom.c | 158 ++++++++++++++++++++++++++++++-----------------
1 file changed, 103 insertions(+), 55 deletions(-)
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index bfdb372..cef62cf 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1024,6 +1024,16 @@ int libxl__domain_resume_device_model(libxl__gc *gc, uint32_t domid)
return 0;
}
+static void domain_suspend_common_wait_guest(libxl__egc *egc,
+ libxl__domain_suspend_state *dss);
+static void domain_suspend_common_guest_suspended(libxl__egc *egc,
+ libxl__domain_suspend_state *dss);
+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);
+
/* calls dss->callback_common_done when done */
static void domain_suspend_callback_common(libxl__egc *egc,
libxl__domain_suspend_state *dss)
@@ -1057,7 +1067,8 @@ static void domain_suspend_callback_common(libxl__egc *egc,
goto err;
}
dss->guest_responded = 1;
- goto guest_suspended;
+ domain_suspend_common_guest_suspended(egc, dss);
+ return;
}
if (dss->hvm && (!hvm_pvdrv || hvm_s_state)) {
@@ -1069,63 +1080,81 @@ static void domain_suspend_callback_common(libxl__egc *egc,
}
/* The guest does not (need to) respond to this sort of request. */
dss->guest_responded = 1;
- } else {
- LOG(DEBUG, "issuing %s suspend request via XenBus control node",
- dss->hvm ? "PVHVM" : "PV");
+ domain_suspend_common_wait_guest(egc, dss);
+ return;
+ }
- libxl__domain_pvcontrol_write(gc, XBT_NULL, domid, "suspend");
+ LOG(DEBUG, "issuing %s suspend request via XenBus control node",
+ dss->hvm ? "PVHVM" : "PV");
- LOG(DEBUG, "wait for the guest to acknowledge suspend request");
- watchdog = 60;
- while (!strcmp(state, "suspend") && watchdog > 0) {
- usleep(100000);
+ libxl__domain_pvcontrol_write(gc, XBT_NULL, domid, "suspend");
- state = libxl__domain_pvcontrol_read(gc, XBT_NULL, domid);
- if (!state) state = "";
+ LOG(DEBUG, "wait for the guest to acknowledge suspend request");
+ watchdog = 60;
+ while (!strcmp(state, "suspend") && watchdog > 0) {
+ usleep(100000);
- watchdog--;
- }
+ state = libxl__domain_pvcontrol_read(gc, XBT_NULL, domid);
+ if (!state) state = "";
- /*
- * Guest appears to not be responding. Cancel the suspend
- * request.
- *
- * We re-read the suspend node and clear it within a
- * transaction in order to handle the case where we race
- * against the guest catching up and acknowledging the request
- * at the last minute.
- */
- if (!strcmp(state, "suspend")) {
- LOG(ERROR, "guest didn't acknowledge suspend, cancelling request");
- retry_transaction:
- t = xs_transaction_start(CTX->xsh);
-
- state = libxl__domain_pvcontrol_read(gc, t, domid);
- if (!state) state = "";
-
- if (!strcmp(state, "suspend"))
- libxl__domain_pvcontrol_write(gc, t, domid, "");
-
- if (!xs_transaction_end(CTX->xsh, t, 0))
- if (errno == EAGAIN)
- goto retry_transaction;
+ watchdog--;
+ }
- }
+ /*
+ * Guest appears to not be responding. Cancel the suspend
+ * request.
+ *
+ * We re-read the suspend node and clear it within a
+ * transaction in order to handle the case where we race
+ * against the guest catching up and acknowledging the request
+ * at the last minute.
+ */
+ if (!strcmp(state, "suspend")) {
+ LOG(ERROR, "guest didn't acknowledge suspend, cancelling request");
+ retry_transaction:
+ t = xs_transaction_start(CTX->xsh);
- /*
- * Final check for guest acknowledgement. The guest may have
- * acknowledged while we were cancelling the request in which
- * case we lost the race while cancelling and should continue.
- */
- if (!strcmp(state, "suspend")) {
- LOG(ERROR, "guest didn't acknowledge suspend, request cancelled");
- goto err;
- }
+ state = libxl__domain_pvcontrol_read(gc, t, domid);
+ if (!state) state = "";
+
+ if (!strcmp(state, "suspend"))
+ libxl__domain_pvcontrol_write(gc, t, domid, "");
+
+ if (!xs_transaction_end(CTX->xsh, t, 0))
+ if (errno == EAGAIN)
+ goto retry_transaction;
- LOG(DEBUG, "guest acknowledged suspend request");
- dss->guest_responded = 1;
}
+ /*
+ * Final check for guest acknowledgement. The guest may have
+ * acknowledged while we were cancelling the request in which
+ * case we lost the race while cancelling and should continue.
+ */
+ if (!strcmp(state, "suspend")) {
+ LOG(ERROR, "guest didn't acknowledge suspend, request cancelled");
+ goto err;
+ }
+
+ LOG(DEBUG, "guest acknowledged suspend request");
+ dss->guest_responded = 1;
+ domain_suspend_common_wait_guest(egc,dss);
+ return;
+
+ err:
+ domain_suspend_common_failed(egc, dss);
+}
+
+static void domain_suspend_common_wait_guest(libxl__egc *egc,
+ libxl__domain_suspend_state *dss)
+{
+ STATE_AO_GC(dss->ao);
+ int ret;
+ int watchdog;
+
+ /* Convenience aliases */
+ const uint32_t domid = dss->domid;
+
LOG(DEBUG, "wait for the guest to suspend");
watchdog = 60;
while (watchdog > 0) {
@@ -1141,7 +1170,8 @@ static void domain_suspend_callback_common(libxl__egc *egc,
& XEN_DOMINF_shutdownmask;
if (shutdown_reason == SHUTDOWN_suspend) {
LOG(DEBUG, "guest has suspended");
- goto guest_suspended;
+ domain_suspend_common_guest_suspended(egc, dss);
+ return;
}
}
@@ -1149,19 +1179,37 @@ static void domain_suspend_callback_common(libxl__egc *egc,
}
LOG(ERROR, "guest did not suspend");
- err:
- dss->callback_common_done(egc, dss, 0);
- return;
+ domain_suspend_common_failed(egc, dss);
+}
+
+static void domain_suspend_common_guest_suspended(libxl__egc *egc,
+ libxl__domain_suspend_state *dss)
+{
+ STATE_AO_GC(dss->ao);
+ int ret;
- guest_suspended:
if (dss->hvm) {
ret = libxl__domain_suspend_device_model(gc, dss);
if (ret) {
LOG(ERROR, "libxl__domain_suspend_device_model failed ret=%d", ret);
- goto err;
+ domain_suspend_common_failed(egc, dss);
+ return;
}
}
- dss->callback_common_done(egc, dss, 1);
+ 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)
+{
+ dss->callback_common_done(egc, dss, ok);
}
static inline char *physmap_path(libxl__gc *gc, uint32_t domid,
--
1.7.10.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 15/22] libxl: suspend: New libxl__domain_pvcontrol_xspath
2014-03-17 13:29 [PATCH v3 RESEND 00/22] libxl: asynchronous suspend Ian Jackson
` (13 preceding siblings ...)
2014-03-17 13:29 ` [PATCH 14/22] libxl: suspend: Reorg domain_suspend_callback_common Ian Jackson
@ 2014-03-17 13:29 ` Ian Jackson
2014-03-17 13:30 ` [PATCH 16/22] libxl: suspend: New domain_suspend_pvcontrol_acked Ian Jackson
` (7 subsequent siblings)
22 siblings, 0 replies; 29+ messages in thread
From: Ian Jackson @ 2014-03-17 13:29 UTC (permalink / raw)
To: xen-devel
Cc: Shriram Rajagopalan, Stefano Stabellini, Ian Jackson,
Ian Campbell, Lai Jiangshan
Factor out the pv control node xenstore path calculation into
libxl__domain_pvcontrol_xspath.
This xs path calculation was open coded in
libxl__domain_pvcontrol_read and _write. This is undesirable because
it duplicates the code and because it makes the path inaccessible to
other parts of libxl (which are soon going to want it).
No functional change.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
tools/libxl/libxl.c | 21 +++++++++++----------
tools/libxl/libxl_internal.h | 1 +
2 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 7b7ffd3..4ecdbbb 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -894,17 +894,23 @@ int libxl__domain_pvcontrol_available(libxl__gc *gc, uint32_t domid)
return !!pvdriver;
}
-char * libxl__domain_pvcontrol_read(libxl__gc *gc, xs_transaction_t t,
- uint32_t domid)
+const char *libxl__domain_pvcontrol_xspath(libxl__gc *gc, uint32_t domid)
{
- const char *shutdown_path;
const char *dom_path;
dom_path = libxl__xs_get_dompath(gc, domid);
if (!dom_path)
return NULL;
- shutdown_path = libxl__sprintf(gc, "%s/control/shutdown", dom_path);
+ return GCSPRINTF("%s/control/shutdown", dom_path);
+}
+
+char * libxl__domain_pvcontrol_read(libxl__gc *gc, xs_transaction_t t,
+ uint32_t domid)
+{
+ const char *shutdown_path;
+
+ shutdown_path = libxl__domain_pvcontrol_xspath(gc, domid);
if (!shutdown_path)
return NULL;
@@ -915,13 +921,8 @@ int libxl__domain_pvcontrol_write(libxl__gc *gc, xs_transaction_t t,
uint32_t domid, const char *cmd)
{
const char *shutdown_path;
- const char *dom_path;
-
- dom_path = libxl__xs_get_dompath(gc, domid);
- if (!dom_path)
- return ERROR_FAIL;
- shutdown_path = libxl__sprintf(gc, "%s/control/shutdown", dom_path);
+ shutdown_path = libxl__domain_pvcontrol_xspath(gc, domid);
if (!shutdown_path)
return ERROR_FAIL;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 923ad27..9ec0d0b 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -999,6 +999,7 @@ _hidden int libxl__domain_resume(libxl__gc *gc, uint32_t domid,
/* returns 0 or 1, or a libxl error code */
_hidden int libxl__domain_pvcontrol_available(libxl__gc *gc, uint32_t domid);
+_hidden const char *libxl__domain_pvcontrol_xspath(libxl__gc*, uint32_t domid);
_hidden char * libxl__domain_pvcontrol_read(libxl__gc *gc,
xs_transaction_t t, uint32_t domid);
_hidden int libxl__domain_pvcontrol_write(libxl__gc *gc, xs_transaction_t t,
--
1.7.10.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 16/22] libxl: suspend: New domain_suspend_pvcontrol_acked
2014-03-17 13:29 [PATCH v3 RESEND 00/22] libxl: asynchronous suspend Ian Jackson
` (14 preceding siblings ...)
2014-03-17 13:29 ` [PATCH 15/22] libxl: suspend: New libxl__domain_pvcontrol_xspath Ian Jackson
@ 2014-03-17 13:30 ` Ian Jackson
2014-03-17 13:30 ` [PATCH 17/22] libxl: suspend: domain_suspend_callback_common xs errs Ian Jackson
` (6 subsequent siblings)
22 siblings, 0 replies; 29+ messages in thread
From: Ian Jackson @ 2014-03-17 13:30 UTC (permalink / raw)
To: xen-devel
Cc: Shriram Rajagopalan, Stefano Stabellini, Ian Jackson,
Ian Campbell, Lai Jiangshan
Factor out domain_suspend_pvcontrol_acked.
This replaces a bunch of open-coded strcmp()s and makes the code
clearer. It also eliminates the need to check for state==NULL each
time it's read, because we can check for NULL once before the strcmp.
No functional change.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v3: Improve comment re xswatch state ENOENT
---
tools/libxl/libxl_dom.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index cef62cf..78f1de7 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1034,6 +1034,12 @@ static void domain_suspend_common_done(libxl__egc *egc,
libxl__domain_suspend_state *dss,
bool ok);
+static bool domain_suspend_pvcontrol_acked(const char *state) {
+ /* any value other than "suspend", including ENOENT (i.e. !state), is OK */
+ if (!state) return 1;
+ return strcmp(state,"suspend");
+}
+
/* calls dss->callback_common_done when done */
static void domain_suspend_callback_common(libxl__egc *egc,
libxl__domain_suspend_state *dss)
@@ -1091,11 +1097,10 @@ static void domain_suspend_callback_common(libxl__egc *egc,
LOG(DEBUG, "wait for the guest to acknowledge suspend request");
watchdog = 60;
- while (!strcmp(state, "suspend") && watchdog > 0) {
+ while (!domain_suspend_pvcontrol_acked(state) && watchdog > 0) {
usleep(100000);
state = libxl__domain_pvcontrol_read(gc, XBT_NULL, domid);
- if (!state) state = "";
watchdog--;
}
@@ -1109,21 +1114,19 @@ static void domain_suspend_callback_common(libxl__egc *egc,
* against the guest catching up and acknowledging the request
* at the last minute.
*/
- if (!strcmp(state, "suspend")) {
+ if (!domain_suspend_pvcontrol_acked(state)) {
LOG(ERROR, "guest didn't acknowledge suspend, cancelling request");
retry_transaction:
t = xs_transaction_start(CTX->xsh);
state = libxl__domain_pvcontrol_read(gc, t, domid);
- if (!state) state = "";
- if (!strcmp(state, "suspend"))
+ if (!domain_suspend_pvcontrol_acked(state))
libxl__domain_pvcontrol_write(gc, t, domid, "");
if (!xs_transaction_end(CTX->xsh, t, 0))
if (errno == EAGAIN)
goto retry_transaction;
-
}
/*
@@ -1131,7 +1134,7 @@ static void domain_suspend_callback_common(libxl__egc *egc,
* acknowledged while we were cancelling the request in which
* case we lost the race while cancelling and should continue.
*/
- if (!strcmp(state, "suspend")) {
+ if (!domain_suspend_pvcontrol_acked(state)) {
LOG(ERROR, "guest didn't acknowledge suspend, request cancelled");
goto err;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 17/22] libxl: suspend: domain_suspend_callback_common xs errs
2014-03-17 13:29 [PATCH v3 RESEND 00/22] libxl: asynchronous suspend Ian Jackson
` (15 preceding siblings ...)
2014-03-17 13:30 ` [PATCH 16/22] libxl: suspend: New domain_suspend_pvcontrol_acked Ian Jackson
@ 2014-03-17 13:30 ` Ian Jackson
2014-03-17 13:30 ` [PATCH 18/22] libxl: suspend: Async xenstore pvcontrol wait Ian Jackson
` (5 subsequent siblings)
22 siblings, 0 replies; 29+ messages in thread
From: Ian Jackson @ 2014-03-17 13:30 UTC (permalink / raw)
To: xen-devel
Cc: Shriram Rajagopalan, Stefano Stabellini, Ian Jackson,
Ian Campbell, Lai Jiangshan
In domain_suspend_callback_common, use libxl__xs_transaction_start in
a loop, rather than xs_transaction_start and a goto label.
This will improve the error handling, but have no other semantic
effect.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
tools/libxl/libxl_dom.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 78f1de7..9fec4e7 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1050,6 +1050,7 @@ static void domain_suspend_callback_common(libxl__egc *egc,
char *state = "suspend";
int watchdog;
xs_transaction_t t;
+ int rc;
/* Convenience aliases */
const uint32_t domid = dss->domid;
@@ -1116,17 +1117,19 @@ static void domain_suspend_callback_common(libxl__egc *egc,
*/
if (!domain_suspend_pvcontrol_acked(state)) {
LOG(ERROR, "guest didn't acknowledge suspend, cancelling request");
- retry_transaction:
- t = xs_transaction_start(CTX->xsh);
+ for (;;) {
+ rc = libxl__xs_transaction_start(gc, &t);
+ if (rc) goto err;
- state = libxl__domain_pvcontrol_read(gc, t, domid);
+ state = libxl__domain_pvcontrol_read(gc, t, domid);
- if (!domain_suspend_pvcontrol_acked(state))
- libxl__domain_pvcontrol_write(gc, t, domid, "");
+ if (!domain_suspend_pvcontrol_acked(state))
+ libxl__domain_pvcontrol_write(gc, t, domid, "");
- if (!xs_transaction_end(CTX->xsh, t, 0))
- if (errno == EAGAIN)
- goto retry_transaction;
+ rc = libxl__xs_transaction_commit(gc, &t);
+ if (!rc) break;
+ if (rc<0) goto err;
+ }
}
/*
--
1.7.10.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 18/22] libxl: suspend: Async xenstore pvcontrol wait
2014-03-17 13:29 [PATCH v3 RESEND 00/22] libxl: asynchronous suspend Ian Jackson
` (16 preceding siblings ...)
2014-03-17 13:30 ` [PATCH 17/22] libxl: suspend: domain_suspend_callback_common xs errs Ian Jackson
@ 2014-03-17 13:30 ` Ian Jackson
2014-03-17 13:39 ` Ian Campbell
2014-03-17 13:30 ` [PATCH 19/22] libxl: suspend: Abolish usleeps in domain suspend wait Ian Jackson
` (4 subsequent siblings)
22 siblings, 1 reply; 29+ messages in thread
From: Ian Jackson @ 2014-03-17 13:30 UTC (permalink / raw)
To: xen-devel
Cc: Shriram Rajagopalan, Stefano Stabellini, Ian Jackson,
Ian Campbell, Lai Jiangshan
When negotiating guest suspend via the xenstore pvcontrol protocol
(ie when the guest does NOT support the evtchn fast suspend protocol):
Replace the use of loops and usleep with a call to libxl__xswait.
Also, replace the xenstore transaction loop with one using
libxl__xs_transaction_start et al.
There is not intended to be any semantic change, other than to make
the algorithm properly asynchronous.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
CC: Ian Campbell <ian.campbell@citrix.com>
---
v3: Add a comment to clarify last minute ack.
v3X: Do NOT rename "pvcontrol" xswait state struct to "guest_wait"
(because we're NOT going to use it for the event channel based wait
too).
---
tools/libxl/libxl_dom.c | 94 ++++++++++++++++++++++++++----------------
tools/libxl/libxl_internal.h | 1 +
2 files changed, 60 insertions(+), 35 deletions(-)
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 9fec4e7..0c48159 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1028,6 +1028,8 @@ static void domain_suspend_common_wait_guest(libxl__egc *egc,
libxl__domain_suspend_state *dss);
static void domain_suspend_common_guest_suspended(libxl__egc *egc,
libxl__domain_suspend_state *dss);
+static void domain_suspend_common_pvcontrol_suspending(libxl__egc *egc,
+ libxl__xswait_state *xswa, int rc, const char *state);
static void domain_suspend_common_failed(libxl__egc *egc,
libxl__domain_suspend_state *dss);
static void domain_suspend_common_done(libxl__egc *egc,
@@ -1047,10 +1049,6 @@ static void domain_suspend_callback_common(libxl__egc *egc,
STATE_AO_GC(dss->ao);
unsigned long hvm_s_state = 0, hvm_pvdrv = 0;
int ret;
- char *state = "suspend";
- int watchdog;
- xs_transaction_t t;
- int rc;
/* Convenience aliases */
const uint32_t domid = dss->domid;
@@ -1096,59 +1094,82 @@ static void domain_suspend_callback_common(libxl__egc *egc,
libxl__domain_pvcontrol_write(gc, XBT_NULL, domid, "suspend");
- LOG(DEBUG, "wait for the guest to acknowledge suspend request");
- watchdog = 60;
- while (!domain_suspend_pvcontrol_acked(state) && watchdog > 0) {
- usleep(100000);
+ dss->pvcontrol.path = libxl__domain_pvcontrol_xspath(gc, domid);
+ if (!dss->pvcontrol.path) goto err;
- state = libxl__domain_pvcontrol_read(gc, XBT_NULL, domid);
+ dss->pvcontrol.ao = ao;
+ dss->pvcontrol.what = "guest acknowledgement of suspend request";
+ dss->pvcontrol.timeout_ms = 60 * 1000;
+ dss->pvcontrol.callback = domain_suspend_common_pvcontrol_suspending;
+ libxl__xswait_start(gc, &dss->pvcontrol);
+ return;
- watchdog--;
- }
+ err:
+ domain_suspend_common_failed(egc, dss);
+}
- /*
- * Guest appears to not be responding. Cancel the suspend
- * request.
- *
- * We re-read the suspend node and clear it within a
- * transaction in order to handle the case where we race
- * against the guest catching up and acknowledging the request
- * at the last minute.
- */
- if (!domain_suspend_pvcontrol_acked(state)) {
- LOG(ERROR, "guest didn't acknowledge suspend, cancelling request");
+static void domain_suspend_common_pvcontrol_suspending(libxl__egc *egc,
+ libxl__xswait_state *xswa, int rc, const char *state)
+{
+ libxl__domain_suspend_state *dss = CONTAINER_OF(xswa, *dss, pvcontrol);
+ STATE_AO_GC(dss->ao);
+ xs_transaction_t t = 0;
+
+ if (!rc && !domain_suspend_pvcontrol_acked(state))
+ /* keep waiting */
+ return;
+
+ libxl__xswait_stop(gc, &dss->pvcontrol);
+
+ if (rc == ERROR_TIMEDOUT) {
+ /*
+ * Guest appears to not be responding. Cancel the suspend
+ * request.
+ *
+ * We re-read the suspend node and clear it within a
+ * transaction in order to handle the case where we race
+ * against the guest catching up and acknowledging the request
+ * at the last minute.
+ */
for (;;) {
rc = libxl__xs_transaction_start(gc, &t);
if (rc) goto err;
- state = libxl__domain_pvcontrol_read(gc, t, domid);
+ rc = libxl__xs_read_checked(gc, t, xswa->path, &state);
+ if (rc) goto err;
+
+ if (domain_suspend_pvcontrol_acked(state))
+ /* last minute ack */
+ break;
- if (!domain_suspend_pvcontrol_acked(state))
- libxl__domain_pvcontrol_write(gc, t, domid, "");
+ rc = libxl__xs_write_checked(gc, t, xswa->path, "");
+ if (rc) goto err;
rc = libxl__xs_transaction_commit(gc, &t);
- if (!rc) break;
+ if (!rc) {
+ LOG(ERROR,
+ "guest didn't acknowledge suspend, cancelling request");
+ goto err;
+ }
if (rc<0) goto err;
}
- }
-
- /*
- * Final check for guest acknowledgement. The guest may have
- * acknowledged while we were cancelling the request in which
- * case we lost the race while cancelling and should continue.
- */
- if (!domain_suspend_pvcontrol_acked(state)) {
- LOG(ERROR, "guest didn't acknowledge suspend, request cancelled");
+ } else if (rc) {
+ /* some error in xswait's read of xenstore, already logged */
goto err;
}
+ assert(domain_suspend_pvcontrol_acked(state));
LOG(DEBUG, "guest acknowledged suspend request");
+
+ libxl__xs_transaction_abort(gc, &t);
dss->guest_responded = 1;
domain_suspend_common_wait_guest(egc,dss);
return;
err:
+ libxl__xs_transaction_abort(gc, &t);
domain_suspend_common_failed(egc, dss);
+ return;
}
static void domain_suspend_common_wait_guest(libxl__egc *egc,
@@ -1215,6 +1236,8 @@ static void domain_suspend_common_done(libxl__egc *egc,
libxl__domain_suspend_state *dss,
bool ok)
{
+ EGC_GC;
+ assert(!libxl__xswait_inuse(&dss->pvcontrol));
dss->callback_common_done(egc, dss, ok);
}
@@ -1400,6 +1423,7 @@ void libxl__domain_suspend(libxl__egc *egc, libxl__domain_suspend_state *dss)
&dss->shs.callbacks.save.a;
logdirty_init(&dss->logdirty);
+ libxl__xswait_init(&dss->pvcontrol);
switch (type) {
case LIBXL_DOMAIN_TYPE_HVM: {
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 9ec0d0b..479edb7 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2444,6 +2444,7 @@ struct libxl__domain_suspend_state {
int hvm;
int xcflags;
int guest_responded;
+ libxl__xswait_state pvcontrol;
const char *dm_savefile;
int interval; /* checkpoint interval (for Remus) */
libxl__save_helper_state shs;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 18/22] libxl: suspend: Async xenstore pvcontrol wait
2014-03-17 13:30 ` [PATCH 18/22] libxl: suspend: Async xenstore pvcontrol wait Ian Jackson
@ 2014-03-17 13:39 ` Ian Campbell
0 siblings, 0 replies; 29+ messages in thread
From: Ian Campbell @ 2014-03-17 13:39 UTC (permalink / raw)
To: Ian Jackson
Cc: Shriram Rajagopalan, xen-devel, Lai Jiangshan, Stefano Stabellini
On Mon, 2014-03-17 at 13:30 +0000, Ian Jackson wrote:
> When negotiating guest suspend via the xenstore pvcontrol protocol
> (ie when the guest does NOT support the evtchn fast suspend protocol):
>
> Replace the use of loops and usleep with a call to libxl__xswait.
>
> Also, replace the xenstore transaction loop with one using
> libxl__xs_transaction_start et al.
>
> There is not intended to be any semantic change, other than to make
> the algorithm properly asynchronous.
>
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 19/22] libxl: suspend: Abolish usleeps in domain suspend wait
2014-03-17 13:29 [PATCH v3 RESEND 00/22] libxl: asynchronous suspend Ian Jackson
` (17 preceding siblings ...)
2014-03-17 13:30 ` [PATCH 18/22] libxl: suspend: Async xenstore pvcontrol wait Ian Jackson
@ 2014-03-17 13:30 ` Ian Jackson
2014-03-17 13:39 ` Ian Campbell
2014-03-17 13:30 ` [PATCH 20/22] libxl: suspend: Fix suspend wait corner cases Ian Jackson
` (3 subsequent siblings)
22 siblings, 1 reply; 29+ messages in thread
From: Ian Jackson @ 2014-03-17 13:30 UTC (permalink / raw)
To: xen-devel
Cc: Shriram Rajagopalan, Stefano Stabellini, Ian Jackson,
Ian Campbell, Lai Jiangshan
Replace the use of a loop with usleep().
Instead, use a xenstore watch and an event system timeout. (xenstore
fires watches on @releaseDomain when a domain shuts down.)
The logic which checks for the state of the domain is unchanged, and
not ideal, but we will leave that for the next patch.
There is not intended to be any semantic change, other than to make
the algorithm properly asynchronous and the consequential waiting be
on xenstore, rather than polling.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
CC: Ian Campbell <ian.campbell@citrix.com>
---
v3: Remove some trailing whitespace
Improve commit message.
v3X: Do NOT use an xswait instead of separate watch and timeout.
---
tools/libxl/libxl_dom.c | 80 ++++++++++++++++++++++++++++++------------
tools/libxl/libxl_internal.h | 2 ++
2 files changed, 60 insertions(+), 22 deletions(-)
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 0c48159..5d6dcc4 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1028,8 +1028,14 @@ static void domain_suspend_common_wait_guest(libxl__egc *egc,
libxl__domain_suspend_state *dss);
static void domain_suspend_common_guest_suspended(libxl__egc *egc,
libxl__domain_suspend_state *dss);
+
static void domain_suspend_common_pvcontrol_suspending(libxl__egc *egc,
libxl__xswait_state *xswa, int rc, const char *state);
+static void suspend_common_wait_guest_watch(libxl__egc *egc,
+ libxl__ev_xswatch *xsw, const char *watch_path, const char *event_path);
+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,
@@ -1176,36 +1182,59 @@ static void domain_suspend_common_wait_guest(libxl__egc *egc,
libxl__domain_suspend_state *dss)
{
STATE_AO_GC(dss->ao);
+ int rc;
+
+ LOG(DEBUG, "wait for the guest to suspend");
+
+ rc = libxl__ev_xswatch_register(gc, &dss->guest_watch,
+ suspend_common_wait_guest_watch,
+ "@releaseDomain");
+ if (rc) goto err;
+
+ rc = libxl__ev_time_register_rel(gc, &dss->guest_timeout,
+ suspend_common_wait_guest_timeout,
+ 60*1000);
+ if (rc) goto err;
+ return;
+
+ err:
+ domain_suspend_common_failed(egc, dss);
+}
+
+static void suspend_common_wait_guest_watch(libxl__egc *egc,
+ libxl__ev_xswatch *xsw, const char *watch_path, const char *event_path)
+{
+ libxl__domain_suspend_state *dss =
+ CONTAINER_OF(xsw, *dss, guest_watch);
+ STATE_AO_GC(dss->ao);
+ xc_domaininfo_t info;
int ret;
- int watchdog;
/* Convenience aliases */
const uint32_t domid = dss->domid;
- LOG(DEBUG, "wait for the guest to suspend");
- watchdog = 60;
- while (watchdog > 0) {
- xc_domaininfo_t info;
-
- usleep(100000);
- ret = xc_domain_getinfolist(CTX->xch, domid, 1, &info);
- if (ret == 1 && info.domain == domid &&
- (info.flags & XEN_DOMINF_shutdown)) {
- int shutdown_reason;
-
- shutdown_reason = (info.flags >> XEN_DOMINF_shutdownshift)
- & XEN_DOMINF_shutdownmask;
- if (shutdown_reason == SHUTDOWN_suspend) {
- LOG(DEBUG, "guest has suspended");
- domain_suspend_common_guest_suspended(egc, dss);
- return;
- }
+ ret = xc_domain_getinfolist(CTX->xch, domid, 1, &info);
+ if (ret == 1 && info.domain == domid &&
+ (info.flags & XEN_DOMINF_shutdown)) {
+ int shutdown_reason;
+
+ shutdown_reason = (info.flags >> XEN_DOMINF_shutdownshift)
+ & XEN_DOMINF_shutdownmask;
+ if (shutdown_reason == SHUTDOWN_suspend) {
+ LOG(DEBUG, "guest has suspended");
+ domain_suspend_common_guest_suspended(egc, dss);
+ return;
}
-
- watchdog--;
}
+ /* otherwise, keep waiting */
+}
- LOG(ERROR, "guest did not suspend");
+static void suspend_common_wait_guest_timeout(libxl__egc *egc,
+ libxl__ev_time *ev, const struct timeval *requested_abs)
+{
+ 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);
}
@@ -1215,6 +1244,9 @@ static void domain_suspend_common_guest_suspended(libxl__egc *egc,
STATE_AO_GC(dss->ao);
int ret;
+ 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) {
@@ -1238,6 +1270,8 @@ static void domain_suspend_common_done(libxl__egc *egc,
{
EGC_GC;
assert(!libxl__xswait_inuse(&dss->pvcontrol));
+ libxl__ev_xswatch_deregister(gc, &dss->guest_watch);
+ libxl__ev_time_deregister(gc, &dss->guest_timeout);
dss->callback_common_done(egc, dss, ok);
}
@@ -1424,6 +1458,8 @@ void libxl__domain_suspend(libxl__egc *egc, libxl__domain_suspend_state *dss)
logdirty_init(&dss->logdirty);
libxl__xswait_init(&dss->pvcontrol);
+ libxl__ev_xswatch_init(&dss->guest_watch);
+ libxl__ev_time_init(&dss->guest_timeout);
switch (type) {
case LIBXL_DOMAIN_TYPE_HVM: {
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 479edb7..a67ea3c 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2445,6 +2445,8 @@ struct libxl__domain_suspend_state {
int xcflags;
int guest_responded;
libxl__xswait_state pvcontrol;
+ libxl__ev_xswatch guest_watch;
+ libxl__ev_time guest_timeout;
const char *dm_savefile;
int interval; /* checkpoint interval (for Remus) */
libxl__save_helper_state shs;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 19/22] libxl: suspend: Abolish usleeps in domain suspend wait
2014-03-17 13:30 ` [PATCH 19/22] libxl: suspend: Abolish usleeps in domain suspend wait Ian Jackson
@ 2014-03-17 13:39 ` Ian Campbell
0 siblings, 0 replies; 29+ messages in thread
From: Ian Campbell @ 2014-03-17 13:39 UTC (permalink / raw)
To: Ian Jackson
Cc: Shriram Rajagopalan, xen-devel, Lai Jiangshan, Stefano Stabellini
On Mon, 2014-03-17 at 13:30 +0000, Ian Jackson wrote:
> Replace the use of a loop with usleep().
>
> Instead, use a xenstore watch and an event system timeout. (xenstore
> fires watches on @releaseDomain when a domain shuts down.)
>
> The logic which checks for the state of the domain is unchanged, and
> not ideal, but we will leave that for the next patch.
>
> There is not intended to be any semantic change, other than to make
> the algorithm properly asynchronous and the consequential waiting be
> on xenstore, rather than polling.
>
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 20/22] libxl: suspend: Fix suspend wait corner cases
2014-03-17 13:29 [PATCH v3 RESEND 00/22] libxl: asynchronous suspend Ian Jackson
` (18 preceding siblings ...)
2014-03-17 13:30 ` [PATCH 19/22] libxl: suspend: Abolish usleeps in domain suspend wait Ian Jackson
@ 2014-03-17 13:30 ` Ian Jackson
2014-03-17 13:30 ` [PATCH 21/22] libxl: suspend: Async evtchn wait Ian Jackson
` (2 subsequent siblings)
22 siblings, 0 replies; 29+ messages in thread
From: Ian Jackson @ 2014-03-17 13:30 UTC (permalink / raw)
To: xen-devel
Cc: Shriram Rajagopalan, Stefano Stabellini, Ian Jackson,
Ian Campbell, Lai Jiangshan
When we are waiting for a guest to suspend, this suspend operation
would continue to wait (until the timeout) if the guest was destroyed
or shut down for another reason, or if xc_domain_getinfolist failed.
Handle these cases correctly, as errors.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v3: Remove unwanted error call after a new "goto err".
---
tools/libxl/libxl_dom.c | 41 +++++++++++++++++++++++++++++------------
1 file changed, 29 insertions(+), 12 deletions(-)
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 5d6dcc4..28855a1 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1209,24 +1209,41 @@ static void suspend_common_wait_guest_watch(libxl__egc *egc,
STATE_AO_GC(dss->ao);
xc_domaininfo_t info;
int ret;
+ int shutdown_reason;
/* Convenience aliases */
const uint32_t domid = dss->domid;
ret = xc_domain_getinfolist(CTX->xch, domid, 1, &info);
- if (ret == 1 && info.domain == domid &&
- (info.flags & XEN_DOMINF_shutdown)) {
- int shutdown_reason;
-
- shutdown_reason = (info.flags >> XEN_DOMINF_shutdownshift)
- & XEN_DOMINF_shutdownmask;
- if (shutdown_reason == SHUTDOWN_suspend) {
- LOG(DEBUG, "guest has suspended");
- domain_suspend_common_guest_suspended(egc, dss);
- return;
- }
+ if (ret < 0) {
+ LOGE(ERROR, "unable to check for status of guest %"PRId32"", domid);
+ goto err;
+ }
+
+ if (!(ret == 1 && info.domain == domid)) {
+ LOGE(ERROR, "guest %"PRId32" we were suspending has been destroyed",
+ domid);
+ goto err;
+ }
+
+ if (!(info.flags & XEN_DOMINF_shutdown))
+ /* keep waiting */
+ return;
+
+ shutdown_reason = (info.flags >> XEN_DOMINF_shutdownshift)
+ & XEN_DOMINF_shutdownmask;
+ if (shutdown_reason != SHUTDOWN_suspend) {
+ LOG(DEBUG, "guest %"PRId32" we were suspending has shut down"
+ " with unexpected reason code %d", domid, shutdown_reason);
+ goto err;
}
- /* otherwise, keep waiting */
+
+ LOG(DEBUG, "guest has suspended");
+ domain_suspend_common_guest_suspended(egc, dss);
+ return;
+
+ err:
+ domain_suspend_common_failed(egc, dss);
}
static void suspend_common_wait_guest_timeout(libxl__egc *egc,
--
1.7.10.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 21/22] libxl: suspend: Async evtchn wait
2014-03-17 13:29 [PATCH v3 RESEND 00/22] libxl: asynchronous suspend Ian Jackson
` (19 preceding siblings ...)
2014-03-17 13:30 ` [PATCH 20/22] libxl: suspend: Fix suspend wait corner cases Ian Jackson
@ 2014-03-17 13:30 ` Ian Jackson
2014-03-17 13:30 ` [PATCH 22/22] libxl: suspend: Apply guest timeout in evtchn case Ian Jackson
2014-03-17 15:55 ` [PATCH v3 RESEND 00/22] libxl: asynchronous suspend Ian Jackson
22 siblings, 0 replies; 29+ messages in thread
From: Ian Jackson @ 2014-03-17 13:30 UTC (permalink / raw)
To: xen-devel
Cc: Shriram Rajagopalan, Stefano Stabellini, Ian Jackson,
Ian Campbell, Lai Jiangshan
When negotiating guest suspend via the evtchn ("fast") protocol,
abolish synchronous wait for domain suspend.
If the guest supports the event channel suspend protocol, we used to
sit in a loop in xc_await_suspend waiting (perhaps indefinitely) for
it to suspend.
Instead, use the new libxl event channel event facility. When we see
that the event is signaled, we look at the domain to see if it has
suspended. (In this patch we do not yet set a timeout; that will come
next.)
So the suspend operation no longer blocks with the libxl ctx lock
held, and instead returns to the event loop. Additionally, domains
which signal the event channel themselves, or undergo other state
changes, will be handled more correctly.
We end up making a few more hypercalls.
Also, if we encounter errors setting up the suspend event channel
(which should not happen), abort the operation rather than falling
back to the xenstore protocol.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v3: Improve commit message.
---
tools/libxl/libxl_dom.c | 78 ++++++++++++++++++++++++++----------------
tools/libxl/libxl_internal.h | 3 +-
2 files changed, 50 insertions(+), 31 deletions(-)
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 28855a1..87c994c 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1031,8 +1031,12 @@ static void domain_suspend_common_guest_suspended(libxl__egc *egc,
static void domain_suspend_common_pvcontrol_suspending(libxl__egc *egc,
libxl__xswait_state *xswa, int rc, const char *state);
+static void domain_suspend_common_wait_guest_evtchn(libxl__egc *egc,
+ libxl__ev_evtchn *evev);
static void suspend_common_wait_guest_watch(libxl__egc *egc,
libxl__ev_xswatch *xsw, const char *watch_path, const char *event_path);
+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);
@@ -1054,7 +1058,7 @@ static void domain_suspend_callback_common(libxl__egc *egc,
{
STATE_AO_GC(dss->ao);
unsigned long hvm_s_state = 0, hvm_pvdrv = 0;
- int ret;
+ int ret, rc;
/* Convenience aliases */
const uint32_t domid = dss->domid;
@@ -1064,21 +1068,19 @@ static void domain_suspend_callback_common(libxl__egc *egc,
xc_get_hvm_param(CTX->xch, domid, HVM_PARAM_ACPI_S_STATE, &hvm_s_state);
}
- if ((hvm_s_state == 0) && (dss->suspend_eventchn >= 0)) {
+ if ((hvm_s_state == 0) && (dss->guest_evtchn.port >= 0)) {
LOG(DEBUG, "issuing %s suspend request via event channel",
dss->hvm ? "PVHVM" : "PV");
- ret = xc_evtchn_notify(dss->xce, dss->suspend_eventchn);
+ ret = xc_evtchn_notify(CTX->xce, dss->guest_evtchn.port);
if (ret < 0) {
LOG(ERROR, "xc_evtchn_notify failed ret=%d", ret);
goto err;
}
- ret = xc_await_suspend(CTX->xch, dss->xce, dss->suspend_eventchn);
- if (ret < 0) {
- LOG(ERROR, "xc_await_suspend failed ret=%d", ret);
- goto err;
- }
- dss->guest_responded = 1;
- domain_suspend_common_guest_suspended(egc, dss);
+
+ dss->guest_evtchn.callback = domain_suspend_common_wait_guest_evtchn;
+ rc = libxl__ev_evtchn_wait(gc, &dss->guest_evtchn);
+ if (rc) goto err;
+
return;
}
@@ -1114,6 +1116,19 @@ static void domain_suspend_callback_common(libxl__egc *egc,
domain_suspend_common_failed(egc, dss);
}
+static void domain_suspend_common_wait_guest_evtchn(libxl__egc *egc,
+ libxl__ev_evtchn *evev)
+{
+ libxl__domain_suspend_state *dss = CONTAINER_OF(evev, *dss, guest_evtchn);
+ 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. */
+ libxl__ev_evtchn_wait(gc, &dss->guest_evtchn);
+ suspend_common_wait_guest_check(egc, dss);
+}
+
static void domain_suspend_common_pvcontrol_suspending(libxl__egc *egc,
libxl__xswait_state *xswa, int rc, const char *state)
{
@@ -1204,8 +1219,13 @@ static void domain_suspend_common_wait_guest(libxl__egc *egc,
static void suspend_common_wait_guest_watch(libxl__egc *egc,
libxl__ev_xswatch *xsw, const char *watch_path, const char *event_path)
{
- libxl__domain_suspend_state *dss =
- CONTAINER_OF(xsw, *dss, guest_watch);
+ libxl__domain_suspend_state *dss = CONTAINER_OF(xsw, *dss, guest_watch);
+ suspend_common_wait_guest_check(egc, dss);
+}
+
+static void suspend_common_wait_guest_check(libxl__egc *egc,
+ libxl__domain_suspend_state *dss)
+{
STATE_AO_GC(dss->ao);
xc_domaininfo_t info;
int ret;
@@ -1261,6 +1281,7 @@ static void domain_suspend_common_guest_suspended(libxl__egc *egc,
STATE_AO_GC(dss->ao);
int ret;
+ libxl__ev_evtchn_cancel(gc, &dss->guest_evtchn);
libxl__ev_xswatch_deregister(gc, &dss->guest_watch);
libxl__ev_time_deregister(gc, &dss->guest_timeout);
@@ -1287,6 +1308,7 @@ static void domain_suspend_common_done(libxl__egc *egc,
{
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);
@@ -1475,6 +1497,7 @@ void libxl__domain_suspend(libxl__egc *egc, libxl__domain_suspend_state *dss)
logdirty_init(&dss->logdirty);
libxl__xswait_init(&dss->pvcontrol);
+ libxl__ev_evtchn_init(&dss->guest_evtchn);
libxl__ev_xswatch_init(&dss->guest_watch);
libxl__ev_time_init(&dss->guest_timeout);
@@ -1503,7 +1526,7 @@ void libxl__domain_suspend(libxl__egc *egc, libxl__domain_suspend_state *dss)
| (debug ? XCFLAGS_DEBUG : 0)
| (dss->hvm ? XCFLAGS_HVM : 0);
- dss->suspend_eventchn = -1;
+ dss->guest_evtchn.port = -1;
dss->guest_evtchn_lockfd = -1;
dss->guest_responded = 0;
dss->dm_savefile = libxl__device_model_savefile(gc, domid);
@@ -1514,20 +1537,17 @@ void libxl__domain_suspend(libxl__egc *egc, libxl__domain_suspend_state *dss)
dss->xcflags |= XCFLAGS_CHECKPOINT_COMPRESS;
}
- dss->xce = xc_evtchn_open(NULL, 0);
- if (dss->xce == NULL)
- goto out;
- else
- {
- port = xs_suspend_evtchn_port(dss->domid);
+ port = xs_suspend_evtchn_port(dss->domid);
- if (port >= 0) {
- dss->suspend_eventchn =
- xc_suspend_evtchn_init_exclusive(CTX->xch, dss->xce,
+ if (port >= 0) {
+ dss->guest_evtchn.port =
+ xc_suspend_evtchn_init_exclusive(CTX->xch, CTX->xce,
dss->domid, port, &dss->guest_evtchn_lockfd);
- if (dss->suspend_eventchn < 0)
- LOG(WARN, "Suspend event channel initialization failed");
+ if (dss->guest_evtchn.port < 0) {
+ LOG(WARN, "Suspend event channel initialization failed");
+ rc = ERROR_FAIL;
+ goto out;
}
}
@@ -1686,11 +1706,11 @@ static void domain_suspend_done(libxl__egc *egc,
/* Convenience aliases */
const uint32_t domid = dss->domid;
- if (dss->suspend_eventchn > 0)
- xc_suspend_evtchn_release(CTX->xch, dss->xce, domid,
- dss->suspend_eventchn, &dss->guest_evtchn_lockfd);
- if (dss->xce != NULL)
- xc_evtchn_close(dss->xce);
+ libxl__ev_evtchn_cancel(gc, &dss->guest_evtchn);
+
+ if (dss->guest_evtchn.port > 0)
+ xc_suspend_evtchn_release(CTX->xch, CTX->xce, domid,
+ dss->guest_evtchn.port, &dss->guest_evtchn_lockfd);
dss->callback(egc, dss, rc);
}
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index a67ea3c..69d2ba8 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2438,8 +2438,7 @@ struct libxl__domain_suspend_state {
int debug;
const libxl_domain_remus_info *remus;
/* private */
- xc_evtchn *xce; /* event channel handle */
- int suspend_eventchn;
+ libxl__ev_evtchn guest_evtchn;
int guest_evtchn_lockfd;
int hvm;
int xcflags;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 22/22] libxl: suspend: Apply guest timeout in evtchn case
2014-03-17 13:29 [PATCH v3 RESEND 00/22] libxl: asynchronous suspend Ian Jackson
` (20 preceding siblings ...)
2014-03-17 13:30 ` [PATCH 21/22] libxl: suspend: Async evtchn wait Ian Jackson
@ 2014-03-17 13:30 ` Ian Jackson
2014-03-17 15:55 ` [PATCH v3 RESEND 00/22] libxl: asynchronous suspend Ian Jackson
22 siblings, 0 replies; 29+ messages in thread
From: Ian Jackson @ 2014-03-17 13:30 UTC (permalink / raw)
To: xen-devel
Cc: Shriram Rajagopalan, Stefano Stabellini, Ian Jackson,
Ian Campbell, Lai Jiangshan
When negotiating guest suspend via the evtchn ("fast") protocol,
the guest may still fail to respond.
So set the timeout. The existing error path will already properly
tear down our (event channel) wait.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
tools/libxl/libxl_dom.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 87c994c..36e70b5 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1081,6 +1081,11 @@ 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,
+ suspend_common_wait_guest_timeout,
+ 60*1000);
+ if (rc) goto err;
+
return;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v3 RESEND 00/22] libxl: asynchronous suspend
2014-03-17 13:29 [PATCH v3 RESEND 00/22] libxl: asynchronous suspend Ian Jackson
` (21 preceding siblings ...)
2014-03-17 13:30 ` [PATCH 22/22] libxl: suspend: Apply guest timeout in evtchn case Ian Jackson
@ 2014-03-17 15:55 ` Ian Jackson
22 siblings, 0 replies; 29+ messages in thread
From: Ian Jackson @ 2014-03-17 15:55 UTC (permalink / raw)
To: xen-devel, Ian Campbell, Lai Jiangshan, Shriram Rajagopalan
Ian Jackson writes ("[PATCH v3 RESEND 00/22] libxl: asynchronous suspend"):
> I think this version is probably ready to go in. It's lacking a
> handful of hopefully-easy acks.
Thanks, I have pushed this series.
Ian.
^ permalink raw reply [flat|nested] 29+ messages in thread