Hi all,
this patch series introduces a new xc_save_id called
XC_SAVE_ID_TOOLSTACK to save/restore toolstack specific information.
Libxl is going to use the new save_id to save and restore qemu''s
physmap.
Changes in v2:
- xc_domain_save frees the buffer allocated by the callback;
- introduce a version number in the libxl save record;
- define libxl__physmap_info and use it to read/store information to the
buffer.
Stefano Stabellini (2):
libxc: introduce XC_SAVE_ID_TOOLSTACK
libxl: save/restore qemu''s physmap
tools/libxc/xc_domain_restore.c | 66 ++++++++++++++------
tools/libxc/xc_domain_save.c | 17 +++++
tools/libxc/xenguest.h | 23 +++++++-
tools/libxc/xg_save_restore.h | 1 +
tools/libxl/libxl_dom.c | 132 ++++++++++++++++++++++++++++++++++++++-
tools/xcutils/xc_restore.c | 3 +-
6 files changed, 220 insertions(+), 22 deletions(-)
Cheers,
Stefano
Stefano Stabellini
2012-Jan-20 17:25 UTC
[PATCH v2 1/2] libxc: introduce XC_SAVE_ID_TOOLSTACK
Introduce a new save_id to save/restore toolstack specific extra
information.
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
tools/libxc/xc_domain_restore.c | 66 +++++++++++++++++++++++++++-----------
tools/libxc/xc_domain_save.c | 17 ++++++++++
tools/libxc/xenguest.h | 23 +++++++++++++-
tools/libxc/xg_save_restore.h | 1 +
tools/libxl/libxl_dom.c | 2 +-
tools/xcutils/xc_restore.c | 3 +-
6 files changed, 90 insertions(+), 22 deletions(-)
diff --git a/tools/libxc/xc_domain_restore.c b/tools/libxc/xc_domain_restore.c
index 14451d1..72b6d5b 100644
--- a/tools/libxc/xc_domain_restore.c
+++ b/tools/libxc/xc_domain_restore.c
@@ -702,7 +702,8 @@ static void pagebuf_free(pagebuf_t* buf)
}
static int pagebuf_get_one(xc_interface *xch, struct restore_ctx *ctx,
- pagebuf_t* buf, int fd, uint32_t dom)
+ pagebuf_t* buf, int fd, uint32_t dom,
+ struct restore_callbacks* callbacks)
{
int count, countpages, oldcount, i;
void* ptmp;
@@ -725,7 +726,7 @@ static int pagebuf_get_one(xc_interface *xch, struct
restore_ctx *ctx,
case XC_SAVE_ID_ENABLE_VERIFY_MODE:
DPRINTF("Entering page verify mode\n");
buf->verify = 1;
- return pagebuf_get_one(xch, ctx, buf, fd, dom);
+ return pagebuf_get_one(xch, ctx, buf, fd, dom, callbacks);
case XC_SAVE_ID_VCPU_INFO:
buf->new_ctxt_format = 1;
@@ -736,7 +737,7 @@ static int pagebuf_get_one(xc_interface *xch, struct
restore_ctx *ctx,
return -1;
}
// DPRINTF("Max VCPU ID: %d, vcpumap: %llx\n",
buf->max_vcpu_id, buf->vcpumap);
- return pagebuf_get_one(xch, ctx, buf, fd, dom);
+ return pagebuf_get_one(xch, ctx, buf, fd, dom, callbacks);
case XC_SAVE_ID_HVM_IDENT_PT:
/* Skip padding 4 bytes then read the EPT identity PT location. */
@@ -747,7 +748,7 @@ static int pagebuf_get_one(xc_interface *xch, struct
restore_ctx *ctx,
return -1;
}
// DPRINTF("EPT identity map address: %llx\n",
buf->identpt);
- return pagebuf_get_one(xch, ctx, buf, fd, dom);
+ return pagebuf_get_one(xch, ctx, buf, fd, dom, callbacks);
case XC_SAVE_ID_HVM_VM86_TSS:
/* Skip padding 4 bytes then read the vm86 TSS location. */
@@ -758,7 +759,7 @@ static int pagebuf_get_one(xc_interface *xch, struct
restore_ctx *ctx,
return -1;
}
// DPRINTF("VM86 TSS location: %llx\n", buf->vm86_tss);
- return pagebuf_get_one(xch, ctx, buf, fd, dom);
+ return pagebuf_get_one(xch, ctx, buf, fd, dom, callbacks);
case XC_SAVE_ID_TMEM:
DPRINTF("xc_domain_restore start tmem\n");
@@ -766,14 +767,14 @@ static int pagebuf_get_one(xc_interface *xch, struct
restore_ctx *ctx,
PERROR("error reading/restoring tmem");
return -1;
}
- return pagebuf_get_one(xch, ctx, buf, fd, dom);
+ return pagebuf_get_one(xch, ctx, buf, fd, dom, callbacks);
case XC_SAVE_ID_TMEM_EXTRA:
if ( xc_tmem_restore_extra(xch, dom, fd) ) {
PERROR("error reading/restoring tmem extra");
return -1;
}
- return pagebuf_get_one(xch, ctx, buf, fd, dom);
+ return pagebuf_get_one(xch, ctx, buf, fd, dom, callbacks);
case XC_SAVE_ID_TSC_INFO:
{
@@ -787,7 +788,7 @@ static int pagebuf_get_one(xc_interface *xch, struct
restore_ctx *ctx,
PERROR("error reading/restoring tsc info");
return -1;
}
- return pagebuf_get_one(xch, ctx, buf, fd, dom);
+ return pagebuf_get_one(xch, ctx, buf, fd, dom, callbacks);
}
case XC_SAVE_ID_HVM_CONSOLE_PFN :
@@ -799,12 +800,12 @@ static int pagebuf_get_one(xc_interface *xch, struct
restore_ctx *ctx,
return -1;
}
// DPRINTF("console pfn location: %llx\n",
buf->console_pfn);
- return pagebuf_get_one(xch, ctx, buf, fd, dom);
+ return pagebuf_get_one(xch, ctx, buf, fd, dom, callbacks);
case XC_SAVE_ID_LAST_CHECKPOINT:
ctx->last_checkpoint = 1;
// DPRINTF("last checkpoint indication received");
- return pagebuf_get_one(xch, ctx, buf, fd, dom);
+ return pagebuf_get_one(xch, ctx, buf, fd, dom, callbacks);
case XC_SAVE_ID_HVM_ACPI_IOPORTS_LOCATION:
/* Skip padding 4 bytes then read the acpi ioport location. */
@@ -814,7 +815,7 @@ static int pagebuf_get_one(xc_interface *xch, struct
restore_ctx *ctx,
PERROR("error read the acpi ioport location");
return -1;
}
- return pagebuf_get_one(xch, ctx, buf, fd, dom);
+ return pagebuf_get_one(xch, ctx, buf, fd, dom, callbacks);
case XC_SAVE_ID_HVM_VIRIDIAN:
/* Skip padding 4 bytes then read the acpi ioport location. */
@@ -824,8 +825,33 @@ static int pagebuf_get_one(xc_interface *xch, struct
restore_ctx *ctx,
PERROR("error read the viridian flag");
return -1;
}
- return pagebuf_get_one(xch, ctx, buf, fd, dom);
+ return pagebuf_get_one(xch, ctx, buf, fd, dom, callbacks);
+ case XC_SAVE_ID_TOOLSTACK:
+ {
+ uint32_t len;
+ uint8_t *buf2;
+ RDEXACT(fd, &len, sizeof(len));
+ buf2 = (uint8_t*) malloc(len);
+ if ( buf2 == NULL )
+ {
+ PERROR("error memory allocation");
+ return -1;
+ }
+ RDEXACT(fd, buf2, len);
+ if ( callbacks != NULL && callbacks->toolstack_restore
!= NULL )
+ {
+ if ( callbacks->toolstack_restore(dom,
+ buf2, len, callbacks->data) < 0 )
+ {
+ PERROR("error calling toolstack_restore");
+ free(buf2);
+ return -1;
+ }
+ }
+ free(buf2);
+ return pagebuf_get_one(xch, ctx, buf, fd, dom, callbacks);
+ }
case XC_SAVE_ID_ENABLE_COMPRESSION:
/* We cannot set compression flag directly in pagebuf structure,
* since this pagebuf still has uncompressed pages that are yet to
@@ -834,7 +860,7 @@ static int pagebuf_get_one(xc_interface *xch, struct
restore_ctx *ctx,
*/
ctx->compressing = 1;
// DPRINTF("compression flag received");
- return pagebuf_get_one(xch, ctx, buf, fd, dom);
+ return pagebuf_get_one(xch, ctx, buf, fd, dom, callbacks);
case XC_SAVE_ID_COMPRESSED_DATA:
@@ -902,7 +928,7 @@ static int pagebuf_get_one(xc_interface *xch, struct
restore_ctx *ctx,
* following a <XC_SAVE_ID_COMPRESSED_DATA, compressedChunkSize>
tuple.
*/
if (buf->compressing)
- return pagebuf_get_one(xch, ctx, buf, fd, dom);
+ return pagebuf_get_one(xch, ctx, buf, fd, dom, callbacks);
oldcount = buf->nr_physpages;
buf->nr_physpages += countpages;
@@ -927,7 +953,8 @@ static int pagebuf_get_one(xc_interface *xch, struct
restore_ctx *ctx,
}
static int pagebuf_get(xc_interface *xch, struct restore_ctx *ctx,
- pagebuf_t* buf, int fd, uint32_t dom)
+ pagebuf_t* buf, int fd, uint32_t dom,
+ struct restore_callbacks *callbacks)
{
int rc;
@@ -935,7 +962,7 @@ static int pagebuf_get(xc_interface *xch, struct restore_ctx
*ctx,
buf->compbuf_pos = buf->compbuf_size = 0;
do {
- rc = pagebuf_get_one(xch, ctx, buf, fd, dom);
+ rc = pagebuf_get_one(xch, ctx, buf, fd, dom, callbacks);
} while (rc > 0);
if (rc < 0)
@@ -1248,7 +1275,8 @@ static int apply_batch(xc_interface *xch, uint32_t dom,
struct restore_ctx *ctx,
int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom,
unsigned int store_evtchn, unsigned long *store_mfn,
unsigned int console_evtchn, unsigned long *console_mfn,
- unsigned int hvm, unsigned int pae, int superpages)
+ unsigned int hvm, unsigned int pae, int superpages,
+ struct restore_callbacks* callbacks)
{
DECLARE_DOMCTL;
int rc = 1, frc, i, j, n, m, pae_extended_cr3 = 0, ext_vcpucontext = 0;
@@ -1427,7 +1455,7 @@ int xc_domain_restore(xc_interface *xch, int io_fd,
uint32_t dom,
if ( !ctx->completed ) {
pagebuf.nr_physpages = pagebuf.nr_pages = 0;
pagebuf.compbuf_pos = pagebuf.compbuf_size = 0;
- if ( pagebuf_get_one(xch, ctx, &pagebuf, io_fd, dom) < 0 ) {
+ if ( pagebuf_get_one(xch, ctx, &pagebuf, io_fd, dom, callbacks)
< 0 ) {
PERROR("Error when reading batch");
goto out;
}
@@ -1542,7 +1570,7 @@ int xc_domain_restore(xc_interface *xch, int io_fd,
uint32_t dom,
// DPRINTF("Buffered checkpoint\n");
- if ( pagebuf_get(xch, ctx, &pagebuf, io_fd, dom) ) {
+ if ( pagebuf_get(xch, ctx, &pagebuf, io_fd, dom, callbacks) ) {
PERROR("error when buffering batch, finishing");
goto finish;
}
diff --git a/tools/libxc/xc_domain_save.c b/tools/libxc/xc_domain_save.c
index a6bb894..67bcfe2 100644
--- a/tools/libxc/xc_domain_save.c
+++ b/tools/libxc/xc_domain_save.c
@@ -1676,6 +1676,23 @@ int xc_domain_save(xc_interface *xch, int io_fd, uint32_t
dom, uint32_t max_iter
}
}
+ if ( callbacks != NULL && callbacks->toolstack_save != NULL )
+ {
+ int id = XC_SAVE_ID_TOOLSTACK;
+ uint8_t *buf;
+ uint32_t len;
+
+ if ( callbacks->toolstack_save(dom, &buf, &len,
callbacks->data) < 0 )
+ {
+ PERROR("Error calling toolstack_save");
+ goto out;
+ }
+ wrexact(io_fd, &id, sizeof(id));
+ wrexact(io_fd, &len, sizeof(len));
+ wrexact(io_fd, buf, len);
+ free(buf);
+ }
+
if ( !callbacks->checkpoint )
{
/*
diff --git a/tools/libxc/xenguest.h b/tools/libxc/xenguest.h
index 4475ee9..3cac30c 100644
--- a/tools/libxc/xenguest.h
+++ b/tools/libxc/xenguest.h
@@ -44,6 +44,14 @@ struct save_callbacks {
/* Enable qemu-dm logging dirty pages to xen */
int (*switch_qemu_logdirty)(int domid, unsigned enable, void *data); /* HVM
only */
+ /* Save toolstack specific data
+ * @param buf the buffer with the data to be saved
+ * @param len the length of the buffer
+ * 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);
+
/* to be provided as the last argument to each callback function */
void* data;
};
@@ -61,6 +69,16 @@ int xc_domain_save(xc_interface *xch, int io_fd, uint32_t
dom, uint32_t max_iter
struct save_callbacks* callbacks, int hvm);
+/* 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);
+
+ /* to be provided as the last argument to each callback function */
+ void* data;
+};
+
/**
* This function will restore a saved domain.
*
@@ -72,12 +90,15 @@ int xc_domain_save(xc_interface *xch, int io_fd, uint32_t
dom, uint32_t max_iter
* @parm hvm non-zero if this is a HVM restore
* @parm pae non-zero if this HVM domain has PAE support enabled
* @parm superpages non-zero to allocate guest memory with superpages
+ * @parm callbacks non-NULL to receive a callback to restore toolstack
+ * specific data
* @return 0 on success, -1 on failure
*/
int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom,
unsigned int store_evtchn, unsigned long *store_mfn,
unsigned int console_evtchn, unsigned long *console_mfn,
- unsigned int hvm, unsigned int pae, int superpages);
+ unsigned int hvm, unsigned int pae, int superpages,
+ struct restore_callbacks *callbacks);
/**
* xc_domain_restore writes a file to disk that contains the device
* model saved state.
diff --git a/tools/libxc/xg_save_restore.h b/tools/libxc/xg_save_restore.h
index c5e0743..68feec5 100644
--- a/tools/libxc/xg_save_restore.h
+++ b/tools/libxc/xg_save_restore.h
@@ -253,6 +253,7 @@
#define XC_SAVE_ID_HVM_VIRIDIAN -11
#define XC_SAVE_ID_COMPRESSED_DATA -12 /* Marker to indicate arrival of
compressed data */
#define XC_SAVE_ID_ENABLE_COMPRESSION -13 /* Marker to enable compression logic
at receiver side */
+#define XC_SAVE_ID_TOOLSTACK -14 /* Optional toolstack specific info
*/
/*
** We process save/restore/migrate in batches of pages; the below
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index c898d89..fd2b051 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -373,7 +373,7 @@ int libxl__domain_restore_common(libxl__gc *gc, uint32_t
domid,
rc = xc_domain_restore(ctx->xch, fd, domid,
state->store_port, &state->store_mfn,
state->console_port, &state->console_mfn,
- hvm, pae, superpages);
+ hvm, pae, superpages, NULL);
if ( rc ) {
LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "restoring domain");
return ERROR_FAIL;
diff --git a/tools/xcutils/xc_restore.c b/tools/xcutils/xc_restore.c
index 985cbec..eb1d31c 100644
--- a/tools/xcutils/xc_restore.c
+++ b/tools/xcutils/xc_restore.c
@@ -46,7 +46,8 @@ main(int argc, char **argv)
superpages = !!hvm;
ret = xc_domain_restore(xch, io_fd, domid, store_evtchn, &store_mfn,
- console_evtchn, &console_mfn, hvm, pae,
superpages);
+ console_evtchn, &console_mfn, hvm, pae,
superpages,
+ NULL);
if ( ret == 0 )
{
--
1.7.2.5
Stefano Stabellini
2012-Jan-20 17:25 UTC
[PATCH v2 2/2] libxl: save/restore qemu''s physmap
Read Qemu''s physmap from xenstore and save it using toolstack_save.
Restore Qemu''s physmap using toolstack_restore.
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
tools/libxl/libxl_dom.c | 132 ++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 131 insertions(+), 1 deletions(-)
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index fd2b051..644d0d2 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -347,6 +347,66 @@ out:
return rc;
}
+struct libxl__physmap_info {
+ uint64_t phys_offset;
+ uint64_t start_addr;
+ uint64_t size;
+ uint32_t namelen;
+ char name[];
+};
+
+#define TOOLSTACK_SAVE_VERSION 1
+
+static int libxl__toolstack_restore(uint32_t domid, uint8_t *buf,
+ uint32_t size, void *data)
+{
+ libxl__gc *gc = (libxl__gc *) data;
+ int i, ret;
+ uint8_t *ptr = buf;
+ uint32_t count = 0, version = 0;
+ struct libxl__physmap_info* pi;
+
+ if (size < sizeof(version) + sizeof(count))
+ return -1;
+
+ memcpy(&version, ptr, sizeof(version));
+ ptr += sizeof(version);
+
+ if (version != TOOLSTACK_SAVE_VERSION)
+ return -1;
+
+ memcpy(&count, ptr, sizeof(count));
+ ptr += sizeof(count);
+
+ if (size < sizeof(version) + sizeof(count) +
+ count * (sizeof(struct libxl__physmap_info)))
+ return -1;
+
+ for (i = 0; i < count; i++) {
+ pi = (struct libxl__physmap_info*) ptr;
+ ptr += sizeof(struct libxl__physmap_info) + pi->namelen;
+
+ ret = libxl__xs_write(gc, 0, libxl__sprintf(gc,
+
"/local/domain/0/device-model/%d/physmap/%"PRIx64"/start_addr",
+ domid, pi->phys_offset), "%"PRIx64,
pi->start_addr);
+ if (ret)
+ return -1;
+ ret = libxl__xs_write(gc, 0, libxl__sprintf(gc,
+
"/local/domain/0/device-model/%d/physmap/%"PRIx64"/size",
+ domid, pi->phys_offset), "%"PRIx64,
pi->size);
+ if (ret)
+ return -1;
+ if (pi->namelen > 0) {
+ ret = libxl__xs_write(gc, 0, libxl__sprintf(gc,
+
"/local/domain/0/device-model/%d/physmap/%"PRIx64"/name",
+ domid, pi->phys_offset), "%s",
pi->name);
+ if (ret)
+ return -1;
+ }
+ }
+ return 0;
+}
+
int libxl__domain_restore_common(libxl__gc *gc, uint32_t domid,
libxl_domain_build_info *info,
libxl__domain_build_state *state,
@@ -356,11 +416,14 @@ int libxl__domain_restore_common(libxl__gc *gc, uint32_t
domid,
/* read signature */
int rc;
int hvm, pae, superpages;
+ struct restore_callbacks callbacks;
switch (info->type) {
case LIBXL_DOMAIN_TYPE_HVM:
hvm = 1;
superpages = 1;
pae = info->u.hvm.pae;
+ callbacks.toolstack_restore = libxl__toolstack_restore;
+ callbacks.data = gc;
break;
case LIBXL_DOMAIN_TYPE_PV:
hvm = 0;
@@ -373,7 +436,7 @@ int libxl__domain_restore_common(libxl__gc *gc, uint32_t
domid,
rc = xc_domain_restore(ctx->xch, fd, domid,
state->store_port, &state->store_mfn,
state->console_port, &state->console_mfn,
- hvm, pae, superpages, NULL);
+ hvm, pae, superpages, &callbacks);
if ( rc ) {
LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "restoring domain");
return ERROR_FAIL;
@@ -528,6 +591,72 @@ static int libxl__domain_suspend_common_callback(void
*data)
return 0;
}
+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;
+ int i = 0;
+ char *start_addr = NULL, *size = NULL, *phys_offset = NULL, *name = NULL;
+ unsigned int num = 0;
+ uint32_t count = 0, version = TOOLSTACK_SAVE_VERSION, namelen = 0;
+ uint8_t *ptr = NULL;
+ char **entries = NULL;
+ struct libxl__physmap_info *pi;
+
+ entries = libxl__xs_directory(gc, 0, libxl__sprintf(gc,
+ "/local/domain/0/device-model/%d/physmap", domid),
&num);
+ count = num;
+
+ *len = sizeof(version) + sizeof(count);
+ *buf = calloc(1, *len);
+ ptr = *buf;
+ if (*buf == NULL)
+ return -1;
+
+ memcpy(ptr, &version, sizeof(version));
+ ptr += sizeof(version);
+ memcpy(ptr, &count, sizeof(count));
+ ptr += sizeof(count);
+
+ for (i = 0; i < count; i++) {
+ unsigned long offset;
+ phys_offset = entries[i];
+ start_addr = libxl__xs_read(gc, 0, libxl__sprintf(gc,
+
"/local/domain/0/device-model/%d/physmap/%s/start_addr",
+ domid, phys_offset));
+ size = libxl__xs_read(gc, 0, libxl__sprintf(gc,
+
"/local/domain/0/device-model/%d/physmap/%s/size",
+ domid, phys_offset));
+ name = libxl__xs_read(gc, 0, libxl__sprintf(gc,
+
"/local/domain/0/device-model/%d/physmap/%s/name",
+ domid, phys_offset));
+
+ if (start_addr == NULL || size == NULL || phys_offset == NULL)
+ return -1;
+
+ if (name == NULL)
+ namelen = 0;
+ else
+ namelen = strlen(name) + 1;
+ *len += namelen + sizeof(struct libxl__physmap_info);
+ offset = ptr - (*buf);
+ *buf = realloc(*buf, *len);
+ if (*buf == NULL)
+ return -1;
+ ptr = (*buf) + offset;
+ pi = (struct libxl__physmap_info *) ptr;
+ pi->phys_offset = strtoll(phys_offset, NULL, 16);
+ pi->start_addr = strtoll(start_addr, NULL, 16);
+ pi->size = strtoll(size, NULL, 16);
+ pi->namelen = namelen;
+ memcpy(pi->name, name, namelen);
+ ptr += sizeof(struct libxl__physmap_info) + namelen;
+ }
+
+ return 0;
+}
+
int libxl__domain_suspend_common(libxl__gc *gc, uint32_t domid, int fd,
libxl_domain_type type,
int live, int debug)
@@ -579,6 +708,7 @@ int libxl__domain_suspend_common(libxl__gc *gc, uint32_t
domid, int fd,
memset(&callbacks, 0, sizeof(callbacks));
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;
rc = xc_domain_save(ctx->xch, fd, domid, 0, 0, flags, &callbacks,
hvm);
--
1.7.2.5
Shriram Rajagopalan
2012-Jan-27 20:12 UTC
Re: [PATCH v2 1/2] libxc: introduce XC_SAVE_ID_TOOLSTACK
On Fri, Jan 20, 2012 at 9:25 AM, Stefano Stabellini < stefano.stabellini@eu.citrix.com> wrote:> Introduce a new save_id to save/restore toolstack specific extra > information. > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > --- > tools/libxc/xc_domain_restore.c | 66 > +++++++++++++++++++++++++++----------- > tools/libxc/xc_domain_save.c | 17 ++++++++++ > tools/libxc/xenguest.h | 23 +++++++++++++- > tools/libxc/xg_save_restore.h | 1 + > tools/libxl/libxl_dom.c | 2 +- > tools/xcutils/xc_restore.c | 3 +- > 6 files changed, 90 insertions(+), 22 deletions(-) > > diff --git a/tools/libxc/xc_domain_restore.c > b/tools/libxc/xc_domain_restore.c > index 14451d1..72b6d5b 100644 > --- a/tools/libxc/xc_domain_restore.c > +++ b/tools/libxc/xc_domain_restore.c > @@ -702,7 +702,8 @@ static void pagebuf_free(pagebuf_t* buf) > } > > static int pagebuf_get_one(xc_interface *xch, struct restore_ctx *ctx, > - pagebuf_t* buf, int fd, uint32_t dom) > + pagebuf_t* buf, int fd, uint32_t dom, > + struct restore_callbacks* callbacks) > { > int count, countpages, oldcount, i; > void* ptmp; > @@ -725,7 +726,7 @@ static int pagebuf_get_one(xc_interface *xch, struct > restore_ctx *ctx, > case XC_SAVE_ID_ENABLE_VERIFY_MODE: > DPRINTF("Entering page verify mode\n"); > buf->verify = 1; > - return pagebuf_get_one(xch, ctx, buf, fd, dom); > + return pagebuf_get_one(xch, ctx, buf, fd, dom, callbacks); > > case XC_SAVE_ID_VCPU_INFO: > buf->new_ctxt_format = 1; > @@ -736,7 +737,7 @@ static int pagebuf_get_one(xc_interface *xch, struct > restore_ctx *ctx, > return -1; > } > // DPRINTF("Max VCPU ID: %d, vcpumap: %llx\n", buf->max_vcpu_id, > buf->vcpumap); > - return pagebuf_get_one(xch, ctx, buf, fd, dom); > + return pagebuf_get_one(xch, ctx, buf, fd, dom, callbacks); > > case XC_SAVE_ID_HVM_IDENT_PT: > /* Skip padding 4 bytes then read the EPT identity PT location. */ > @@ -747,7 +748,7 @@ static int pagebuf_get_one(xc_interface *xch, struct > restore_ctx *ctx, > return -1; > } > // DPRINTF("EPT identity map address: %llx\n", buf->identpt); > - return pagebuf_get_one(xch, ctx, buf, fd, dom); > + return pagebuf_get_one(xch, ctx, buf, fd, dom, callbacks); > > case XC_SAVE_ID_HVM_VM86_TSS: > /* Skip padding 4 bytes then read the vm86 TSS location. */ > @@ -758,7 +759,7 @@ static int pagebuf_get_one(xc_interface *xch, struct > restore_ctx *ctx, > return -1; > } > // DPRINTF("VM86 TSS location: %llx\n", buf->vm86_tss); > - return pagebuf_get_one(xch, ctx, buf, fd, dom); > + return pagebuf_get_one(xch, ctx, buf, fd, dom, callbacks); > > case XC_SAVE_ID_TMEM: > DPRINTF("xc_domain_restore start tmem\n"); > @@ -766,14 +767,14 @@ static int pagebuf_get_one(xc_interface *xch, struct > restore_ctx *ctx, > PERROR("error reading/restoring tmem"); > return -1; > } > - return pagebuf_get_one(xch, ctx, buf, fd, dom); > + return pagebuf_get_one(xch, ctx, buf, fd, dom, callbacks); > > case XC_SAVE_ID_TMEM_EXTRA: > if ( xc_tmem_restore_extra(xch, dom, fd) ) { > PERROR("error reading/restoring tmem extra"); > return -1; > } > - return pagebuf_get_one(xch, ctx, buf, fd, dom); > + return pagebuf_get_one(xch, ctx, buf, fd, dom, callbacks); > > case XC_SAVE_ID_TSC_INFO: > { > @@ -787,7 +788,7 @@ static int pagebuf_get_one(xc_interface *xch, struct > restore_ctx *ctx, > PERROR("error reading/restoring tsc info"); > return -1; > } > - return pagebuf_get_one(xch, ctx, buf, fd, dom); > + return pagebuf_get_one(xch, ctx, buf, fd, dom, callbacks); > } > > case XC_SAVE_ID_HVM_CONSOLE_PFN : > @@ -799,12 +800,12 @@ static int pagebuf_get_one(xc_interface *xch, struct > restore_ctx *ctx, > return -1; > } > // DPRINTF("console pfn location: %llx\n", buf->console_pfn); > - return pagebuf_get_one(xch, ctx, buf, fd, dom); > + return pagebuf_get_one(xch, ctx, buf, fd, dom, callbacks); > > case XC_SAVE_ID_LAST_CHECKPOINT: > ctx->last_checkpoint = 1; > // DPRINTF("last checkpoint indication received"); > - return pagebuf_get_one(xch, ctx, buf, fd, dom); > + return pagebuf_get_one(xch, ctx, buf, fd, dom, callbacks); > > case XC_SAVE_ID_HVM_ACPI_IOPORTS_LOCATION: > /* Skip padding 4 bytes then read the acpi ioport location. */ > @@ -814,7 +815,7 @@ static int pagebuf_get_one(xc_interface *xch, struct > restore_ctx *ctx, > PERROR("error read the acpi ioport location"); > return -1; > } > - return pagebuf_get_one(xch, ctx, buf, fd, dom); > + return pagebuf_get_one(xch, ctx, buf, fd, dom, callbacks); > > case XC_SAVE_ID_HVM_VIRIDIAN: > /* Skip padding 4 bytes then read the acpi ioport location. */ > @@ -824,8 +825,33 @@ static int pagebuf_get_one(xc_interface *xch, struct > restore_ctx *ctx, > PERROR("error read the viridian flag"); > return -1; > } > - return pagebuf_get_one(xch, ctx, buf, fd, dom); > + return pagebuf_get_one(xch, ctx, buf, fd, dom, callbacks); > > + case XC_SAVE_ID_TOOLSTACK: > + { > + uint32_t len; > + uint8_t *buf2; > + RDEXACT(fd, &len, sizeof(len)); > + buf2 = (uint8_t*) malloc(len); > + if ( buf2 == NULL ) > + { > + PERROR("error memory allocation"); > + return -1; > + } > + RDEXACT(fd, buf2, len); > + if ( callbacks != NULL && callbacks->toolstack_restore !> NULL ) > + { > + if ( callbacks->toolstack_restore(dom, > + buf2, len, callbacks->data) < 0 ) > + { >Pagebuf() shouldnt be modifying any domain state until the entire memory buffer is obtained, esp the device state. See below. @@ -1542,7 +1570,7 @@ int xc_domain_restore(xc_interface *xch, int io_fd,> uint32_t dom, > > // DPRINTF("Buffered checkpoint\n"); > > - if ( pagebuf_get(xch, ctx, &pagebuf, io_fd, dom) ) { > + if ( pagebuf_get(xch, ctx, &pagebuf, io_fd, dom, callbacks) ) { > PERROR("error when buffering batch, finishing"); > goto finish; > } >If there is an error in applying the toolstack state, then pagebuf_get returns -1 and the rest of the code would still attempt to resume the domain, with possibly inconsistent device model state. Also, lets say there was no error in applying the toolstack state. If a network error occurs while receiving the next XC_SAVE_ID or so, then again, the code following the above snippet would attempt to resume the domain, with a memory state inconsistent with the device state. The right place to call the callbacks->toolstack_restore would be after the finish_hvm: label, where currently the old QEMU device state is restored. shriram _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini
2012-Jan-30 14:16 UTC
Re: [PATCH v2 1/2] libxc: introduce XC_SAVE_ID_TOOLSTACK
On Fri, 27 Jan 2012, Shriram Rajagopalan wrote:> If there is an error in applying the toolstack state, then pagebuf_get returns -1 and > the rest of the code would still attempt to resume the domain, with possibly > inconsistent device model state.This sounds like an unhandled error in the caller of pagebuf_get. I think that if pagebuf_get returns an error, the error should be propagated and the migration should be aborted, right? After all I don''t think we can successfully resume the domain if we fail to read XC_SAVE_ID_TSC_INFO, for example. I think we need something like this: @@ -1589,9 +1616,10 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom, // DPRINTF("Buffered checkpoint\n"); - if ( pagebuf_get(xch, ctx, &pagebuf, io_fd, dom) ) { + if ( pagebuf_get(xch, ctx, &pagebuf, io_fd, dom) < 0 ) { PERROR("error when buffering batch, finishing"); - goto finish; + goto out; } memset(&tmptail, 0, sizeof(tmptail)); tmptail.ishvm = hvm;> Also, lets say there was no error in applying the toolstack state. If a network > error occurs while receiving the next XC_SAVE_ID or so, then again, the code following > the above snippet would attempt to resume the domain, with a memory state inconsistent > with the device state.This seems wrong to me, but I am not very familiar with this protocol. As I wrote above, why should we continue if pagebuf_get returns an error?> The right place to call the callbacks->toolstack_restore would be after the finish_hvm: > label, where currently the old QEMU device state is restored.Are you suggesting that we should read the toolstack data from pagebuf_get but only call the callback after finish_hvm? I can do that but honestly it doesn''t make too much sense to me.
Shriram Rajagopalan
2012-Jan-30 16:06 UTC
Re: [PATCH v2 1/2] libxc: introduce XC_SAVE_ID_TOOLSTACK
On 2012-01-30, at 6:16 AM, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote:> On Fri, 27 Jan 2012, Shriram Rajagopalan wrote: >> If there is an error in applying the toolstack state, then pagebuf_get returns -1 and >> the rest of the code would still attempt to resume the domain, with possibly >> inconsistent device model state. > > This sounds like an unhandled error in the caller of pagebuf_get.Nope. pagebuf_get is used only when Remus is on. It is called after applying the last memory checkpoint. So when it returns an error, we simply discard whatever we buffered and wrap up. For migration we have a special xc_save_id_last_checkpoint that terminates the code after buffering the first pagebuf.> I think that if pagebuf_get returns an error, the error should be > propagated and the migration should be aborted, right? > After all I don''t think we can successfully resume the domain if we fail > to read XC_SAVE_ID_TSC_INFO, for example. > I think we need something like this: > > > @@ -1589,9 +1616,10 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom, > > // DPRINTF("Buffered checkpoint\n"); > > - if ( pagebuf_get(xch, ctx, &pagebuf, io_fd, dom) ) { > + if ( pagebuf_get(xch, ctx, &pagebuf, io_fd, dom) < 0 ) { > PERROR("error when buffering batch, finishing"); > - goto finish; > + goto out; > } > memset(&tmptail, 0, sizeof(tmptail)); > tmptail.ishvm = hvm; > > >> Also, lets say there was no error in applying the toolstack state. If a network >> error occurs while receiving the next XC_SAVE_ID or so, then again, the code following >> the above snippet would attempt to resume the domain, with a memory state inconsistent >> with the device state. > > This seems wrong to me, but I am not very familiar with this protocol. > As I wrote above, why should we continue if pagebuf_get returns an > error? > > >> The right place to call the callbacks->toolstack_restore would be after the finish_hvm: >> label, where currently the old QEMU device state is restored. > > Are you suggesting that we should read the toolstack data from > pagebuf_get but only call the callback after finish_hvm?Yep. I hope the above explanation made some sense.> I can do that but honestly it doesn''t make too much sense to me. >
Stefano Stabellini
2012-Jan-30 16:43 UTC
Re: [PATCH v2 1/2] libxc: introduce XC_SAVE_ID_TOOLSTACK
On Mon, 30 Jan 2012, Shriram Rajagopalan wrote:> On 2012-01-30, at 6:16 AM, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote: > > > On Fri, 27 Jan 2012, Shriram Rajagopalan wrote: > >> If there is an error in applying the toolstack state, then pagebuf_get returns -1 and > >> the rest of the code would still attempt to resume the domain, with possibly > >> inconsistent device model state. > > > > This sounds like an unhandled error in the caller of pagebuf_get. > > Nope. pagebuf_get is used only when Remus is on. > It is called after applying the last memory checkpoint. > So when it returns an error, we simply discard whatever we buffered > and wrap up. For migration we > have a special xc_save_id_last_checkpoint > that terminates the code after > buffering the first pagebuf. > > > I think that if pagebuf_get returns an error, the error should be > > propagated and the migration should be aborted, right? > > After all I don''t think we can successfully resume the domain if we fail > > to read XC_SAVE_ID_TSC_INFO, for example. > > I think we need something like this: > > > > > > @@ -1589,9 +1616,10 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom, > > > > // DPRINTF("Buffered checkpoint\n"); > > > > - if ( pagebuf_get(xch, ctx, &pagebuf, io_fd, dom) ) { > > + if ( pagebuf_get(xch, ctx, &pagebuf, io_fd, dom) < 0 ) { > > PERROR("error when buffering batch, finishing"); > > - goto finish; > > + goto out; > > } > > memset(&tmptail, 0, sizeof(tmptail)); > > tmptail.ishvm = hvm; > > > > > >> Also, lets say there was no error in applying the toolstack state. If a network > >> error occurs while receiving the next XC_SAVE_ID or so, then again, the code following > >> the above snippet would attempt to resume the domain, with a memory state inconsistent > >> with the device state. > > > > This seems wrong to me, but I am not very familiar with this protocol. > > As I wrote above, why should we continue if pagebuf_get returns an > > error? > > > > > >> The right place to call the callbacks->toolstack_restore would be after the finish_hvm: > >> label, where currently the old QEMU device state is restored. > > > > Are you suggesting that we should read the toolstack data from > > pagebuf_get but only call the callback after finish_hvm? > > Yep. I hope the above explanation made some sense.All right, I''ll do that.