Ian Jackson
2012-May-30 16:16 UTC
[PATCH v2 00/15] libxl: domain save/restore: run in a separate process
This is v2 of my series to asyncify save/restore. It appears to work for me. Patches 00-05 were posted before. 06-15 are new. Preparatory work: 01/15 libxc: xc_domain_restore, make toolstack_restore const-correct 02/15 libxl: domain save: rename variables etc. 03/15 libxl: domain restore: reshuffle, preparing for ao 04/15 libxl: domain save: API changes for asynchrony The meat: 05/15 libxl: domain save/restore: run in a separate process Some fixups: 06/15 libxl: rename libxl_dom:save_helper to physmap_path 07/15 libxl: provide libxl__xs_*_checked and libxl__xs_transaction_* 08/15 libxl: wait for qemu to acknowledge logdirty command Asyncify writing of qemu save file, too: 09/15 libxl: datacopier: provide "prefix data" facilit 10/15 libxl: prepare for asynchronous writing of qemu save file 11/15 libxl: Make libxl__domain_save_device_model asynchronous Unrelated bugfixes found during testing: 12/15 xl: Handle return value from libxl_domain_suspend correctly 13/15 libxl: do not leak dms->saved_state 14/15 libxl: do not leak spawned middle children 15/15 libxl: do not leak an event struct on ignored ao progress Thanks, Ian.
Ian Jackson
2012-May-30 16:16 UTC
[PATCH 01/15] libxc: xc_domain_restore, make toolstack_restore const-correct
Also provide typedefs for the nontrivial function callback types.
Update the one provider of this callback, in libxl.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
tools/libxc/xenguest.h | 14 ++++++++++----
tools/libxl/libxl_dom.c | 4 ++--
2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/tools/libxc/xenguest.h b/tools/libxc/xenguest.h
index 91d53f7..328a2a5 100644
--- a/tools/libxc/xenguest.h
+++ b/tools/libxc/xenguest.h
@@ -31,6 +31,10 @@
#define X86_64_B_SIZE 64
#define X86_32_B_SIZE 32
+typedef int xc_switch_qemu_logdirty_cb(int domid, unsigned enable, void *data);
+typedef int xc_toolstack_save_cb(uint32_t domid, uint8_t **buf,
+ uint32_t *len, void *data);
+
/* callbacks provided by xc_domain_save */
struct save_callbacks {
/* Called after expiration of checkpoint interval,
@@ -61,7 +65,7 @@ struct save_callbacks {
int (*checkpoint)(void* data);
/* Enable qemu-dm logging dirty pages to xen */
- int (*switch_qemu_logdirty)(int domid, unsigned enable, void *data); /* HVM
only */
+ xc_switch_qemu_logdirty_cb *switch_qemu_logdirty; /* HVM only */
/* Save toolstack specific data
* @param buf the buffer with the data to be saved
@@ -69,7 +73,7 @@ struct save_callbacks {
* The callee allocates the buffer, the caller frees it (buffer must
* be free''able).
*/
- int (*toolstack_save)(uint32_t domid, uint8_t **buf, uint32_t *len, void
*data);
+ xc_toolstack_save_cb *toolstack_save;
/* to be provided as the last argument to each callback function */
void* data;
@@ -89,11 +93,13 @@ int xc_domain_save(xc_interface *xch, int io_fd, uint32_t
dom, uint32_t max_iter
unsigned long vm_generationid_addr);
+typedef int xc_toolstack_restore_cb(uint32_t domid, const uint8_t *buf,
+ uint32_t size, void* data);
+
/* callbacks provided by xc_domain_restore */
struct restore_callbacks {
/* callback to restore toolstack specific data */
- int (*toolstack_restore)(uint32_t domid, uint8_t *buf,
- uint32_t size, void* data);
+ xc_toolstack_restore_cb *toolstack_restore;
/* to be provided as the last argument to each callback function */
void* data;
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 06dbc92..929fbf2 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -465,13 +465,13 @@ static inline char *restore_helper(libxl__gc *gc, uint32_t
domid,
domid, phys_offset, node);
}
-static int libxl__toolstack_restore(uint32_t domid, uint8_t *buf,
+static int libxl__toolstack_restore(uint32_t domid, const uint8_t *buf,
uint32_t size, void *data)
{
libxl__gc *gc = (libxl__gc *) data;
libxl_ctx *ctx = gc->owner;
int i, ret;
- uint8_t *ptr = buf;
+ const uint8_t *ptr = buf;
uint32_t count = 0, version = 0;
struct libxl__physmap_info* pi;
char *xs_path;
--
1.7.2.5
Preparatory work for making domain suspend asynchronous:
* Rename `struct suspendinfo'' to `libxl__domain_suspend_state''
and move it to libxl_internal.h.
* Rename variables `si'' to `dss''.
* Change the stack-allocated state and callbacks from
struct suspendinfo si;
struct save_callbacks callbacks;
struct restore_callbacks callbacks;
to
libxl__domain_suspend_state dss[1];
struct save_callbacks callbacks[1];
struct restore_callbacks callbacks[1];
so that it may be referred to as a pointer variable everywhere.
* Rename the variable `flags'' (in libxl__domain_suspend_common
and in libxl__domain_suspend_state) to `xcflags'', to help
distinguish it from the other `flags'' which is passed in
from the calling application in libxl_domain_suspend_info.
* Move the prototypes of suspend-related functions in libxl_internal.h
to after the definition of the state struct.
* Replace several ctx variables with gc variables and
consequently references to ctx with CTX. Change references
to `dss->gc'' in the functional code to simply `gc''.
* Use LOG* rather than LIBXL__LOG* in a number of places.
* In libxl__domain_save_device_model use `rc'' instead of
`ret''.
* Introduce and use `gc'' and `domid'' in
libxl__domain_suspend_common_callback.
* Wrap some long lines.
* Add an extra pair of parens for clarity in a flag test.
* Remove two pointless casts from void* to a struct*.
No functional change whatsoever.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Change in v2:
* Make callbacks into arrays (for pointerisation) too.
* Updated to cope with new remus code.
* Fixed typo in commit message.
---
tools/libxl/libxl_dom.c | 254 ++++++++++++++++++++----------------------
tools/libxl/libxl_internal.h | 30 +++++-
2 files changed, 149 insertions(+), 135 deletions(-)
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 929fbf2..231ca40 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -468,7 +468,7 @@ static inline char *restore_helper(libxl__gc *gc, uint32_t
domid,
static int libxl__toolstack_restore(uint32_t domid, const uint8_t *buf,
uint32_t size, void *data)
{
- libxl__gc *gc = (libxl__gc *) data;
+ libxl__gc *gc = data;
libxl_ctx *ctx = gc->owner;
int i, ret;
const uint8_t *ptr = buf;
@@ -529,7 +529,7 @@ int libxl__domain_restore_common(libxl__gc *gc, uint32_t
domid,
/* read signature */
int rc;
int hvm, pae, superpages;
- struct restore_callbacks callbacks;
+ struct restore_callbacks callbacks[1];
int no_incr_generationid;
switch (info->type) {
case LIBXL_DOMAIN_TYPE_HVM:
@@ -537,8 +537,8 @@ int libxl__domain_restore_common(libxl__gc *gc, uint32_t
domid,
superpages = 1;
pae = libxl_defbool_val(info->u.hvm.pae);
no_incr_generationid =
!libxl_defbool_val(info->u.hvm.incr_generationid);
- callbacks.toolstack_restore = libxl__toolstack_restore;
- callbacks.data = gc;
+ callbacks->toolstack_restore = libxl__toolstack_restore;
+ callbacks->data = gc;
break;
case LIBXL_DOMAIN_TYPE_PV:
hvm = 0;
@@ -554,7 +554,7 @@ int libxl__domain_restore_common(libxl__gc *gc, uint32_t
domid,
state->store_domid, state->console_port,
&state->console_mfn, state->console_domid,
hvm, pae, superpages, no_incr_generationid,
- &state->vm_generationid_addr,
&callbacks);
+ &state->vm_generationid_addr, callbacks);
if ( rc ) {
LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "restoring domain");
return ERROR_FAIL;
@@ -562,33 +562,23 @@ int libxl__domain_restore_common(libxl__gc *gc, uint32_t
domid,
return 0;
}
-struct suspendinfo {
- libxl__gc *gc;
- xc_evtchn *xce; /* event channel handle */
- int suspend_eventchn;
- int domid;
- int hvm;
- unsigned int flags;
- int guest_responded;
- int save_fd; /* Migration stream fd (for Remus) */
- int interval; /* checkpoint interval (for Remus) */
-};
-
-static int libxl__domain_suspend_common_switch_qemu_logdirty(int domid,
unsigned int enable, void *data)
+static int libxl__domain_suspend_common_switch_qemu_logdirty
+ (int domid, unsigned int enable, void *data)
{
- struct suspendinfo *si = data;
- libxl_ctx *ctx = libxl__gc_owner(si->gc);
+ libxl__domain_suspend_state *dss = data;
+ libxl__gc *gc = dss->gc;
char *path;
bool rc;
- path = libxl__sprintf(si->gc,
"/local/domain/0/device-model/%u/logdirty/cmd", domid);
+ path = libxl__sprintf(gc,
+ "/local/domain/0/device-model/%u/logdirty/cmd",
domid);
if (!path)
return 1;
if (enable)
- rc = xs_write(ctx->xsh, XBT_NULL, path, "enable",
strlen("enable"));
+ rc = xs_write(CTX->xsh, XBT_NULL, path, "enable",
strlen("enable"));
else
- rc = xs_write(ctx->xsh, XBT_NULL, path, "disable",
strlen("disable"));
+ rc = xs_write(CTX->xsh, XBT_NULL, path, "disable",
strlen("disable"));
return rc ? 0 : 1;
}
@@ -643,53 +633,56 @@ int libxl__domain_resume_device_model(libxl__gc *gc,
uint32_t domid)
static int libxl__domain_suspend_common_callback(void *data)
{
- struct suspendinfo *si = data;
+ libxl__domain_suspend_state *dss = data;
+ libxl__gc *gc = dss->gc;
unsigned long hvm_s_state = 0, hvm_pvdrv = 0;
int ret;
char *state = "suspend";
int watchdog;
- libxl_ctx *ctx = libxl__gc_owner(si->gc);
xs_transaction_t t;
- if (si->hvm) {
- xc_get_hvm_param(ctx->xch, si->domid, HVM_PARAM_CALLBACK_IRQ,
&hvm_pvdrv);
- xc_get_hvm_param(ctx->xch, si->domid, HVM_PARAM_ACPI_S_STATE,
&hvm_s_state);
+ /* Convenience aliases */
+ const uint32_t domid = dss->domid;
+
+ if (dss->hvm) {
+ xc_get_hvm_param(CTX->xch, domid, HVM_PARAM_CALLBACK_IRQ,
&hvm_pvdrv);
+ xc_get_hvm_param(CTX->xch, domid, HVM_PARAM_ACPI_S_STATE,
&hvm_s_state);
}
- if ((hvm_s_state == 0) && (si->suspend_eventchn >= 0)) {
- LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "issuing %s suspend request via
event channel",
- si->hvm ? "PVHVM" : "PV");
- ret = xc_evtchn_notify(si->xce, si->suspend_eventchn);
+ if ((hvm_s_state == 0) && (dss->suspend_eventchn >= 0)) {
+ LOG(DEBUG, "issuing %s suspend request via event channel",
+ dss->hvm ? "PVHVM" : "PV");
+ ret = xc_evtchn_notify(dss->xce, dss->suspend_eventchn);
if (ret < 0) {
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "xc_evtchn_notify failed
ret=%d", ret);
+ LOG(ERROR, "xc_evtchn_notify failed ret=%d", ret);
return 0;
}
- ret = xc_await_suspend(ctx->xch, si->xce,
si->suspend_eventchn);
+ ret = xc_await_suspend(CTX->xch, dss->xce,
dss->suspend_eventchn);
if (ret < 0) {
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "xc_await_suspend failed
ret=%d", ret);
+ LOG(ERROR, "xc_await_suspend failed ret=%d", ret);
return 0;
}
- si->guest_responded = 1;
+ dss->guest_responded = 1;
goto guest_suspended;
}
- if (si->hvm && (!hvm_pvdrv || hvm_s_state)) {
- LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Calling xc_domain_shutdown on
HVM domain");
- xc_domain_shutdown(ctx->xch, si->domid, SHUTDOWN_suspend);
+ if (dss->hvm && (!hvm_pvdrv || hvm_s_state)) {
+ LOG(DEBUG, "Calling xc_domain_shutdown on HVM domain");
+ xc_domain_shutdown(CTX->xch, domid, SHUTDOWN_suspend);
/* The guest does not (need to) respond to this sort of request. */
- si->guest_responded = 1;
+ dss->guest_responded = 1;
} else {
- LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "issuing %s suspend request via
XenBus control node",
- si->hvm ? "PVHVM" : "PV");
+ LOG(DEBUG, "issuing %s suspend request via XenBus control
node",
+ dss->hvm ? "PVHVM" : "PV");
- libxl__domain_pvcontrol_write(si->gc, XBT_NULL, si->domid,
"suspend");
+ libxl__domain_pvcontrol_write(gc, XBT_NULL, domid,
"suspend");
- LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "wait for the guest to
acknowledge suspend request");
+ LOG(DEBUG, "wait for the guest to acknowledge suspend
request");
watchdog = 60;
while (!strcmp(state, "suspend") && watchdog > 0)
{
usleep(100000);
- state = libxl__domain_pvcontrol_read(si->gc, XBT_NULL,
si->domid);
+ state = libxl__domain_pvcontrol_read(gc, XBT_NULL, domid);
if (!state) state = "";
watchdog--;
@@ -705,17 +698,17 @@ static int libxl__domain_suspend_common_callback(void
*data)
* at the last minute.
*/
if (!strcmp(state, "suspend")) {
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "guest didn''t
acknowledge suspend, cancelling request");
+ LOG(ERROR, "guest didn''t acknowledge suspend,
cancelling request");
retry_transaction:
- t = xs_transaction_start(ctx->xsh);
+ t = xs_transaction_start(CTX->xsh);
- state = libxl__domain_pvcontrol_read(si->gc, t, si->domid);
+ state = libxl__domain_pvcontrol_read(gc, t, domid);
if (!state) state = "";
if (!strcmp(state, "suspend"))
- libxl__domain_pvcontrol_write(si->gc, t, si->domid,
"");
+ libxl__domain_pvcontrol_write(gc, t, domid, "");
- if (!xs_transaction_end(ctx->xsh, t, 0))
+ if (!xs_transaction_end(CTX->xsh, t, 0))
if (errno == EAGAIN)
goto retry_transaction;
@@ -727,27 +720,29 @@ static int libxl__domain_suspend_common_callback(void
*data)
* case we lost the race while cancelling and should continue.
*/
if (!strcmp(state, "suspend")) {
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "guest didn''t
acknowledge suspend, request cancelled");
+ LOG(ERROR, "guest didn''t acknowledge suspend, request
cancelled");
return 0;
}
- LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "guest acknowledged suspend
request");
- si->guest_responded = 1;
+ LOG(DEBUG, "guest acknowledged suspend request");
+ dss->guest_responded = 1;
}
- LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "wait for the guest to
suspend");
+ 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, si->domid, 1, &info);
- if (ret == 1 && info.domain == si->domid &&
info.flags & XEN_DOMINF_shutdown) {
+ 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;
+ shutdown_reason = (info.flags >> XEN_DOMINF_shutdownshift)
+ & XEN_DOMINF_shutdownmask;
if (shutdown_reason == SHUTDOWN_suspend) {
- LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "guest has
suspended");
+ LOG(DEBUG, "guest has suspended");
goto guest_suspended;
}
}
@@ -755,15 +750,14 @@ static int libxl__domain_suspend_common_callback(void
*data)
watchdog--;
}
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "guest did not suspend");
+ LOG(ERROR, "guest did not suspend");
return 0;
guest_suspended:
- if (si->hvm) {
- ret = libxl__domain_suspend_device_model(si->gc, si->domid);
+ if (dss->hvm) {
+ ret = libxl__domain_suspend_device_model(dss->gc, dss->domid);
if (ret) {
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
- "libxl__domain_suspend_device_model failed
ret=%d", ret);
+ LOG(ERROR, "libxl__domain_suspend_device_model failed
ret=%d", ret);
return 0;
}
}
@@ -781,9 +775,8 @@ static inline char *save_helper(libxl__gc *gc, uint32_t
domid,
static int libxl__toolstack_save(uint32_t domid, uint8_t **buf,
uint32_t *len, void *data)
{
- struct suspendinfo *si = (struct suspendinfo *) data;
- libxl__gc *gc = (libxl__gc *) si->gc;
- libxl_ctx *ctx = gc->owner;
+ libxl__domain_suspend_state *dss = data;
+ libxl__gc *gc = dss->gc;
int i = 0;
char *start_addr = NULL, *size = NULL, *phys_offset = NULL, *name = NULL;
unsigned int num = 0;
@@ -812,21 +805,21 @@ static int libxl__toolstack_save(uint32_t domid, uint8_t
**buf,
char *xs_path;
phys_offset = entries[i];
if (phys_offset == NULL) {
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "phys_offset %d is
NULL", i);
+ LOG(ERROR, "phys_offset %d is NULL", i);
return -1;
}
xs_path = save_helper(gc, domid, phys_offset, "start_addr");
start_addr = libxl__xs_read(gc, 0, xs_path);
if (start_addr == NULL) {
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "%s is NULL", xs_path);
+ LOG(ERROR, "%s is NULL", xs_path);
return -1;
}
xs_path = save_helper(gc, domid, phys_offset, "size");
size = libxl__xs_read(gc, 0, xs_path);
if (size == NULL) {
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "%s is NULL", xs_path);
+ LOG(ERROR, "%s is NULL", xs_path);
return -1;
}
@@ -862,11 +855,11 @@ static int libxl__remus_domain_suspend_callback(void
*data)
static int libxl__remus_domain_resume_callback(void *data)
{
- struct suspendinfo *si = data;
- libxl_ctx *ctx = libxl__gc_owner(si->gc);
+ libxl__domain_suspend_state *dss = data;
+ libxl__gc *gc = dss->gc;
/* Resumes the domain and the device model */
- if (libxl_domain_resume(ctx, si->domid, /* Fast Suspend */1))
+ if (libxl_domain_resume(CTX, dss->domid, /* Fast Suspend */1))
return 0;
/* TODO: Deal with disk. Start a new network output buffer */
@@ -875,15 +868,15 @@ static int libxl__remus_domain_resume_callback(void *data)
static int libxl__remus_domain_checkpoint_callback(void *data)
{
- struct suspendinfo *si = data;
+ libxl__domain_suspend_state *dss = data;
/* This would go into tailbuf. */
- if (si->hvm &&
- libxl__domain_save_device_model(si->gc, si->domid,
si->save_fd))
+ if (dss->hvm &&
+ libxl__domain_save_device_model(dss->gc, dss->domid,
dss->save_fd))
return 0;
/* TODO: Wait for disk and memory ack, release network buffer */
- usleep(si->interval * 1000);
+ usleep(dss->interval * 1000);
return 1;
}
@@ -892,11 +885,10 @@ int libxl__domain_suspend_common(libxl__gc *gc, uint32_t
domid, int fd,
int live, int debug,
const libxl_domain_remus_info *r_info)
{
- libxl_ctx *ctx = libxl__gc_owner(gc);
- int flags;
+ int xcflags;
int port;
- struct save_callbacks callbacks;
- struct suspendinfo si;
+ struct save_callbacks callbacks[1];
+ libxl__domain_suspend_state dss[1];
int hvm, rc = ERROR_FAIL;
unsigned long vm_generationid_addr;
@@ -921,71 +913,72 @@ int libxl__domain_suspend_common(libxl__gc *gc, uint32_t
domid, int fd,
return ERROR_INVAL;
}
- memset(&si, 0, sizeof(si));
- flags = (live) ? XCFLAGS_LIVE : 0
+ xcflags = (live) ? XCFLAGS_LIVE : 0
| (debug) ? XCFLAGS_DEBUG : 0
| (hvm) ? XCFLAGS_HVM : 0;
+ dss->domid = domid;
+ dss->xcflags = xcflags;
+ dss->hvm = hvm;
+ dss->gc = gc;
+ dss->suspend_eventchn = -1;
+ dss->guest_responded = 0;
+
if (r_info != NULL) {
- si.interval = r_info->interval;
+ dss->interval = r_info->interval;
if (r_info->compression)
- flags |= XCFLAGS_CHECKPOINT_COMPRESS;
- si.save_fd = fd;
+ xcflags |= XCFLAGS_CHECKPOINT_COMPRESS;
+ dss->save_fd = fd;
}
else
- si.save_fd = -1;
-
- si.domid = domid;
- si.flags = flags;
- si.hvm = hvm;
- si.gc = gc;
- si.suspend_eventchn = -1;
- si.guest_responded = 0;
+ dss->save_fd = -1;
- si.xce = xc_evtchn_open(NULL, 0);
- if (si.xce == NULL)
+ dss->xce = xc_evtchn_open(NULL, 0);
+ if (dss->xce == NULL)
goto out;
else
{
- port = xs_suspend_evtchn_port(si.domid);
+ port = xs_suspend_evtchn_port(dss->domid);
if (port >= 0) {
- si.suspend_eventchn = xc_suspend_evtchn_init(ctx->xch, si.xce,
si.domid, port);
+ dss->suspend_eventchn +
xc_suspend_evtchn_init(CTX->xch, dss->xce, dss->domid, port);
- if (si.suspend_eventchn < 0)
- LIBXL__LOG(ctx, LIBXL__LOG_WARNING, "Suspend event channel
initialization failed");
+ if (dss->suspend_eventchn < 0)
+ LOG(WARN, "Suspend event channel initialization
failed");
}
}
- memset(&callbacks, 0, sizeof(callbacks));
+ memset(callbacks, 0, sizeof(*callbacks));
if (r_info != NULL) {
- callbacks.suspend = libxl__remus_domain_suspend_callback;
- callbacks.postcopy = libxl__remus_domain_resume_callback;
- callbacks.checkpoint = libxl__remus_domain_checkpoint_callback;
+ callbacks->suspend = libxl__remus_domain_suspend_callback;
+ 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_common_callback;
- callbacks.switch_qemu_logdirty =
libxl__domain_suspend_common_switch_qemu_logdirty;
- callbacks.toolstack_save = libxl__toolstack_save;
- callbacks.data = &si;
+ callbacks->switch_qemu_logdirty =
libxl__domain_suspend_common_switch_qemu_logdirty;
+ callbacks->toolstack_save = libxl__toolstack_save;
+ callbacks->data = dss;
- rc = xc_domain_save(ctx->xch, fd, domid, 0, 0, flags, &callbacks,
+ rc = xc_domain_save(CTX->xch, fd, domid, 0, 0, xcflags, callbacks,
hvm, vm_generationid_addr);
if ( rc ) {
- LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "saving domain: %s",
- si.guest_responded ?
+ LOGE(ERROR, "saving domain: %s",
+ dss->guest_responded ?
"domain responded to suspend request" :
"domain did not respond to suspend
request");
- if ( !si.guest_responded )
+ if ( !dss->guest_responded )
rc = ERROR_GUEST_TIMEDOUT;
else
rc = ERROR_FAIL;
}
- if (si.suspend_eventchn > 0)
- xc_suspend_evtchn_release(ctx->xch, si.xce, domid,
si.suspend_eventchn);
- if (si.xce != NULL)
- xc_evtchn_close(si.xce);
+ if (dss->suspend_eventchn > 0)
+ xc_suspend_evtchn_release(CTX->xch, dss->xce, domid,
+ dss->suspend_eventchn);
+ if (dss->xce != NULL)
+ xc_evtchn_close(dss->xce);
out:
return rc;
@@ -993,8 +986,7 @@ out:
int libxl__domain_save_device_model(libxl__gc *gc, uint32_t domid, int fd)
{
- libxl_ctx *ctx = libxl__gc_owner(gc);
- int ret, fd2 = -1, c;
+ int rc, fd2 = -1, c;
char buf[1024];
const char *filename = libxl__device_model_savefile(gc, domid);
struct stat st;
@@ -1002,46 +994,46 @@ int libxl__domain_save_device_model(libxl__gc *gc,
uint32_t domid, int fd)
if (stat(filename, &st) < 0)
{
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Unable to stat qemu save
file\n");
- ret = ERROR_FAIL;
+ LOG(ERROR, "Unable to stat qemu save file\n");
+ rc = ERROR_FAIL;
goto out;
}
qemu_state_len = st.st_size;
- LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Qemu state is %d bytes\n",
qemu_state_len);
+ LOG(DEBUG, "Qemu state is %d bytes\n", qemu_state_len);
- ret = libxl_write_exactly(ctx, fd, QEMU_SIGNATURE, strlen(QEMU_SIGNATURE),
+ rc = libxl_write_exactly(CTX, fd, QEMU_SIGNATURE, strlen(QEMU_SIGNATURE),
"saved-state file", "qemu
signature");
- if (ret)
+ if (rc)
goto out;
- ret = libxl_write_exactly(ctx, fd, &qemu_state_len,
sizeof(qemu_state_len),
+ rc = libxl_write_exactly(CTX, fd, &qemu_state_len,
sizeof(qemu_state_len),
"saved-state file", "saved-state
length");
- if (ret)
+ if (rc)
goto out;
fd2 = open(filename, O_RDONLY);
if (fd2 < 0) {
- LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Unable to open qemu save
file\n");
+ LOGE(ERROR, "Unable to open qemu save file\n");
goto out;
}
while ((c = read(fd2, buf, sizeof(buf))) != 0) {
if (c < 0) {
if (errno == EINTR)
continue;
- ret = errno;
+ rc = errno;
goto out;
}
- ret = libxl_write_exactly(
- ctx, fd, buf, c, "saved-state file", "qemu
state");
- if (ret)
+ rc = libxl_write_exactly(
+ CTX, fd, buf, c, "saved-state file", "qemu
state");
+ if (rc)
goto out;
}
- ret = 0;
+ rc = 0;
out:
if (fd2 >= 0) close(fd2);
unlink(filename);
- return ret;
+ return rc;
}
char *libxl__uuid2string(libxl__gc *gc, const libxl_uuid uuid)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index a5edea4..b67e317 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -783,10 +783,6 @@ _hidden int libxl__domain_restore_common(libxl__gc *gc,
uint32_t domid,
libxl_domain_build_info *info,
libxl__domain_build_state *state,
int fd);
-_hidden int libxl__domain_suspend_common(libxl__gc *gc, uint32_t domid, int fd,
- libxl_domain_type type,
- int live, int debug,
- const libxl_domain_remus_info
*r_info);
_hidden const char *libxl__device_model_savefile(libxl__gc *gc, uint32_t
domid);
_hidden int libxl__domain_suspend_device_model(libxl__gc *gc, uint32_t domid);
_hidden int libxl__domain_resume_device_model(libxl__gc *gc, uint32_t domid);
@@ -1779,6 +1775,23 @@ _hidden void
libxl__datacopier_kill(libxl__datacopier_state *dc);
_hidden int libxl__datacopier_start(libxl__datacopier_state *dc);
+/*----- Domain suspend (save) state structure -----*/
+
+typedef struct libxl__domain_suspend_state libxl__domain_suspend_state;
+
+struct libxl__domain_suspend_state {
+ libxl__gc *gc;
+ xc_evtchn *xce; /* event channel handle */
+ int suspend_eventchn;
+ int domid;
+ int hvm;
+ unsigned int xcflags;
+ int guest_responded;
+ int save_fd; /* Migration stream fd (for Remus) */
+ int interval; /* checkpoint interval (for Remus) */
+};
+
+
/*----- openpty -----*/
/*
@@ -1889,6 +1902,15 @@ struct libxl__domain_create_state {
* for the non-stubdom device model. */
};
+/*----- Domain suspend (save) functions -----*/
+
+_hidden int libxl__domain_suspend_common(libxl__gc *gc, uint32_t domid, int fd,
+ libxl_domain_type type,
+ int live, int debug,
+ const libxl_domain_remus_info
*r_info);
+
+
+
/*
* Convenience macros.
*/
--
1.7.2.5
Ian Jackson
2012-May-30 16:16 UTC
[PATCH 03/15] libxl: domain restore: reshuffle, preparing for ao
We are going to arrange that libxl, instead of calling
xc_domain_restore, calls a stub function which forks and execs a
helper program, so that restore can be asynchronous rather than
blocking the whole toolstack.
This stub function will be called libxl__xc_domain_restore.
However, its prospective call site is unsuitable for a function which
needs to make a callback, and is buried in two nested single-call-site
functions which are logically part of the domain creation procedure.
So we first abolish those single-call-site functions, integrate their
contents into domain creation in their proper temporal order, and
break out libxl__xc_domain_restore ready for its reimplementation.
No functional change - just the following reorganisation:
* Abolish libxl__domain_restore_common, as it had only one caller.
Move its contents into (what was) domain_restore.
* There is a new stage function domcreate_rebuild_done containing what
used to be the bulk of domcreate_bootloader_done, since
domcreate_bootloader_done now simply starts the restore (or does the
rebuild) and arranges to call the next stage.
* Move the contents of domain_restore into its correct place in the
domain creation sequence. We put it inside
domcreate_bootloader_done, which now either calls
libxl__xc_domain_restore which will call the new function
domcreate_rebuild_done.
* Various general-purpose local variables (`i'' etc.) and convenience
alias variables need to be shuffled about accordingly.
* Consequently libxl__toolstack_restore needs to gain external linkage
as it is now in a different file to its user.
* Move the xc_domain_save callbacks struct from the stack into
libxl__domain_create_state.
In general the moved code remains almost identical. Two returns in
what used to be libxl__domain_restore_common have been changed to set
the return value and "goto out", and the call sites for the abolished
and new functions have been adjusted.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Changes in v2:
* Also move the save callbacks
---
tools/libxl/Makefile | 1 +
tools/libxl/libxl_create.c | 244 +++++++++++++++++++++++--------------
tools/libxl/libxl_dom.c | 45 +-------
tools/libxl/libxl_internal.h | 19 +++-
tools/libxl/libxl_save_callout.c | 37 ++++++
5 files changed, 206 insertions(+), 140 deletions(-)
create mode 100644 tools/libxl/libxl_save_callout.c
diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index 5d9227e..9b84421 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -66,6 +66,7 @@ LIBXL_OBJS = flexarray.o libxl.o libxl_create.o libxl_dm.o
libxl_pci.o \
libxl_dom.o libxl_exec.o libxl_xshelp.o libxl_device.o \
libxl_internal.o libxl_utils.o libxl_uuid.o \
libxl_json.o libxl_aoutils.o \
+ libxl_save_callout.o \
libxl_qmp.o libxl_event.o libxl_fork.o $(LIBXL_OBJS-y)
LIBXL_OBJS += _libxl_types.o libxl_flask.o _libxl_types_internal.o
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 77acecc..44455f9 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -20,7 +20,6 @@
#include "libxl_internal.h"
#include <xc_dom.h>
-#include <xenguest.h>
void libxl_domain_config_init(libxl_domain_config *d_config)
{
@@ -316,89 +315,6 @@ out:
return ret;
}
-static int domain_restore(libxl__gc *gc, libxl_domain_build_info *info,
- uint32_t domid, int fd,
- libxl__domain_build_state *state)
-{
- libxl_ctx *ctx = libxl__gc_owner(gc);
- char **vments = NULL, **localents = NULL;
- struct timeval start_time;
- int i, ret, esave, flags;
-
- ret = libxl__build_pre(gc, domid, info, state);
- if (ret)
- goto out;
-
- ret = libxl__domain_restore_common(gc, domid, info, state, fd);
- if (ret)
- goto out;
-
- gettimeofday(&start_time, NULL);
-
- switch (info->type) {
- case LIBXL_DOMAIN_TYPE_HVM:
- vments = libxl__calloc(gc, 7, sizeof(char *));
- vments[0] = "rtc/timeoffset";
- vments[1] = (info->u.hvm.timeoffset) ? info->u.hvm.timeoffset :
"";
- vments[2] = "image/ostype";
- vments[3] = "hvm";
- vments[4] = "start_time";
- vments[5] = libxl__sprintf(gc, "%lu.%02d",
start_time.tv_sec,(int)start_time.tv_usec/10000);
- break;
- case LIBXL_DOMAIN_TYPE_PV:
- vments = libxl__calloc(gc, 11, sizeof(char *));
- i = 0;
- vments[i++] = "image/ostype";
- vments[i++] = "linux";
- vments[i++] = "image/kernel";
- vments[i++] = (char *) state->pv_kernel.path;
- vments[i++] = "start_time";
- vments[i++] = libxl__sprintf(gc, "%lu.%02d",
start_time.tv_sec,(int)start_time.tv_usec/10000);
- if (state->pv_ramdisk.path) {
- vments[i++] = "image/ramdisk";
- vments[i++] = (char *) state->pv_ramdisk.path;
- }
- if (state->pv_cmdline) {
- vments[i++] = "image/cmdline";
- vments[i++] = (char *) state->pv_cmdline;
- }
- break;
- default:
- ret = ERROR_INVAL;
- goto out;
- }
- ret = libxl__build_post(gc, domid, info, state, vments, localents);
- if (ret)
- goto out;
-
- if (info->type == LIBXL_DOMAIN_TYPE_HVM) {
- ret = asprintf(&state->saved_state,
- XC_DEVICE_MODEL_RESTORE_FILE".%d", domid);
- ret = (ret < 0) ? ERROR_FAIL : 0;
- }
-
-out:
- if (info->type == LIBXL_DOMAIN_TYPE_PV) {
- libxl__file_reference_unmap(&state->pv_kernel);
- libxl__file_reference_unmap(&state->pv_ramdisk);
- }
-
- esave = errno;
-
- flags = fcntl(fd, F_GETFL);
- if (flags == -1) {
- LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "unable to get flags on
restore fd");
- } else {
- flags &= ~O_NONBLOCK;
- if (fcntl(fd, F_SETFL, flags) == -1)
- LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "unable to put restore
fd"
- " back to blocking mode");
- }
-
- errno = esave;
- return ret;
-}
-
int libxl__domain_make(libxl__gc *gc, libxl_domain_create_info *info,
uint32_t *domid)
{
@@ -579,10 +495,13 @@ static void
domcreate_bootloader_console_available(libxl__egc *egc,
static void domcreate_bootloader_done(libxl__egc *egc,
libxl__bootloader_state *bl,
int rc);
-
static void domcreate_console_available(libxl__egc *egc,
libxl__domain_create_state *dcs);
+static void domcreate_rebuild_done(libxl__egc *egc,
+ libxl__domain_create_state *dcs,
+ int ret);
+
/* Our own function to clean up and call the user''s callback.
* The final call in the sequence. */
static void domcreate_complete(libxl__egc *egc,
@@ -670,20 +589,20 @@ static void domcreate_console_available(libxl__egc *egc,
static void domcreate_bootloader_done(libxl__egc *egc,
libxl__bootloader_state *bl,
- int ret)
+ int rc)
{
libxl__domain_create_state *dcs = CONTAINER_OF(bl, *dcs, bl);
STATE_AO_GC(bl->ao);
- int i;
/* convenience aliases */
const uint32_t domid = dcs->guest_domid;
libxl_domain_config *const d_config = dcs->guest_config;
+ libxl_domain_build_info *const info = &d_config->b_info;
const int restore_fd = dcs->restore_fd;
libxl__domain_build_state *const state = &dcs->build_state;
- libxl_ctx *const ctx = CTX;
+ struct restore_callbacks *const callbacks = &dcs->callbacks;
- if (ret) goto error_out;
+ if (rc) domcreate_rebuild_done(egc, dcs, rc);
/* consume bootloader outputs. state->pv_{kernel,ramdisk} have
* been initialised by the bootloader already.
@@ -699,12 +618,153 @@ static void domcreate_bootloader_done(libxl__egc *egc,
dcs->dmss.dm.callback = domcreate_devmodel_started;
dcs->dmss.callback = domcreate_devmodel_started;
- if ( restore_fd >= 0 ) {
- ret = domain_restore(gc, &d_config->b_info, domid, restore_fd,
state);
+ if ( restore_fd < 0 ) {
+ rc = libxl__domain_build(gc, &d_config->b_info, domid, state);
+ domcreate_rebuild_done(egc, dcs, rc);
+ return;
+ }
+
+ /* Restore */
+
+ rc = libxl__build_pre(gc, domid, info, state);
+ if (rc)
+ goto out;
+
+ /* read signature */
+ int hvm, pae, superpages;
+ int no_incr_generationid;
+ switch (info->type) {
+ case LIBXL_DOMAIN_TYPE_HVM:
+ hvm = 1;
+ superpages = 1;
+ pae = libxl_defbool_val(info->u.hvm.pae);
+ no_incr_generationid =
!libxl_defbool_val(info->u.hvm.incr_generationid);
+ callbacks->toolstack_restore = libxl__toolstack_restore;
+ callbacks->data = gc;
+ break;
+ case LIBXL_DOMAIN_TYPE_PV:
+ hvm = 0;
+ superpages = 0;
+ pae = 1;
+ no_incr_generationid = 0;
+ break;
+ default:
+ rc = ERROR_INVAL;
+ goto out;
+ }
+ libxl__xc_domain_restore(egc, dcs,
+ hvm, pae, superpages, no_incr_generationid);
+ return;
+
+ out:
+ libxl__xc_domain_restore_done(egc, dcs, rc, 0, 0);
+}
+
+void libxl__xc_domain_restore_done(libxl__egc *egc,
+ libxl__domain_create_state *dcs,
+ int ret, int retval, int errnoval)
+{
+ STATE_AO_GC(dcs->ao);
+ libxl_ctx *ctx = libxl__gc_owner(gc);
+ char **vments = NULL, **localents = NULL;
+ struct timeval start_time;
+ int i, esave, flags;
+
+ /* convenience aliases */
+ const uint32_t domid = dcs->guest_domid;
+ libxl_domain_config *const d_config = dcs->guest_config;
+ libxl_domain_build_info *const info = &d_config->b_info;
+ libxl__domain_build_state *const state = &dcs->build_state;
+ const int fd = dcs->restore_fd;
+
+ if (ret)
+ goto out;
+
+ if (retval) {
+ LOGEV(ERROR, errnoval, "restoring domain");
+ ret = ERROR_FAIL;
+ goto out;
+ }
+
+ gettimeofday(&start_time, NULL);
+
+ switch (info->type) {
+ case LIBXL_DOMAIN_TYPE_HVM:
+ vments = libxl__calloc(gc, 7, sizeof(char *));
+ vments[0] = "rtc/timeoffset";
+ vments[1] = (info->u.hvm.timeoffset) ? info->u.hvm.timeoffset :
"";
+ vments[2] = "image/ostype";
+ vments[3] = "hvm";
+ vments[4] = "start_time";
+ vments[5] = libxl__sprintf(gc, "%lu.%02d",
start_time.tv_sec,(int)start_time.tv_usec/10000);
+ break;
+ case LIBXL_DOMAIN_TYPE_PV:
+ vments = libxl__calloc(gc, 11, sizeof(char *));
+ i = 0;
+ vments[i++] = "image/ostype";
+ vments[i++] = "linux";
+ vments[i++] = "image/kernel";
+ vments[i++] = (char *) state->pv_kernel.path;
+ vments[i++] = "start_time";
+ vments[i++] = libxl__sprintf(gc, "%lu.%02d",
start_time.tv_sec,(int)start_time.tv_usec/10000);
+ if (state->pv_ramdisk.path) {
+ vments[i++] = "image/ramdisk";
+ vments[i++] = (char *) state->pv_ramdisk.path;
+ }
+ if (state->pv_cmdline) {
+ vments[i++] = "image/cmdline";
+ vments[i++] = (char *) state->pv_cmdline;
+ }
+ break;
+ default:
+ ret = ERROR_INVAL;
+ goto out;
+ }
+ ret = libxl__build_post(gc, domid, info, state, vments, localents);
+ if (ret)
+ goto out;
+
+ if (info->type == LIBXL_DOMAIN_TYPE_HVM) {
+ ret = asprintf(&state->saved_state,
+ XC_DEVICE_MODEL_RESTORE_FILE".%d", domid);
+ ret = (ret < 0) ? ERROR_FAIL : 0;
+ }
+
+out:
+ if (info->type == LIBXL_DOMAIN_TYPE_PV) {
+ libxl__file_reference_unmap(&state->pv_kernel);
+ libxl__file_reference_unmap(&state->pv_ramdisk);
+ }
+
+ esave = errno;
+
+ flags = fcntl(fd, F_GETFL);
+ if (flags == -1) {
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "unable to get flags on
restore fd");
} else {
- ret = libxl__domain_build(gc, &d_config->b_info, domid, state);
+ flags &= ~O_NONBLOCK;
+ if (fcntl(fd, F_SETFL, flags) == -1)
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "unable to put restore
fd"
+ " back to blocking mode");
}
+ errno = esave;
+ domcreate_rebuild_done(egc, dcs, ret);
+}
+
+static void domcreate_rebuild_done(libxl__egc *egc,
+ libxl__domain_create_state *dcs,
+ int ret)
+{
+ STATE_AO_GC(dcs->ao);
+ int i;
+
+ /* convenience aliases */
+ const uint32_t domid = dcs->guest_domid;
+ libxl_domain_config *const d_config = dcs->guest_config;
+ libxl__domain_build_state *const state = &dcs->build_state;
+ libxl_ctx *const ctx = CTX;
+
if (ret) {
LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot (re-)build domain:
%d", ret);
ret = ERROR_FAIL;
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 231ca40..3bd1cb9 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -19,7 +19,6 @@
#include <xenctrl.h>
#include <xc_dom.h>
-#include <xenguest.h>
#include <xen/hvm/hvm_info_table.h>
@@ -465,7 +464,7 @@ static inline char *restore_helper(libxl__gc *gc, uint32_t
domid,
domid, phys_offset, node);
}
-static int libxl__toolstack_restore(uint32_t domid, const uint8_t *buf,
+int libxl__toolstack_restore(uint32_t domid, const uint8_t *buf,
uint32_t size, void *data)
{
libxl__gc *gc = data;
@@ -520,48 +519,6 @@ static int libxl__toolstack_restore(uint32_t domid, const
uint8_t *buf,
return 0;
}
-int libxl__domain_restore_common(libxl__gc *gc, uint32_t domid,
- libxl_domain_build_info *info,
- libxl__domain_build_state *state,
- int fd)
-{
- libxl_ctx *ctx = libxl__gc_owner(gc);
- /* read signature */
- int rc;
- int hvm, pae, superpages;
- struct restore_callbacks callbacks[1];
- int no_incr_generationid;
- switch (info->type) {
- case LIBXL_DOMAIN_TYPE_HVM:
- hvm = 1;
- superpages = 1;
- pae = libxl_defbool_val(info->u.hvm.pae);
- no_incr_generationid =
!libxl_defbool_val(info->u.hvm.incr_generationid);
- callbacks->toolstack_restore = libxl__toolstack_restore;
- callbacks->data = gc;
- break;
- case LIBXL_DOMAIN_TYPE_PV:
- hvm = 0;
- superpages = 0;
- pae = 1;
- no_incr_generationid = 0;
- break;
- default:
- return ERROR_INVAL;
- }
- rc = xc_domain_restore(ctx->xch, fd, domid,
- state->store_port, &state->store_mfn,
- state->store_domid, state->console_port,
- &state->console_mfn, state->console_domid,
- hvm, pae, superpages, no_incr_generationid,
- &state->vm_generationid_addr, callbacks);
- if ( rc ) {
- LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "restoring domain");
- return ERROR_FAIL;
- }
- return 0;
-}
-
static int libxl__domain_suspend_common_switch_qemu_logdirty
(int domid, unsigned int enable, void *data)
{
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index b67e317..ae4b99d 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -46,6 +46,7 @@
#include <xenstore.h>
#include <xenctrl.h>
+#include <xenguest.h>
#include "xentoollog.h"
@@ -779,10 +780,8 @@ _hidden int libxl__domain_rename(libxl__gc *gc, uint32_t
domid,
const char *old_name, const char *new_name,
xs_transaction_t trans);
-_hidden int libxl__domain_restore_common(libxl__gc *gc, uint32_t domid,
- libxl_domain_build_info *info,
- libxl__domain_build_state *state,
- int fd);
+_hidden int libxl__toolstack_restore(uint32_t domid, const uint8_t *buf,
+ uint32_t size, void *data);
_hidden const char *libxl__device_model_savefile(libxl__gc *gc, uint32_t
domid);
_hidden int libxl__domain_suspend_device_model(libxl__gc *gc, uint32_t domid);
_hidden int libxl__domain_resume_device_model(libxl__gc *gc, uint32_t domid);
@@ -1900,6 +1899,7 @@ struct libxl__domain_create_state {
libxl__stub_dm_spawn_state dmss;
/* If we''re not doing stubdom, we use only dmss.dm,
* for the non-stubdom device model. */
+ struct restore_callbacks callbacks;
};
/*----- Domain suspend (save) functions -----*/
@@ -1909,6 +1909,17 @@ _hidden int libxl__domain_suspend_common(libxl__gc *gc,
uint32_t domid, int fd,
int live, int debug,
const libxl_domain_remus_info
*r_info);
+/* calls libxl__xc_domain_restore_done when done */
+_hidden void libxl__xc_domain_restore(libxl__egc *egc,
+ libxl__domain_create_state *dcs,
+ int hvm, int pae, int superpages,
+ int no_incr_generationid);
+/* If rc==0 then retval is the return value from xc_domain_save
+ * and errnoval is the errno value it provided.
+ * If rc!=0, retval and errnoval are undefined. */
+_hidden void libxl__xc_domain_restore_done(libxl__egc *egc,
+ libxl__domain_create_state *dcs,
+ int rc, int retval, int errnoval);
/*
diff --git a/tools/libxl/libxl_save_callout.c b/tools/libxl/libxl_save_callout.c
new file mode 100644
index 0000000..2f8db9f
--- /dev/null
+++ b/tools/libxl/libxl_save_callout.c
@@ -0,0 +1,37 @@
+/*
+ * Copyright (C) 2012 Citrix Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; version 2.1 only. with the special
+ * exception on linking described in file LICENSE.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Lesser General Public License for more details.
+ */
+
+#include "libxl_osdeps.h"
+
+#include "libxl_internal.h"
+
+void libxl__xc_domain_restore(libxl__egc *egc, libxl__domain_create_state *dcs,
+ int hvm, int pae, int superpages,
+ int no_incr_generationid)
+{
+ STATE_AO_GC(dcs->ao);
+
+ /* Convenience aliases */
+ const uint32_t domid = dcs->guest_domid;
+ const int restore_fd = dcs->restore_fd;
+ libxl__domain_build_state *const state = &dcs->build_state;
+
+ int r = xc_domain_restore(CTX->xch, restore_fd, domid,
+ state->store_port, &state->store_mfn,
+ state->store_domid, state->console_port,
+ &state->console_mfn,
state->console_domid,
+ hvm, pae, superpages, no_incr_generationid,
+ &state->vm_generationid_addr,
&dcs->callbacks);
+ libxl__xc_domain_restore_done(egc, dcs, 0, r, errno);
+}
--
1.7.2.5
Ian Jackson
2012-May-30 16:16 UTC
[PATCH 04/15] libxl: domain save: API changes for asynchrony
Change the internal and external APIs for domain save (suspend) to be
capable of asynchronous operation. The implementation remains
synchronous. The interfaces surrounding device model saving are still
synchronous.
Public API changes:
* libxl_domain_save takes an ao_how.
* libxl_domain_remus_start takes an ao_how. If the
libxl_domain_remus_info is NULL, we abort rather than returning an
error.
* The `suspend_callback'' function passed to libxl_domain_save is
never called by the existing implementation in libxl. Abolish it.
* libxl_domain_save takes its flags parameter as an argument.
Thus libxl_domain_suspend_info is abolished.
* XL_SUSPEND_* flags renamed to LIBXL_SAVE_*.
* Callers in xl updated.
Internal code restructuring:
* libxl__domain_suspend_state member types and names rationalised.
* libxl__domain_suspend renamed from libxl__domain_suspend_common.
(_common here actually meant "internal function").
* libxl__domain_suspend takes a libxl__domain_suspend_state, which
where the parameters to the operation are filled in by the caller.
* xc_domain_save is now called via libxl__xc_domain_save which can
itself become asynchronous.
* Consequently, libxl__domain_suspend is split into two functions at
the callback boundary; the second half is
libxl__xc_domain_save_done.
* libxl__domain_save_device_model is now called by the actual
implementation rather than by the public wrapper. It is already in
its proper place in the domain save execution sequence. So
officially make it part of that execution sequence, renaming it to
domain_save_device_model.
* Effectively, rewrite the public wrapper functions
libxl_domain_suspend and libxl_domain_remus_start.
* Remove a needless #include <xenctrl.h>
* libxl__domain_suspend aborts on unexpected domain types rather
than mysteriously returning EINVAL.
* struct save_callbacks moved from the stack to the dss.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Changes in v2:
* Move save_callbacks too.
* Merge with Remus changes.
* Improvements to commit message.
* Do not rename libxl_domain_suspend any more.
---
tools/libxl/libxl.c | 89 +++++++++++++++++++--------
tools/libxl/libxl.h | 22 ++++----
tools/libxl/libxl_dom.c | 122 ++++++++++++++++++++++++++-----------
tools/libxl/libxl_internal.h | 45 ++++++++++++---
tools/libxl/libxl_save_callout.c | 12 ++++
tools/libxl/xl_cmdimpl.c | 9 +--
6 files changed, 212 insertions(+), 87 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 8a18fdf..53626ae 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -619,27 +619,44 @@ libxl_vminfo * libxl_list_vm(libxl_ctx *ctx, int *nb_vm)
return ptr;
}
+static void remus_crashed_cb(libxl__egc *egc,
+ libxl__domain_suspend_state *dss, int rc);
+
/* TODO: Explicit Checkpoint acknowledgements via recv_fd. */
int libxl_domain_remus_start(libxl_ctx *ctx, libxl_domain_remus_info *info,
- uint32_t domid, int send_fd, int recv_fd)
+ uint32_t domid, int send_fd, int recv_fd,
+ const libxl_asyncop_how *ao_how)
{
- GC_INIT(ctx);
+ AO_CREATE(ctx, domid, ao_how);
+ libxl__domain_suspend_state *dss;
+
libxl_domain_type type = libxl__domain_type(gc, domid);
- int rc = 0;
+ /* TODO: check error return from libxl__domain_type */
- if (info == NULL) {
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
- "No remus_info structure supplied for domain %d",
domid);
- rc = ERROR_INVAL;
- goto remus_fail;
- }
+ GCNEW(dss);
+ dss->ao = ao;
+ dss->callback = remus_crashed_cb;
+ dss->domid = domid;
+ dss->fd = send_fd;
+ /* TODO do something with recv_fd */
+ dss->type = type;
+ dss->live = 1;
+ dss->debug = 0;
+ dss->remus = info;
+
+ assert(info);
/* TBD: Remus setup - i.e. attach qdisc, enable disk buffering, etc */
/* Point of no return */
- rc = libxl__domain_suspend_common(gc, domid, send_fd, type, /* live */ 1,
- /* debug */ 0, info);
+ libxl__domain_suspend(egc, dss);
+ return AO_INPROGRESS;
+}
+static void remus_crashed_cb(libxl__egc *egc,
+ libxl__domain_suspend_state *dss, int rc)
+{
+ STATE_AO_GC(dss->ao);
/*
* With Remus, if we reach this point, it means either
* backup died or some network error occurred preventing us
@@ -649,27 +666,47 @@ int libxl_domain_remus_start(libxl_ctx *ctx,
libxl_domain_remus_info *info,
/* TBD: Remus cleanup - i.e. detach qdisc, release other
* resources.
*/
- remus_fail:
- GC_FREE;
- return rc;
+ libxl__ao_complete(egc, ao, rc);
}
-int libxl_domain_suspend(libxl_ctx *ctx, libxl_domain_suspend_info *info,
- uint32_t domid, int fd)
+static void domain_suspend_cb(libxl__egc *egc,
+ libxl__domain_suspend_state *dss, int rc)
{
- GC_INIT(ctx);
+ STATE_AO_GC(dss->ao);
+ libxl__ao_complete(egc,ao,rc);
+
+}
+
+int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd, int flags,
+ const libxl_asyncop_how *ao_how)
+{
+ AO_CREATE(ctx, domid, ao_how);
+ int rc;
+
libxl_domain_type type = libxl__domain_type(gc, domid);
- int live = info != NULL && info->flags & XL_SUSPEND_LIVE;
- int debug = info != NULL && info->flags & XL_SUSPEND_DEBUG;
- int rc = 0;
+ if (type < 0) {
+ LOG(ERROR,"domain %"PRIu32": unable to determine domain
type", domid);
+ rc = ERROR_FAIL;
+ goto out_err;
+ }
- rc = libxl__domain_suspend_common(gc, domid, fd, type, live, debug,
- /* No Remus */ NULL);
+ libxl__domain_suspend_state *dss;
+ GCNEW(dss);
- if (!rc && type == LIBXL_DOMAIN_TYPE_HVM)
- rc = libxl__domain_save_device_model(gc, domid, fd);
- GC_FREE;
- return rc;
+ dss->ao = ao;
+ dss->callback = domain_suspend_cb;
+
+ dss->domid = domid;
+ dss->fd = fd;
+ dss->type = type;
+ dss->live = flags & LIBXL_SUSPEND_LIVE;
+ dss->debug = flags & LIBXL_SUSPEND_DEBUG;
+
+ libxl__domain_suspend(egc, dss);
+ return AO_INPROGRESS;
+
+ out_err:
+ return AO_ABORT(rc);
}
int libxl_domain_pause(libxl_ctx *ctx, uint32_t domid)
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 3a0d71d..4f1f4fd 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -347,13 +347,6 @@ typedef struct libxl__ctx libxl_ctx;
const libxl_version_info* libxl_get_version_info(libxl_ctx *ctx);
-typedef struct {
-#define XL_SUSPEND_DEBUG 1
-#define XL_SUSPEND_LIVE 2
- int flags;
- int (*suspend_callback)(void *, int);
-} libxl_domain_suspend_info;
-
enum {
ERROR_NONSPECIFIC = -1,
ERROR_VERSION = -2,
@@ -514,16 +507,23 @@ int libxl_domain_create_restore(libxl_ctx *ctx,
libxl_domain_config *d_config,
void libxl_domain_config_init(libxl_domain_config *d_config);
void libxl_domain_config_dispose(libxl_domain_config *d_config);
-int libxl_domain_remus_start(libxl_ctx *ctx, libxl_domain_remus_info *info,
- uint32_t domid, int send_fd, int recv_fd);
-int libxl_domain_suspend(libxl_ctx *ctx, libxl_domain_suspend_info *info,
- uint32_t domid, int fd);
+
+int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd,
+ int flags, /* LIBXL_SUSPEND_* */
+ const libxl_asyncop_how *ao_how);
+#define LIBXL_SUSPEND_DEBUG 1
+#define LIBXL_SUSPEND_LIVE 2
/* @param suspend_cancel [from xenctrl.h:xc_domain_resume( @param fast )]
* If this parameter is true, use co-operative resume. The guest
* must support this.
*/
int libxl_domain_resume(libxl_ctx *ctx, uint32_t domid, int suspend_cancel);
+
+int libxl_domain_remus_start(libxl_ctx *ctx, libxl_domain_remus_info *info,
+ uint32_t domid, int send_fd, int recv_fd,
+ const libxl_asyncop_how *ao_how);
+
int libxl_domain_shutdown(libxl_ctx *ctx, uint32_t domid);
int libxl_domain_reboot(libxl_ctx *ctx, uint32_t domid);
int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid);
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 3bd1cb9..8cac6d5 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -17,13 +17,11 @@
#include <glob.h>
-#include <xenctrl.h>
-#include <xc_dom.h>
+#include "libxl_internal.h"
+#include <xc_dom.h>
#include <xen/hvm/hvm_info_table.h>
-#include "libxl_internal.h"
-
libxl_domain_type libxl__domain_type(libxl__gc *gc, uint32_t domid)
{
libxl_ctx *ctx = libxl__gc_owner(gc);
@@ -519,11 +517,18 @@ int libxl__toolstack_restore(uint32_t domid, const uint8_t
*buf,
return 0;
}
-static int libxl__domain_suspend_common_switch_qemu_logdirty
+/*==================== Domain suspend (save) ====================*/
+
+static void domain_suspend_done(libxl__egc *egc,
+ libxl__domain_suspend_state *dss, int rc);
+
+/*----- callbacks, called by xc_domain_save -----*/
+
+int libxl__domain_suspend_common_switch_qemu_logdirty
(int domid, unsigned int enable, void *data)
{
libxl__domain_suspend_state *dss = data;
- libxl__gc *gc = dss->gc;
+ STATE_AO_GC(dss->ao);
char *path;
bool rc;
@@ -588,10 +593,10 @@ int libxl__domain_resume_device_model(libxl__gc *gc,
uint32_t domid)
return 0;
}
-static int libxl__domain_suspend_common_callback(void *data)
+int libxl__domain_suspend_common_callback(void *data)
{
libxl__domain_suspend_state *dss = data;
- libxl__gc *gc = dss->gc;
+ STATE_AO_GC(dss->ao);
unsigned long hvm_s_state = 0, hvm_pvdrv = 0;
int ret;
char *state = "suspend";
@@ -712,7 +717,7 @@ static int libxl__domain_suspend_common_callback(void *data)
guest_suspended:
if (dss->hvm) {
- ret = libxl__domain_suspend_device_model(dss->gc, dss->domid);
+ ret = libxl__domain_suspend_device_model(gc, dss->domid);
if (ret) {
LOG(ERROR, "libxl__domain_suspend_device_model failed
ret=%d", ret);
return 0;
@@ -729,11 +734,11 @@ static inline char *save_helper(libxl__gc *gc, uint32_t
domid,
domid, phys_offset, node);
}
-static int libxl__toolstack_save(uint32_t domid, uint8_t **buf,
+int libxl__toolstack_save(uint32_t domid, uint8_t **buf,
uint32_t *len, void *data)
{
libxl__domain_suspend_state *dss = data;
- libxl__gc *gc = dss->gc;
+ STATE_AO_GC(dss->ao);
int i = 0;
char *start_addr = NULL, *size = NULL, *phys_offset = NULL, *name = NULL;
unsigned int num = 0;
@@ -804,6 +809,8 @@ static int libxl__toolstack_save(uint32_t domid, uint8_t
**buf,
return 0;
}
+/*----- remus callbacks -----*/
+
static int libxl__remus_domain_suspend_callback(void *data)
{
/* TODO: Issue disk and network checkpoint reqs. */
@@ -813,7 +820,7 @@ static int libxl__remus_domain_suspend_callback(void *data)
static int libxl__remus_domain_resume_callback(void *data)
{
libxl__domain_suspend_state *dss = data;
- libxl__gc *gc = dss->gc;
+ STATE_AO_GC(dss->ao);
/* Resumes the domain and the device model */
if (libxl_domain_resume(CTX, dss->domid, /* Fast Suspend */1))
@@ -826,10 +833,11 @@ static int libxl__remus_domain_resume_callback(void *data)
static int libxl__remus_domain_checkpoint_callback(void *data)
{
libxl__domain_suspend_state *dss = data;
+ STATE_AO_GC(dss->ao);
/* This would go into tailbuf. */
if (dss->hvm &&
- libxl__domain_save_device_model(dss->gc, dss->domid,
dss->save_fd))
+ libxl__domain_save_device_model(gc, dss->domid, dss->fd))
return 0;
/* TODO: Wait for disk and memory ack, release network buffer */
@@ -837,18 +845,24 @@ static int libxl__remus_domain_checkpoint_callback(void
*data)
return 1;
}
-int libxl__domain_suspend_common(libxl__gc *gc, uint32_t domid, int fd,
- libxl_domain_type type,
- int live, int debug,
- const libxl_domain_remus_info *r_info)
+/*----- main code for suspending, in order of execution -----*/
+
+void libxl__domain_suspend(libxl__egc *egc, libxl__domain_suspend_state *dss)
{
+ STATE_AO_GC(dss->ao);
int xcflags;
int port;
- struct save_callbacks callbacks[1];
- libxl__domain_suspend_state dss[1];
int hvm, rc = ERROR_FAIL;
unsigned long vm_generationid_addr;
+ /* Convenience aliases */
+ const uint32_t domid = dss->domid;
+ const libxl_domain_type type = dss->type;
+ const int live = dss->live;
+ const int debug = dss->debug;
+ const libxl_domain_remus_info *const r_info = dss->remus;
+ struct save_callbacks *const callbacks = &dss->callbacks;
+
switch (type) {
case LIBXL_DOMAIN_TYPE_HVM: {
char *path;
@@ -867,17 +881,14 @@ int libxl__domain_suspend_common(libxl__gc *gc, uint32_t
domid, int fd,
hvm = 0;
break;
default:
- return ERROR_INVAL;
+ abort();
}
xcflags = (live) ? XCFLAGS_LIVE : 0
| (debug) ? XCFLAGS_DEBUG : 0
| (hvm) ? XCFLAGS_HVM : 0;
- dss->domid = domid;
- dss->xcflags = xcflags;
dss->hvm = hvm;
- dss->gc = gc;
dss->suspend_eventchn = -1;
dss->guest_responded = 0;
@@ -885,10 +896,7 @@ int libxl__domain_suspend_common(libxl__gc *gc, uint32_t
domid, int fd,
dss->interval = r_info->interval;
if (r_info->compression)
xcflags |= XCFLAGS_CHECKPOINT_COMPRESS;
- dss->save_fd = fd;
}
- else
- dss->save_fd = -1;
dss->xce = xc_evtchn_open(NULL, 0);
if (dss->xce == NULL)
@@ -918,10 +926,28 @@ int libxl__domain_suspend_common(libxl__gc *gc, uint32_t
domid, int fd,
callbacks->toolstack_save = libxl__toolstack_save;
callbacks->data = dss;
- rc = xc_domain_save(CTX->xch, fd, domid, 0, 0, xcflags, callbacks,
- hvm, vm_generationid_addr);
- if ( rc ) {
- LOGE(ERROR, "saving domain: %s",
+ libxl__xc_domain_save(egc, dss, xcflags, hvm, vm_generationid_addr);
+ return;
+
+ out:
+ domain_suspend_done(egc, dss, rc);
+}
+
+void libxl__xc_domain_save_done(libxl__egc *egc,
+ libxl__domain_suspend_state *dss,
+ int rc, int retval, int errnoval)
+{
+ STATE_AO_GC(dss->ao);
+
+ /* Convenience aliases */
+ const libxl_domain_type type = dss->type;
+ const uint32_t domid = dss->domid;
+
+ if (rc)
+ goto out;
+
+ if (retval) {
+ LOGEV(ERROR, errnoval, "saving domain: %s",
dss->guest_responded ?
"domain responded to suspend request" :
"domain did not respond to suspend
request");
@@ -929,16 +955,21 @@ int libxl__domain_suspend_common(libxl__gc *gc, uint32_t
domid, int fd,
rc = ERROR_GUEST_TIMEDOUT;
else
rc = ERROR_FAIL;
+ goto out;
}
- if (dss->suspend_eventchn > 0)
- xc_suspend_evtchn_release(CTX->xch, dss->xce, domid,
- dss->suspend_eventchn);
- if (dss->xce != NULL)
- xc_evtchn_close(dss->xce);
+ if (type == LIBXL_DOMAIN_TYPE_HVM) {
+ rc = libxl__domain_suspend_device_model(gc, domid);
+ if (rc) goto out;
+
+ rc = libxl__domain_save_device_model(gc, domid, dss->fd);
+ if (rc) goto out;
+ }
+
+ rc = 0;
out:
- return rc;
+ domain_suspend_done(egc, dss, rc);
}
int libxl__domain_save_device_model(libxl__gc *gc, uint32_t domid, int fd)
@@ -993,6 +1024,25 @@ out:
return rc;
}
+static void domain_suspend_done(libxl__egc *egc,
+ libxl__domain_suspend_state *dss, int rc)
+{
+ STATE_AO_GC(dss->ao);
+
+ /* 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);
+ if (dss->xce != NULL)
+ xc_evtchn_close(dss->xce);
+
+ dss->callback(egc, dss, rc);
+}
+
+/*==================== Miscellaneous ====================*/
+
char *libxl__uuid2string(libxl__gc *gc, const libxl_uuid uuid)
{
char *s = libxl__sprintf(gc, LIBXL_UUID_FMT, LIBXL_UUID_BYTES(uuid));
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index ae4b99d..63da9b0 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1778,16 +1778,27 @@ _hidden int
libxl__datacopier_start(libxl__datacopier_state *dc);
typedef struct libxl__domain_suspend_state libxl__domain_suspend_state;
+typedef void libxl__domain_suspend_cb(libxl__egc*,
+ libxl__domain_suspend_state*, int rc);
+
struct libxl__domain_suspend_state {
- libxl__gc *gc;
+ /* set by caller of libxl__domain_suspend */
+ libxl__ao *ao;
+ libxl__domain_suspend_cb *callback;
+
+ uint32_t domid;
+ int fd;
+ libxl_domain_type type;
+ int live;
+ int debug;
+ const libxl_domain_remus_info *remus;
+ /* private */
xc_evtchn *xce; /* event channel handle */
int suspend_eventchn;
- int domid;
int hvm;
- unsigned int xcflags;
int guest_responded;
- int save_fd; /* Migration stream fd (for Remus) */
int interval; /* checkpoint interval (for Remus) */
+ struct save_callbacks callbacks;
};
@@ -1904,10 +1915,28 @@ struct libxl__domain_create_state {
/*----- Domain suspend (save) functions -----*/
-_hidden int libxl__domain_suspend_common(libxl__gc *gc, uint32_t domid, int fd,
- libxl_domain_type type,
- int live, int debug,
- const libxl_domain_remus_info
*r_info);
+/* calls callback when done */
+_hidden void libxl__domain_suspend(libxl__egc *egc,
+ libxl__domain_suspend_state *dss);
+
+
+/* calls libxl__xc_domain_suspend_done when done */
+_hidden void libxl__xc_domain_save(libxl__egc*, libxl__domain_suspend_state*,
+ int xcflags, int hvm,
+ unsigned long vm_generationid_addr);
+/* If rc==0 then retval is the return value from xc_domain_save
+ * and errnoval is the errno value it provided.
+ * If rc!=0, retval and errnoval are undefined. */
+_hidden void libxl__xc_domain_save_done(libxl__egc*,
+ libxl__domain_suspend_state*,
+ int rc, int retval, int errnoval);
+
+_hidden int libxl__domain_suspend_common_callback(void *data);
+_hidden int 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,
+ uint32_t *len, void *data);
+
/* calls libxl__xc_domain_restore_done when done */
_hidden void libxl__xc_domain_restore(libxl__egc *egc,
diff --git a/tools/libxl/libxl_save_callout.c b/tools/libxl/libxl_save_callout.c
index 2f8db9f..d8defa9 100644
--- a/tools/libxl/libxl_save_callout.c
+++ b/tools/libxl/libxl_save_callout.c
@@ -35,3 +35,15 @@ void libxl__xc_domain_restore(libxl__egc *egc,
libxl__domain_create_state *dcs,
&state->vm_generationid_addr,
&dcs->callbacks);
libxl__xc_domain_restore_done(egc, dcs, 0, r, errno);
}
+
+void libxl__xc_domain_save(libxl__egc *egc, libxl__domain_suspend_state *dss,
+ int xcflags, int hvm,
+ unsigned long vm_generationid_addr)
+{
+ STATE_AO_GC(dss->ao);
+ int r;
+
+ r = xc_domain_save(CTX->xch, dss->fd, dss->domid, 0, 0, xcflags,
+ &dss->callbacks, hvm, vm_generationid_addr);
+ libxl__xc_domain_save_done(egc, dss, 0, r, errno);
+}
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 040cefc..9b4a80e 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -2820,7 +2820,7 @@ static int save_domain(const char *p, const char
*filename, int checkpoint,
save_domain_core_writeconfig(fd, filename, config_data, config_len);
- CHK_ERRNO(libxl_domain_suspend(ctx, NULL, domid, fd));
+ CHK_ERRNO(libxl_domain_suspend(ctx, domid, fd, 0, NULL));
close(fd);
if (checkpoint)
@@ -2982,7 +2982,6 @@ static void migrate_domain(const char *domain_spec, const
char *rune,
pid_t child = -1;
int rc;
int send_fd = -1, recv_fd = -1;
- libxl_domain_suspend_info suspinfo;
char *away_domname;
char rc_buf;
uint8_t *config_data;
@@ -3004,9 +3003,7 @@ static void migrate_domain(const char *domain_spec, const
char *rune,
xtl_stdiostream_adjust_flags(logger, XTL_STDIOSTREAM_HIDE_PROGRESS, 0);
- memset(&suspinfo, 0, sizeof(suspinfo));
- suspinfo.flags |= XL_SUSPEND_LIVE;
- rc = libxl_domain_suspend(ctx, &suspinfo, domid, send_fd);
+ rc = libxl_domain_suspend(ctx, domid, send_fd, LIBXL_SUSPEND_LIVE, NULL);
if (rc) {
fprintf(stderr, "migration sender: libxl_domain_suspend
failed"
" (rc=%d)\n", rc);
@@ -6623,7 +6620,7 @@ int main_remus(int argc, char **argv)
}
/* Point of no return */
- rc = libxl_domain_remus_start(ctx, &r_info, domid, send_fd, recv_fd);
+ rc = libxl_domain_remus_start(ctx, &r_info, domid, send_fd, recv_fd,
0);
/* If we are here, it means backup has failed/domain suspend failed.
* Try to resume the domain and exit gracefully.
--
1.7.2.5
Ian Jackson
2012-May-30 16:16 UTC
[PATCH 05/15] libxl: domain save/restore: run in a separate process
libxenctrl expects to be able to simply run the save or restore
operation synchronously. This won''t work well in a process which is
trying to handle multiple domains.
The options are:
- Block such a whole process (eg, the whole of libvirt) while
migration completes (or until it fails).
- Create a thread to run xc_domain_save and xc_domain_restore on.
This is quite unpalatable. Multithreaded programming is error
prone enough without generating threads in libraries, particularly
if the thread does some very complex operation.
- Fork and run the operation in the child without execing. This is
no good because we would need to negotiate with the caller about
fds we would inherit (and we might be a very large process).
- Fork and exec a helper.
Of these options the latter is the most palatable.
Consequently:
* A new helper program libxl-save-helper (which does both save and
restore). It will be installed in /usr/lib/xen/bin. It does not
link against libxl, only libxc, and its error handling does not
need to be very advanced. It does contain a plumbing through of
the logging interface into the callback stream.
* A small ad-hoc protocol between the helper and libxl which allows
log messages and the libxc callbacks to be passed up and down.
Protocol doc comment is in libxl_save_helper.c.
* To avoid a lot of tedium the marshalling boilerplate (stubs for the
helper and the callback decoder for libxl) is generated with a
small perl script.
* Implement new functionality to spawn the helper, monitor its
output, provide responses, and check on its exit status.
* The functions libxl__xc_domain_restore_done and
libxl__xc_domain_save_done now turn out to want be called in the
same place. So make their state argument a void* so that the two
functions are type compatible.
The domain save path still writes the qemu savefile synchronously.
This will need to be fixed in a subsequent patch.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Changes in v2:
* Helper path can be overridden by an environment variable for testing.
* Add a couple of debug logging messages re toolstack data.
* Fixes from testing.
* Helper protocol message lengths (and numbers) are 16-bit which
more clearly avoids piling lots of junk on the stack.
* Merged with remus changes.
* Callback implementations in libxl now called via pointers
so remus can have its own callbacks.
* Better namespace prefixes on autogenerated names etc.
* Autogenerator can generate debugging printfs too.
---
.gitignore | 1 +
.hgignore | 2 +
tools/libxl/Makefile | 21 ++-
tools/libxl/libxl_create.c | 22 ++-
tools/libxl/libxl_dom.c | 36 ++--
tools/libxl/libxl_internal.h | 55 +++++-
tools/libxl/libxl_save_callout.c | 332 +++++++++++++++++++++++++++++-
tools/libxl/libxl_save_helper.c | 279 +++++++++++++++++++++++++
tools/libxl/libxl_save_msgs_gen.pl | 393 ++++++++++++++++++++++++++++++++++++
9 files changed, 1103 insertions(+), 38 deletions(-)
create mode 100644 tools/libxl/libxl_save_helper.c
create mode 100755 tools/libxl/libxl_save_msgs_gen.pl
diff --git a/.gitignore b/.gitignore
index 7770e54..3451e52 100644
--- a/.gitignore
+++ b/.gitignore
@@ -353,6 +353,7 @@ tools/libxl/_*.[ch]
tools/libxl/testidl
tools/libxl/testidl.c
tools/libxl/*.pyc
+tools/libxl/libxl-save-helper
tools/blktap2/control/tap-ctl
tools/firmware/etherboot/eb-roms.h
tools/firmware/etherboot/gpxe-git-snapshot.tar.gz
diff --git a/.hgignore b/.hgignore
index 27d8f79..05304ea 100644
--- a/.hgignore
+++ b/.hgignore
@@ -180,9 +180,11 @@
^tools/libxl/_.*\.c$
^tools/libxl/libxlu_cfg_y\.output$
^tools/libxl/xl$
+^tools/libxl/libxl-save-helper$
^tools/libxl/testidl$
^tools/libxl/testidl\.c$
^tools/libxl/tmp\..*$
+^tools/libxl/.*\.new$
^tools/libvchan/vchan-node[12]$
^tools/libaio/src/.*\.ol$
^tools/libaio/src/.*\.os$
diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index 9b84421..e11354f 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -66,25 +66,30 @@ LIBXL_OBJS = flexarray.o libxl.o libxl_create.o libxl_dm.o
libxl_pci.o \
libxl_dom.o libxl_exec.o libxl_xshelp.o libxl_device.o \
libxl_internal.o libxl_utils.o libxl_uuid.o \
libxl_json.o libxl_aoutils.o \
- libxl_save_callout.o \
+ libxl_save_callout.o _libxl_save_msgs_callout.o \
libxl_qmp.o libxl_event.o libxl_fork.o $(LIBXL_OBJS-y)
LIBXL_OBJS += _libxl_types.o libxl_flask.o _libxl_types_internal.o
$(LIBXL_OBJS): CFLAGS += $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest)
$(CFLAGS_libxenstore) $(CFLAGS_libblktapctl) -include $(XEN_ROOT)/tools/config.h
-AUTOINCS= libxlu_cfg_y.h libxlu_cfg_l.h _libxl_list.h
+AUTOINCS= libxlu_cfg_y.h libxlu_cfg_l.h _libxl_list.h \
+ _libxl_save_msgs_callout.h _libxl_save_msgs_helper.h
AUTOSRCS= libxlu_cfg_y.c libxlu_cfg_l.c
+AUTOSRCS += _libxl_save_msgs_callout.c _libxl_save_msgs_helper.c
LIBXLU_OBJS = libxlu_cfg_y.o libxlu_cfg_l.o libxlu_cfg.o \
libxlu_disk_l.o libxlu_disk.o libxlu_vif.o libxlu_pci.o
$(LIBXLU_OBJS): CFLAGS += $(CFLAGS_libxenctrl) # For xentoollog.h
-CLIENTS = xl testidl
+CLIENTS = xl testidl libxl-save-helper
XL_OBJS = xl.o xl_cmdimpl.o xl_cmdtable.o xl_sxp.o
$(XL_OBJS): CFLAGS += $(CFLAGS_libxenctrl) # For xentoollog.h
$(XL_OBJS): CFLAGS += $(CFLAGS_libxenlight)
$(XL_OBJS): CFLAGS += -include $(XEN_ROOT)/tools/config.h # libxl_json.h needs
it.
+SAVE_HELPER_OBJS = libxl_save_helper.o _libxl_save_msgs_helper.o
+$(SAVE_HELPER_OBJS): CFLAGS += $(CFLAGS_libxenctrl) $(CFLAGS_libxenlight)
+
testidl.o: CFLAGS += $(CFLAGS_libxenctrl) $(CFLAGS_libxenlight)
testidl.c: libxl_types.idl gentest.py libxl.h $(AUTOINCS)
$(PYTHON) gentest.py libxl_types.idl testidl.c.new
@@ -116,6 +121,12 @@ _libxl_list.h:
$(XEN_INCLUDE)/xen-external/bsd-sys-queue-h-seddery $(XEN_INCLUDE
perl $^ --prefix=libxl >$@.new
$(call move-if-changed,$@.new,$@)
+_libxl_save_msgs_helper.c _libxl_save_msgs_callout.c \
+_libxl_save_msgs_helper.h _libxl_save_msgs_callout.h: \
+ libxl_save_msgs_gen.pl
+ $(PERL) -w $< $@ >$@.new
+ $(call move-if-changed,$@.new,$@)
+
libxl.h: _libxl_types.h
libxl_json.h: _libxl_types_json.h
libxl_internal.h: _libxl_types_internal.h _libxl_paths.h
@@ -157,6 +168,9 @@ libxlutil.a: $(LIBXLU_OBJS)
xl: $(XL_OBJS) libxlutil.so libxenlight.so
$(CC) $(LDFLAGS) -o $@ $(XL_OBJS) libxlutil.so $(LDLIBS_libxenlight)
$(LDLIBS_libxenctrl) -lyajl $(APPEND_LDFLAGS)
+libxl-save-helper: $(SAVE_HELPER_OBJS) libxenlight.so
+ $(CC) $(LDFLAGS) -o $@ $(SAVE_HELPER_OBJS) $(LDLIBS_libxenctrl)
$(LDLIBS_libxenguest) $(APPEND_LDFLAGS)
+
testidl: testidl.o libxlutil.so libxenlight.so
$(CC) $(LDFLAGS) -o $@ testidl.o libxlutil.so $(LDLIBS_libxenlight)
$(LDLIBS_libxenctrl) $(APPEND_LDFLAGS)
@@ -168,6 +182,7 @@ install: all
$(INSTALL_DIR) $(DESTDIR)$(BASH_COMPLETION_DIR)
$(INSTALL_DIR) $(DESTDIR)$(XEN_RUN_DIR)
$(INSTALL_PROG) xl $(DESTDIR)$(SBINDIR)
+ $(INSTALL_PROG) libxl-save-helper $(DESTDIR)$(PRIVATE_BINDIR)
$(INSTALL_PROG) libxenlight.so.$(MAJOR).$(MINOR) $(DESTDIR)$(LIBDIR)
ln -sf libxenlight.so.$(MAJOR).$(MINOR)
$(DESTDIR)$(LIBDIR)/libxenlight.so.$(MAJOR)
ln -sf libxenlight.so.$(MAJOR) $(DESTDIR)$(LIBDIR)/libxenlight.so
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 44455f9..eaf378b 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -600,7 +600,8 @@ static void domcreate_bootloader_done(libxl__egc *egc,
libxl_domain_build_info *const info = &d_config->b_info;
const int restore_fd = dcs->restore_fd;
libxl__domain_build_state *const state = &dcs->build_state;
- struct restore_callbacks *const callbacks = &dcs->callbacks;
+ libxl__srm_restore_autogen_callbacks *const callbacks +
&dcs->shs.callbacks.restore.a;
if (rc) domcreate_rebuild_done(egc, dcs, rc);
@@ -640,7 +641,6 @@ static void domcreate_bootloader_done(libxl__egc *egc,
pae = libxl_defbool_val(info->u.hvm.pae);
no_incr_generationid =
!libxl_defbool_val(info->u.hvm.incr_generationid);
callbacks->toolstack_restore = libxl__toolstack_restore;
- callbacks->data = gc;
break;
case LIBXL_DOMAIN_TYPE_PV:
hvm = 0;
@@ -660,10 +660,24 @@ static void domcreate_bootloader_done(libxl__egc *egc,
libxl__xc_domain_restore_done(egc, dcs, rc, 0, 0);
}
-void libxl__xc_domain_restore_done(libxl__egc *egc,
- libxl__domain_create_state *dcs,
+void libxl__srm_callout_callback_restore_results(unsigned long store_mfn,
+ unsigned long console_mfn, unsigned long genidad, void *user)
+{
+ libxl__save_helper_state *shs = user;
+ libxl__domain_create_state *dcs = CONTAINER_OF(shs, *dcs, shs);
+ STATE_AO_GC(dcs->ao);
+ libxl__domain_build_state *const state = &dcs->build_state;
+
+ state->store_mfn = store_mfn;
+ state->console_mfn = console_mfn;
+ state->vm_generationid_addr = genidad;
+ shs->need_results = 0;
+}
+
+void libxl__xc_domain_restore_done(libxl__egc *egc, void *dcs_void,
int ret, int retval, int errnoval)
{
+ libxl__domain_create_state *dcs = dcs_void;
STATE_AO_GC(dcs->ao);
libxl_ctx *ctx = libxl__gc_owner(gc);
char **vments = NULL, **localents = NULL;
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 8cac6d5..716cf4b 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -463,16 +463,20 @@ static inline char *restore_helper(libxl__gc *gc, uint32_t
domid,
}
int libxl__toolstack_restore(uint32_t domid, const uint8_t *buf,
- uint32_t size, void *data)
+ uint32_t size, void *user)
{
- libxl__gc *gc = data;
- libxl_ctx *ctx = gc->owner;
+ libxl__save_helper_state *shs = user;
+ libxl__domain_create_state *dcs = CONTAINER_OF(shs, *dcs, shs);
+ STATE_AO_GC(dcs->ao);
+ libxl_ctx *ctx = CTX;
int i, ret;
const uint8_t *ptr = buf;
uint32_t count = 0, version = 0;
struct libxl__physmap_info* pi;
char *xs_path;
+ LOG(DEBUG,"domain=%"PRIu32" toolstack data
size=%"PRIu32, domid, size);
+
if (size < sizeof(version) + sizeof(count)) {
LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "wrong size");
return -1;
@@ -525,9 +529,10 @@ static void domain_suspend_done(libxl__egc *egc,
/*----- callbacks, called by xc_domain_save -----*/
int libxl__domain_suspend_common_switch_qemu_logdirty
- (int domid, unsigned int enable, void *data)
+ (int domid, unsigned enable, void *user)
{
- libxl__domain_suspend_state *dss = data;
+ libxl__save_helper_state *shs = user;
+ libxl__domain_suspend_state *dss = CONTAINER_OF(shs, *dss, shs);
STATE_AO_GC(dss->ao);
char *path;
bool rc;
@@ -593,9 +598,10 @@ int libxl__domain_resume_device_model(libxl__gc *gc,
uint32_t domid)
return 0;
}
-int libxl__domain_suspend_common_callback(void *data)
+int libxl__domain_suspend_common_callback(void *user)
{
- libxl__domain_suspend_state *dss = data;
+ 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;
@@ -735,9 +741,9 @@ static inline char *save_helper(libxl__gc *gc, uint32_t
domid,
}
int libxl__toolstack_save(uint32_t domid, uint8_t **buf,
- uint32_t *len, void *data)
+ uint32_t *len, void *dss_void)
{
- libxl__domain_suspend_state *dss = data;
+ libxl__domain_suspend_state *dss = dss_void;
STATE_AO_GC(dss->ao);
int i = 0;
char *start_addr = NULL, *size = NULL, *phys_offset = NULL, *name = NULL;
@@ -806,6 +812,8 @@ int libxl__toolstack_save(uint32_t domid, uint8_t **buf,
ptr += sizeof(struct libxl__physmap_info) + namelen;
}
+ LOG(DEBUG,"domain=%"PRIu32" toolstack data
size=%"PRIu32, domid, *len);
+
return 0;
}
@@ -861,7 +869,8 @@ void libxl__domain_suspend(libxl__egc *egc,
libxl__domain_suspend_state *dss)
const int live = dss->live;
const int debug = dss->debug;
const libxl_domain_remus_info *const r_info = dss->remus;
- struct save_callbacks *const callbacks = &dss->callbacks;
+ libxl__srm_save_autogen_callbacks *const callbacks +
&dss->shs.callbacks.save.a;
switch (type) {
case LIBXL_DOMAIN_TYPE_HVM: {
@@ -923,8 +932,7 @@ void libxl__domain_suspend(libxl__egc *egc,
libxl__domain_suspend_state *dss)
callbacks->suspend = libxl__domain_suspend_common_callback;
callbacks->switch_qemu_logdirty =
libxl__domain_suspend_common_switch_qemu_logdirty;
- callbacks->toolstack_save = libxl__toolstack_save;
- callbacks->data = dss;
+ dss->shs.callbacks.save.toolstack_save = libxl__toolstack_save;
libxl__xc_domain_save(egc, dss, xcflags, hvm, vm_generationid_addr);
return;
@@ -933,10 +941,10 @@ void libxl__domain_suspend(libxl__egc *egc,
libxl__domain_suspend_state *dss)
domain_suspend_done(egc, dss, rc);
}
-void libxl__xc_domain_save_done(libxl__egc *egc,
- libxl__domain_suspend_state *dss,
+void libxl__xc_domain_save_done(libxl__egc *egc, void *dss_void,
int rc, int retval, int errnoval)
{
+ libxl__domain_suspend_state *dss = dss_void;
STATE_AO_GC(dss->ao);
/* Convenience aliases */
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 63da9b0..e4c3f34 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -54,6 +54,7 @@
#include "libxl.h"
#include "_libxl_paths.h"
+#include "_libxl_save_msgs_callout.h"
#if __GNUC__ > 3 || (__GNUC__ == 3 && __GNUC_MINOR__ >= 1)
#define _hidden __attribute__((visibility("hidden")))
@@ -1774,6 +1775,50 @@ _hidden void
libxl__datacopier_kill(libxl__datacopier_state *dc);
_hidden int libxl__datacopier_start(libxl__datacopier_state *dc);
+/*----- Save/restore helper (used by creation and suspend) -----*/
+
+typedef struct libxl__srm_save_callbacks {
+ libxl__srm_save_autogen_callbacks a;
+ xc_toolstack_save_cb *toolstack_save;
+} libxl__srm_save_callbacks;
+
+typedef struct libxl__srm_restore_callbacks {
+ libxl__srm_restore_autogen_callbacks a;
+} libxl__srm_restore_callbacks;
+
+/* a pointer to this struct is also passed as "user" to the
+ * save callout helper callback functions */
+typedef struct libxl__save_helper_state {
+ /* public, caller of run_helper initialises */
+ libxl__ao *ao;
+ uint32_t domid;
+ union {
+ libxl__srm_save_callbacks save;
+ libxl__srm_restore_callbacks restore;
+ } callbacks;
+ int (*recv_callback)(const unsigned char *msg, uint32_t len, void *user);
+ void (*completion_callback)(libxl__egc *egc, void *caller_state,
+ int rc, int retval, int errnoval);
+ void *caller_state;
+ int need_results; /* set to 0 or 1 by caller of run_helper;
+ * if set to 1 then the ultimate caller''s
+ * results function must set it to 0 */
+ /* private */
+ int rc;
+ int completed; /* retval/errnoval valid iff completed */
+ int retval, errnoval; /* from xc_domain_save / xc_domain_restore */
+ libxl__carefd *pipes[2]; /* 0 = helper''s stdin, 1 =
helper''s stdout */
+ libxl__ev_fd readable;
+ libxl__ev_child child;
+ const char *stdin_what, *stdout_what;
+ FILE *toolstack_data_file;
+
+ libxl__egc *egc; /* valid only for duration of each event callback;
+ * is here in this struct for the benefit of the
+ * marshalling and xc callback functions */
+} libxl__save_helper_state;
+
+
/*----- Domain suspend (save) state structure -----*/
typedef struct libxl__domain_suspend_state libxl__domain_suspend_state;
@@ -1798,7 +1843,7 @@ struct libxl__domain_suspend_state {
int hvm;
int guest_responded;
int interval; /* checkpoint interval (for Remus) */
- struct save_callbacks callbacks;
+ libxl__save_helper_state shs;
};
@@ -1910,7 +1955,7 @@ struct libxl__domain_create_state {
libxl__stub_dm_spawn_state dmss;
/* If we''re not doing stubdom, we use only dmss.dm,
* for the non-stubdom device model. */
- struct restore_callbacks callbacks;
+ libxl__save_helper_state shs;
};
/*----- Domain suspend (save) functions -----*/
@@ -1927,8 +1972,7 @@ _hidden void libxl__xc_domain_save(libxl__egc*,
libxl__domain_suspend_state*,
/* If rc==0 then retval is the return value from xc_domain_save
* and errnoval is the errno value it provided.
* If rc!=0, retval and errnoval are undefined. */
-_hidden void libxl__xc_domain_save_done(libxl__egc*,
- libxl__domain_suspend_state*,
+_hidden void libxl__xc_domain_save_done(libxl__egc*, void *dss_void,
int rc, int retval, int errnoval);
_hidden int libxl__domain_suspend_common_callback(void *data);
@@ -1946,8 +1990,7 @@ _hidden void libxl__xc_domain_restore(libxl__egc *egc,
/* If rc==0 then retval is the return value from xc_domain_save
* and errnoval is the errno value it provided.
* If rc!=0, retval and errnoval are undefined. */
-_hidden void libxl__xc_domain_restore_done(libxl__egc *egc,
- libxl__domain_create_state *dcs,
+_hidden void libxl__xc_domain_restore_done(libxl__egc *egc, void *dcs_void,
int rc, int retval, int errnoval);
diff --git a/tools/libxl/libxl_save_callout.c b/tools/libxl/libxl_save_callout.c
index d8defa9..ff7dfb6 100644
--- a/tools/libxl/libxl_save_callout.c
+++ b/tools/libxl/libxl_save_callout.c
@@ -16,6 +16,19 @@
#include "libxl_internal.h"
+static void run_helper(libxl__egc *egc, libxl__save_helper_state *shs,
+ const char *mode_arg, int data_fd,
+ const unsigned long *argnums, int num_argnums);
+
+static void helper_failed(libxl__egc*, libxl__save_helper_state *shs, int rc);
+static void helper_stdout_readable(libxl__egc *egc, libxl__ev_fd *ev,
+ int fd, short events, short revents);
+static void helper_exited(libxl__egc *egc, libxl__ev_child *ch,
+ pid_t pid, int status);
+static void helper_done(libxl__egc *egc, libxl__save_helper_state *shs);
+
+/*----- entrypoints -----*/
+
void libxl__xc_domain_restore(libxl__egc *egc, libxl__domain_create_state *dcs,
int hvm, int pae, int superpages,
int no_incr_generationid)
@@ -27,13 +40,28 @@ void libxl__xc_domain_restore(libxl__egc *egc,
libxl__domain_create_state *dcs,
const int restore_fd = dcs->restore_fd;
libxl__domain_build_state *const state = &dcs->build_state;
- int r = xc_domain_restore(CTX->xch, restore_fd, domid,
- state->store_port, &state->store_mfn,
- state->store_domid, state->console_port,
- &state->console_mfn,
state->console_domid,
- hvm, pae, superpages, no_incr_generationid,
- &state->vm_generationid_addr,
&dcs->callbacks);
- libxl__xc_domain_restore_done(egc, dcs, 0, r, errno);
+ unsigned cbflags = libxl__srm_callout_enumcallbacks_restore
+ (&dcs->shs.callbacks.restore.a);
+
+ const unsigned long argnums[] = {
+ restore_fd, domid,
+ state->store_port,
+ state->store_domid, state->console_port,
+ state->console_domid,
+ hvm, pae, superpages, no_incr_generationid,
+ cbflags,
+ };
+
+ dcs->shs.ao = ao;
+ dcs->shs.domid = domid;
+ dcs->shs.recv_callback = libxl__srm_callout_received_restore;
+ dcs->shs.completion_callback = libxl__xc_domain_restore_done;
+ dcs->shs.caller_state = dcs;
+ dcs->shs.need_results = 1;
+ dcs->shs.toolstack_data_file = 0;
+
+ run_helper(egc, &dcs->shs, "--restore-domain", restore_fd,
+ argnums, ARRAY_SIZE(argnums));
}
void libxl__xc_domain_save(libxl__egc *egc, libxl__domain_suspend_state *dss,
@@ -41,9 +69,291 @@ void libxl__xc_domain_save(libxl__egc *egc,
libxl__domain_suspend_state *dss,
unsigned long vm_generationid_addr)
{
STATE_AO_GC(dss->ao);
- int r;
+ int r, rc;
+ uint32_t toolstack_data_len;
+
+ /* Resources we need to free */
+ uint8_t *toolstack_data_buf = 0;
+
+ unsigned cbflags = libxl__srm_callout_enumcallbacks_save
+ (&dss->shs.callbacks.save.a);
+
+ r = libxl__toolstack_save(dss->domid, &toolstack_data_buf,
+ &toolstack_data_len, dss);
+ if (r) { rc = ERROR_FAIL; goto out; }
+
+ dss->shs.toolstack_data_file = tmpfile();
+ if (!dss->shs.toolstack_data_file) {
+ LOGE(ERROR, "cannot create toolstack data tmpfile");
+ rc = ERROR_FAIL;
+ goto out;
+ }
+ int toolstack_data_fd = fileno(dss->shs.toolstack_data_file);
+
+ r = libxl_write_exactly(CTX, toolstack_data_fd,
+ toolstack_data_buf, toolstack_data_len,
+ "toolstack data tmpfile", 0);
+ if (r) { rc = ERROR_FAIL; goto out; }
+
+ r = lseek(toolstack_data_fd, 0, SEEK_SET);
+ if (r) {
+ LOGE(ERROR, "rewind toolstack data tmpfile");
+ rc = ERROR_FAIL;
+ goto out;
+ }
+
+ const unsigned long argnums[] = {
+ dss->fd, dss->domid, 0, 0, xcflags, hvm, vm_generationid_addr,
+ toolstack_data_fd, toolstack_data_len,
+ cbflags,
+ };
+
+ dss->shs.ao = ao;
+ dss->shs.domid = dss->domid;
+ dss->shs.recv_callback = libxl__srm_callout_received_save;
+ dss->shs.completion_callback = libxl__xc_domain_save_done;
+ dss->shs.caller_state = dss;
+ dss->shs.need_results = 0;
+
+ free(toolstack_data_buf);
+
+ run_helper(egc, &dss->shs, "--save-domain", dss->fd,
+ argnums, ARRAY_SIZE(argnums));
+ return;
+
+ out:
+ free(toolstack_data_buf);
+ if (dss->shs.toolstack_data_file)
fclose(dss->shs.toolstack_data_file);
+
+ libxl__xc_domain_save_done(egc, dss, rc, 0, 0);
+}
+
+
+/*----- helper execution -----*/
+
+static void run_helper(libxl__egc *egc, libxl__save_helper_state *shs,
+ const char *mode_arg, int data_fd,
+ const unsigned long *argnums, int num_argnums)
+{
+ STATE_AO_GC(shs->ao);
+ const char *args[3 + num_argnums];
+ const char **arg = args;
+ int i, rc;
+
+ /* Resources we must free */
+ libxl__carefd *childs_pipes[2] = { 0,0 };
+
+ /* Convenience aliases */
+ const uint32_t domid = shs->domid;
+
+ shs->rc = 0;
+ shs->completed = 0;
+ shs->pipes[0] = shs->pipes[1] = 0;
+ libxl__ev_fd_init(&shs->readable);
+ libxl__ev_child_init(&shs->child);
+
+ shs->stdin_what = GCSPRINTF("domain %"PRIu32"
save/restore helper"
+ " stdin pipe", domid);
+ shs->stdout_what = GCSPRINTF("domain %"PRIu32"
save/restore helper"
+ " stdout pipe", domid);
+
+ *arg++ = getenv("LIBXL_SAVE_HELPER") ?: LIBEXEC "/"
"libxl-save-helper";
+ *arg++ = mode_arg;
+ for (i=0; i<num_argnums; i++)
+ *arg++ = GCSPRINTF("%lu", argnums[i]);
+ *arg++ = 0;
+ assert(arg == args + ARRAY_SIZE(args));
+
+ libxl__carefd_begin();
+ for (i=0; i<2; i++) {
+ int fds[2];
+ if (libxl_pipe(CTX,fds)) { rc = ERROR_FAIL; goto out; }
+ childs_pipes[i] = libxl__carefd_record(CTX, fds[i]);
+ shs->pipes[i] = libxl__carefd_record(CTX, fds[!i]);
+ }
+ libxl__carefd_unlock();
+
+ pid_t pid = libxl__ev_child_fork(gc, &shs->child, helper_exited);
+ if (!pid) {
+ libxl_fd_set_cloexec(CTX, data_fd, 0);
+ libxl__exec(gc,
+ libxl__carefd_fd(childs_pipes[0]),
+ libxl__carefd_fd(childs_pipes[1]),
+ -1,
+ args[0], (char**)args, 0);
+ }
+
+ libxl__carefd_close(childs_pipes[0]);
+ libxl__carefd_close(childs_pipes[1]);
+
+ rc = libxl__ev_fd_register(gc, &shs->readable,
helper_stdout_readable,
+ libxl__carefd_fd(shs->pipes[1]),
POLLIN|POLLPRI);
+ if (rc) goto out;
+ return;
+
+ out:
+ libxl__carefd_close(childs_pipes[0]);
+ libxl__carefd_close(childs_pipes[1]);
+ helper_failed(egc, shs, rc);;
+}
+
+static void helper_failed(libxl__egc *egc, libxl__save_helper_state *shs,
+ int rc)
+{
+ STATE_AO_GC(shs->ao);
+
+ if (!shs->rc)
+ shs->rc = rc;
+
+ libxl__ev_fd_deregister(gc, &shs->readable);
+
+ if (!libxl__ev_child_inuse(&shs->child)) {
+ helper_done(egc, shs);
+ return;
+ }
+
+ int r = kill(shs->child.pid, SIGKILL);
+ if (r) LOGE(WARN, "failed to kill save/restore helper [%lu]",
+ (unsigned long)shs->child.pid);
+}
+
+static void helper_stdout_readable(libxl__egc *egc, libxl__ev_fd *ev,
+ int fd, short events, short revents)
+{
+ libxl__save_helper_state *shs = CONTAINER_OF(ev, *shs, readable);
+ STATE_AO_GC(shs->ao);
+ int rc, errnoval;
+
+ if (revents & POLLPRI) {
+ LOG(ERROR, "%s signaled POLLERR", shs->stdout_what);
+ rc = ERROR_FAIL;
+ out:
+ /* this is here because otherwise we bypass the decl of msg[] */
+ helper_failed(egc, shs, rc);
+ return;
+ }
+
+ uint16_t msglen;
+ errnoval = libxl_read_exactly(CTX, fd, &msglen, sizeof(msglen),
+ shs->stdout_what, "ipc msg
header");
+ if (errnoval) { rc = ERROR_FAIL; goto out; }
+
+ unsigned char msg[msglen];
+ errnoval = libxl_read_exactly(CTX, fd, msg, msglen,
+ shs->stdout_what, "ipc msg
body");
+ if (errnoval) { rc = ERROR_FAIL; goto out; }
+
+ shs->egc = egc;
+ shs->recv_callback(msg, msglen, shs);
+ shs->egc = 0;
+ return;
+}
+
+static void helper_exited(libxl__egc *egc, libxl__ev_child *ch,
+ pid_t pid, int status)
+{
+ libxl__save_helper_state *shs = CONTAINER_OF(ch, *shs, child);
+ STATE_AO_GC(shs->ao);
+
+ /* Convenience aliases */
+ const uint32_t domid = shs->domid;
+
+ const char *what + GCSPRINTF("domain %"PRIu32"
save/restore helper", domid);
+
+ if (status) {
+ libxl_report_child_exitstatus(CTX, XTL_ERROR, what, pid, status);
+ shs->rc = ERROR_FAIL;
+ }
+
+ if (shs->need_results) {
+ if (!shs->rc)
+ LOG(ERROR,"%s exited without providing results",what);
+ shs->rc = ERROR_FAIL;
+ }
+
+ if (!shs->completed) {
+ if (!shs->rc)
+ LOG(ERROR,"%s exited without signaling completion",what);
+ shs->rc = ERROR_FAIL;
+ }
+
+ helper_done(egc, shs);
+ return;
+}
+
+static void helper_done(libxl__egc *egc, libxl__save_helper_state *shs)
+{
+ STATE_AO_GC(shs->ao);
+
+ libxl__ev_fd_deregister(gc, &shs->readable);
+ libxl__carefd_close(shs->pipes[0]); shs->pipes[0] = 0;
+ libxl__carefd_close(shs->pipes[1]); shs->pipes[1] = 0;
+ assert(!libxl__ev_child_inuse(&shs->child));
+ if (shs->toolstack_data_file) fclose(shs->toolstack_data_file);
+
+ shs->egc = egc;
+ shs->completion_callback(egc, shs->caller_state,
+ shs->rc, shs->retval, shs->errnoval);
+ shs->egc = 0;
+}
+
+/*----- generic helpers for the autogenerated code -----*/
+
+const libxl__srm_save_autogen_callbacks*
+libxl__srm_callout_get_callbacks_save(void *user)
+{
+ libxl__save_helper_state *shs = user;
+ return &shs->callbacks.save.a;
+}
+
+const libxl__srm_restore_autogen_callbacks*
+libxl__srm_callout_get_callbacks_restore(void *user)
+{
+ libxl__save_helper_state *shs = user;
+ return &shs->callbacks.restore.a;
+}
+
+void libxl__srm_callout_sendreply(int r, void *user)
+{
+ libxl__save_helper_state *shs = user;
+ libxl__egc *egc = shs->egc;
+ STATE_AO_GC(shs->ao);
+ int errnoval;
+
+ errnoval = libxl_write_exactly(CTX, libxl__carefd_fd(shs->pipes[0]),
+ &r, sizeof(r), shs->stdin_what,
+ "callback return value");
+ if (errnoval)
+ helper_failed(egc, shs, ERROR_FAIL);
+}
+
+void libxl__srm_callout_callback_log(uint32_t level, uint32_t errnoval,
+ const char *context, const char *formatted, void *user)
+{
+ libxl__save_helper_state *shs = user;
+ STATE_AO_GC(shs->ao);
+ xtl_log(CTX->lg, level, errnoval, context, "%s", formatted);
+}
+
+void libxl__srm_callout_callback_progress(const char *context,
+ const char *doing_what, unsigned long done,
+ unsigned long total, void *user)
+{
+ libxl__save_helper_state *shs = user;
+ STATE_AO_GC(shs->ao);
+ xtl_progress(CTX->lg, context, doing_what, done, total);
+}
+
+int libxl__srm_callout_callback_complete(int retval, int errnoval,
+ void *user)
+{
+ libxl__save_helper_state *shs = user;
+ STATE_AO_GC(shs->ao);
- r = xc_domain_save(CTX->xch, dss->fd, dss->domid, 0, 0, xcflags,
- &dss->callbacks, hvm, vm_generationid_addr);
- libxl__xc_domain_save_done(egc, dss, 0, r, errno);
+ shs->completed = 1;
+ shs->retval = retval;
+ shs->errnoval = errnoval;
+ libxl__ev_fd_deregister(gc, &shs->readable);
+ return 0;
}
diff --git a/tools/libxl/libxl_save_helper.c b/tools/libxl/libxl_save_helper.c
new file mode 100644
index 0000000..0481ebf
--- /dev/null
+++ b/tools/libxl/libxl_save_helper.c
@@ -0,0 +1,279 @@
+/*
+ * Copyright (C) 2012 Citrix Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; version 2.1 only. with the special
+ * exception on linking described in file LICENSE.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Lesser General Public License for more details.
+ */
+
+/*
+ * The libxl-save-helper utility speaks a protocol to its caller for
+ * the callbacks. The protocol is as follows.
+ *
+ * The helper talks on stdin and stdout, in binary in machine
+ * endianness. The helper speaks first, and only when it has a
+ * callback to make. It writes a 16-bit number being the message
+ * length, and then the message body.
+ *
+ * Each message starts with a 16-bit number indicating which of the
+ * messages it is, and then some arguments in a binary marshalled form.
+ * If the callback does not need a reply (it returns void), the helper
+ * just continues. Otherwise the helper waits for its caller to send a
+ * single int which is to be the return value from the callback.
+ *
+ * Where feasible the stubs and callbacks have prototypes identical to
+ * those required by xc_domain_save and xc_domain_restore, so that the
+ * autogenerated functions can be used/provided directly.
+ *
+ * The actual messages are in the array @msgs in libxl_save_msgs_gen.pl
+ */
+
+#include "libxl_osdeps.h"
+
+#include <stdlib.h>
+#include <unistd.h>
+#include <assert.h>
+#include <inttypes.h>
+
+#include "libxl.h"
+
+#include "xenctrl.h"
+#include "xenguest.h"
+#include "_libxl_save_msgs_helper.h"
+
+/*----- globals -----*/
+
+static const char *program = "libxl-save-helper";
+static xentoollog_logger *logger;
+static xc_interface *xch;
+
+/*----- error handling -----*/
+
+static void fail(int errnoval, const char *fmt, ...)
+ __attribute__((noreturn,format(printf,2,3)));
+static void fail(int errnoval, const char *fmt, ...)
+{
+ va_list al;
+ va_start(al,fmt);
+ xtl_logv(logger,XTL_ERROR,errnoval,program,fmt,al);
+ exit(-1);
+}
+
+static int read_exactly(int fd, void *buf, size_t len)
+/* returns 0 if we get eof, even if we got it midway through; 1 if ok */
+{
+ while (len) {
+ ssize_t r = read(fd, buf, len);
+ if (r<=0) return r;
+ assert(r <= len);
+ len -= r;
+ buf = (char*)buf + r;
+ }
+ return 1;
+}
+
+static void *xmalloc(size_t sz)
+{
+ if (!sz) return 0;
+ void *r = malloc(sz);
+ if (!r) { perror("memory allocation failed"); exit(-1); }
+ return r;
+}
+
+/*----- logger -----*/
+
+typedef struct {
+ xentoollog_logger vtable;
+} xentoollog_logger_tellparent;
+
+static void tellparent_vmessage(xentoollog_logger *logger_in,
+ xentoollog_level level,
+ int errnoval,
+ const char *context,
+ const char *format,
+ va_list al)
+{
+ char *formatted;
+ int r = vasprintf(&formatted, format, al);
+ if (r < 0) { perror("memory allocation failed during
logging"); exit(-1); }
+ helper_stub_log(level, errnoval, context, formatted, 0);
+ free(formatted);
+}
+
+static void tellparent_progress(struct xentoollog_logger *logger_in,
+ const char *context,
+ const char *doing_what, int percent,
+ unsigned long done, unsigned long total)
+{
+ helper_stub_progress(context, doing_what, done, total, 0);
+}
+
+static void tellparent_destroy(struct xentoollog_logger *logger_in)
+{
+ abort();
+}
+
+static xentoollog_logger_tellparent *createlogger_tellparent(void)
+{
+ xentoollog_logger_tellparent newlogger;
+ return XTL_NEW_LOGGER(tellparent, newlogger);
+}
+
+/*----- helper functions called by autogenerated stubs -----*/
+
+unsigned char * helper_allocbuf(int len, void *user)
+{
+ return xmalloc(len);
+}
+
+static void transmit(const unsigned char *msg, int len, void *user)
+{
+ while (len) {
+ int r = write(1, msg, len);
+ if (r<0) { perror("write"); exit(-1); }
+ assert(r >= 0);
+ assert(r <= len);
+ len -= r;
+ msg += r;
+ }
+}
+
+void helper_transmitmsg(unsigned char *msg_freed, int len_in, void *user)
+{
+ assert(len_in < 64*1024);
+ uint16_t len = len_in;
+ transmit((const void*)&len, sizeof(len), user);
+ transmit(msg_freed, len, user);
+ free(msg_freed);
+}
+
+int helper_getreply(void *user)
+{
+ int v;
+ int r = read_exactly(0, &v, sizeof(v));
+ if (r<=0) exit(-2);
+ return v;
+}
+
+/*----- other callbacks -----*/
+
+static int toolstack_save_fd;
+static uint32_t toolstack_save_len;
+
+static int toolstack_save_cb(uint32_t domid, uint8_t **buf,
+ uint32_t *len, void *data)
+{
+ assert(toolstack_save_fd > 0);
+
+ *buf = xmalloc(toolstack_save_len);
+ int r = read_exactly(toolstack_save_fd, *buf, toolstack_save_len);
+ if (r<0) fail(errno,"read toolstack data");
+ if (r==0) fail(0,"read toolstack data eof");
+
+ toolstack_save_fd = -1;
+ *len = toolstack_save_len;
+ return 0;
+}
+
+static void startup(const char *op) {
+ logger = (xentoollog_logger*)createlogger_tellparent();
+ if (!logger) {
+ fprintf(stderr, "%s: cannot initialise logger\n", program);
+ exit(-1);
+ }
+
+ xtl_log(logger,XTL_DEBUG,0,program,"starting %s",op);
+
+ xch = xc_interface_open(logger,logger,0);
+ if (!xch) fail(errno,"xc_interface_open failed");
+}
+
+static void complete(int retval) {
+ int errnoval = errno;
+ xtl_log(logger,XTL_DEBUG,errnoval,program,"complete
r=%d",retval);
+ helper_stub_complete(retval,errnoval,0);
+ exit(0);
+}
+
+static struct save_callbacks helper_save_callbacks;
+static struct restore_callbacks helper_restore_callbacks;
+
+int main(int argc, char **argv)
+{
+ int r;
+
+#define NEXTARG (++argv, assert(*argv), *argv)
+
+ const char *mode = *++argv;
+ assert(mode);
+
+ if (!strcmp(mode,"--save-domain")) {
+
+ int io_fd = atoi(NEXTARG);
+ uint32_t dom = strtoul(NEXTARG,0,10);
+ uint32_t max_iters = strtoul(NEXTARG,0,10);
+ uint32_t max_factor = strtoul(NEXTARG,0,10);
+ uint32_t flags = strtoul(NEXTARG,0,10);
+ int hvm = atoi(NEXTARG);
+ unsigned long genidad = strtoul(NEXTARG,0,10);
+ toolstack_save_fd = atoi(NEXTARG);
+ toolstack_save_len = strtoul(NEXTARG,0,10);
+ unsigned cbflags = strtoul(NEXTARG,0,10);
+ assert(!*++argv);
+
+ helper_save_callbacks.toolstack_save = toolstack_save_cb;
+ helper_setcallbacks_save(&helper_save_callbacks, cbflags);
+
+ startup("save");
+ r = xc_domain_save(xch, io_fd, dom, max_iters, max_factor, flags,
+ &helper_save_callbacks, hvm, genidad);
+ complete(r);
+
+ } else if (!strcmp(mode,"--restore-domain")) {
+
+ int io_fd = atoi(NEXTARG);
+ uint32_t dom = strtoul(NEXTARG,0,10);
+ unsigned store_evtchn = strtoul(NEXTARG,0,10);
+ domid_t store_domid = strtoul(NEXTARG,0,10);
+ unsigned console_evtchn = strtoul(NEXTARG,0,10);
+ domid_t console_domid = strtoul(NEXTARG,0,10);
+ unsigned int hvm = strtoul(NEXTARG,0,10);
+ unsigned int pae = strtoul(NEXTARG,0,10);
+ int superpages = strtoul(NEXTARG,0,10);
+ int no_incr_genidad = strtoul(NEXTARG,0,10);
+ unsigned cbflags = strtoul(NEXTARG,0,10);
+ assert(!*++argv);
+
+ helper_setcallbacks_restore(&helper_restore_callbacks, cbflags);
+
+ unsigned long store_mfn = 0;
+ unsigned long console_mfn = 0;
+ unsigned long genidad = 0;
+
+ startup("restore");
+ r = xc_domain_restore(xch, io_fd, dom, store_evtchn, &store_mfn,
+ store_domid, console_evtchn, &console_mfn,
+ console_domid, hvm, pae, superpages,
+ no_incr_genidad, &genidad,
+ &helper_restore_callbacks);
+ helper_stub_restore_results(store_mfn,console_mfn,genidad,0);
+ complete(r);
+
+ } else {
+ assert(!"unexpected mode argument");
+ }
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/tools/libxl/libxl_save_msgs_gen.pl
b/tools/libxl/libxl_save_msgs_gen.pl
new file mode 100755
index 0000000..cd0837e
--- /dev/null
+++ b/tools/libxl/libxl_save_msgs_gen.pl
@@ -0,0 +1,393 @@
+#!/usr/bin/perl -w
+
+use warnings;
+use strict;
+use POSIX;
+
+our $debug = 0; # produce copius debugging output at run-time?
+
+our @msgs = (
+ # flags:
+ # s - applicable to save
+ # r - applicable to restore
+ # c - function pointer in callbacks struct rather than fixed function
+ # x - function pointer is in struct {save,restore}_callbacks
+ # and its null-ness needs to be passed through to the
helper''s xc
+ # W - needs a return value; callback is synchronous
+ [ 1, ''sr'', "log",
[qw(uint32_t level
+ uint32_t errnoval
+ STRING context
+ STRING formatted)] ],
+ [ 2, ''sr'', "progress",
[qw(STRING context
+ STRING doing_what),
+ ''unsigned
long'', ''done'',
+ ''unsigned
long'', ''total''] ],
+ [ 3, ''scxW'', "suspend", [] ],
+ [ 4, ''scxW'', "postcopy", [] ],
+ [ 5, ''scxW'', "checkpoint", [] ],
+ [ 6, ''scxW'', "switch_qemu_logdirty",
[qw(int domid
+ unsigned enable)] ],
+ # toolstack_save done entirely `by hand''
+ [ 7, ''rcxW'', "toolstack_restore",
[qw(uint32_t domid
+ BLOCK tsdata)] ],
+ [ 8, ''r'', "restore_results",
[''unsigned long'', ''store_mfn'',
+ ''unsigned
long'', ''console_mfn'',
+ ''unsigned
long'', ''genidad''] ],
+ [ 9, ''srW'', "complete",
[qw(int retval
+ int errnoval)] ],
+);
+
+#----------------------------------------
+
+our %cbs;
+our %func;
+our %func_ah;
+our @outfuncs;
+our %out_decls;
+our %out_body;
+our %msgnum_used;
+
+die unless @ARGV==1;
+die if $ARGV[0] =~ m/^-/;
+
+our ($intendedout) = @ARGV;
+
+$intendedout =~ m/([a-z]+)\.([ch])$/ or die;
+my ($want_ah, $ch) = ($1, $2);
+
+my $declprefix = '''';
+
+foreach my $ah (qw(callout helper)) {
+ $out_body{$ah} .= <<END.($ah eq ''callout'' ?
<<END : <<END);
+#include "libxl_osdeps.h"
+
+#include <assert.h>
+#include <string.h>
+#include <stdint.h>
+#include <limits.h>
+END
+
+#include "libxl_internal.h"
+
+END
+
+#include "_libxl_save_msgs_${ah}.h"
+#include <xenctrl.h>
+#include <xenguest.h>
+
+END
+}
+
+die $want_ah unless defined $out_body{$want_ah};
+
+sub f_decl ($$$$) {
+ my ($name, $ah, $c_rtype, $c_decl) = @_;
+ $out_decls{$name} = "${declprefix}$c_rtype $name$c_decl;\n";
+ $func{$name} = "$c_rtype $name$c_decl\n{\n" . ($func{$name} ||
'''');
+ $func_ah{$name} = $ah;
+}
+
+sub f_more ($$) {
+ my ($name, $addbody) = @_;
+ $func{$name} ||= '''';
+ $func{$name} .= $addbody;
+ push @outfuncs, $name;
+}
+
+our $libxl = "libxl__srm";
+our $callback = "${libxl}_callout_callback";
+our $receiveds = "${libxl}_callout_received";
+our $sendreply = "${libxl}_callout_sendreply";
+our $getcallbacks = "${libxl}_callout_get_callbacks";
+our $enumcallbacks = "${libxl}_callout_enumcallbacks";
+sub cbtype ($) { "${libxl}_".$_[0]."_autogen_callbacks"; };
+
+f_decl($sendreply, ''callout'', ''void'',
"(int r, void *user)");
+
+our $helper = "helper";
+our $encode = "${helper}_stub";
+our $allocbuf = "${helper}_allocbuf";
+our $transmit = "${helper}_transmitmsg";
+our $getreply = "${helper}_getreply";
+our $setcallbacks = "${helper}_setcallbacks";
+
+f_decl($allocbuf, ''helper'', ''unsigned char
*'', ''(int len, void *user)'');
+f_decl($transmit, ''helper'', ''void'',
+ ''(unsigned char *msg_freed, int len, void *user)'');
+f_decl($getreply, ''helper'', ''int'',
''(void *user)'');
+
+sub typeid ($) { my ($t) = @_; $t =~ s/\W/_/; return $t; };
+
+$out_body{''callout''} .= <<END;
+static int bytes_get(const unsigned char **msg,
+ const unsigned char *const endmsg,
+ void *result, int rlen)
+{
+ if (endmsg - *msg < rlen) return 0;
+ memcpy(result,*msg,rlen);
+ *msg += rlen;
+ return 1;
+}
+
+END
+$out_body{''helper''} .= <<END;
+static void bytes_put(unsigned char *const buf, int *len,
+ const void *value, int vlen)
+{
+ assert(vlen < INT_MAX/2 - *len);
+ if (buf)
+ memcpy(buf + *len, value, vlen);
+ *len += vlen;
+}
+
+END
+
+foreach my $simpletype (qw(int uint16_t uint32_t unsigned), ''unsigned
long'') {
+ my $typeid = typeid($simpletype);
+ $out_body{''callout''} .= <<END;
+static int ${typeid}_get(const unsigned char **msg,
+ const unsigned char *const endmsg,
+ $simpletype *result)
+{
+ return bytes_get(msg, endmsg, result, sizeof(*result));
+}
+
+END
+ $out_body{''helper''} .= <<END;
+static void ${typeid}_put(unsigned char *const buf, int *len,
+ const $simpletype value)
+{
+ bytes_put(buf, len, &value, sizeof(value));
+}
+
+END
+}
+
+$out_body{''callout''} .= <<END;
+static int BLOCK_get(const unsigned char **msg,
+ const unsigned char *const endmsg,
+ const uint8_t **result, uint32_t *result_size)
+{
+ if (!uint32_t_get(msg,endmsg,result_size)) return 0;
+ if (endmsg - *msg < *result_size) return 0;
+ *result = (const void*)*msg;
+ *msg += *result_size;
+ return 1;
+}
+
+static int STRING_get(const unsigned char **msg,
+ const unsigned char *const endmsg,
+ const char **result)
+{
+ const uint8_t *data;
+ uint32_t datalen;
+ if (!BLOCK_get(msg,endmsg,&data,&datalen)) return 0;
+ if (datalen == 0) return 0;
+ if (data[datalen-1] != ''\\0'') return 0;
+ *result = (const void*)data;
+ return 1;
+}
+
+END
+$out_body{''helper''} .= <<END;
+static void BLOCK_put(unsigned char *const buf,
+ int *len,
+ const uint8_t *bytes, uint32_t size)
+{
+ 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)
+{
+ size_t slen = strlen(string);
+ assert(slen < INT_MAX / 4);
+ assert(slen < (uint32_t)0x40000000);
+ BLOCK_put(buf, len, (const void*)string, slen+1);
+}
+
+END
+
+foreach my $sr (qw(save restore)) {
+ f_decl("${getcallbacks}_${sr}", ''callout'',
+ "const ".cbtype($sr)." *",
+ "(void *data)");
+
+ f_decl("${receiveds}_${sr}", ''callout'',
''int'',
+ "(const unsigned char *msg, uint32_t len, void *user)");
+
+ f_decl("${enumcallbacks}_${sr}", ''callout'',
''unsigned'',
+ "(const ".cbtype($sr)." *cbs)");
+ f_more("${enumcallbacks}_${sr}", " unsigned cbflags =
0;\n");
+
+ f_decl("${setcallbacks}_${sr}", ''helper'',
''void'',
+ "(struct ${sr}_callbacks *cbs, unsigned cbflags)");
+
+ f_more("${receiveds}_${sr}", <<END.($debug ? <<END :
'''').<<END);
+ const unsigned char *const endmsg = msg + len;
+ uint16_t mtype;
+ if (!uint16_t_get(&msg,endmsg,&mtype)) return 0;
+END
+ fprintf(stderr,"libxl callout receiver: got len=%u
mtype=%u\\n",len,mtype);
+END
+ switch (mtype) {
+
+END
+
+ $cbs{$sr} = "typedef struct ".cbtype($sr)." {\n";
+}
+
+foreach my $msginfo (@msgs) {
+ my ($msgnum, $flags, $name, $args) = @$msginfo;
+ die if $msgnum_used{$msgnum}++;
+
+ my $f_more_sr = sub {
+ my ($contents_spec, $fnamebase) = @_;
+ $fnamebase ||= "${receiveds}";
+ foreach my $sr (qw(save restore)) {
+ $sr =~ m/^./;
+ next unless $flags =~ m/$&/;
+ my $contents = (!ref $contents_spec) ? $contents_spec :
+ $contents_spec->($sr);
+ f_more("${fnamebase}_${sr}", $contents);
+ }
+ };
+
+ $f_more_sr->(" case $msgnum: { /* $name */\n");
+ if ($flags =~ m/W/) {
+ $f_more_sr->(" int r;\n");
+ }
+
+ my $c_rtype_helper = $flags =~ m/W/ ? ''int'' :
''void'';
+ my $c_rtype_callout = $flags =~ m/W/ ? ''int'' :
''void'';
+ my $c_decl = ''('';
+ my $c_callback_args = '''';
+
+ f_more("${encode}_$name", <<END.($debug ? <<END :
'''').<<END);
+ unsigned char *buf = 0;
+ int len = 0, allocd = 0;
+
+END
+ fprintf(stderr,"libxl-save-helper: encoding $name\\n");
+END
+ for (;;) {
+ uint16_t_put(buf, &len, $msgnum /* $name */);
+END
+
+ my @args = @$args;
+ my $c_recv = '''';
+ my ($argtype, $arg);
+ while (($argtype, $arg, @args) = @args) {
+ my $typeid = typeid($argtype);
+ my $c_args = "$arg";
+ my $c_get_args = "&$arg";
+ if ($argtype eq ''STRING'') {
+ $c_decl .= "const char *$arg, ";
+ $f_more_sr->(" const char *$arg;\n");
+ } elsif ($argtype eq ''BLOCK'') {
+ $c_decl .= "const uint8_t *$arg, uint32_t ${arg}_size, ";
+ $c_args .= ", ${arg}_size";
+ $c_get_args .= ",&${arg}_size";
+ $f_more_sr->(" const uint8_t *$arg;\n".
+ " uint32_t ${arg}_size;\n");
+ } else {
+ $c_decl .= "$argtype $arg, ";
+ $f_more_sr->(" $argtype $arg;\n");
+ }
+ $c_callback_args .= "$c_args, ";
+ $c_recv.+ " if
(!${typeid}_get(&msg,endmsg,$c_get_args)) return 0;\n";
+ f_more("${encode}_$name", " ${typeid}_put(buf, &len,
$c_args);\n");
+ }
+ $f_more_sr->($c_recv);
+ $c_decl .= "void *user)";
+ $c_callback_args .= "user";
+
+ $f_more_sr->(" if (msg != endmsg) return 0;\n");
+
+ my $c_callback;
+ if ($flags !~ m/c/) {
+ $c_callback = "${callback}_$name";
+ } else {
+ $f_more_sr->(sub {
+ my ($sr) = @_;
+ $cbs{$sr} .= " $c_rtype_callout (*${name})$c_decl;\n";
+ return
+ " const ".cbtype($sr)." *const cbs =\n".
+ " ${getcallbacks}_${sr}(user);\n";
+ });
+ $c_callback = "cbs->${name}";
+ }
+ my $c_make_callback = "$c_callback($c_callback_args)";
+ if ($flags !~ m/W/) {
+ $f_more_sr->(" $c_make_callback;\n");
+ } else {
+ $f_more_sr->(" r = $c_make_callback;\n".
+ " $sendreply(r, user);\n");
+ f_decl($sendreply, ''callout'', ''void'',
''(int r, void *user)'');
+ }
+ if ($flags =~ m/x/) {
+ my $c_v = "(1u<<$msgnum)";
+ my $c_cb = "cbs->$name";
+ $f_more_sr->(" if ($c_cb) cbflags |= $c_v;\n",
$enumcallbacks);
+ $f_more_sr->(" $c_cb = (cbflags & $c_v) ?
${encode}_${name} : 0;\n",
+ $setcallbacks);
+ }
+ $f_more_sr->(" return 1;\n }\n\n");
+ f_decl("${callback}_$name", ''callout'',
$c_rtype_callout, $c_decl);
+ f_decl("${encode}_$name", ''helper'',
$c_rtype_helper, $c_decl);
+ f_more("${encode}_$name",
+" if (buf) break;
+ buf = ${helper}_allocbuf(len, user);
+ assert(buf);
+ allocd = len;
+ len = 0;
+ }
+ assert(len == allocd);
+ ${transmit}(buf, len, user);
+");
+ if ($flags =~ m/W/) {
+ f_more("${encode}_$name", (<<END.($debug ? <<END :
'''').<<END));
+ int r = ${helper}_getreply(user);
+END
+ fprintf(stderr,"libxl-save-helper: $name got reply %d\\n",r);
+END
+ return r;
+END
+ }
+}
+
+print "/* AUTOGENERATED by $0 DO NOT EDIT */\n\n" or die $!;
+
+foreach my $sr (qw(save restore)) {
+ f_more("${enumcallbacks}_${sr}",
+ " return cbflags;\n");
+ f_more("${receiveds}_${sr}",
+ " default:\n".
+ " return 0;\n".
+ " }");
+ $cbs{$sr} .= "} ".cbtype($sr).";\n\n";
+ if ($ch eq ''h'') {
+ print $cbs{$sr} or die $!;
+ print "struct ${sr}_callbacks;\n";
+ }
+}
+
+if ($ch eq ''c'') {
+ foreach my $name (@outfuncs) {
+ next unless defined $func{$name};
+ $func{$name} .= "}\n\n";
+ $out_body{$func_ah{$name}} .= $func{$name};
+ delete $func{$name};
+ }
+ print $out_body{$want_ah} or die $!;
+} else {
+ foreach my $name (sort keys %out_decls) {
+ next unless $func_ah{$name} eq $want_ah;
+ print $out_decls{$name} or die $!;
+ }
+}
+
+close STDOUT or die $!;
--
1.7.2.5
Ian Jackson
2012-May-30 16:16 UTC
[PATCH 06/15] libxl: rename libxl_dom:save_helper to physmap_path
"save_helper" isn''t very descriptive. Also it is now
confusing
because it reads like it might refer to the libxl-save-helper
executable which runs xc_domain_save and xc_domain_restore.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
tools/libxl/libxl_dom.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 716cf4b..3e89dd7 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -732,7 +732,7 @@ int libxl__domain_suspend_common_callback(void *user)
return 1;
}
-static inline char *save_helper(libxl__gc *gc, uint32_t domid,
+static inline char *physmap_path(libxl__gc *gc, uint32_t domid,
char *phys_offset, char *node)
{
return libxl__sprintf(gc,
@@ -777,21 +777,21 @@ int libxl__toolstack_save(uint32_t domid, uint8_t **buf,
return -1;
}
- xs_path = save_helper(gc, domid, phys_offset, "start_addr");
+ xs_path = physmap_path(gc, domid, phys_offset, "start_addr");
start_addr = libxl__xs_read(gc, 0, xs_path);
if (start_addr == NULL) {
LOG(ERROR, "%s is NULL", xs_path);
return -1;
}
- xs_path = save_helper(gc, domid, phys_offset, "size");
+ xs_path = physmap_path(gc, domid, phys_offset, "size");
size = libxl__xs_read(gc, 0, xs_path);
if (size == NULL) {
LOG(ERROR, "%s is NULL", xs_path);
return -1;
}
- xs_path = save_helper(gc, domid, phys_offset, "name");
+ xs_path = physmap_path(gc, domid, phys_offset, "name");
name = libxl__xs_read(gc, 0, xs_path);
if (name == NULL)
namelen = 0;
--
1.7.2.5
Ian Jackson
2012-May-30 16:16 UTC
[PATCH 07/15] libxl: provide libxl__xs_*_checked and libxl__xs_transaction_*
These useful utility functions make dealing with xenstore a little
less painful.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
tools/libxl/libxl_internal.h | 38 +++++++++++++++++++++
tools/libxl/libxl_xshelp.c | 76 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 114 insertions(+), 0 deletions(-)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index e4c3f34..ee7f66a 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -498,6 +498,44 @@ _hidden bool libxl__xs_mkdir(libxl__gc *gc,
xs_transaction_t t,
_hidden char *libxl__xs_libxl_path(libxl__gc *gc, uint32_t domid);
+
+/*----- "checked" xenstore access functions -----*/
+/* Each of these functions will check that it succeeded; if it
+ * fails it logs and returns ERROR_FAIL.
+ */
+
+/* On success, *result_out came from the gc.
+ * On error, *result_out is undefined.
+ * ENOENT counts as success but sets *result_out=0
+ */
+int libxl__xs_read_checked(libxl__gc *gc, xs_transaction_t t,
+ const char *path, const char **result_out);
+
+/* Does not include a trailing null.
+ * May usefully be combined with GCSPRINTF if the format string
+ * behaviour of libxl__xs_write is desirable. */
+int libxl__xs_write_checked(libxl__gc *gc, xs_transaction_t t,
+ const char *path, const char *string);
+
+/* ENOENT is not an error (even if the parent directories don''t exist)
*/
+int libxl__xs_rm_checked(libxl__gc *gc, xs_transaction_t t, const char *path);
+
+/* Transaction functions, best used together.
+ * The caller should initialise *t to 0 (XBT_NULL) before calling start.
+ * Each function leaves *t!=0 iff the transaction needs cleaning up.
+ *
+ * libxl__xs_transaction_commit returns:
+ * <0 failure - a libxl error code
+ * +1 commit conflict; transaction has been destroyed and caller
+ * must go round again (call _start again and retry)
+ * 0 commited successfully
+ */
+int libxl__xs_transaction_start(libxl__gc *gc, xs_transaction_t *t);
+int libxl__xs_transaction_commit(libxl__gc *gc, xs_transaction_t *t);
+void libxl__xs_transaction_abort(libxl__gc *gc, xs_transaction_t *t);
+
+
+
/*
* This is a recursive delete, from top to bottom. What this function does
* is remove empty folders that contained the deleted entry.
diff --git a/tools/libxl/libxl_xshelp.c b/tools/libxl/libxl_xshelp.c
index c5b5364..5f359bc 100644
--- a/tools/libxl/libxl_xshelp.c
+++ b/tools/libxl/libxl_xshelp.c
@@ -135,6 +135,82 @@ char *libxl__xs_libxl_path(libxl__gc *gc, uint32_t domid)
return s;
}
+int libxl__xs_read_checked(libxl__gc *gc, xs_transaction_t t,
+ const char *path, const char **result_out)
+{
+ char *result = libxl__xs_read(gc, t, path);
+ if (!result) {
+ if (errno != ENOENT) {
+ LOGE(ERROR, "xenstore read failed: `%s''", path);
+ return ERROR_FAIL;
+ }
+ }
+ *result_out = result;
+ return 0;
+}
+
+int libxl__xs_write_checked(libxl__gc *gc, xs_transaction_t t,
+ const char *path, const char *string)
+{
+ size_t length = strlen(string);
+ if (!xs_write(CTX->xsh, t, path, string, length)) {
+ LOGE(ERROR, "xenstore write failed: `%s'' =
`%s''", path, string);
+ return ERROR_FAIL;
+ }
+ return 0;
+}
+
+int libxl__xs_rm_checked(libxl__gc *gc, xs_transaction_t t, const char *path)
+{
+ if (!xs_rm(CTX->xsh, t, path)) {
+ if (errno == ENOENT)
+ return 0;
+
+ LOGE(ERROR, "xenstore rm failed: `%s''", path);
+ return ERROR_FAIL;
+ }
+ return 0;
+}
+
+int libxl__xs_transaction_start(libxl__gc *gc, xs_transaction_t *t)
+{
+ assert(!*t);
+ *t = xs_transaction_start(CTX->xsh);
+ if (!*t) {
+ LOGE(ERROR, "could not create xenstore transaction");
+ return ERROR_FAIL;
+ }
+ return 0;
+}
+
+int libxl__xs_transaction_commit(libxl__gc *gc, xs_transaction_t *t)
+{
+ assert(*t);
+
+ if (!xs_transaction_end(CTX->xsh, *t, 0)) {
+ if (errno == EAGAIN)
+ return +1;
+
+ *t = 0;
+ LOGE(ERROR, "could not commit xenstore transacton");
+ return ERROR_FAIL;
+ }
+
+ *t = 0;
+ return 0;
+}
+
+void libxl__xs_transaction_abort(libxl__gc *gc, xs_transaction_t *t)
+{
+ if (!*t)
+ return;
+
+ if (!xs_transaction_end(CTX->xsh, *t, 1))
+ LOGE(ERROR, "could not abort xenstore transacton");
+
+ *t = 0;
+}
+
int libxl__xs_path_cleanup(libxl__gc *gc, xs_transaction_t t, char *user_path)
{
unsigned int nb = 0;
--
1.7.2.5
Ian Jackson
2012-May-30 16:16 UTC
[PATCH 08/15] libxl: wait for qemu to acknowledge logdirty command
The current migration code in libxl instructs qemu to start or stop
logdirty, but it does not wait for an acknowledgement from qemu before
continuing. This might lead to memory corruption (!)
Fix this by waiting for qemu to acknowledge the command.
Unfortunately the necessary ao arrangements for waiting for this
command are unique because qemu has a special protocol for this
particular operation.
Also, this change means that the switch_qemu_logdirty callback
implementation in libxl can no longer synchronously produce its return
value, as it now needs to wait for xenstore. So we tell the
marshalling code generator that it is a message which does not need a
reply. This turns the callback function called by the marshaller into
one which returns void; the callback function arranges to later
explicitly sends the reply to the helper, when the xs watch triggers
and the appropriate value is read from xenstore.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
tools/libxl/libxl_dom.c | 176 +++++++++++++++++++++++++++++++++---
tools/libxl/libxl_internal.h | 18 ++++-
tools/libxl/libxl_save_callout.c | 8 ++
tools/libxl/libxl_save_msgs_gen.pl | 7 +-
4 files changed, 193 insertions(+), 16 deletions(-)
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 3e89dd7..b13ed51 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -526,30 +526,180 @@ 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);
-/*----- callbacks, called by xc_domain_save -----*/
+/*----- complicated callback, called by xc_domain_save -----*/
+
+/*
+ * We implement the other end of protocol for controlling qemu-dm''s
+ * logdirty. There is no documentation for this protocol, but our
+ * counterparty''s implementation is in
+ * qemu-xen-traditional.git:xenstore.c in the function
+ * xenstore_process_logdirty_event
+ */
+
+static void switch_logdirty_timeout(libxl__egc *egc, libxl__ev_time *ev,
+ const struct timeval *requested_abs);
+static void switch_logdirty_xswatch(libxl__egc *egc, libxl__ev_xswatch*,
+ const char *watch_path, const char *event_path);
+static void switch_logdirty_done(libxl__egc *egc,
+ libxl__domain_suspend_state *dss, int ok);
-int libxl__domain_suspend_common_switch_qemu_logdirty
+static void logdirty_init(libxl__logdirty_switch *lds)
+{
+ lds->cmd_path = 0;
+ libxl__ev_xswatch_init(&lds->watch);
+ libxl__ev_time_init(&lds->timeout);
+}
+
+void libxl__domain_suspend_common_switch_qemu_logdirty
(int domid, unsigned enable, void *user)
{
libxl__save_helper_state *shs = user;
+ libxl__egc *egc = shs->egc;
libxl__domain_suspend_state *dss = CONTAINER_OF(shs, *dss, shs);
+ libxl__logdirty_switch *lds = &dss->logdirty;
STATE_AO_GC(dss->ao);
- char *path;
- bool rc;
+ int rc;
+ xs_transaction_t t = 0;
+ const char *got;
- path = libxl__sprintf(gc,
+ if (!lds->cmd_path) {
+ lds->cmd_path = GCSPRINTF(
"/local/domain/0/device-model/%u/logdirty/cmd",
domid);
- if (!path)
- return 1;
+ lds->ret_path = GCSPRINTF(
+ "/local/domain/0/device-model/%u/logdirty/ret",
domid);
+ }
+ lds->cmd = enable ? "enable" : "disable";
- if (enable)
- rc = xs_write(CTX->xsh, XBT_NULL, path, "enable",
strlen("enable"));
- else
- rc = xs_write(CTX->xsh, XBT_NULL, path, "disable",
strlen("disable"));
+ rc = libxl__ev_xswatch_register(gc, &lds->watch,
+ switch_logdirty_xswatch, lds->ret_path);
+ if (rc) goto out;
+
+ rc = libxl__ev_time_register_rel(gc, &lds->timeout,
+ switch_logdirty_timeout, 10*1000);
+ if (rc) goto out;
+
+ for (;;) {
+ rc = libxl__xs_transaction_start(gc, &t);
+ if (rc) goto out;
+
+ rc = libxl__xs_read_checked(gc, t, lds->cmd_path, &got);
+ if (rc) goto out;
+
+ if (got) {
+ const char *got_ret;
+ rc = libxl__xs_read_checked(gc, t, lds->ret_path, &got_ret);
+ if (rc) goto out;
- return rc ? 0 : 1;
+ if (strcmp(got, got_ret)) {
+ LOG(ERROR,"controlling logdirty: qemu was already
sent"
+ " command `%s'' (xenstore path `%s'')
but result is `%s''",
+ got, lds->cmd_path, got_ret ? got_ret :
"<none>");
+ rc = ERROR_FAIL;
+ goto out;
+ }
+ rc = libxl__xs_rm_checked(gc, t, lds->cmd_path);
+ if (rc) goto out;
+ }
+
+ rc = libxl__xs_rm_checked(gc, t, lds->ret_path);
+ if (rc) goto out;
+
+ rc = libxl__xs_write_checked(gc, t, lds->cmd_path, lds->cmd);
+ if (rc) goto out;
+
+ rc = libxl__xs_transaction_commit(gc, &t);
+ if (!rc) break;
+ if (rc<0) goto out;
+ }
+
+ /* OK, wait for some callback */
+ return;
+
+ out:
+ LOG(ERROR,"logdirty switch failed (rc=%d), aborting suspend",rc);
+ switch_logdirty_done(egc,dss,0);
+}
+
+static void switch_logdirty_timeout(libxl__egc *egc, libxl__ev_time *ev,
+ const struct timeval *requested_abs)
+{
+ libxl__domain_suspend_state *dss = CONTAINER_OF(ev, *dss,
logdirty.timeout);
+ STATE_AO_GC(dss->ao);
+ LOG(ERROR,"logdirty switch: wait for device model timed out");
+ switch_logdirty_done(egc,dss,0);
}
+static void switch_logdirty_xswatch(libxl__egc *egc, libxl__ev_xswatch *watch,
+ const char *watch_path, const char *event_path)
+{
+ libxl__domain_suspend_state *dss + CONTAINER_OF(watch, *dss,
logdirty.watch);
+ libxl__logdirty_switch *lds = &dss->logdirty;
+ STATE_AO_GC(dss->ao);
+ const char *got;
+ xs_transaction_t t = 0;
+ int rc;
+
+ for (;;) {
+ rc = libxl__xs_transaction_start(gc, &t);
+ if (rc) goto out;
+
+ rc = libxl__xs_read_checked(gc, t, lds->ret_path, &got);
+ if (rc) goto out;
+
+ if (!got) {
+ rc = +1;
+ goto out;
+ }
+
+ if (strcmp(got, lds->cmd)) {
+ LOG(ERROR,"logdirty switch: sent command `%s'' but got
reply `%s''"
+ " (xenstore paths `%s'' / `%s'')",
lds->cmd, got,
+ lds->cmd_path, lds->ret_path);
+ rc = ERROR_FAIL;
+ goto out;
+ }
+
+ rc = libxl__xs_rm_checked(gc, t, lds->cmd_path);
+ if (rc) goto out;
+
+ rc = libxl__xs_rm_checked(gc, t, lds->ret_path);
+ if (rc) goto out;
+
+ rc = libxl__xs_transaction_commit(gc, &t);
+ if (!rc) break;
+ if (rc<0) goto out;
+ }
+
+ out:
+ /* rc < 0: error
+ * rc == 0: ok, we are done
+ * rc == +1: need to keep waiting
+ */
+ libxl__xs_transaction_abort(gc, &t);
+
+ if (!rc) {
+ switch_logdirty_done(egc,dss,1);
+ } else if (rc < 0) {
+ LOG(ERROR,"logdirty switch: response read failed
(rc=%d)",rc);
+ switch_logdirty_done(egc,dss,0);
+ }
+}
+
+static void switch_logdirty_done(libxl__egc *egc,
+ libxl__domain_suspend_state *dss, int ok)
+{
+ STATE_AO_GC(dss->ao);
+ libxl__logdirty_switch *lds = &dss->logdirty;
+
+ libxl__ev_xswatch_deregister(gc, &lds->watch);
+ libxl__ev_time_deregister(gc, &lds->timeout);
+
+ libxl__xc_domain_saverestore_async_callback_done(egc, &dss->shs,
ok);
+}
+
+/*----- callbacks, called by xc_domain_save -----*/
+
int libxl__domain_suspend_device_model(libxl__gc *gc, uint32_t domid)
{
libxl_ctx *ctx = libxl__gc_owner(gc);
@@ -872,6 +1022,8 @@ void libxl__domain_suspend(libxl__egc *egc,
libxl__domain_suspend_state *dss)
libxl__srm_save_autogen_callbacks *const callbacks
&dss->shs.callbacks.save.a;
+ logdirty_init(&dss->logdirty);
+
switch (type) {
case LIBXL_DOMAIN_TYPE_HVM: {
char *path;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index ee7f66a..8f4f45d 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1864,6 +1864,14 @@ typedef struct libxl__domain_suspend_state
libxl__domain_suspend_state;
typedef void libxl__domain_suspend_cb(libxl__egc*,
libxl__domain_suspend_state*, int rc);
+typedef struct libxl__logdirty_switch {
+ const char *cmd;
+ const char *cmd_path;
+ const char *ret_path;
+ libxl__ev_xswatch watch;
+ libxl__ev_time timeout;
+} libxl__logdirty_switch;
+
struct libxl__domain_suspend_state {
/* set by caller of libxl__domain_suspend */
libxl__ao *ao;
@@ -1882,6 +1890,7 @@ struct libxl__domain_suspend_state {
int guest_responded;
int interval; /* checkpoint interval (for Remus) */
libxl__save_helper_state shs;
+ libxl__logdirty_switch logdirty;
};
@@ -2013,8 +2022,15 @@ _hidden void libxl__xc_domain_save(libxl__egc*,
libxl__domain_suspend_state*,
_hidden void libxl__xc_domain_save_done(libxl__egc*, void *dss_void,
int rc, int retval, int errnoval);
+/* Used by asynchronous callbacks: ie ones which xc regards as
+ * returning a value, but which we want to handle asynchronously.
+ * Such functions'' actual callback function return void in libxl
+ * When they are ready to indicate completion, they call this. */
+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_common_switch_qemu_logdirty
+_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,
uint32_t *len, void *data);
diff --git a/tools/libxl/libxl_save_callout.c b/tools/libxl/libxl_save_callout.c
index ff7dfb6..4413e97 100644
--- a/tools/libxl/libxl_save_callout.c
+++ b/tools/libxl/libxl_save_callout.c
@@ -129,6 +129,14 @@ void libxl__xc_domain_save(libxl__egc *egc,
libxl__domain_suspend_state *dss,
}
+void libxl__xc_domain_saverestore_async_callback_done(libxl__egc *egc,
+ libxl__save_helper_state *shs, int return_value)
+{
+ shs->egc = egc;
+ libxl__srm_callout_sendreply(return_value, shs);
+ shs->egc = 0;
+}
+
/*----- helper execution -----*/
static void run_helper(libxl__egc *egc, libxl__save_helper_state *shs,
diff --git a/tools/libxl/libxl_save_msgs_gen.pl
b/tools/libxl/libxl_save_msgs_gen.pl
index cd0837e..8832c46 100755
--- a/tools/libxl/libxl_save_msgs_gen.pl
+++ b/tools/libxl/libxl_save_msgs_gen.pl
@@ -14,6 +14,7 @@ our @msgs = (
# x - function pointer is in struct {save,restore}_callbacks
# and its null-ness needs to be passed through to the
helper''s xc
# W - needs a return value; callback is synchronous
+ # A - needs a return value; callback is asynchronous
[ 1, ''sr'', "log",
[qw(uint32_t level
uint32_t errnoval
STRING context
@@ -25,7 +26,7 @@ our @msgs = (
[ 3, ''scxW'', "suspend", [] ],
[ 4, ''scxW'', "postcopy", [] ],
[ 5, ''scxW'', "checkpoint", [] ],
- [ 6, ''scxW'', "switch_qemu_logdirty",
[qw(int domid
+ [ 6, ''scxA'', "switch_qemu_logdirty",
[qw(int domid
unsigned enable)] ],
# toolstack_save done entirely `by hand''
[ 7, ''rcxW'', "toolstack_restore",
[qw(uint32_t domid
@@ -260,7 +261,7 @@ foreach my $msginfo (@msgs) {
$f_more_sr->(" int r;\n");
}
- my $c_rtype_helper = $flags =~ m/W/ ? ''int'' :
''void'';
+ my $c_rtype_helper = $flags =~ m/[WA]/ ? ''int'' :
''void'';
my $c_rtype_callout = $flags =~ m/W/ ? ''int'' :
''void'';
my $c_decl = ''('';
my $c_callback_args = '''';
@@ -348,7 +349,7 @@ END
assert(len == allocd);
${transmit}(buf, len, user);
");
- if ($flags =~ m/W/) {
+ if ($flags =~ m/[WA]/) {
f_more("${encode}_$name", (<<END.($debug ? <<END :
'''').<<END));
int r = ${helper}_getreply(user);
END
--
1.7.2.5
Ian Jackson
2012-May-30 16:16 UTC
[PATCH 09/15] libxl: datacopier: provide "prefix data" facilit
This will be used to write the qemu data banner to the save/migration
stream.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
tools/libxl/libxl_aoutils.c | 15 +++++++++++++++
tools/libxl/libxl_internal.h | 6 ++++++
2 files changed, 21 insertions(+), 0 deletions(-)
diff --git a/tools/libxl/libxl_aoutils.c b/tools/libxl/libxl_aoutils.c
index ee0df57..208b34b 100644
--- a/tools/libxl/libxl_aoutils.c
+++ b/tools/libxl/libxl_aoutils.c
@@ -74,6 +74,21 @@ static void datacopier_check_state(libxl__egc *egc,
libxl__datacopier_state *dc)
}
}
+void libxl__datacopier_prefixdata(libxl__egc *egc, libxl__datacopier_state *dc,
+ const void *data, size_t len)
+{
+ libxl__datacopier_buf *buf;
+
+ assert(len < dc->maxsz - dc->used);
+
+ buf = libxl__zalloc(0, sizeof(*buf) - sizeof(buf->buf) + len);
+ buf->used = len;
+ memcpy(buf->buf, data, len);
+
+ dc->used += len;
+ LIBXL_TAILQ_INSERT_TAIL(&dc->bufs, buf, entry);
+}
+
static void datacopier_readable(libxl__egc *egc, libxl__ev_fd *ev,
int fd, short events, short revents) {
libxl__datacopier_state *dc = CONTAINER_OF(ev, *dc, toread);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 8f4f45d..b41e72f 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1812,6 +1812,12 @@ _hidden void
libxl__datacopier_init(libxl__datacopier_state *dc);
_hidden void libxl__datacopier_kill(libxl__datacopier_state *dc);
_hidden int libxl__datacopier_start(libxl__datacopier_state *dc);
+/* Inserts literal data into the output stream.
+ * May safely be used only immediately after libxl__datacopier_start.
+ * (But may be called multiple times.) The data is copied.
+ * NB exceeding maxsz will fail an assertion! */
+_hidden void libxl__datacopier_prefixdata(libxl__egc*,
libxl__datacopier_state*,
+ const void *data, size_t len);
/*----- Save/restore helper (used by creation and suspend) -----*/
--
1.7.2.5
Ian Jackson
2012-May-30 16:16 UTC
[PATCH 10/15] libxl: prepare for asynchronous writing of qemu save file
* Combine the various calls to libxl__device_model_savefile into one
at the start of libxl__domain_suspend, storing the result in the
dss. Consequently a few functions take a dss instead of some or all
of their other arguments.
* Make libxl__domain_save_device_model''s API into an asynchronous
style which takes a callback. The function is, however, still
synchronous; it will be made actually async in the next patch.
* Consequently make libxl__remus_domain_checkpoint_callback into an
asynchronous callback.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
tools/libxl/libxl_dom.c | 54 ++++++++++++++++++++++++++----------
tools/libxl/libxl_internal.h | 18 ++++++++++--
tools/libxl/libxl_save_msgs_gen.pl | 2 +-
3 files changed, 55 insertions(+), 19 deletions(-)
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index b13ed51..708304e 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -700,11 +700,13 @@ static void switch_logdirty_done(libxl__egc *egc,
/*----- callbacks, called by xc_domain_save -----*/
-int libxl__domain_suspend_device_model(libxl__gc *gc, uint32_t domid)
+int libxl__domain_suspend_device_model(libxl__gc *gc,
+ libxl__domain_suspend_state *dss)
{
libxl_ctx *ctx = libxl__gc_owner(gc);
int ret = 0;
- const char *filename = libxl__device_model_savefile(gc, domid);
+ uint32_t const domid = dss->domid;
+ const char *const filename = dss->dm_savefile;
switch (libxl__device_model_version_running(gc, domid)) {
case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL: {
@@ -873,7 +875,7 @@ int libxl__domain_suspend_common_callback(void *user)
guest_suspended:
if (dss->hvm) {
- ret = libxl__domain_suspend_device_model(gc, dss->domid);
+ ret = libxl__domain_suspend_device_model(gc, dss);
if (ret) {
LOG(ERROR, "libxl__domain_suspend_device_model failed
ret=%d", ret);
return 0;
@@ -988,19 +990,32 @@ static int libxl__remus_domain_resume_callback(void *data)
return 1;
}
-static int libxl__remus_domain_checkpoint_callback(void *data)
+/*----- remus asynchronous checkpoint callback -----*/
+
+static void remus_checkpoint_dm_saved(libxl__egc *egc,
+ libxl__domain_suspend_state *dss, int
rc);
+
+static void libxl__remus_domain_checkpoint_callback(void *data)
{
libxl__domain_suspend_state *dss = data;
+ libxl__egc *egc = dss->shs.egc;
STATE_AO_GC(dss->ao);
/* This would go into tailbuf. */
- if (dss->hvm &&
- libxl__domain_save_device_model(gc, dss->domid, dss->fd))
- return 0;
+ if (dss->hvm) {
+ libxl__domain_save_device_model(egc, dss, remus_checkpoint_dm_saved);
+ } else {
+ remus_checkpoint_dm_saved(egc, dss, 0);
+ }
+}
+static void remus_checkpoint_dm_saved(libxl__egc *egc,
+ libxl__domain_suspend_state *dss, int rc)
+{
/* TODO: Wait for disk and memory ack, release network buffer */
+ /* TODO: make this asynchronous */
usleep(dss->interval * 1000);
- return 1;
+ libxl__xc_domain_saverestore_async_callback_done(egc, &dss->shs, 1);
}
/*----- main code for suspending, in order of execution -----*/
@@ -1052,6 +1067,7 @@ void libxl__domain_suspend(libxl__egc *egc,
libxl__domain_suspend_state *dss)
dss->hvm = hvm;
dss->suspend_eventchn = -1;
dss->guest_responded = 0;
+ dss->dm_savefile = libxl__device_model_savefile(gc, domid);
if (r_info != NULL) {
dss->interval = r_info->interval;
@@ -1101,7 +1117,6 @@ void libxl__xc_domain_save_done(libxl__egc *egc, void
*dss_void,
/* Convenience aliases */
const libxl_domain_type type = dss->type;
- const uint32_t domid = dss->domid;
if (rc)
goto out;
@@ -1119,11 +1134,11 @@ void libxl__xc_domain_save_done(libxl__egc *egc, void
*dss_void,
}
if (type == LIBXL_DOMAIN_TYPE_HVM) {
- rc = libxl__domain_suspend_device_model(gc, domid);
+ rc = libxl__domain_suspend_device_model(gc, dss);
if (rc) goto out;
- rc = libxl__domain_save_device_model(gc, domid, dss->fd);
- if (rc) goto out;
+ libxl__domain_save_device_model(egc, dss, domain_suspend_done);
+ return;
}
rc = 0;
@@ -1132,14 +1147,22 @@ out:
domain_suspend_done(egc, dss, rc);
}
-int libxl__domain_save_device_model(libxl__gc *gc, uint32_t domid, int fd)
+void libxl__domain_save_device_model(libxl__egc *egc,
+ libxl__domain_suspend_state *dss,
+ libxl__save_device_model_cb *callback)
{
+ STATE_AO_GC(dss->ao);
int rc, fd2 = -1, c;
char buf[1024];
- const char *filename = libxl__device_model_savefile(gc, domid);
struct stat st;
uint32_t qemu_state_len;
+ dss->save_dm_callback = callback;
+
+ /* Convenience aliases */
+ const char *const filename = dss->dm_savefile;
+ const int fd = dss->fd;
+
if (stat(filename, &st) < 0)
{
LOG(ERROR, "Unable to stat qemu save file\n");
@@ -1181,7 +1204,8 @@ int libxl__domain_save_device_model(libxl__gc *gc,
uint32_t domid, int fd)
out:
if (fd2 >= 0) close(fd2);
unlink(filename);
- return rc;
+
+ dss->save_dm_callback(egc, dss, rc);
}
static void domain_suspend_done(libxl__egc *egc,
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index b41e72f..cc4c731 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -821,10 +821,8 @@ _hidden int libxl__domain_rename(libxl__gc *gc, uint32_t
domid,
_hidden int libxl__toolstack_restore(uint32_t domid, const uint8_t *buf,
uint32_t size, void *data);
-_hidden const char *libxl__device_model_savefile(libxl__gc *gc, uint32_t
domid);
-_hidden int libxl__domain_suspend_device_model(libxl__gc *gc, uint32_t domid);
_hidden int libxl__domain_resume_device_model(libxl__gc *gc, uint32_t domid);
-_hidden int libxl__domain_save_device_model(libxl__gc *gc, uint32_t domid, int
fd);
+
_hidden void libxl__userdata_destroyall(libxl__gc *gc, uint32_t domid);
_hidden int libxl__domain_pvcontrol_available(libxl__gc *gc, uint32_t domid);
@@ -1869,6 +1867,8 @@ typedef struct libxl__domain_suspend_state
libxl__domain_suspend_state;
typedef void libxl__domain_suspend_cb(libxl__egc*,
libxl__domain_suspend_state*, int rc);
+typedef void libxl__save_device_model_cb(libxl__egc*,
+ libxl__domain_suspend_state*, int rc);
typedef struct libxl__logdirty_switch {
const char *cmd;
@@ -1894,9 +1894,12 @@ struct libxl__domain_suspend_state {
int suspend_eventchn;
int hvm;
int guest_responded;
+ const char *dm_savefile;
int interval; /* checkpoint interval (for Remus) */
libxl__save_helper_state shs;
libxl__logdirty_switch logdirty;
+ /* private for libxl__domain_save_device_model */
+ libxl__save_device_model_cb *save_dm_callback;
};
@@ -2053,6 +2056,15 @@ _hidden void libxl__xc_domain_restore(libxl__egc *egc,
_hidden void libxl__xc_domain_restore_done(libxl__egc *egc, void *dcs_void,
int rc, int retval, int errnoval);
+/* Each time the dm needs to be saved, we must call suspend and then save */
+_hidden int libxl__domain_suspend_device_model(libxl__gc *gc,
+ libxl__domain_suspend_state *dss);
+_hidden void libxl__domain_save_device_model(libxl__egc *egc,
+ libxl__domain_suspend_state *dss,
+ libxl__save_device_model_cb *callback);
+
+_hidden const char *libxl__device_model_savefile(libxl__gc *gc, uint32_t
domid);
+
/*
* Convenience macros.
diff --git a/tools/libxl/libxl_save_msgs_gen.pl
b/tools/libxl/libxl_save_msgs_gen.pl
index 8832c46..3ac6f80 100755
--- a/tools/libxl/libxl_save_msgs_gen.pl
+++ b/tools/libxl/libxl_save_msgs_gen.pl
@@ -25,7 +25,7 @@ our @msgs = (
''unsigned
long'', ''total''] ],
[ 3, ''scxW'', "suspend", [] ],
[ 4, ''scxW'', "postcopy", [] ],
- [ 5, ''scxW'', "checkpoint", [] ],
+ [ 5, ''scxA'', "checkpoint", [] ],
[ 6, ''scxA'', "switch_qemu_logdirty",
[qw(int domid
unsigned enable)] ],
# toolstack_save done entirely `by hand''
--
1.7.2.5
Ian Jackson
2012-May-30 16:16 UTC
[PATCH 11/15] libxl: Make libxl__domain_save_device_model asynchronous
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
tools/libxl/libxl_dom.c | 98 +++++++++++++++++++++++++++--------------
tools/libxl/libxl_internal.h | 1 +
2 files changed, 65 insertions(+), 34 deletions(-)
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 708304e..2b9e886 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1147,15 +1147,17 @@ out:
domain_suspend_done(egc, dss, rc);
}
+static void save_device_model_datacopier_done(libxl__egc *egc,
+ libxl__datacopier_state *dc, int onwrite, int errnoval);
+
void libxl__domain_save_device_model(libxl__egc *egc,
libxl__domain_suspend_state *dss,
libxl__save_device_model_cb *callback)
{
STATE_AO_GC(dss->ao);
- int rc, fd2 = -1, c;
- char buf[1024];
struct stat st;
uint32_t qemu_state_len;
+ int rc;
dss->save_dm_callback = callback;
@@ -1163,49 +1165,77 @@ void libxl__domain_save_device_model(libxl__egc *egc,
const char *const filename = dss->dm_savefile;
const int fd = dss->fd;
- if (stat(filename, &st) < 0)
+ libxl__datacopier_state *dc = &dss->save_dm_datacopier;
+ memset(dc, 0, sizeof(*dc));
+ dc->readwhat = GCSPRINTF("qemu save file %s", filename);
+ dc->ao = ao;
+ dc->readfd = -1;
+ dc->writefd = fd;
+ dc->maxsz = INT_MAX;
+ dc->copywhat = GCSPRINTF("qemu save file for domain %"PRIu32,
dss->domid);
+ dc->writewhat = "save/migration stream";
+ dc->callback = save_device_model_datacopier_done;
+
+ dc->readfd = open(filename, O_RDONLY);
+ if (dc->readfd < 0) {
+ LOGE(ERROR, "unable to open %s", dc->readwhat);
+ goto out;
+ }
+
+ if (fstat(dc->readfd, &st))
{
- LOG(ERROR, "Unable to stat qemu save file\n");
- rc = ERROR_FAIL;
+ LOGE(ERROR, "unable to fstat %s", dc->readwhat);
+ goto out;
+ }
+
+ if (!S_ISREG(st.st_mode)) {
+ LOG(ERROR, "%s is not a plain file!", dc->readwhat);
goto out;
}
qemu_state_len = st.st_size;
LOG(DEBUG, "Qemu state is %d bytes\n", qemu_state_len);
- rc = libxl_write_exactly(CTX, fd, QEMU_SIGNATURE, strlen(QEMU_SIGNATURE),
- "saved-state file", "qemu
signature");
- if (rc)
- goto out;
+ rc = libxl__datacopier_start(dc);
+ if (rc) goto out;
- rc = libxl_write_exactly(CTX, fd, &qemu_state_len,
sizeof(qemu_state_len),
- "saved-state file", "saved-state
length");
- if (rc)
- goto out;
+ libxl__datacopier_prefixdata(egc, dc,
+ QEMU_SIGNATURE, strlen(QEMU_SIGNATURE));
- fd2 = open(filename, O_RDONLY);
- if (fd2 < 0) {
- LOGE(ERROR, "Unable to open qemu save file\n");
- goto out;
- }
- while ((c = read(fd2, buf, sizeof(buf))) != 0) {
- if (c < 0) {
- if (errno == EINTR)
- continue;
- rc = errno;
- goto out;
- }
- rc = libxl_write_exactly(
- CTX, fd, buf, c, "saved-state file", "qemu
state");
- if (rc)
- goto out;
+ libxl__datacopier_prefixdata(egc, dc,
+ &qemu_state_len, sizeof(qemu_state_len));
+ return;
+
+ out:
+ save_device_model_datacopier_done(egc, dc, -1, 0);
+}
+
+static void save_device_model_datacopier_done(libxl__egc *egc,
+ libxl__datacopier_state *dc, int onwrite, int errnoval)
+{
+ libxl__domain_suspend_state *dss + CONTAINER_OF(dc, *dss,
save_dm_datacopier);
+ STATE_AO_GC(dss->ao);
+
+ /* Convenience aliases */
+ const char *const filename = dss->dm_savefile;
+ int our_rc = 0;
+ int rc;
+
+ libxl__datacopier_kill(dc);
+
+ if (onwrite || errnoval)
+ our_rc = ERROR_FAIL;
+
+ if (dc->readfd >= 0) {
+ close(dc->readfd);
+ dc->readfd = -1;
}
- rc = 0;
-out:
- if (fd2 >= 0) close(fd2);
- unlink(filename);
- dss->save_dm_callback(egc, dss, rc);
+ rc = libxl__remove_file(gc, filename);
+ if (!our_rc) our_rc = rc;
+
+ dss->save_dm_callback(egc, dss, our_rc);
}
static void domain_suspend_done(libxl__egc *egc,
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index cc4c731..22013fd 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1900,6 +1900,7 @@ struct libxl__domain_suspend_state {
libxl__logdirty_switch logdirty;
/* private for libxl__domain_save_device_model */
libxl__save_device_model_cb *save_dm_callback;
+ libxl__datacopier_state save_dm_datacopier;
};
--
1.7.2.5
Ian Jackson
2012-May-30 16:16 UTC
[PATCH 12/15] xl: Handle return value from libxl_domain_suspend correctly
libxl_domain_suspend returns a libxl error code. So it must be
wrapped with MUST and not CHK_ERRNO.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
tools/libxl/xl_cmdimpl.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 9b4a80e..8c5f147 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -2820,7 +2820,7 @@ static int save_domain(const char *p, const char
*filename, int checkpoint,
save_domain_core_writeconfig(fd, filename, config_data, config_len);
- CHK_ERRNO(libxl_domain_suspend(ctx, domid, fd, 0, NULL));
+ MUST(libxl_domain_suspend(ctx, domid, fd, 0, NULL));
close(fd);
if (checkpoint)
--
1.7.2.5
This was allocated using asprintf but never freed. Use GCSPRINTF.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
tools/libxl/libxl_create.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index eaf378b..6babdb0 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -739,9 +739,8 @@ void libxl__xc_domain_restore_done(libxl__egc *egc, void
*dcs_void,
goto out;
if (info->type == LIBXL_DOMAIN_TYPE_HVM) {
- ret = asprintf(&state->saved_state,
+ state->saved_state = GCSPRINTF(
XC_DEVICE_MODEL_RESTORE_FILE".%d", domid);
- ret = (ret < 0) ? ERROR_FAIL : 0;
}
out:
--
1.7.2.5
Ian Jackson
2012-May-30 16:16 UTC
[PATCH 14/15] libxl: do not leak spawned middle children
libxl__spawn_spawn would, when libxl__spawn_detach was called, make
the spawn become idle immediately. However it still has a child
process which needs to be waited for: the `detachable'' spawned
child.
This is wrong because the ultimate in-libxl caller may return to the
application, with a child process still forked but not reaped libxl
contrary to the documented behaviour of libxl.
Instead, replace libxl__spawn_detach with libxl__spawn_initiate_detach
which is asynchronous. The detachable spawned children are abolished;
instead, we defer calling back to the in-libxl user until the middle
child has been reaped.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
tools/libxl/libxl_dm.c | 14 ++++-
tools/libxl/libxl_exec.c | 124 ++++++++++++++++++++++++------------------
tools/libxl/libxl_internal.h | 40 +++++++-------
3 files changed, 103 insertions(+), 75 deletions(-)
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index d987347..284847a 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -904,6 +904,8 @@ static void device_model_confirm(libxl__egc *egc,
libxl__spawn_state *spawn,
const char *xsdata);
static void device_model_startup_failed(libxl__egc *egc,
libxl__spawn_state *spawn);
+static void device_model_detached(libxl__egc *egc,
+ libxl__spawn_state *spawn);
/* our "next step" function, called from those callbacks and
elsewhere */
static void device_model_spawn_outcome(libxl__egc *egc,
@@ -1011,6 +1013,7 @@ retry_transaction:
spawn->midproc_cb = libxl__spawn_record_pid;
spawn->confirm_cb = device_model_confirm;
spawn->failure_cb = device_model_startup_failed;
+ spawn->detached_cb = device_model_detached;
rc = libxl__spawn_spawn(egc, spawn);
if (rc < 0)
@@ -1044,9 +1047,7 @@ static void device_model_confirm(libxl__egc *egc,
libxl__spawn_state *spawn,
if (strcmp(xsdata, "running"))
return;
- libxl__spawn_detach(gc, spawn);
-
- device_model_spawn_outcome(egc, dmss, 0);
+ libxl__spawn_initiate_detach(gc, spawn);
}
static void device_model_startup_failed(libxl__egc *egc,
@@ -1056,6 +1057,13 @@ static void device_model_startup_failed(libxl__egc *egc,
device_model_spawn_outcome(egc, dmss, ERROR_FAIL);
}
+static void device_model_detached(libxl__egc *egc,
+ libxl__spawn_state *spawn)
+{
+ libxl__dm_spawn_state *dmss = CONTAINER_OF(spawn, *dmss, spawn);
+ device_model_spawn_outcome(egc, dmss, 0);
+}
+
static void device_model_spawn_outcome(libxl__egc *egc,
libxl__dm_spawn_state *dmss,
int rc)
diff --git a/tools/libxl/libxl_exec.c b/tools/libxl/libxl_exec.c
index 082bf44..bb85682 100644
--- a/tools/libxl/libxl_exec.c
+++ b/tools/libxl/libxl_exec.c
@@ -238,15 +238,16 @@ err:
/*
* Full set of possible states of a libxl__spawn_state and its _detachable:
*
- * ss-> ss-> ss-> | ssd->
ssd->
- * timeout xswatch ssd | mid ss
- * - Undefined undef undef no | - -
- * - Idle Idle Idle no | - -
- * - Active Active Active yes | Active yes
- * - Partial Active/Idle Active/Idle maybe | Active/Idle yes (if
exists)
- * - Detached - - - | Active no
+ * detaching failed mid timeout xswatch
+ * - Undefined undef undef - undef undef
+ * - Idle any any Idle Idle Idle
+ * - Attached OK 0 0 Active Active Active
+ * - Attached Failed 0 1 Active Idle Idle
+ * - Detaching 1 maybe Active Idle Idle
+ * - Partial any any Idle Active/Idle Active/Idle
*
- * When in state Detached, the middle process has been sent a SIGKILL.
+ * When in states Detaching or Attached Failed, the middle process has
+ * been sent a SIGKILL.
*/
/* Event callbacks. */
@@ -257,19 +258,18 @@ static void spawn_timeout(libxl__egc *egc, libxl__ev_time
*ev,
static void spawn_middle_death(libxl__egc *egc, libxl__ev_child *childw,
pid_t pid, int status);
-/* Precondition: Partial. Results: Detached. */
+/* Precondition: Partial. Results: Idle. */
static void spawn_cleanup(libxl__gc *gc, libxl__spawn_state *ss);
-/* Precondition: Partial; caller has logged failure reason.
- * Results: Caller notified of failure;
- * after return, ss may be completely invalid as caller may reuse it */
-static void spawn_failed(libxl__egc *egc, libxl__spawn_state *ss);
+/* Precondition: Attached or Detaching; caller has logged failure reason.
+ * Results: Detaching, or Attached Failing */
+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);
- ss->ssd = 0;
}
int libxl__spawn_spawn(libxl__egc *egc, libxl__spawn_state *ss)
@@ -280,8 +280,7 @@ int libxl__spawn_spawn(libxl__egc *egc, libxl__spawn_state
*ss)
int status, rc;
libxl__spawn_init(ss);
- ss->ssd = libxl__zalloc(0, sizeof(*ss->ssd));
- libxl__ev_child_init(&ss->ssd->mid);
+ ss->failed = ss->detaching = 0;
rc = libxl__ev_time_register_rel(gc, &ss->timeout,
spawn_timeout, ss->timeout_ms);
@@ -291,7 +290,7 @@ int libxl__spawn_spawn(libxl__egc *egc, libxl__spawn_state
*ss)
spawn_watch_event, ss->xspath);
if (rc) goto out_err;
- pid_t middle = libxl__ev_child_fork(gc, &ss->ssd->mid,
spawn_middle_death);
+ pid_t middle = libxl__ev_child_fork(gc, &ss->mid,
spawn_middle_death);
if (middle ==-1) { rc = ERROR_FAIL; goto out_err; }
if (middle) {
@@ -344,54 +343,64 @@ 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);
+}
+
+static void spawn_detach(libxl__gc *gc, libxl__spawn_state *ss)
+/* Precondition: Attached or Detaching, but caller must have just set
+ * at least one of detaching or failed.
+ * Results: Detaching or Attached Failed */
+{
int r;
+ assert(libxl__ev_child_inuse(&ss->mid));
libxl__ev_time_deregister(gc, &ss->timeout);
libxl__ev_xswatch_deregister(gc, &ss->xswatch);
- libxl__spawn_state_detachable *ssd = ss->ssd;
- if (ssd) {
- if (libxl__ev_child_inuse(&ssd->mid)) {
- pid_t child = ssd->mid.pid;
- r = kill(child, SIGKILL);
- if (r && errno != ESRCH)
- LOGE(WARN, "%s: failed to kill intermediate child
(pid=%lu)",
- ss->what, (unsigned long)child);
- }
+ pid_t child = ss->mid.pid;
+ r = kill(child, SIGKILL);
+ if (r && errno != ESRCH)
+ LOGE(WARN, "%s: failed to kill intermediate child (pid=%lu)",
+ ss->what, (unsigned long)child);
+}
- /* disconnect the ss and ssd from each other */
- ssd->ss = 0;
- ss->ssd = 0;
- }
+void libxl__spawn_initiate_detach(libxl__gc *gc, libxl__spawn_state *ss)
+{
+ ss->detaching = 1;
+ spawn_detach(gc, ss);
}
-static void spawn_failed(libxl__egc *egc, libxl__spawn_state *ss)
+static void spawn_fail(libxl__egc *egc, libxl__spawn_state *ss)
+/* Caller must have logged. Must be last thing in calling function,
+ * as it may make the callback. Precondition: Attached or Detaching. */
{
EGC_GC;
- spawn_cleanup(gc, ss);
- ss->failure_cb(egc, ss); /* must be last; callback may do anything to ss
*/
+ ss->failed = 1;
+ spawn_detach(gc, ss);
}
static void spawn_timeout(libxl__egc *egc, libxl__ev_time *ev,
const struct timeval *requested_abs)
{
- /* Before event, was Active; is now Partial. */
+ /* Before event, was Attached. */
EGC_GC;
libxl__spawn_state *ss = CONTAINER_OF(ev, *ss, timeout);
LOG(ERROR, "%s: startup timed out", ss->what);
- spawn_failed(egc, ss); /* must be last */
+ 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)
{
- /* On entry, is Active. */
+ /* 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);
- spawn_failed(egc, ss); /* must be last */
+ spawn_fail(egc, ss); /* must be last */
return;
}
ss->confirm_cb(egc, ss, p); /* must be last */
@@ -399,20 +408,22 @@ static void spawn_watch_event(libxl__egc *egc,
libxl__ev_xswatch *xsw,
static void spawn_middle_death(libxl__egc *egc, libxl__ev_child *childw,
pid_t pid, int status)
- /* Before event, was Active or Detached;
- * is now Active or Detached except that ssd->mid is Idle */
+ /* On entry, is Attached or Detaching */
{
EGC_GC;
- libxl__spawn_state_detachable *ssd = CONTAINER_OF(childw, *ssd, mid);
- libxl__spawn_state *ss = ssd->ss;
-
- if (!WIFEXITED(status)) {
+ libxl__spawn_state *ss = CONTAINER_OF(childw, *ss, mid);
+
+ if ((ss->failed || ss->detaching) &&
+ ((WIFEXITED(status) && WEXITSTATUS(status)==0) ||
+ (WIFSIGNALED(status) && WTERMSIG(status)==SIGKILL))) {
+ /* as expected */
+ } else if (!WIFEXITED(status)) {
+ int loglevel = ss->detaching ? XTL_WARN : XTL_ERROR;
const char *what - GCSPRINTF("%s intermediate process
(startup monitor)",
- ss ? ss->what : "(detached)");
- int loglevel = ss ? XTL_ERROR : XTL_WARN;
+ GCSPRINTF("%s intermediate process (startup monitor)",
ss->what);
libxl_report_child_exitstatus(CTX, loglevel, what, pid, status);
- } else if (ss) { /* otherwise it was supposed to be a daemon by now */
+ ss->failed = 1;
+ } else {
if (!status)
LOG(ERROR, "%s [%ld]: unexpectedly exited with exit status
0,"
" when we were waiting for it to confirm startup",
@@ -430,15 +441,22 @@ static void spawn_middle_death(libxl__egc *egc,
libxl__ev_child *childw,
LOG(ERROR, "%s [%ld]: died during startup due to unknown
fatal"
" signal number %d", ss->what, (unsigned
long)pid, sig);
}
- ss->ssd = 0; /* detatch the ssd to make the ss be in state Partial
*/
- spawn_failed(egc, ss); /* must be last use of ss */
+ ss->failed = 1;
}
- free(ssd);
-}
-void libxl__spawn_detach(libxl__gc *gc, libxl__spawn_state *ss)
-{
spawn_cleanup(gc, ss);
+
+ if (ss->failed && !ss->detaching) {
+ ss->failure_cb(egc, ss); /* must be last */
+ return;
+ }
+
+ if (ss->failed && ss->detaching)
+ LOG(WARN,"%s underlying machinery seemed to fail,"
+ " but its function seems to have been successful",
ss->what);
+
+ assert(ss->detaching);
+ ss->detached_cb(egc, ss);
}
/*
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 22013fd..2592caa 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -981,8 +981,8 @@ _hidden int libxl__device_pci_destroy_all(libxl__gc *gc,
uint32_t domid);
*
* Higher-level double-fork and separate detach eg as for device models
*
- * Each libxl__spawn_state is in one of the conventional states
- * Undefined, Idle, Active
+ * Each libxl__spawn_state is in one of these states
+ * Undefined, Idle, Attached, Detaching
*/
typedef struct libxl__obsolete_spawn_starting libxl__spawn_starting;
@@ -1023,15 +1023,15 @@ _hidden void libxl__spawn_init(libxl__spawn_state*);
* intermediate or final child; an error message will have been
* logged.
*
- * confirm_cb and failure_cb will not be called reentrantly from
- * within libxl__spawn_spawn.
+ * confirm_cb, failure_cb and detached_cb will not be called
+ * reentrantly from within libxl__spawn_spawn.
*
* what: string describing the spawned process, used for logging
*
* Logs errors. A copy of "what" is taken.
* Return values:
* < 0 error, *spawn is now Idle and need not be detached
- * +1 caller is the parent, *spawn is Active and must eventually be
detached
+ * +1 caller is the parent, *spawn is Attached and must be detached
* 0 caller is now the inner child, should probably call libxl__exec
*
* The spawn state must be Undefined or Idle on entry.
@@ -1039,12 +1039,15 @@ _hidden void libxl__spawn_init(libxl__spawn_state*);
_hidden int libxl__spawn_spawn(libxl__egc *egc, libxl__spawn_state *spawn);
/*
- * libxl__spawn_detach - Detaches the daemonic child.
+ * libxl__spawn_request_detach - Detaches the daemonic child.
*
* Works by killing the intermediate process from spawn_spawn.
* After this function returns, failures of either child are no
* longer reported via failure_cb.
*
+ * This is not synchronous: there will be a further callback when
+ * the detach is complete.
+ *
* If called before the inner child has been created, this may prevent
* it from running at all. Thus this should be called only when the
* inner child has notified that it is ready. Normally it will be
@@ -1052,10 +1055,10 @@ _hidden int libxl__spawn_spawn(libxl__egc *egc,
libxl__spawn_state *spawn);
*
* Logs errors.
*
- * The spawn state must be Active or Idle on entry and will be Idle
+ * The spawn state must be Attached entry and will be Detaching
* on return.
*/
-_hidden void libxl__spawn_detach(libxl__gc *gc, libxl__spawn_state*);
+_hidden void libxl__spawn_initiate_detach(libxl__gc *gc, libxl__spawn_state*);
/*
* If successful, this should return 0.
@@ -1092,15 +1095,11 @@ typedef void libxl__spawn_failure_cb(libxl__egc*,
libxl__spawn_state*);
typedef void libxl__spawn_confirm_cb(libxl__egc*, libxl__spawn_state*,
const char *xsdata);
-typedef struct {
- /* Private to the spawn implementation.
- */
- /* This separate struct, from malloc, allows us to "detach"
- * the child and reap it later, when our user has gone
- * away and freed its libxl__spawn_state */
- struct libxl__spawn_state *ss;
- libxl__ev_child mid;
-} libxl__spawn_state_detachable;
+/*
+ * Called when the detach (requested by libxl__spawn_initiate_detach) has
+ * completed. On entry to the callback the spawn state is Idle.
+ */
+typedef void libxl__spawn_detached_cb(libxl__egc*, libxl__spawn_state*);
struct libxl__spawn_state {
/* must be filled in by user and remain valid */
@@ -1112,15 +1111,18 @@ struct libxl__spawn_state {
libxl__spawn_midproc_cb *midproc_cb;
libxl__spawn_failure_cb *failure_cb;
libxl__spawn_confirm_cb *confirm_cb;
+ libxl__spawn_detached_cb *detached_cb;
/* remaining fields are private to libxl_spawn_... */
+ 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__spawn_state_detachable *ssd;
};
static inline int libxl__spawn_inuse(libxl__spawn_state *ss)
- { return !!ss->ssd; }
+ { return libxl__ev_child_inuse(&ss->mid); }
/*
* libxl_spawner_record_pid - Record given pid in xenstore
--
1.7.2.5
Ian Jackson
2012-May-30 16:16 UTC
[PATCH 15/15] libxl: do not leak an event struct on ignored ao progress
On entry to libxl__ao_progress_report, the caller has allocated an
event. If the progress report is to be ignored, we need to free it.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
tools/libxl/libxl_event.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c
index 565d2c2..dd1a9ca 100644
--- a/tools/libxl/libxl_event.c
+++ b/tools/libxl/libxl_event.c
@@ -1601,6 +1601,7 @@ void libxl__ao_progress_report(libxl__egc *egc, libxl__ao
*ao,
ev->for_user = how->for_event;
if (how->callback == dummy_asyncprogress_callback_ignore) {
LOG(DEBUG,"ao %p: progress report: ignored",ao);
+ libxl_event_free(CTX,ev);
/* ignore */
} else if (how->callback) {
libxl__aop_occurred *aop = libxl__zalloc(&egc->gc,
sizeof(*aop));
--
1.7.2.5
Ian Campbell
2012-May-30 17:26 UTC
Re: [PATCH 01/15] libxc: xc_domain_restore, make toolstack_restore const-correct
On Wed, 2012-05-30 at 17:16 +0100, Ian Jackson wrote:> Also provide typedefs for the nontrivial function callback types.Are there any other uses of these type other than the one of each inlined in the structs? If not then I''m not sure this makes things clearer -- you now have to look at both the callback struct and then go find the corresponding typedef.> Update the one provider of this callback, in libxl. > > Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com> > --- > tools/libxc/xenguest.h | 14 ++++++++++---- > tools/libxl/libxl_dom.c | 4 ++-- > 2 files changed, 12 insertions(+), 6 deletions(-) > > diff --git a/tools/libxc/xenguest.h b/tools/libxc/xenguest.h > index 91d53f7..328a2a5 100644 > --- a/tools/libxc/xenguest.h > +++ b/tools/libxc/xenguest.h > @@ -31,6 +31,10 @@ > #define X86_64_B_SIZE 64 > #define X86_32_B_SIZE 32 > > +typedef int xc_switch_qemu_logdirty_cb(int domid, unsigned enable, void *data); > +typedef int xc_toolstack_save_cb(uint32_t domid, uint8_t **buf, > + uint32_t *len, void *data); > + > /* callbacks provided by xc_domain_save */ > struct save_callbacks { > /* Called after expiration of checkpoint interval, > @@ -61,7 +65,7 @@ struct save_callbacks { > int (*checkpoint)(void* data); > > /* Enable qemu-dm logging dirty pages to xen */ > - int (*switch_qemu_logdirty)(int domid, unsigned enable, void *data); /* HVM only */ > + xc_switch_qemu_logdirty_cb *switch_qemu_logdirty; /* HVM only */ > > /* Save toolstack specific data > * @param buf the buffer with the data to be saved > @@ -69,7 +73,7 @@ struct save_callbacks { > * The callee allocates the buffer, the caller frees it (buffer must > * be free''able). > */ > - int (*toolstack_save)(uint32_t domid, uint8_t **buf, uint32_t *len, void *data); > + xc_toolstack_save_cb *toolstack_save; > > /* to be provided as the last argument to each callback function */ > void* data; > @@ -89,11 +93,13 @@ int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom, uint32_t max_iter > unsigned long vm_generationid_addr); > > > +typedef int xc_toolstack_restore_cb(uint32_t domid, const uint8_t *buf, > + uint32_t size, void* data); > + > /* callbacks provided by xc_domain_restore */ > struct restore_callbacks { > /* callback to restore toolstack specific data */ > - int (*toolstack_restore)(uint32_t domid, uint8_t *buf, > - uint32_t size, void* data); > + xc_toolstack_restore_cb *toolstack_restore; > > /* to be provided as the last argument to each callback function */ > void* data; > diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c > index 06dbc92..929fbf2 100644 > --- a/tools/libxl/libxl_dom.c > +++ b/tools/libxl/libxl_dom.c > @@ -465,13 +465,13 @@ static inline char *restore_helper(libxl__gc *gc, uint32_t domid, > domid, phys_offset, node); > } > > -static int libxl__toolstack_restore(uint32_t domid, uint8_t *buf, > +static int libxl__toolstack_restore(uint32_t domid, const uint8_t *buf, > uint32_t size, void *data) > { > libxl__gc *gc = (libxl__gc *) data; > libxl_ctx *ctx = gc->owner; > int i, ret; > - uint8_t *ptr = buf; > + const uint8_t *ptr = buf; > uint32_t count = 0, version = 0; > struct libxl__physmap_info* pi; > char *xs_path;
Ian Campbell
2012-May-30 17:32 UTC
Re: [PATCH 02/15] libxl: domain save: rename variables etc.
> @@ -921,71 +913,72 @@ int libxl__domain_suspend_common(libxl__gc *gc, uint32_t domid, int fd, > return ERROR_INVAL; > } > > - memset(&si, 0, sizeof(si)); > - flags = (live) ? XCFLAGS_LIVE : 0 > + xcflags = (live) ? XCFLAGS_LIVE : 0 > | (debug) ? XCFLAGS_DEBUG : 0 > | (hvm) ? XCFLAGS_HVM : 0; > > + dss->domid = domid; > + dss->xcflags = xcflags; > + dss->hvm = hvm; > + dss->gc = gc; > + dss->suspend_eventchn = -1; > + dss->guest_responded = 0; > + > if (r_info != NULL) { > - si.interval = r_info->interval; > + dss->interval = r_info->interval; > if (r_info->compression) > - flags |= XCFLAGS_CHECKPOINT_COMPRESS; > - si.save_fd = fd; > + xcflags |= XCFLAGS_CHECKPOINT_COMPRESS;You now do this after the "dss->xcflags = xcflags" (since you moved that block up), won''t that mean you loose it? (xcflags gets used again below in the call to xc_domain_save, but I guess that''ll go away during asyncification).> + dss->save_fd = fd; > } > else > - si.save_fd = -1; > - > - si.domid = domid; > - si.flags = flags; > - si.hvm = hvm; > - si.gc = gc; > - si.suspend_eventchn = -1; > - si.guest_responded = 0; > + dss->save_fd = -1;
Ian Campbell
2012-May-30 17:44 UTC
Re: [PATCH 06/15] libxl: rename libxl_dom:save_helper to physmap_path
On Wed, 2012-05-30 at 17:16 +0100, Ian Jackson wrote:> "save_helper" isn''t very descriptive. Also it is now confusing > because it reads like it might refer to the libxl-save-helper > executable which runs xc_domain_save and xc_domain_restore. > > Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>Acked-by: Ian Campbell <ian.campbell@citrix.com>
Ian Campbell
2012-May-30 17:48 UTC
Re: [PATCH 07/15] libxl: provide libxl__xs_*_checked and libxl__xs_transaction_*
> +int libxl__xs_read_checked(libxl__gc *gc, xs_transaction_t t, > + const char *path, const char **result_out) > +{ > + char *result = libxl__xs_read(gc, t, path); > + if (!result) { > + if (errno != ENOENT) {Can''t you combine these with && ? [...]> +int libxl__xs_transaction_start(libxl__gc *gc, xs_transaction_t *t) > +{ > + assert(!*t); > + *t = xs_transaction_start(CTX->xsh); > + if (!*t) { > + LOGE(ERROR, "could not create xenstore transaction"); > + return ERROR_FAIL; > + } > + return 0; > +} > + > +int libxl__xs_transaction_commit(libxl__gc *gc, xs_transaction_t *t) > +{ > + assert(*t); > + > + if (!xs_transaction_end(CTX->xsh, *t, 0)) { > + if (errno == EAGAIN) > + return +1; > + > + *t = 0; > + LOGE(ERROR, "could not commit xenstore transacton");transaction It would be much more useful if this function took a "const char *what" (even just __func__ from the caller) and logged it here. To a lesser extent this applies to all these helpers.> + return ERROR_FAIL; > + } > + > + *t = 0; > + return 0; > +} > + > +void libxl__xs_transaction_abort(libxl__gc *gc, xs_transaction_t *t) > +{ > + if (!*t) > + return; > + > + if (!xs_transaction_end(CTX->xsh, *t, 1)) > + LOGE(ERROR, "could not abort xenstore transacton");transaction> + > + *t = 0; > +}
Ian Campbell
2012-May-31 07:52 UTC
Re: [PATCH 09/15] libxl: datacopier: provide "prefix data" facilit
In the subject: "facility" On Wed, 2012-05-30 at 17:16 +0100, Ian Jackson wrote:> This will be used to write the qemu data banner to the save/migration > stream. > > Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com> > --- > tools/libxl/libxl_aoutils.c | 15 +++++++++++++++ > tools/libxl/libxl_internal.h | 6 ++++++ > 2 files changed, 21 insertions(+), 0 deletions(-) > > diff --git a/tools/libxl/libxl_aoutils.c b/tools/libxl/libxl_aoutils.c > index ee0df57..208b34b 100644 > --- a/tools/libxl/libxl_aoutils.c > +++ b/tools/libxl/libxl_aoutils.c > @@ -74,6 +74,21 @@ static void datacopier_check_state(libxl__egc *egc, libxl__datacopier_state *dc) > } > } > > +void libxl__datacopier_prefixdata(libxl__egc *egc, libxl__datacopier_state *dc, > + const void *data, size_t len) > +{ > + libxl__datacopier_buf *buf; > + > + assert(len < dc->maxsz - dc->used); > + > + buf = libxl__zalloc(0, sizeof(*buf) - sizeof(buf->buf) + len); > + buf->used = len; > + memcpy(buf->buf, data, len); > + > + dc->used += len; > + LIBXL_TAILQ_INSERT_TAIL(&dc->bufs, buf, entry); > +} > + > static void datacopier_readable(libxl__egc *egc, libxl__ev_fd *ev, > int fd, short events, short revents) { > libxl__datacopier_state *dc = CONTAINER_OF(ev, *dc, toread); > diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h > index 8f4f45d..b41e72f 100644 > --- a/tools/libxl/libxl_internal.h > +++ b/tools/libxl/libxl_internal.h > @@ -1812,6 +1812,12 @@ _hidden void libxl__datacopier_init(libxl__datacopier_state *dc); > _hidden void libxl__datacopier_kill(libxl__datacopier_state *dc); > _hidden int libxl__datacopier_start(libxl__datacopier_state *dc); > > +/* Inserts literal data into the output stream. > + * May safely be used only immediately after libxl__datacopier_start.After datacopier_start the fds are registered, is there not a race between those events firing (perhaps in another thread which has called into libxl) and this thread? Is the safe place not between datacopier_init and the rest of datacopier_start?> + * (But may be called multiple times.) The data is copied. > + * NB exceeding maxsz will fail an assertion! */ > +_hidden void libxl__datacopier_prefixdata(libxl__egc*, libxl__datacopier_state*, > + const void *data, size_t len); > > /*----- Save/restore helper (used by creation and suspend) -----*/ >
Ian Campbell
2012-May-31 07:53 UTC
Re: [PATCH 12/15] xl: Handle return value from libxl_domain_suspend correctly
On Wed, 2012-05-30 at 17:16 +0100, Ian Jackson wrote:> libxl_domain_suspend returns a libxl error code. So it must be > wrapped with MUST and not CHK_ERRNO. > > Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>Acked-by: Ian Campbell <ian.campbell@citrix.com>> --- > tools/libxl/xl_cmdimpl.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c > index 9b4a80e..8c5f147 100644 > --- a/tools/libxl/xl_cmdimpl.c > +++ b/tools/libxl/xl_cmdimpl.c > @@ -2820,7 +2820,7 @@ static int save_domain(const char *p, const char *filename, int checkpoint, > > save_domain_core_writeconfig(fd, filename, config_data, config_len); > > - CHK_ERRNO(libxl_domain_suspend(ctx, domid, fd, 0, NULL)); > + MUST(libxl_domain_suspend(ctx, domid, fd, 0, NULL)); > close(fd); > > if (checkpoint)
On Wed, 2012-05-30 at 17:16 +0100, Ian Jackson wrote:> This was allocated using asprintf but never freed. Use GCSPRINTF. > > Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>Acked-by: Ian Campbell <ian.campbell@citrix.com>> --- > tools/libxl/libxl_create.c | 3 +-- > 1 files changed, 1 insertions(+), 2 deletions(-) > > diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c > index eaf378b..6babdb0 100644 > --- a/tools/libxl/libxl_create.c > +++ b/tools/libxl/libxl_create.c > @@ -739,9 +739,8 @@ void libxl__xc_domain_restore_done(libxl__egc *egc, void *dcs_void, > goto out; > > if (info->type == LIBXL_DOMAIN_TYPE_HVM) { > - ret = asprintf(&state->saved_state, > + state->saved_state = GCSPRINTF( > XC_DEVICE_MODEL_RESTORE_FILE".%d", domid); > - ret = (ret < 0) ? ERROR_FAIL : 0; > } > > out:
Ian Campbell
2012-May-31 07:55 UTC
Re: [PATCH 15/15] libxl: do not leak an event struct on ignored ao progress
On Wed, 2012-05-30 at 17:16 +0100, Ian Jackson wrote:> On entry to libxl__ao_progress_report, the caller has allocated an > event. If the progress report is to be ignored, we need to free it. > > Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>Acked-by: Ian Campbell <ian.campbell@citrix.com>> --- > tools/libxl/libxl_event.c | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c > index 565d2c2..dd1a9ca 100644 > --- a/tools/libxl/libxl_event.c > +++ b/tools/libxl/libxl_event.c > @@ -1601,6 +1601,7 @@ void libxl__ao_progress_report(libxl__egc *egc, libxl__ao *ao, > ev->for_user = how->for_event; > if (how->callback == dummy_asyncprogress_callback_ignore) { > LOG(DEBUG,"ao %p: progress report: ignored",ao); > + libxl_event_free(CTX,ev); > /* ignore */ > } else if (how->callback) { > libxl__aop_occurred *aop = libxl__zalloc(&egc->gc, sizeof(*aop));
Ian Jackson
2012-May-31 09:55 UTC
Re: [PATCH 01/15] libxc: xc_domain_restore, make toolstack_restore const-correct
Ian Campbell writes ("Re: [Xen-devel] [PATCH 01/15] libxc:
xc_domain_restore, make toolstack_restore
const-correct"):> On Wed, 2012-05-30 at 17:16 +0100, Ian Jackson wrote:
> > Also provide typedefs for the nontrivial function callback types.
>
> Are there any other uses of these type other than the one of each
> inlined in the structs?
There is one, later in my patch series, in libxl.
> If not then I''m not sure this makes things clearer -- you now have
to
> look at both the callback struct and then go find the corresponding
> typedef.
I''m not particularly bothered about this and could write the type out
again in its other location in libxl_internal.h. The compiler will
check it after all.
Let me know what you think best.
Ian.
Ian Jackson
2012-May-31 10:02 UTC
Re: [PATCH 02/15] libxl: domain save: rename variables etc.
Ian Campbell writes ("Re: [Xen-devel] [PATCH 02/15] libxl: domain save:
rename variables etc."):>
> > @@ -921,71 +913,72 @@ int libxl__domain_suspend_common(libxl__gc *gc,
uint32_t domid, int fd,
...> > + xcflags = (live) ? XCFLAGS_LIVE : 0
> > | (debug) ? XCFLAGS_DEBUG : 0
> > | (hvm) ? XCFLAGS_HVM : 0;
...> > + dss->xcflags = xcflags;
...> > + xcflags |= XCFLAGS_CHECKPOINT_COMPRESS;
>
> You now do this after the "dss->xcflags = xcflags" (since you
moved that
> block up), won''t that mean you loose it?
Yes.
> (xcflags gets used again below in the call to xc_domain_save, but I
> guess that''ll go away during asyncification).
I should have abolished the local variable entirely. Given that I''m
renaming the variable I might as well change all the references to use
dss->xcflags which eliminates the possibility to make this kind of
mistake.
So I will do that.
Ian.
Ian Jackson
2012-May-31 10:22 UTC
Re: [PATCH 07/15] libxl: provide libxl__xs_*_checked and libxl__xs_transaction_*
Ian Campbell writes ("Re: [Xen-devel] [PATCH 07/15] libxl: provide
libxl__xs_*_checked and libxl__xs_transaction_*"):>
> > +int libxl__xs_read_checked(libxl__gc *gc, xs_transaction_t t,
> > + const char *path, const char **result_out)
> > +{
> > + char *result = libxl__xs_read(gc, t, path);
> > + if (!result) {
> > + if (errno != ENOENT) {
>
> Can''t you combine these with && ?
Yes, but this matches better the way I think of it. I can change it
if you like.
> > +int libxl__xs_transaction_commit(libxl__gc *gc, xs_transaction_t *t)
> > +{
...> > + LOGE(ERROR, "could not commit xenstore
transacton");
>
> transaction
Oops.
> It would be much more useful if this function took a "const char
> *what" (even just __func__ from the caller) and logged it here.
I think that this call can only fail due to general failure of
xenstore (or our xenstore handle). So a `what'' here wouldn''t
help
much. The other helpers are more likely to fail (permissions, for
example) but they can log the path.
I played about a bit with adding more information supplied by the
callers. Ultimately what I seem to end up with is something that''s
rather too clumsy to use without helper macros which I don''t think
will assist with clarity.
Eg,
if (got) {
const char *got_ret;
rc = XLXS_READ(t, lds->ret_path, &got_ret);
if (rc) goto out;
which is all very well but hardly a model of clarity.
An alternative would be to replace `xs_transaction_t t'' with a struct
that contains at least the file and function name and a `what'', which
would be passed to the transaction start helper.
So I think I would prefer to keep things as they are.
Ian.
Ian Jackson
2012-May-31 10:32 UTC
Re: [PATCH 09/15] libxl: datacopier: provide "prefix data" facilit
Ian Campbell writes ("Re: [Xen-devel] [PATCH 09/15] libxl: datacopier:
provide "prefix data" facilit"):> In the subject: "facility"
Oops, fixed.
> On Wed, 2012-05-30 at 17:16 +0100, Ian Jackson wrote:
> > @@ -1812,6 +1812,12 @@ _hidden void
libxl__datacopier_init(libxl__datacopier_state *dc);
> > _hidden void libxl__datacopier_kill(libxl__datacopier_state *dc);
> > _hidden int libxl__datacopier_start(libxl__datacopier_state *dc);
> >
> > +/* Inserts literal data into the output stream.
> > + * May safely be used only immediately after libxl__datacopier_start.
>
> After datacopier_start the fds are registered, is there not a race
> between those events firing (perhaps in another thread which has called
> into libxl) and this thread?
No, because we have the ctx mutex held all of the time.
And there isn''t a reentrancy hazard either. As the comments in
libxl_internal.h have it:
* int libxl__ev_KIND_register(libxl__gc *gc, libxl__ev_KIND *GEN,
* libxl__ev_KIND_callback *FUNC,
* DETAILS);
* [...]. FUNC will
* not be called from within the call to _register. [...]
I think given your question this warrants a comment:
@@ -78,6 +78,13 @@ void libxl__datacopier_prefixdata(libxl__egc *egc,
libxl__datacopier_state *dc,
const void *data, size_t len)
{
libxl__datacopier_buf *buf;
+ /*
+ * It is safe for this to be called immediately after _start, as
+ * is documented in the public comment. _start''s caller must
have
+ * the mutex locked, so other threads don''t get to mess with the
+ * contents, and the fd events cannot happen reentrantly. So we
+ * are guaranteed to beat the first data from the read fd.
+ */
> Is the safe place not between datacopier_init and the rest of
> datacopier_start?
No, because _start calls _init (just like all the
libxl__*_make_events_happen functions do).
Ian.
Ian Campbell
2012-May-31 11:16 UTC
Re: [PATCH 01/15] libxc: xc_domain_restore, make toolstack_restore const-correct
On Thu, 2012-05-31 at 10:55 +0100, Ian Jackson wrote:> Ian Campbell writes ("Re: [Xen-devel] [PATCH 01/15] libxc: xc_domain_restore, make toolstack_restore const-correct"): > > On Wed, 2012-05-30 at 17:16 +0100, Ian Jackson wrote: > > > Also provide typedefs for the nontrivial function callback types. > > > > Are there any other uses of these type other than the one of each > > inlined in the structs? > > There is one, later in my patch series, in libxl.Ah, I had a look up to ptch 5 (the originally posted set) but didn''t spot it, sorry.> > If not then I''m not sure this makes things clearer -- you now have to > > look at both the callback struct and then go find the corresponding > > typedef. > > I''m not particularly bothered about this and could write the type out > again in its other location in libxl_internal.h. The compiler will > check it after all. > > Let me know what you think best.I prefer to write it out. Especially since the docs (such as they are) are in the struct too. Ian.
Ian Campbell
2012-May-31 11:17 UTC
Re: [PATCH 07/15] libxl: provide libxl__xs_*_checked and libxl__xs_transaction_*
On Thu, 2012-05-31 at 11:22 +0100, Ian Jackson wrote:> Ian Campbell writes ("Re: [Xen-devel] [PATCH 07/15] libxl: provide libxl__xs_*_checked and libxl__xs_transaction_*"): > > > > > +int libxl__xs_read_checked(libxl__gc *gc, xs_transaction_t t, > > > + const char *path, const char **result_out) > > > +{ > > > + char *result = libxl__xs_read(gc, t, path); > > > + if (!result) { > > > + if (errno != ENOENT) { > > > > Can''t you combine these with && ? > > Yes, but this matches better the way I think of it. I can change it > if you like.It''s fine, your reason for writing it that way is as good as any.> > It would be much more useful if this function took a "const char > > *what" (even just __func__ from the caller) and logged it here. > > I think that this call can only fail due to general failure of > xenstore (or our xenstore handle). So a `what'' here wouldn''t help > much. The other helpers are more likely to fail (permissions, for > example) but they can log the path.[...]> So I think I would prefer to keep things as they are.Based on your argument I agree. Ian.
Ian Campbell
2012-May-31 11:20 UTC
Re: [PATCH 09/15] libxl: datacopier: provide "prefix data" facilit
On Thu, 2012-05-31 at 11:32 +0100, Ian Jackson wrote:> > On Wed, 2012-05-30 at 17:16 +0100, Ian Jackson wrote: > > > @@ -1812,6 +1812,12 @@ _hidden void libxl__datacopier_init(libxl__datacopier_state *dc); > > > _hidden void libxl__datacopier_kill(libxl__datacopier_state *dc); > > > _hidden int libxl__datacopier_start(libxl__datacopier_state *dc); > > > > > > +/* Inserts literal data into the output stream. > > > + * May safely be used only immediately after libxl__datacopier_start. > > > > After datacopier_start the fds are registered, is there not a race > > between those events firing (perhaps in another thread which has called > > into libxl) and this thread? > > No, because we have the ctx mutex held all of the time.Ah!.> > And there isn''t a reentrancy hazard either. As the comments in > libxl_internal.h have it: > > * int libxl__ev_KIND_register(libxl__gc *gc, libxl__ev_KIND *GEN, > * libxl__ev_KIND_callback *FUNC, > * DETAILS); > * [...]. FUNC will > * not be called from within the call to _register. [...] > > I think given your question this warrants a comment: > > @@ -78,6 +78,13 @@ void libxl__datacopier_prefixdata(libxl__egc *egc, libxl__datacopier_state *dc, > const void *data, size_t len) > { > libxl__datacopier_buf *buf; > + /* > + * It is safe for this to be called immediately after _start, as > + * is documented in the public comment. _start''s caller must have > + * the mutex locked, so other threads don''t get to mess with the > + * contents, and the fd events cannot happen reentrantly.Perhaps this could be more explicit about having to hold the mutex from before _start until after any _prefixdata calls? Perhaps "...called immediately after _start, while still holding the mutex, as is documented in the public ..." ? "The mutex" here is the CTX lock, right?> So we > + * are guaranteed to beat the first data from the read fd. > + */ > > > Is the safe place not between datacopier_init and the rest of > > datacopier_start? > > No, because _start calls _init (just like all the > libxl__*_make_events_happen functions do).I spotted that, which is why I said between the _init and the "rest of" _start, but my original premise was wrong anyway so never mind...> > Ian.
Ian Jackson
2012-May-31 14:00 UTC
Re: [PATCH 01/15] libxc: xc_domain_restore, make toolstack_restore const-correct
Ian Campbell writes ("Re: [Xen-devel] [PATCH 01/15] libxc:
xc_domain_restore, make toolstack_restore
const-correct"):> On Thu, 2012-05-31 at 10:55 +0100, Ian Jackson wrote:
> > I''m not particularly bothered about this and could write the
type out
> > again in its other location in libxl_internal.h. The compiler will
> > check it after all.
> >
> > Let me know what you think best.
>
> I prefer to write it out. Especially since the docs (such as they are)
> are in the struct too.
OK.
Ian.
Ian Jackson
2012-May-31 14:07 UTC
Re: [PATCH 09/15] libxl: datacopier: provide "prefix data" facilit
Ian Campbell writes ("Re: [Xen-devel] [PATCH 09/15] libxl: datacopier:
provide "prefix data" facilit"):> On Thu, 2012-05-31 at 11:32 +0100, Ian Jackson wrote:
> > + * It is safe for this to be called immediately after _start,
as
> > + * is documented in the public comment. _start''s
caller must have
> > + * the mutex locked, so other threads don''t get to
mess with the
> > + * contents, and the fd events cannot happen reentrantly.
>
> Perhaps this could be more explicit about having to hold the mutex from
> before _start until after any _prefixdata calls?
OK:
/* Inserts literal data into the output stream. The data is copied.
* May safely be used only immediately after libxl__datacopier_start
* (before the ctx is unlocked). But may be called multiple times.
* NB exceeding maxsz will fail an assertion! */
_hidden void libxl__datacopier_prefixdata(libxl__egc*,
libxl__datacopier_state*,
const void *data, size_t len);
> "The mutex" here is the CTX lock, right?
Yes. I''ve clarified that by writing `ctx locked'' instead.
Ian.