[PATCH v4 0/3] libxl: save/restore qemu physmap

[PATCH v4 0/3] libxl: save/restore qemu physmap

[Stefano Stabellini]

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.

Important note: the second and third patches only make sense if the
corresponding QEMU's patches[1] are accepted.


[1] http://marc.info/?l=qemu-devel&m=132749682330542&w=2


Changes in v4:

- addressed Shriram's comments about the first patch: saving/restoring
toolstack extra information should work we Remus too now;

- added a new patch to use QMP "save_devices" command rather than
"migrate" to save QEMU's device state.



Changes in v3:

- rebased the series;

- xc_domain_restore: read the toolstack data in pagebuf_get_one, call
the callback at finish_hvm;



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 (3):
      libxc: introduce XC_SAVE_ID_TOOLSTACK
      libxl: save/restore qemu's physmap
      libxl_qmp: remove libxl__qmp_migrate, introduce libxl__qmp_save

 tools/libxc/xc_domain_restore.c |   46 ++++++++++++-
 tools/libxc/xc_domain_save.c    |   17 +++++
 tools/libxc/xenguest.h          |   23 ++++++-
 tools/libxc/xg_save_restore.h   |    1 +
 tools/libxl/libxl_dom.c         |  143 ++++++++++++++++++++++++++++++++++++---
 tools/libxl/libxl_internal.h    |    2 +-
 tools/libxl/libxl_qmp.c         |   82 +---------------------
 tools/xcutils/xc_restore.c      |    2 +-
 8 files changed, 222 insertions(+), 94 deletions(-)


Cheers,

Stefano

[PATCH v4 1/3] libxc: introduce XC_SAVE_ID_TOOLSTACK

[Stefano Stabellini]

Introduce a new save_id to save/restore toolstack specific extra
information.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>

---
 tools/libxc/xc_domain_restore.c |   46 ++++++++++++++++++++++++++++++++++++++-
 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      |    2 +-
 6 files changed, 87 insertions(+), 4 deletions(-)

diff --git a/tools/libxc/xc_domain_restore.c b/tools/libxc/xc_domain_restore.c
index 3fda6f8..958534c 100644
--- a/tools/libxc/xc_domain_restore.c
+++ b/tools/libxc/xc_domain_restore.c
@@ -659,6 +659,11 @@ static void tailbuf_free(tailbuf_t *buf)
         tailbuf_free_pv(&buf->u.pv);
 }
 
