[PATCH 0/3] Migration on QEMU upstream

[PATCH 0/3] Migration on QEMU upstream

[Anthony PERARD]

Hi all,

This patch series introduce migration with QEMU upstream.

A patch series for QEMU will be sent later meant to "fix" the default behavior
of QEMU during save/restore.

There is two new QMP commands implemented:
  - getfd: This give a file descriptor to QEMU through the unix socket.
  - migration: This ask QEMU to save its states in a fd previously sent.

In order to call "getfd", an alternative qmp_send have been implemented in
libxl.

Regards,


Anthony PERARD (3):
  libxl_qmp: Cut qmp_send function.
  libxl_qmp: Command migrate.
  libxl: Introduce migrate with the new QEMU.

 tools/libxl/libxl_dm.c       |    6 +-
 tools/libxl/libxl_dom.c      |   28 +++++++-
 tools/libxl/libxl_internal.h |    2 +
 tools/libxl/libxl_qmp.c      |  161 +++++++++++++++++++++++++++++++++++++----
 4 files changed, 176 insertions(+), 21 deletions(-)

-- 
Anthony PERARD

[PATCH 1/3] libxl_qmp: Cut qmp_send function.

[Anthony PERARD]

This patch prepare for the next patch, that will introduce an alternative send
function.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/libxl/libxl_qmp.c |   56 +++++++++++++++++++++++++++++++---------------
 1 files changed, 38 insertions(+), 18 deletions(-)

diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index f749e01..99ab4fa 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -446,13 +446,14 @@ static int qmp_next(libxl__gc *gc, libxl__qmp_handler *qmp)
     return rc;
 }
 
