Xen project Mailing List

On Wed, Jan 25, 2012 at 3:08 AM, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:

On Mon, 2012-01-23 at 22:46 +0000, rshriram@xxxxxxxxx wrote:
> # HG changeset patch
> # User Shriram Rajagopalan <rshriram@xxxxxxxxx>
> # Date 1327358638 28800
> # Node ID 11fb1dfda7de4d6759dec87d80cd16cf137f7369
> # Parent 80fdf2182bc62ca358ba2f1a3513b47a4f8d9dfd
> tools/libxl: QEMU device model suspend/resume
>
> * Refactor the libxl__domain_save_device_model.
> * Add support for suspend/resume QEMU device model
> (both traditional xen and QMP).
>
>
> Signed-off-by: Shriram Rajagopalan <rshriram@xxxxxxxxx>
>
> diff -r 80fdf2182bc6 -r 11fb1dfda7de tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c Sat Jan 21 17:15:40 2012 +0000
> +++ b/tools/libxl/libxl.c Mon Jan 23 14:43:58 2012 -0800
> @@ -477,7 +477,7 @@
>
> rc = libxl__domain_suspend_common(gc, domid, fd, type, live, debug);
> if (!rc && type == LIBXL_DOMAIN_TYPE_HVM)
> - rc = libxl__domain_save_device_model(gc, domid, fd);
> + rc = libxl__domain_save_device_model(gc, domid, fd, /* No Remus */ 0);
> GC_FREE;
> return rc;
> }
> diff -r 80fdf2182bc6 -r 11fb1dfda7de tools/libxl/libxl_dom.c
> --- a/tools/libxl/libxl_dom.c Sat Jan 21 17:15:40 2012 +0000
> +++ b/tools/libxl/libxl_dom.c Mon Jan 23 14:43:58 2012 -0800
> @@ -534,6 +534,53 @@
> return 0;
> }
>
> +static int libxl__remus_domain_suspend_qemu(libxl__gc *gc, uint32_t domid)
> +{
> +
> + switch (libxl__device_model_version_running(gc, domid)) {
> + case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL: {
> + char *path = NULL;
> + path = libxl__sprintf(gc, "/local/domain/0/device-model/%d/command",
> + domid);
> + libxl__xs_write(gc, XBT_NULL, path, "save");
> + libxl__wait_for_device_model(gc, domid, "paused", NULL, NULL, NULL);

This and libxl__remus_domain_resume_qemu, qemu_pci_add_xenstore,
qemu_pci_remove_xenstore, libxl__domain_save_device_model and
libxl_domain_unpause would probably benefit from a helper function to
send a command to a traditional qemu.

> + break;
> + }
> + case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
> + /* Stop QEMU */
> + if (libxl__qmp_stop(gc, domid))
> + return ERROR_FAIL;
> + break;
> + default:
> + return ERROR_INVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int libxl__remus_domain_resume_qemu(libxl__gc *gc, uint32_t domid)
> +{
> +
> + switch (libxl__device_model_version_running(gc, domid)) {
> + case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL: {
> + char *path = NULL;
> + path = libxl__sprintf(gc, "/local/domain/0/device-model/%d/command",
> + domid);
> + libxl__xs_write(gc, XBT_NULL, path, "continue");
> + break;
> + }
> + case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
> + /* Stop QEMU */
> + if (libxl__qmp_resume(gc, domid))
> + return ERROR_FAIL;
> + break;
> + default:
> + return ERROR_INVAL;
> + }
> +
> + return 0;
> +}
> +
> int libxl__domain_suspend_common(libxl__gc *gc, uint32_t domid, int fd,
> libxl_domain_type type,
> int live, int debug)
> @@ -620,7 +667,7 @@
> return rc;
> }
>
> -int libxl__domain_save_device_model(libxl__gc *gc, uint32_t domid, int fd)
> +int libxl__domain_save_device_model(libxl__gc *gc, uint32_t domid, int fd, int remus)
> {
> libxl_ctx *ctx = libxl__gc_owner(gc);
> int ret, fd2 = -1, c;
> @@ -631,13 +678,12 @@
>
> switch (libxl__device_model_version_running(gc, domid)) {
> case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL: {
> - char *path = NULL;
> - LIBXL__LOG(ctx, LIBXL__LOG_DEBUG,
> - "Saving device model state to %s", filename);
> - path = libxl__sprintf(gc, "/local/domain/0/device-model/%d/command",
> - domid);
> - libxl__xs_write(gc, XBT_NULL, path, "save");
> - libxl__wait_for_device_model(gc, domid, "paused", NULL, NULL, NULL);
> + /* For Remus,we issue suspend_qemu call separately */

Why?

In remus, save and transmit device model phases are decoupled. This code
(sending the saved device model to the target) is executed after the domain is
resumed.

The control flow in current Remus is like this:
1 suspend vm
1.1 suspend qemu (save command, that saves the device model to a file)
2 copy out dirty memory into a buffer
3 resume vm
3.1 resume qemu
4 send the data in the buffer
5 send the saved device model
(xc_domain_restore extracts this from the tailbuf)

How does this interact with Stefano's XC_SAVEID_TOOLSTACK patches?

I couldnt find anything online by this name. Can you point me to the patch series?

I think some sort of high level description of the control flow used by
Remus and how/why it differs from normal save/restore would be useful
for review.

> + if (!remus) {
> + c = libxl__remus_domain_suspend_qemu(gc, domid);

It seems odd to call this function libxl__remus_FOO and the use it
when !remus. The function doesn't look to be inherently specific to
either remus or !remus anyhow.

> + if (c)
> + return c;
> + }
> break;
> }
> case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
> @@ -668,8 +714,9 @@
> qemu_state_len = st.st_size;
> LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Qemu state is %d bytes\n", qemu_state_len);
>
> - ret = libxl_write_exactly(ctx, fd, QEMU_SIGNATURE, strlen(QEMU_SIGNATURE),
> - "saved-state file", "qemu signature");
> + ret = libxl_write_exactly(ctx, fd, remus ? REMUS_QEMU_SIGNATURE : QEMU_SIGNATURE,
> + remus ? strlen(REMUS_QEMU_SIGNATURE): strlen(QEMU_SIGNATURE),
> + "saved-state file", "qemu signature");

QEMU_SIGNATURE is "DeviceModelRecord0002" which I believe libxc treats
identically to the Remus signature?

Thanks. Yep, the remus_signature is redundant.

Again consideration of how this interacts with Stefano's patch would be
useful. If possible we'd quite like to pull the qemu-restore our of
xc_domain_restore for consistency with how xc_domain_save saves it, the
current layering is quite mad.

Ian.

Re: [Xen-devel] [PATCH 1 of 3] tools/libxl: QEMU device model suspend/resume