+struct toolstack_data_t {
+    uint8_t *data;
+    uint32_t len;
+};
+
 typedef struct {
     void* pages;
     /* pages is of length nr_physpages, pfn_types is of length nr_pages */
@@ -682,6 +687,8 @@ typedef struct {
     uint64_t acpi_ioport_location;
     uint64_t viridian;
     uint64_t vm_generationid_addr;
+
+    struct toolstack_data_t tdata;
 } pagebuf_t;
 
 static int pagebuf_init(pagebuf_t* buf)
@@ -692,6 +699,10 @@ static int pagebuf_init(pagebuf_t* buf)
 
 static void pagebuf_free(pagebuf_t* buf)
 {
+    if (buf->tdata.data != NULL) {
+        free(buf->tdata.data);
+        buf->tdata.data = NULL;
+    }
     if (buf->pages) {
         free(buf->pages);
         buf->pages = NULL;
@@ -827,6 +838,19 @@ static int pagebuf_get_one(xc_interface *xch, struct restore_ctx *ctx,
         }
         return pagebuf_get_one(xch, ctx, buf, fd, dom);
 
+    case XC_SAVE_ID_TOOLSTACK:
+        {
+            RDEXACT(fd, &buf->tdata.len, sizeof(buf->tdata.len));
+            buf->tdata.data = (uint8_t*) realloc(buf->tdata.data, buf->tdata.len);
+            if ( buf->tdata.data == NULL )
+            {
+                PERROR("error memory allocation");
+                return -1;
+            }
+            RDEXACT(fd, buf->tdata.data, buf->tdata.len);
+            return pagebuf_get_one(xch, ctx, buf, fd, dom);
+        }
+
     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
@@ -1262,7 +1286,8 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom,
                       unsigned int console_evtchn, unsigned long *console_mfn,
                       unsigned int hvm, unsigned int pae, int superpages,
                       int no_incr_generationid,
-                      unsigned long *vm_generationid_addr)
+                      unsigned long *vm_generationid_addr,
+                      struct restore_callbacks *callbacks)
 {
     DECLARE_DOMCTL;
     int rc = 1, frc, i, j, n, m, pae_extended_cr3 = 0, ext_vcpucontext = 0;
@@ -1310,6 +1335,7 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom,
 
     pagebuf_t pagebuf;
     tailbuf_t tailbuf, tmptail;
+    struct toolstack_data_t tdata, tdatatmp;
     void* vcpup;
     uint64_t console_pfn = 0;
 
@@ -1322,6 +1348,7 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom,
     pagebuf_init(&pagebuf);
     memset(&tailbuf, 0, sizeof(tailbuf));
     tailbuf.ishvm = hvm;
+    memset(&tdata, 0, sizeof(tdata));
 
     memset(ctx, 0, sizeof(*ctx));
 
@@ -1581,6 +1608,10 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom,
         ERROR("Error, unknow acpi ioport location (%i)", pagebuf.acpi_ioport_location);
     }
 
+    tdatatmp = tdata;
+    tdata = pagebuf.tdata;
+    pagebuf.tdata = tdatatmp;
+
     if ( ctx->last_checkpoint )
     {
         // DPRINTF("Last checkpoint, finishing\n");
@@ -2023,6 +2054,19 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom,
     goto out;
 
   finish_hvm:
+    if ( callbacks != NULL && callbacks->toolstack_restore != NULL &&
+            tdata.data != NULL )
+    {
+        if ( callbacks->toolstack_restore(dom, tdata.data, tdata.len,
+                    callbacks->data) < 0 )
+        {
+            PERROR("error calling toolstack_restore");
+            free(tdata.data);
+            goto out;
+        }
+    }
+    free(tdata.data);
+
     /* Dump the QEMU state to a state file for QEMU to load */
     if ( dump_qemu(xch, dom, &tailbuf.u.hvm) ) {
         PERROR("Error dumping QEMU state to file");
diff --git a/tools/libxc/xc_domain_save.c b/tools/libxc/xc_domain_save.c
index f473dd7..318c433 100644
--- a/tools/libxc/xc_domain_save.c
+++ b/tools/libxc/xc_domain_save.c
@@ -1687,6 +1687,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 6026370..76aa475 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;
 };
@@ -62,6 +70,16 @@ int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom, uint32_t max_iter
                    unsigned long vm_generationid_addr);
 
 
+/* 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.
  *
@@ -75,6 +93,8 @@ int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom, uint32_t max_iter
  * @parm superpages non-zero to allocate guest memory with superpages
  * @parm no_incr_generationid non-zero if generation id is NOT to be incremented
  * @parm vm_generationid_addr returned with the address of the generation id buffer
+ * @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,
@@ -82,7 +102,8 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom,
                       unsigned int console_evtchn, unsigned long *console_mfn,
                       unsigned int hvm, unsigned int pae, int superpages,
                       int no_incr_generationid,
-		      unsigned long *vm_generationid_addr);
+                      unsigned long *vm_generationid_addr,
+                      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 6286b68..46fdeaa 100644
--- a/tools/libxc/xg_save_restore.h
+++ b/tools/libxc/xg_save_restore.h
@@ -254,6 +254,7 @@
 #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_HVM_GENERATION_ID_ADDR -14
+#define XC_SAVE_ID_TOOLSTACK          -15 /* 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 91643a2..2c5eec5 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -379,7 +379,7 @@ int libxl__domain_restore_common(libxl__gc *gc, uint32_t domid,
                            state->store_port, &state->store_mfn,
                            state->console_port, &state->console_mfn,
                            hvm, pae, superpages, no_incr_generationid,
-                           &state->vm_generationid_addr);
+                           &state->vm_generationid_addr, 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 63d53a8..306a10e 100644
--- a/tools/xcutils/xc_restore.c
+++ b/tools/xcutils/xc_restore.c
@@ -47,7 +47,7 @@ main(int argc, char **argv)
 
     ret = xc_domain_restore(xch, io_fd, domid, store_evtchn, &store_mfn,
                             console_evtchn, &console_mfn, hvm, pae, superpages,
-                            0, NULL);
+                            0, NULL, NULL);
 
     if ( ret == 0 )
     {
-- 
1.7.2.5

Re: [PATCH v4 1/3] libxc: introduce XC_SAVE_ID_TOOLSTACK

[Shriram Rajagopalan]

[-- Attachment #1.1: Type: text/plain, Size: 464 bytes --]

On Wed, Feb 1, 2012 at 10:10 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>
> Acked-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>
>
>
cc-ing Ian Jackson.
These patches (and other follow ups including mine) have been acked.
Ian, are there any issues blocking these patches ?

thanks
shriram

[-- Attachment #1.2: Type: text/html, Size: 870 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH v4 1/3] libxc: introduce XC_SAVE_ID_TOOLSTACK

[Stefano Stabellini]

On Fri, 17 Feb 2012, Shriram Rajagopalan wrote:
> 
> On Wed, Feb 1, 2012 at 10:10 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>
> Acked-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>
> 
> 
> cc-ing Ian Jackson.
> These patches (and other follow ups including mine) have been acked.
> Ian, are there any issues blocking these patches ?

The issue are the QEMU side acks. Let me ask them again for feedback.

Re: [PATCH v4 1/3] libxc: introduce XC_SAVE_ID_TOOLSTACK

[Shriram Rajagopalan]

On 2012-02-17, at 3:45 AM, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote:

> On Fri, 17 Feb 2012, Shriram Rajagopalan wrote:
>> 
>> On Wed, Feb 1, 2012 at 10:10 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>
>> Acked-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>
>> 
>> 
>> cc-ing Ian Jackson.
>> These patches (and other follow ups including mine) have been acked.
>> Ian, are there any issues blocking these patches ?
> 
> The issue are the QEMU side acks. Let me ask them again for feedback.
> 

Any update yet ?
 Or should I respin my patches without the qemu_save part?

Shriram

Re: [PATCH v4 1/3] libxc: introduce XC_SAVE_ID_TOOLSTACK

[Stefano Stabellini]

On Sat, 25 Feb 2012, Shriram Rajagopalan wrote:
> On 2012-02-17, at 3:45 AM, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote:
> 
> > On Fri, 17 Feb 2012, Shriram Rajagopalan wrote:
> >> 
> >> On Wed, Feb 1, 2012 at 10:10 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>
> >> Acked-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>
> >> 
> >> 
> >> cc-ing Ian Jackson.
> >> These patches (and other follow ups including mine) have been acked.
> >> Ian, are there any issues blocking these patches ?
> > 
> > The issue are the QEMU side acks. Let me ask them again for feedback.
> > 
> 
> Any update yet ?
>  Or should I respin my patches without the qemu_save part?

I was promised a review last Friday, give few more days please.

[PATCH v4 2/3] libxl: save/restore qemu's physmap

[Stefano Stabellini]

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 2c5eec5..a6eb714 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -349,6 +349,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,
@@ -358,6 +418,7 @@ int libxl__domain_restore_common(libxl__gc *gc, uint32_t domid,
     /* read signature */
     int rc;
     int hvm, pae, superpages;
+    struct restore_callbacks callbacks;
     int no_incr_generationid;
     switch (info->type) {
     case LIBXL_DOMAIN_TYPE_HVM:
@@ -365,6 +426,8 @@ int libxl__domain_restore_common(libxl__gc *gc, uint32_t domid,
         superpages = 1;
         pae = info->u.hvm.pae;
         no_incr_generationid = info->u.hvm.no_incr_generationid;
+        callbacks.toolstack_restore = libxl__toolstack_restore;
+        callbacks.data = gc;
         break;
     case LIBXL_DOMAIN_TYPE_PV:
         hvm = 0;
@@ -379,7 +442,7 @@ int libxl__domain_restore_common(libxl__gc *gc, uint32_t domid,
                            state->store_port, &state->store_mfn,
                            state->console_port, &state->console_mfn,
                            hvm, pae, superpages, no_incr_generationid,
-                           &state->vm_generationid_addr, NULL);
+                           &state->vm_generationid_addr, &callbacks);
     if ( rc ) {
         LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "restoring domain");
         return ERROR_FAIL;
@@ -534,6 +597,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)
@@ -596,6 +725,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,
-- 
1.7.2.5

[PATCH v4 3/3] libxl_qmp: remove libxl__qmp_migrate, introduce libxl__qmp_save

[Stefano Stabellini]

Following the recent changes to upstream Qemu, the best monitor command
to suit or needs is "save_devices" rather than "migrate".
This patch removes libxl__qmp_migrate and introduces libxl__qmp_save
instead, that uses "save_devices".

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 tools/libxl/libxl_dom.c      |   11 +-----
 tools/libxl/libxl_internal.h |    2 +-
 tools/libxl/libxl_qmp.c      |   82 ++----------------------------------------
 3 files changed, 5 insertions(+), 90 deletions(-)

diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index a6eb714..40ebcd1 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -771,18 +771,9 @@ int libxl__domain_save_device_model(libxl__gc *gc, uint32_t domid, int fd)
         break;
     }
     case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