-static int qmp_send(libxl__qmp_handler *qmp,
-                    const char *cmd, libxl_key_value_list *args,
-                    qmp_callback_t callback, void *opaque,
-                    qmp_request_context *context)
+static char *qmp_send_prepare(libxl__gc *gc, libxl__qmp_handler *qmp,
+                              const char *cmd, libxl_key_value_list *args,
+                              qmp_callback_t callback, void *opaque,
+                              qmp_request_context *context)
 {
     yajl_gen_config conf = { 0, NULL };
-    const unsigned char *buf;
+    const unsigned char *buf = NULL;
+    char *ret = NULL;
     unsigned int len = 0;
     yajl_gen_status s;
     yajl_gen hand;
@@ -460,7 +461,7 @@ static int qmp_send(libxl__qmp_handler *qmp,
 
     hand = yajl_gen_alloc(&conf, NULL);
     if (!hand) {
-        return -1;
+        return NULL;
     }
 
     yajl_gen_map_open(hand);
@@ -479,14 +480,14 @@ static int qmp_send(libxl__qmp_handler *qmp,
     if (s) {
         LIBXL__LOG(qmp->ctx, LIBXL__LOG_ERROR,
                    "Failed to generate a qmp command");
-        return -1;
+        goto out;
     }
 
     elm = malloc(sizeof (callback_id_pair));
     if (elm == NULL) {
         LIBXL__LOG_ERRNO(qmp->ctx, LIBXL__LOG_ERROR,
                          "Failed to allocate a QMP callback");
-        goto error;
+        goto out;
     }
     elm->id = qmp->last_id_used;
     elm->callback = callback;
@@ -494,22 +495,41 @@ static int qmp_send(libxl__qmp_handler *qmp,
     elm->context = context;
     SIMPLEQ_INSERT_TAIL(&qmp->callback_list, elm, next);
 
+    ret = libxl__strndup(gc, (const char*)buf, len);
+
     LIBXL__LOG(qmp->ctx, LIBXL__LOG_DEBUG, "next qmp command: '%s'", buf);
 
-    if (libxl_write_exactly(qmp->ctx, qmp->qmp_fd, buf, len,
+out:
+    yajl_gen_free(hand);
+    return ret;
+}
+
+static int qmp_send(libxl__qmp_handler *qmp,
+                    const char *cmd, libxl_key_value_list *args,
+                    qmp_callback_t callback, void *opaque,
+                    qmp_request_context *context)
+{
+    char *buf = NULL;
+    int rc = -1;
+    libxl__gc gc = LIBXL_INIT_GC(qmp->ctx);
+
+    buf = qmp_send_prepare(&gc, qmp, cmd, args, callback, opaque, context);
+
+    if (buf == NULL) {
+        goto out;
+    }
+
+    if (libxl_write_exactly(qmp->ctx, qmp->qmp_fd, buf, strlen(buf),
                             "QMP command", "QMP socket"))
-        goto error;
+        goto out;
     if (libxl_write_exactly(qmp->ctx, qmp->qmp_fd, "\r\n", 2,
                             "CRLF", "QMP socket"))
-        goto error;
+        goto out;
 
-    yajl_gen_free(hand);
-
-    return qmp->last_id_used;
-
-error:
-    yajl_gen_free(hand);
-    return -1;
+    rc = qmp->last_id_used;
+out:
+    libxl__free_all(&gc);
+    return rc;
 }
 
 static int qmp_synchronous_send(libxl__qmp_handler *qmp, const char *cmd,
-- 
Anthony PERARD

[PATCH 2/3] libxl_qmp: Command migrate.

[Anthony PERARD]

This command works in two steps. First, a fd is sent to QEMU through the QMP
socket. And then, the second command "migrate" use the fd previously sent to
ask QEMU to save its states.

This come with an alternative qmp_send function that can send a fd.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/libxl/libxl_internal.h |    2 +
 tools/libxl/libxl_qmp.c      |  109 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 111 insertions(+), 0 deletions(-)

diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 84da6b1..e4afd2b 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -528,6 +528,8 @@ _hidden int libxl__qmp_query_serial(libxl__qmp_handler *qmp);
 _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);
 /* 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 99ab4fa..891ca9e 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -532,6 +532,52 @@ 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,
@@ -768,6 +814,69 @@ 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)
+{
+#define MIGRATE_FD_NAME "dm-migrate"
+    libxl__qmp_handler *qmp = NULL;
+    flexarray_t *parameters = NULL;
+    libxl_key_value_list args = NULL;
+    int rc = 0;
+
+    qmp = libxl__qmp_initialize(libxl__gc_owner(gc), domid);
+    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);
+    args = libxl__xs_kvs_of_flexarray(gc, parameters, parameters->count);
+    if (!args) {
+        rc = ERROR_NOMEM;
+        goto out2;
+    }
+
+    rc = qmp_synchronous_send(qmp, "migrate", &args,
+                              NULL, NULL, qmp->timeout);
+
+out2:
+    flexarray_free(parameters);
+out:
+    libxl__qmp_close(qmp);
+    return rc;
+}
+
 int libxl__qmp_initializations(libxl_ctx *ctx, uint32_t domid)
 {
     libxl__qmp_handler *qmp = NULL;
-- 
Anthony PERARD

[PATCH 3/3] libxl: Introduce migrate with the new QEMU.

[Anthony PERARD]

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/libxl/libxl_dm.c  |    6 ++++--
 tools/libxl/libxl_dom.c |   28 +++++++++++++++++++++++++---
 2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index bf38877..d1e023b 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -408,8 +408,10 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
         }
     }
     if (info->saved_state) {
-        flexarray_append(dm_args, "-loadvm");
-        flexarray_append(dm_args, info->saved_state);
+        /* This file descriptor is meant to be used by QEMU */
+        int migration_fd = open(info->saved_state, O_RDONLY);
+        flexarray_append(dm_args, "-incoming");
+        flexarray_append(dm_args, libxl__sprintf(gc, "fd:%d", migration_fd));
     }
     for (i = 0; info->extra && info->extra[i] != NULL; i++)
         flexarray_append(dm_args, info->extra[i]);
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 718281a..3cef616 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -595,9 +595,31 @@ int libxl__domain_save_device_model(libxl__gc *gc, uint32_t domid, int fd)
     struct stat st;
     uint32_t qemu_state_len;
 
-    LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Saving device model state to %s", filename);
-    libxl__xs_write(gc, XBT_NULL, libxl__sprintf(gc, "/local/domain/0/device-model/%d/command", domid), "save");
-    libxl__wait_for_device_model(gc, domid, "paused", NULL, NULL, NULL);
+    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);
+        break;
+    }
+    case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
+        fd2 = open(filename, O_WRONLY | O_CREAT | O_TRUNC);
+        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 */
+        if (libxl__qmp_migrate(gc, domid, fd2))
+            return ERROR_FAIL;
+        break;
+    default:
+        return ERROR_INVAL;
+    }
 
     if (stat(filename, &st) < 0)
     {
-- 
Anthony PERARD

Re: [PATCH 3/3] libxl: Introduce migrate with the new QEMU.

[Olaf Hering]

On Wed, Nov 23, Anthony PERARD wrote:

> +    case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
> +        fd2 = open(filename, O_WRONLY | O_CREAT | O_TRUNC);

I havent checked wether automated testing caught this, but:

In function 'open',
    inlined from 'libxl__domain_save_device_model' at libxl_dom.c:630:
/usr/include/bits/fcntl2.h:51: error: call to '__open_missing_mode' declared with attribute error: open with O_CREAT in second argument needs 3 arguments
make[3]: *** [libxl_dom.o] Error 1

Olaf

Re: [PATCH 3/3] libxl: Introduce migrate with the new QEMU.

[Ian Campbell]

On Mon, 2011-12-05 at 18:16 +0000, Olaf Hering wrote:
> On Wed, Nov 23, Anthony PERARD wrote:
> 
> > +    case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
> > +        fd2 = open(filename, O_WRONLY | O_CREAT | O_TRUNC);
> 
> I havent checked wether automated testing caught this, but:
> 
> In function 'open',
>     inlined from 'libxl__domain_save_device_model' at libxl_dom.c:630:
> /usr/include/bits/fcntl2.h:51: error: call to '__open_missing_mode' declared with attribute error: open with O_CREAT in second argument needs 3 arguments
> make[3]: *** [libxl_dom.o] Error 1

Was a fix for this ever posted?

Ian.

Re: [PATCH 3/3] libxl: Introduce migrate with the new QEMU.

[Anthony PERARD]

On Wed, Dec 21, 2011 at 09:48, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Mon, 2011-12-05 at 18:16 +0000, Olaf Hering wrote:
>> On Wed, Nov 23, Anthony PERARD wrote:
>>
>> > +    case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
>> > +        fd2 = open(filename, O_WRONLY | O_CREAT | O_TRUNC);
>>
>> I havent checked wether automated testing caught this, but:
>>
>> In function 'open',
>>     inlined from 'libxl__domain_save_device_model' at libxl_dom.c:630:
>> /usr/include/bits/fcntl2.h:51: error: call to '__open_missing_mode' declared with attribute error: open with O_CREAT in second argument needs 3 arguments
>> make[3]: *** [libxl_dom.o] Error 1
>
> Was a fix for this ever posted?

I don't think so. I will do one.

-- 
Anthony PERARD

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

Re: [PATCH 0/3] Migration on QEMU upstream

[Ian Campbell]

On Wed, 2011-11-23 at 17:40 +0000, Anthony PERARD wrote:
> Hi all,
> 
> This patch series introduce migration with QEMU upstream.
> 
> A patch series for QEMU will be sent later meant to "fix" the default behavior
> of QEMU during save/restore.
> 
> There is two new QMP commands implemented:
>   - getfd: This give a file descriptor to QEMU through the unix socket.

This sounds more like a setfd command.

>   - migration: This ask QEMU to save its states in a fd previously sent.

Why can the fd not be included in this message directly? How is a given
{s,g}etfd call associated with the migration command? What happens if
some other command also wants to receive an fd in the future?

I presume that a bunch of this will become  more obvious when the qemu
side is posted, there's a qemu protocol spec in that right?

> In order to call "getfd", an alternative qmp_send have been implemented in
> libxl.

You could also have added optional msg_control{len} parameters to the
existing command. I don't know if that is better though.

Ian.

> 
> Regards,
> 
> 
> Anthony PERARD (3):
>   libxl_qmp: Cut qmp_send function.
>   libxl_qmp: Command migrate.
>   libxl: Introduce migrate with the new QEMU.
> 
>  tools/libxl/libxl_dm.c       |    6 +-
>  tools/libxl/libxl_dom.c      |   28 +++++++-
>  tools/libxl/libxl_internal.h |    2 +
>  tools/libxl/libxl_qmp.c      |  161 +++++++++++++++++++++++++++++++++++++----
>  4 files changed, 176 insertions(+), 21 deletions(-)
> 

Re: [PATCH 0/3] Migration on QEMU upstream

[Anthony PERARD]

On Thu, Nov 24, 2011 at 09:22, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Wed, 2011-11-23 at 17:40 +0000, Anthony PERARD wrote:
>> Hi all,
>>
>> This patch series introduce migration with QEMU upstream.
>>
>> A patch series for QEMU will be sent later meant to "fix" the default behavior
>> of QEMU during save/restore.
>>
>> There is two new QMP commands implemented:
>>   - getfd: This give a file descriptor to QEMU through the unix socket.
>
> This sounds more like a setfd command.

Well, it's the name of the QMP command. I just respect the
"convention" I have in the rest of the file that is
/(libxl__)?qmp_(?<command_name>.+)/


>>   - migration: This ask QEMU to save its states in a fd previously sent.
>
> Why can the fd not be included in this message directly?

The migrate command only accept "fd:namedfd" as output (among other
way). And to have a named fd, we have to call "getfd" fist. But here,
I do both with one call from the wild.

> How is a given {s,g}etfd call associated with the migration command?

libxl__qmp_migrate will call a static function qmp_getfd(fd, "migration-fd")

> What happens if some other command also wants to receive an fd in the future?

The futur command can just call the qmp_getfd function, or we can
export qmp_getfd.

> I presume that a bunch of this will become  more obvious when the qemu
> side is posted, there's a qemu protocol spec in that right?

Mmh, not really, there is nothing to had in QEMU side. getfd and
migrate are both QMP command. You can look in the file qmp-commands.hx
in QEMU for the command migrate and getfd; and look in the file
docs/migration.txt to know wich "Types of migration" QEMU use.

The QEMU patch series I'm about to send is just a "fix" for the Xen case:
 - do not save the RAM.
 - at restore time, do not try to "allocate" (populate_physmap) memory
that should already be allocated, and access to the right guest
address in case this one have been "moved" (true for the VideoRAM)
("moved" using add_to_physmap).

>> In order to call "getfd", an alternative qmp_send have been implemented in
>> libxl.
>
> You could also have added optional msg_control{len} parameters to the
> existing command. I don't know if that is better though.

Well, this will be an extra parameter that will be used in only one
case (I think). And this parameter will be a bit obscure. So, probably
both are good. Also, the code that prepare the message (string) to be
sent is in a separate function now, so both qmp_send and qmp_send_fd
do not have anything in common (almost).

Thanks,

-- 
Anthony PERARD

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

Re: [PATCH 0/3] Migration on QEMU upstream

[Ian Campbell]

On Thu, 2011-11-24 at 12:09 +0000, Anthony PERARD wrote:
[...]
> Mmh, not really, there is nothing to had in QEMU side. getfd and
> migrate are both QMP command. You can look in the file qmp-commands.hx
> in QEMU for the command migrate and getfd; and look in the file
> docs/migration.txt to know wich "Types of migration" QEMU use.

Oh, I didn't realise these were all existing qmp commands, I assumed a
patch adding them was forthcoming. You can ignore most of what I said
then...

> 
> The QEMU patch series I'm about to send is just a "fix" for the Xen case:
>  - do not save the RAM.
>  - at restore time, do not try to "allocate" (populate_physmap) memory
> that should already be allocated, and access to the right guest
> address in case this one have been "moved" (true for the VideoRAM)
> ("moved" using add_to_physmap).
> 
> >> In order to call "getfd", an alternative qmp_send have been implemented in
> >> libxl.
> >
> > You could also have added optional msg_control{len} parameters to the
> > existing command. I don't know if that is better though.
> 
> Well, this will be an extra parameter that will be used in only one
> case (I think). And this parameter will be a bit obscure. So, probably
> both are good. Also, the code that prepare the message (string) to be
> sent is in a separate function now, so both qmp_send and qmp_send_fd
> do not have anything in common (almost).
> 
> Thanks,
> 

Re: [PATCH 0/3] Migration on QEMU upstream

[Ian Jackson]

Anthony PERARD writes ("[Xen-devel] [PATCH 0/3] Migration on QEMU upstream"):
> This patch series introduce migration with QEMU upstream.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>