-        fd2 = open(filename, O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR);
-        if (fd2 < 0) {
-            LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
-                             "Unable to create a QEMU save file\n");
-            return ERROR_FAIL;
-        }
-        /* Save DM state into fd2 */
-        ret = libxl__qmp_migrate(gc, domid, fd2);
+        ret = libxl__qmp_save(gc, domid, (char *)filename);
         if (ret)
             goto out;
-        close(fd2);
-        fd2 = -1;
         break;
     default:
         return ERROR_INVAL;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 3c8da45..6d11cfe 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -619,7 +619,7 @@ _hidden int libxl__qmp_pci_add(libxl__gc *gc, int d, libxl_device_pci *pcidev);
 _hidden int libxl__qmp_pci_del(libxl__gc *gc, int domid,
                                libxl_device_pci *pcidev);
 /* Save current QEMU state into fd. */
-_hidden int libxl__qmp_migrate(libxl__gc *gc, int domid, int fd);
+_hidden int libxl__qmp_save(libxl__gc *gc, int domid, const char *filename);
 /* close and free the QMP handler */
 _hidden void libxl__qmp_close(libxl__qmp_handler *qmp);
 /* remove the socket file, if the file has already been removed,
diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index 6d401b7..19bf699 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -533,52 +533,6 @@ out:
     return rc;
 }
 
-static int qmp_send_fd(libxl__gc *gc, libxl__qmp_handler *qmp,
-                       libxl_key_value_list *args,
-                       qmp_callback_t callback, void *opaque,
-                       qmp_request_context *context,
-                       int fd)
-{
-    struct msghdr msg = { 0 };
-    struct cmsghdr *cmsg;
-    char control[CMSG_SPACE(sizeof (fd))];
-    struct iovec iov;
-    char *buf = NULL;
-
-    buf = qmp_send_prepare(gc, qmp, "getfd", args, callback, opaque, context);
-
-    /* Response data */
-    iov.iov_base = buf;
-    iov.iov_len  = strlen(buf);
-
-    /* compose the message */
-    msg.msg_iov = &iov;
-    msg.msg_iovlen = 1;
-    msg.msg_control = control;
-    msg.msg_controllen = sizeof (control);
-
-    /* attach open fd */
-    cmsg = CMSG_FIRSTHDR(&msg);
-    cmsg->cmsg_level = SOL_SOCKET;
-    cmsg->cmsg_type = SCM_RIGHTS;
-    cmsg->cmsg_len = CMSG_LEN(sizeof (fd));
-    *(int *)CMSG_DATA(cmsg) = fd;
-
-    msg.msg_controllen = cmsg->cmsg_len;
-
-    if (sendmsg(qmp->qmp_fd, &msg, 0) < 0) {
-        LIBXL__LOG_ERRNO(qmp->ctx, LIBXL__LOG_ERROR,
-                         "Failed to send a QMP message to QEMU.");
-        return ERROR_FAIL;
-    }
-    if (libxl_write_exactly(qmp->ctx, qmp->qmp_fd, "\r\n", 2,
-                            "CRLF", "QMP socket")) {
-        return ERROR_FAIL;
-    }
-
-    return qmp->last_id_used;
-}
-
 static int qmp_synchronous_send(libxl__qmp_handler *qmp, const char *cmd,
                                 libxl_key_value_list *args,
                                 qmp_callback_t callback, void *opaque,
@@ -815,34 +769,8 @@ int libxl__qmp_pci_del(libxl__gc *gc, int domid, libxl_device_pci *pcidev)
     return qmp_device_del(gc, domid, id);
 }
 
-static int qmp_getfd(libxl__gc *gc, libxl__qmp_handler *qmp,
-                     int fd, const char *name)
-{
-    flexarray_t *parameters = NULL;
-    libxl_key_value_list args = NULL;
-    int rc = 0;
-
-    parameters = flexarray_make(2, 1);
-    if (!parameters)
-        return ERROR_NOMEM;
-    flexarray_append_pair(parameters, "fdname", (char*)name);
-    args = libxl__xs_kvs_of_flexarray(gc, parameters, parameters->count);
-    if (!args) {
-        rc = ERROR_NOMEM;
-        goto out;
-    }
-
-    if (qmp_send_fd(gc, qmp, &args, NULL, NULL, NULL, fd) < 0) {
-        rc = ERROR_FAIL;
-    }
-out:
-    flexarray_free(parameters);
-    return rc;
-}
-
-int libxl__qmp_migrate(libxl__gc *gc, int domid, int fd)
+int libxl__qmp_save(libxl__gc *gc, int domid, const char *filename)
 {
-#define MIGRATE_FD_NAME "dm-migrate"
     libxl__qmp_handler *qmp = NULL;
     flexarray_t *parameters = NULL;
     libxl_key_value_list args = NULL;
@@ -852,23 +780,19 @@ int libxl__qmp_migrate(libxl__gc *gc, int domid, int fd)
     if (!qmp)
         return ERROR_FAIL;
 
-    rc = qmp_getfd(gc, qmp, fd, MIGRATE_FD_NAME);
-    if (rc)
-        goto out;
-
     parameters = flexarray_make(2, 1);
     if (!parameters) {
         rc = ERROR_NOMEM;
         goto out;
     }
-    flexarray_append_pair(parameters, "uri", "fd:" MIGRATE_FD_NAME);
+    flexarray_append_pair(parameters, "filename", (char *)filename);
     args = libxl__xs_kvs_of_flexarray(gc, parameters, parameters->count);
     if (!args) {
         rc = ERROR_NOMEM;
         goto out2;
     }
 
-    rc = qmp_synchronous_send(qmp, "migrate", &args,
+    rc = qmp_synchronous_send(qmp, "save_devices", &args,
                               NULL, NULL, qmp->timeout);
 
 out2:
-- 
1.7.2.5

Re: [PATCH v4 3/3] libxl_qmp: remove libxl__qmp_migrate, introduce libxl__qmp_save

[Ian Campbell]

On Wed, 2012-02-01 at 18:10 +0000, Stefano Stabellini wrote:
> Following the recent changes to upstream Qemu, the best monitor command
> to suit or needs is "save_devices" rather than "migrate".
> This patch removes libxl__qmp_migrate and introduces libxl__qmp_save
> instead, that uses "save_devices".
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> ---
>  tools/libxl/libxl_dom.c      |   11 +-----
>  tools/libxl/libxl_internal.h |    2 +-
>  tools/libxl/libxl_qmp.c      |   82 ++----------------------------------------
>  3 files changed, 5 insertions(+), 90 deletions(-)
> 
> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> index a6eb714..40ebcd1 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -771,18 +771,9 @@ int libxl__domain_save_device_model(libxl__gc *gc, uint32_t domid, int fd)
>          break;
>      }
>      case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
> -        fd2 = open(filename, O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR);
> -        if (fd2 < 0) {
> -            LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
> -                             "Unable to create a QEMU save file\n");
> -            return ERROR_FAIL;
> -        }
> -        /* Save DM state into fd2 */
> -        ret = libxl__qmp_migrate(gc, domid, fd2);
> +        ret = libxl__qmp_save(gc, domid, (char *)filename);

I don't think you need this cast, your qmp_save takes a const char *.

Otherwise this all looks alright to me.

Ian.

>          if (ret)
>              goto out;
> -        close(fd2);
> -        fd2 = -1;
>          break;
>      default:
>          return ERROR_INVAL;
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 3c8da45..6d11cfe 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -619,7 +619,7 @@ _hidden int libxl__qmp_pci_add(libxl__gc *gc, int d, libxl_device_pci *pcidev);
>  _hidden int libxl__qmp_pci_del(libxl__gc *gc, int domid,
>                                 libxl_device_pci *pcidev);
>  /* Save current QEMU state into fd. */
> -_hidden int libxl__qmp_migrate(libxl__gc *gc, int domid, int fd);
> +_hidden int libxl__qmp_save(libxl__gc *gc, int domid, const char *filename);
>  /* close and free the QMP handler */
>  _hidden void libxl__qmp_close(libxl__qmp_handler *qmp);
>  /* remove the socket file, if the file has already been removed,
> diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
> index 6d401b7..19bf699 100644
> --- a/tools/libxl/libxl_qmp.c
> +++ b/tools/libxl/libxl_qmp.c
> @@ -533,52 +533,6 @@ out:
>      return rc;
>  }
>  
> -static int qmp_send_fd(libxl__gc *gc, libxl__qmp_handler *qmp,
> -                       libxl_key_value_list *args,
> -                       qmp_callback_t callback, void *opaque,
> -                       qmp_request_context *context,
> -                       int fd)
> -{
> -    struct msghdr msg = { 0 };
> -    struct cmsghdr *cmsg;
> -    char control[CMSG_SPACE(sizeof (fd))];
> -    struct iovec iov;
> -    char *buf = NULL;
> -
> -    buf = qmp_send_prepare(gc, qmp, "getfd", args, callback, opaque, context);
> -
> -    /* Response data */
> -    iov.iov_base = buf;
> -    iov.iov_len  = strlen(buf);
> -
> -    /* compose the message */
> -    msg.msg_iov = &iov;
> -    msg.msg_iovlen = 1;
> -    msg.msg_control = control;
> -    msg.msg_controllen = sizeof (control);
> -
> -    /* attach open fd */
> -    cmsg = CMSG_FIRSTHDR(&msg);
> -    cmsg->cmsg_level = SOL_SOCKET;
> -    cmsg->cmsg_type = SCM_RIGHTS;
> -    cmsg->cmsg_len = CMSG_LEN(sizeof (fd));
> -    *(int *)CMSG_DATA(cmsg) = fd;
> -
> -    msg.msg_controllen = cmsg->cmsg_len;
> -
> -    if (sendmsg(qmp->qmp_fd, &msg, 0) < 0) {
> -        LIBXL__LOG_ERRNO(qmp->ctx, LIBXL__LOG_ERROR,
> -                         "Failed to send a QMP message to QEMU.");
> -        return ERROR_FAIL;
> -    }
> -    if (libxl_write_exactly(qmp->ctx, qmp->qmp_fd, "\r\n", 2,
> -                            "CRLF", "QMP socket")) {
> -        return ERROR_FAIL;
> -    }
> -
> -    return qmp->last_id_used;
> -}
> -
>  static int qmp_synchronous_send(libxl__qmp_handler *qmp, const char *cmd,
>                                  libxl_key_value_list *args,
>                                  qmp_callback_t callback, void *opaque,
> @@ -815,34 +769,8 @@ int libxl__qmp_pci_del(libxl__gc *gc, int domid, libxl_device_pci *pcidev)
>      return qmp_device_del(gc, domid, id);
>  }
>  
> -static int qmp_getfd(libxl__gc *gc, libxl__qmp_handler *qmp,
> -                     int fd, const char *name)
> -{
> -    flexarray_t *parameters = NULL;
> -    libxl_key_value_list args = NULL;
> -    int rc = 0;
> -
> -    parameters = flexarray_make(2, 1);
> -    if (!parameters)
> -        return ERROR_NOMEM;
> -    flexarray_append_pair(parameters, "fdname", (char*)name);
> -    args = libxl__xs_kvs_of_flexarray(gc, parameters, parameters->count);
> -    if (!args) {
> -        rc = ERROR_NOMEM;
> -        goto out;
> -    }
> -
> -    if (qmp_send_fd(gc, qmp, &args, NULL, NULL, NULL, fd) < 0) {
> -        rc = ERROR_FAIL;
> -    }
> -out:
> -    flexarray_free(parameters);
> -    return rc;
> -}
> -
> -int libxl__qmp_migrate(libxl__gc *gc, int domid, int fd)
> +int libxl__qmp_save(libxl__gc *gc, int domid, const char *filename)
>  {
> -#define MIGRATE_FD_NAME "dm-migrate"
>      libxl__qmp_handler *qmp = NULL;
>      flexarray_t *parameters = NULL;
>      libxl_key_value_list args = NULL;
> @@ -852,23 +780,19 @@ int libxl__qmp_migrate(libxl__gc *gc, int domid, int fd)
>      if (!qmp)
>          return ERROR_FAIL;
>  
> -    rc = qmp_getfd(gc, qmp, fd, MIGRATE_FD_NAME);
> -    if (rc)
> -        goto out;
> -
>      parameters = flexarray_make(2, 1);
>      if (!parameters) {
>          rc = ERROR_NOMEM;
>          goto out;
>      }
> -    flexarray_append_pair(parameters, "uri", "fd:" MIGRATE_FD_NAME);
> +    flexarray_append_pair(parameters, "filename", (char *)filename);
>      args = libxl__xs_kvs_of_flexarray(gc, parameters, parameters->count);
>      if (!args) {
>          rc = ERROR_NOMEM;
>          goto out2;
>      }
>  
> -    rc = qmp_synchronous_send(qmp, "migrate", &args,
> +    rc = qmp_synchronous_send(qmp, "save_devices", &args,
>                                NULL, NULL, qmp->timeout);
>  
>  out2: