[Qemu-devel] [PATCH 0/3] Add 'inline-fd:' protocol for migration

[Qemu-devel] [PATCH 0/3] Add 'inline-fd:' protocol for migration

[Yury Kotov]

Hi,

I've added 'inline-fd:' proto to simplify migration to/from fd.
I thought about modifying the existing 'fd:' proto but I'm not sure it's a
good idea to change it's contract.

Existing 'fd:' proto works with previously added fd by getfd or add-fd commands.
If client doesn't want to work with this fd before or after migration then it's
easier to send an fd with the migrate-* command. Also, client shouldn't maintain
this fd.

Usage:
{ 'execute': 'migrate', 'arguments': { 'uri': 'inline-fd:' } }
{ 'execute': 'migrate-incoming', 'arguments': { 'uri': 'inline-fd:' } }

Regards,
Yury

Yury Kotov (3):
  monitor: Add monitor_recv_fd function to work with sent fds
  migration: Add inline-fd protocol
  migration-test: Add a test for inline_fd protocol

 include/monitor/monitor.h          |   1 +
 migration/Makefile.objs            |   2 +-
 migration/inline-fd.c              |  89 ++++++++++++++++++++++
 migration/inline-fd.h              |  22 ++++++
 migration/migration.c              |  15 ++++
 migration/trace-events             |   4 +
 monitor.c                          |  15 +++-
 tests/libqtest.c                   |  83 +++++++++++++++++++-
 tests/libqtest.h                   |  51 ++++++++++++-
 tests/migration-test.c             | 117 ++++++++++++++++++++++++++---
 tests/qemu-seccomp                 | Bin 0 -> 1266168 bytes
 tests/test-query-device-blockstats | Bin 0 -> 1272608 bytes
 12 files changed, 379 insertions(+), 20 deletions(-)
 create mode 100644 migration/inline-fd.c
 create mode 100644 migration/inline-fd.h
 create mode 100755 tests/qemu-seccomp
 create mode 100755 tests/test-query-device-blockstats

-- 
2.21.0

[Qemu-devel] [PATCH 1/3] monitor: Add monitor_recv_fd function to work with sent fds

[Yury Kotov]

Signed-off-by: Yury Kotov <yury-kotov@yandex-team.ru>
---
 include/monitor/monitor.h |  1 +
 monitor.c                 | 15 +++++++++++----
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index c1b40a9cac..9b9e593fb3 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -25,6 +25,7 @@ void monitor_cleanup(void);
 int monitor_suspend(Monitor *mon);
 void monitor_resume(Monitor *mon);
 
+int monitor_recv_fd(Monitor *mon, Error **errp);
 int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp);
 int monitor_fd_param(Monitor *mon, const char *fdname, Error **errp);
 
diff --git a/monitor.c b/monitor.c
index 4807bbe811..3ebbc08e5c 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2206,9 +2206,8 @@ void qmp_getfd(const char *fdname, Error **errp)
     mon_fd_t *monfd;
     int fd, tmp_fd;
 
-    fd = qemu_chr_fe_get_msgfd(&cur_mon->chr);
+    fd = monitor_recv_fd(cur_mon, errp);
     if (fd == -1) {
-        error_setg(errp, QERR_FD_NOT_SUPPLIED);
         return;
     }
 
@@ -2266,6 +2265,15 @@ void qmp_closefd(const char *fdname, Error **errp)
     error_setg(errp, QERR_FD_NOT_FOUND, fdname);
 }
 
+int monitor_recv_fd(Monitor *mon, Error **errp)
+{
+    int fd = qemu_chr_fe_get_msgfd(&cur_mon->chr);
+    if (fd == -1) {
+        error_setg(errp, QERR_FD_NOT_SUPPLIED);
+    }
+    return fd;
+}
+
 int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp)
 {
     mon_fd_t *monfd;
@@ -2335,9 +2343,8 @@ AddfdInfo *qmp_add_fd(bool has_fdset_id, int64_t fdset_id, bool has_opaque,
     Monitor *mon = cur_mon;
     AddfdInfo *fdinfo;
 
-    fd = qemu_chr_fe_get_msgfd(&mon->chr);
+    fd = monitor_recv_fd(mon, errp);
     if (fd == -1) {
-        error_setg(errp, QERR_FD_NOT_SUPPLIED);
         goto error;
     }
 
-- 
2.21.0

[Qemu-devel] [PATCH 2/3] migration: Add inline-fd protocol

[Yury Kotov]

Existing 'fd:' proto works with previously added fd by getfd or add-fd commands.
If client doesn't want to work with this fd before or after migration then it's
easier to send an fd with the migrate-* command. Also, client shouldn't maintain
this fd.
So, add 'inline-fd:' proto to work with sent fd.

Usage:
{ 'execute': 'migrate', 'arguments': { 'uri': 'inline-fd:' } }
{ 'execute': 'migrate-incoming', 'arguments': { 'uri': 'inline-fd:' } }

Signed-off-by: Yury Kotov <yury-kotov@yandex-team.ru>
---
 migration/Makefile.objs |  2 +-
 migration/inline-fd.c   | 89 +++++++++++++++++++++++++++++++++++++++++
 migration/inline-fd.h   | 22 ++++++++++
 migration/migration.c   | 15 +++++++
 migration/trace-events  |  4 ++
 5 files changed, 131 insertions(+), 1 deletion(-)
 create mode 100644 migration/inline-fd.c
 create mode 100644 migration/inline-fd.h

diff --git a/migration/Makefile.objs b/migration/Makefile.objs
index a4f3bafd86..f4bb6c5803 100644
--- a/migration/Makefile.objs
+++ b/migration/Makefile.objs
@@ -1,4 +1,4 @@
-common-obj-y += migration.o socket.o fd.o exec.o
+common-obj-y += migration.o socket.o fd.o inline-fd.o exec.o
 common-obj-y += tls.o channel.o savevm.o
 common-obj-y += colo.o colo-failover.o
 common-obj-y += vmstate.o vmstate-types.o page_cache.o
diff --git a/migration/inline-fd.c b/migration/inline-fd.c
new file mode 100644
index 0000000000..90a0dc079f
--- /dev/null
+++ b/migration/inline-fd.c
@@ -0,0 +1,89 @@
+/*
+ * QEMU live migration via generic fd passed with command
+ *
+ * Copyright Yandex, Inc. 2019
+ *
+ * Authors:
+ *  Yury Kotov <yury-kotov@yandex-team.ru>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * Contributions after 2012-01-13 are licensed under the terms of the
+ * GNU GPL, version 2 or (at your option) any later version.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "channel.h"
+#include "inline-fd.h"
+#include "monitor/monitor.h"
+#include "io/channel-util.h"
+#include "trace.h"
+
+
+void inline_fd_start_outgoing_migration(MigrationState *s, Error **errp)
+{
+    QIOChannel *ioc;
+    int fd;
+
+    if (!cur_mon) {
+        error_setg(errp, "Monitor is disabled");
+        return;
+    }
+
+    fd = monitor_recv_fd(cur_mon, errp);
+    if (fd == -1) {
+        return;
+    }
+
+    trace_migration_inline_fd_outgoing(fd);
+    ioc = qio_channel_new_fd(fd, errp);
+    if (!ioc) {
+        close(fd);
+        return;
+    }
+
+    qio_channel_set_name(QIO_CHANNEL(ioc), "migration-infd-outgoing");
+    migration_channel_connect(s, ioc, NULL, NULL);
+    object_unref(OBJECT(ioc));
+}
+
+static gboolean inline_fd_accept_incoming_migration(QIOChannel *ioc,
+                                                    GIOCondition condition,
+                                                    gpointer opaque)
+{
+    migration_channel_process_incoming(ioc);
+    object_unref(OBJECT(ioc));
+    return G_SOURCE_REMOVE;
+}
+
+void inline_fd_start_incoming_migration(Error **errp)
+{
+    QIOChannel *ioc;
+    int fd;
+
+    if (!cur_mon) {
+        error_setg(errp, "Monitor is disabled");
+        return;
+    }
+
+    fd = monitor_recv_fd(cur_mon, errp);
+    if (fd == -1) {
+        return;
+    }
+
+    trace_migration_inline_fd_incoming(fd);
+    ioc = qio_channel_new_fd(fd, errp);
+    if (!ioc) {
+        close(fd);
+        return;
+    }
+
+    qio_channel_set_name(QIO_CHANNEL(ioc), "migration-infd-incoming");
+    qio_channel_add_watch(ioc,
+                          G_IO_IN,
+                          inline_fd_accept_incoming_migration,
+                          NULL,
+                          NULL);
+}
diff --git a/migration/inline-fd.h b/migration/inline-fd.h
new file mode 100644
index 0000000000..5b23ce314d
--- /dev/null
+++ b/migration/inline-fd.h
@@ -0,0 +1,22 @@
+/*
+ * QEMU live migration via generic fd passed with command
+ *
+ * Copyright Yandex, Inc. 2019
+ *
+ * Authors:
+ *  Yury Kotov <yury-kotov@yandex-team.ru>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * Contributions after 2012-01-13 are licensed under the terms of the
+ * GNU GPL, version 2 or (at your option) any later version.
+ */
+
+#ifndef QEMU_MIGRATION_INLINE_FD_H
+#define QEMU_MIGRATION_INLINE_FD_H
+
+void inline_fd_start_incoming_migration(Error **errp);
+void inline_fd_start_outgoing_migration(MigrationState *s, Error **errp);
+
+#endif
diff --git a/migration/migration.c b/migration/migration.c
index 609e0df5d0..7b9fafe218 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -19,6 +19,7 @@
 #include "migration/blocker.h"
 #include "exec.h"
 #include "fd.h"
+#include "inline-fd.h"
 #include "socket.h"
 #include "rdma.h"
 #include "ram.h"
@@ -364,6 +365,13 @@ void qemu_start_incoming_migration(const char *uri, Error **errp)
         unix_start_incoming_migration(p, errp);
     } else if (strstart(uri, "fd:", &p)) {
         fd_start_incoming_migration(p, errp);
+    } else if (strstart(uri, "inline-fd:", &p)) {
+        if (!*p) {
+            inline_fd_start_incoming_migration(errp);
+        } else {
+            error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "uri",
+                       "an empty path for 'inline-fd:' protocol");
+        }
     } else {
         error_setg(errp, "unknown migration protocol: %s", uri);
     }
@@ -1924,6 +1932,13 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
         unix_start_outgoing_migration(s, p, &local_err);
     } else if (strstart(uri, "fd:", &p)) {
         fd_start_outgoing_migration(s, p, &local_err);
+    } else if (strstart(uri, "inline-fd:", &p)) {
+        if (!*p) {
+            inline_fd_start_outgoing_migration(s, &local_err);
+        } else {
+            error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "uri",
+                       "an empty path for 'inline-fd:' protocol");
+        }
     } else {
         error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "uri",
                    "a valid migration protocol");
diff --git a/migration/trace-events b/migration/trace-events
index de2e136e57..1afd11eab5 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -261,6 +261,10 @@ migration_exec_incoming(const char *cmd) "cmd=%s"
 migration_fd_outgoing(int fd) "fd=%d"
 migration_fd_incoming(int fd) "fd=%d"
 
+# inline-fd.c
+migration_inline_fd_outgoing(int fd) "fd=%d"
+migration_inline_fd_incoming(int fd) "fd=%d"
+
 # socket.c
 migration_socket_incoming_accepted(void) ""
 migration_socket_outgoing_connected(const char *hostname) "hostname=%s"
-- 
2.21.0

[Qemu-devel] [PATCH 3/3] migration-test: Add a test for inline_fd protocol

[Yury Kotov]

Signed-off-by: Yury Kotov <yury-kotov@yandex-team.ru>
---
 tests/libqtest.c       |  83 +++++++++++++++++++++++++++--
 tests/libqtest.h       |  51 +++++++++++++++++-
 tests/migration-test.c | 117 +++++++++++++++++++++++++++++++++++++----
 3 files changed, 236 insertions(+), 15 deletions(-)

diff --git a/tests/libqtest.c b/tests/libqtest.c
index c49b85482d..2da9310052 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -32,6 +32,7 @@
 
 #define MAX_IRQ 256
 #define SOCKET_TIMEOUT 50
+#define SOCKET_MAX_FDS 16
 
 QTestState *global_qtest;
 
@@ -391,6 +392,43 @@ static void GCC_FMT_ATTR(2, 3) qtest_sendf(QTestState *s, const char *fmt, ...)
     va_end(ap);
 }
 
+static void socket_send_fds(int fd, int *fds, size_t fds_num,
+                            const char *buf, size_t buf_size)
+{
+#ifndef WIN32
+    ssize_t ret;
+    struct msghdr msg = { 0 };
+    char control[CMSG_SPACE(sizeof(int) * SOCKET_MAX_FDS)] = { 0 };
+    size_t fdsize = sizeof(int) * fds_num;
+    struct cmsghdr *cmsg;
+    struct iovec iov = { .iov_base = (char *)buf, .iov_len = buf_size };
+
+    msg.msg_iov = &iov;
+    msg.msg_iovlen = 1;
+
+    if (fds && fds_num > 0) {
+        g_assert_cmpuint(fds_num, <, SOCKET_MAX_FDS);
+
+        msg.msg_control = control;
+        msg.msg_controllen = CMSG_SPACE(fdsize);
+
+        cmsg = CMSG_FIRSTHDR(&msg);
+        cmsg->cmsg_len = CMSG_LEN(fdsize);
+        cmsg->cmsg_level = SOL_SOCKET;
+        cmsg->cmsg_type = SCM_RIGHTS;
+        memcpy(CMSG_DATA(cmsg), fds, fdsize);
+    }
+
+    do {
+        ret = sendmsg(fd, &msg, 0);
+    } while (ret < 0 && errno == EINTR);
+    g_assert_cmpint(ret, >, 0);
+#else
+    g_test_skip("sendmsg is not supported under Win32");
+    return;
+#endif
+}
+
 static GString *qtest_recv_line(QTestState *s)
 {
     GString *line;
@@ -545,7 +583,8 @@ QDict *qtest_qmp_receive(QTestState *s)
  * in the case that they choose to discard all replies up until
  * a particular EVENT is received.
  */
-void qmp_fd_vsend(int fd, const char *fmt, va_list ap)
+void qmp_fd_vsend_fds(int fd, int *fds, size_t fds_num,
+                      const char *fmt, va_list ap)
 {
     QObject *qobj;
 
@@ -569,25 +608,49 @@ void qmp_fd_vsend(int fd, const char *fmt, va_list ap)
             fprintf(stderr, "%s", str);
         }
         /* Send QMP request */
-        socket_send(fd, str, qstring_get_length(qstr));
+        if (fds && fds_num > 0) {
+            socket_send_fds(fd, fds, fds_num, str, qstring_get_length(qstr));
+        } else {
+            socket_send(fd, str, qstring_get_length(qstr));
+        }
 
         qobject_unref(qstr);
         qobject_unref(qobj);
     }
 }
 
+void qmp_fd_vsend(int fd, const char *fmt, va_list ap)
+{
+    qmp_fd_vsend_fds(fd, NULL, 0, fmt, ap);
+}
+
+void qtest_qmp_vsend_fds(QTestState *s, int *fds, size_t fds_num,
+                         const char *fmt, va_list ap)
+{
+    qmp_fd_vsend_fds(s->qmp_fd, fds, fds_num, fmt, ap);
+}
+
 void qtest_qmp_vsend(QTestState *s, const char *fmt, va_list ap)
 {
-    qmp_fd_vsend(s->qmp_fd, fmt, ap);
+    qmp_fd_vsend_fds(s->qmp_fd, NULL, 0, fmt, ap);
 }
 
 QDict *qmp_fdv(int fd, const char *fmt, va_list ap)
 {
-    qmp_fd_vsend(fd, fmt, ap);
+    qmp_fd_vsend_fds(fd, NULL, 0, fmt, ap);
 
     return qmp_fd_receive(fd);
 }
 
+QDict *qtest_vqmp_fds(QTestState *s, int *fds, size_t fds_num,
+                      const char *fmt, va_list ap)
+{
+    qtest_qmp_vsend_fds(s, fds, fds_num, fmt, ap);
+
+    /* Receive reply */
+    return qtest_qmp_receive(s);
+}
+
 QDict *qtest_vqmp(QTestState *s, const char *fmt, va_list ap)
 {
     qtest_qmp_vsend(s, fmt, ap);
@@ -616,6 +679,18 @@ void qmp_fd_send(int fd, const char *fmt, ...)
     va_end(ap);
 }
 
+QDict *qtest_qmp_fds(QTestState *s, int *fds, size_t fds_num,
+                     const char *fmt, ...)
+{
+    va_list ap;
+    QDict *response;
+
+    va_start(ap, fmt);
+    response = qtest_vqmp_fds(s, fds, fds_num, fmt, ap);
+    va_end(ap);
+    return response;
+}
+
 QDict *qtest_qmp(QTestState *s, const char *fmt, ...)
 {
     va_list ap;
diff --git a/tests/libqtest.h b/tests/libqtest.h
index a16acd58a6..b471b9eeab 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -84,6 +84,21 @@ QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd);
  */
 void qtest_quit(QTestState *s);
 
+/**
+ * qtest_qmp_fds:
+ * @s: #QTestState instance to operate on.
+ * @fds: array of file descriptors
+ * @fds_num: number of elements in @fds
+ * @fmt...: QMP message to send to qemu, formatted like
+ * qobject_from_jsonf_nofail().  See parse_escape() for what's
+ * supported after '%'.
+ *
+ * Sends a QMP message to QEMU with fds and returns the response.
+ */
+QDict *qtest_qmp_fds(QTestState *s, int *fds, size_t fds_num,
+                     const char *fmt, ...)
+    GCC_FMT_ATTR(4, 5);
+
 /**
  * qtest_qmp:
  * @s: #QTestState instance to operate on.
@@ -120,7 +135,23 @@ void qtest_qmp_send_raw(QTestState *s, const char *fmt, ...)
     GCC_FMT_ATTR(2, 3);
 
 /**
- * qtest_qmpv:
+ * qtest_vqmp_fds:
+ * @s: #QTestState instance to operate on.
+ * @fds: array of file descriptors
+ * @fds_num: number of elements in @fds
+ * @fmt: QMP message to send to QEMU, formatted like
+ * qobject_from_jsonf_nofail().  See parse_escape() for what's
+ * supported after '%'.
+ * @ap: QMP message arguments
+ *
+ * Sends a QMP message to QEMU with fds and returns the response.
+ */
+QDict *qtest_vqmp_fds(QTestState *s, int *fds, size_t fds_num,
+                      const char *fmt, va_list ap)
+    GCC_FMT_ATTR(4, 0);
+
+/**
+ * qtest_vqmp:
  * @s: #QTestState instance to operate on.
  * @fmt: QMP message to send to QEMU, formatted like
  * qobject_from_jsonf_nofail().  See parse_escape() for what's
@@ -132,6 +163,22 @@ void qtest_qmp_send_raw(QTestState *s, const char *fmt, ...)
 QDict *qtest_vqmp(QTestState *s, const char *fmt, va_list ap)
     GCC_FMT_ATTR(2, 0);
 
+/**
+ * qtest_qmp_vsend_fds:
+ * @s: #QTestState instance to operate on.
+ * @fds: array of file descriptors
+ * @fds_num: number of elements in @fds
+ * @fmt: QMP message to send to QEMU, formatted like
+ * qobject_from_jsonf_nofail().  See parse_escape() for what's
+ * supported after '%'.
+ * @ap: QMP message arguments
+ *
+ * Sends a QMP message to QEMU and leaves the response in the stream.
+ */
+void qtest_qmp_vsend_fds(QTestState *s, int *fds, size_t fds_num,
+                         const char *fmt, va_list ap)
+    GCC_FMT_ATTR(4, 0);
+
 /**
  * qtest_qmp_vsend:
  * @s: #QTestState instance to operate on.
@@ -985,6 +1032,8 @@ static inline int64_t clock_set(int64_t val)
 }
 
 QDict *qmp_fd_receive(int fd);
+void qmp_fd_vsend_fds(int fd, int *fds, size_t fds_num,
+                      const char *fmt, va_list ap) GCC_FMT_ATTR(4, 0);
 void qmp_fd_vsend(int fd, const char *fmt, va_list ap) GCC_FMT_ATTR(2, 0);
 void qmp_fd_send(int fd, const char *fmt, ...) GCC_FMT_ATTR(2, 3);
 void qmp_fd_send_raw(int fd, const char *fmt, ...) GCC_FMT_ATTR(2, 3);
diff --git a/tests/migration-test.c b/tests/migration-test.c
index bd3f5c3125..538fffcc33 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -15,6 +15,7 @@
 #include "libqtest.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qstring.h"
 #include "qemu/option.h"
 #include "qemu/range.h"
 #include "qemu/sockets.h"
@@ -174,6 +175,21 @@ static void stop_cb(void *opaque, const char *name, QDict *data)
     }
 }
 
+/*
+ * Events can get in the way of responses we are actually waiting for.
+ */
+GCC_FMT_ATTR(3, 4)
+static QDict *wait_command_fd(QTestState *who, int fd, const char *command, ...)
+{
+    va_list ap;
+
+    va_start(ap, command);
+    qtest_qmp_vsend_fds(who, &fd, 1, command, ap);
+    va_end(ap);
+
+    return qtest_qmp_receive_success(who, stop_cb, NULL);
+}
+
 /*
  * Events can get in the way of responses we are actually waiting for.
  */
@@ -461,6 +477,42 @@ static void migrate_set_capability(QTestState *who, const char *capability,
     qobject_unref(rsp);
 }
 
+GCC_FMT_ATTR(4, 0)
+static void migrate_to_fdv(QTestState *who, int fd, const char *uri,
+                           const char *fmt, va_list ap)
+{
+    QDict *args, *rsp;
+
+    args = qdict_from_vjsonf_nofail(fmt, ap);
+    g_assert(!qdict_haskey(args, "uri"));
+    qdict_put_str(args, "uri", uri);
+
+    if (fd != -1) {
+        rsp = qtest_qmp_fds(who, &fd, 1,
+                            "{ 'execute': 'migrate', 'arguments': %p}", args);
+    } else {
+        rsp = qtest_qmp(who, "{ 'execute': 'migrate', 'arguments': %p}", args);
+    }
+
+    g_assert(qdict_haskey(rsp, "return"));
+    qobject_unref(rsp);
+}
+
+/*
+ * Send QMP command "migrate" with fd.
+ * Arguments are built from @fmt... (formatted like
+ * qobject_from_jsonf_nofail()) with "uri": @uri spliced in.
+ */
+GCC_FMT_ATTR(4, 5)
+static void migrate_to_fd(QTestState *who, int fd, const char *uri,
+                          const char *fmt, ...)
+{
+    va_list ap;
+    va_start(ap, fmt);
+    migrate_to_fdv(who, fd, uri, fmt, ap);
+    va_end(ap);
+}
+
 /*
  * Send QMP command "migrate".
  * Arguments are built from @fmt... (formatted like
@@ -470,18 +522,9 @@ GCC_FMT_ATTR(3, 4)
 static void migrate(QTestState *who, const char *uri, const char *fmt, ...)
 {
     va_list ap;
-    QDict *args, *rsp;
-
     va_start(ap, fmt);
-    args = qdict_from_vjsonf_nofail(fmt, ap);
+    migrate_to_fdv(who, -1, uri, fmt, ap);
     va_end(ap);
-
-    g_assert(!qdict_haskey(args, "uri"));
-    qdict_put_str(args, "uri", uri);
-
-    rsp = qmp("{ 'execute': 'migrate', 'arguments': %p}", args);
-    g_assert(qdict_haskey(rsp, "return"));
-    qobject_unref(rsp);
 }
 
 static void migrate_postcopy_start(QTestState *from, QTestState *to)
@@ -1027,6 +1070,59 @@ static void test_precopy_tcp(void)
     g_free(uri);
 }
 
+static void test_migrate_inline_fd(void)
+{
+    QTestState *from, *to;
+    int ret;
+    int pair[2];
+    QDict *rsp;
+
+    if (test_migrate_start(&from, &to, "defer", false, false)) {
+        return;
+    }
+
+    /*
+     * We want to pick a speed slow enough that the test completes
+     * quickly, but that it doesn't complete precopy even on a slow
+     * machine, so also set the downtime.
+     */
+    /* 1 ms should make it not converge*/
+    migrate_set_parameter(from, "downtime-limit", 1);
+    /* 1GB/s */
+    migrate_set_parameter(from, "max-bandwidth", 1000000000);
+
+    /* Wait for the first serial output from the source */
+    wait_for_serial("src_serial");
+
+    ret = socketpair(PF_LOCAL, SOCK_STREAM, 0, pair);
+    g_assert_cmpint(ret, ==, 0);
+
+    rsp = wait_command_fd(to, pair[0],
+                          "{ 'execute': 'migrate-incoming',"
+                          "  'arguments': { 'uri': 'inline-fd:' }}");
+    qobject_unref(rsp);
+
+    migrate_to_fd(from, pair[1], "inline-fd:", "{}");
+
+    close(pair[0]);
+    close(pair[1]);
+
+    wait_for_migration_pass(from);
+
+    /* 300ms should converge */
+    migrate_set_parameter(from, "downtime-limit", 300);
+
+    if (!got_stop) {
+        qtest_qmp_eventwait(from, "STOP");
+    }
+    qtest_qmp_eventwait(to, "RESUME");
+
+    wait_for_serial("dest_serial");
+    wait_for_migration_complete(from);
+
+    test_migrate_end(from, to, true);
+}
+
 int main(int argc, char **argv)
 {
     char template[] = "/tmp/migration-test-XXXXXX";
@@ -1081,6 +1177,7 @@ int main(int argc, char **argv)
     qtest_add_func("/migration/precopy/tcp", test_precopy_tcp);
     /* qtest_add_func("/migration/ignore_shared", test_ignore_shared); */
     qtest_add_func("/migration/xbzrle/unix", test_xbzrle_unix);
+    qtest_add_func("/migration/inline_fd", test_migrate_inline_fd);
 
     ret = g_test_run();
 
-- 
2.21.0

Re: [Qemu-devel] [PATCH 0/3] Add 'inline-fd:' protocol for migration

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1568 bytes --]

On 4/12/19 7:20 AM, Yury Kotov wrote:
> Hi,
> 
> I've added 'inline-fd:' proto to simplify migration to/from fd.
> I thought about modifying the existing 'fd:' proto but I'm not sure it's a
> good idea to change it's contract.
> 
> Existing 'fd:' proto works with previously added fd by getfd or add-fd commands.
> If client doesn't want to work with this fd before or after migration then it's
> easier to send an fd with the migrate-* command. Also, client shouldn't maintain
> this fd.

While the sentiment of making it easier by having fewer QMP commands
might be worthwhile, I'm worried about whether this scales. Having just
a limited set of commands that take an fd over SCM rights, and every
other command wired to automagically work with existing named fds,
scales a lot easier than having to teach individual commands how to take
an fd inline.

At the very least, if we DO decide we like this, then I'd at least hope
that you made this addition introspectible (how can management learn
whether the 'inline-fd:' protocol is supported, and even more
importantly, which commands require an inline fd via SCM rights to be a
valid command based, especially if accepting or rejecting an inline fd
is conditional on other arguments to the command).

> 
> Usage:
> { 'execute': 'migrate', 'arguments': { 'uri': 'inline-fd:' } }
> { 'execute': 'migrate-incoming', 'arguments': { 'uri': 'inline-fd:' } }
> 
-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Qemu-devel] [PATCH 0/3] Add 'inline-fd:' protocol for migration

[Daniel P. Berrangé]

On Fri, Apr 12, 2019 at 12:13:51PM -0500, Eric Blake wrote:
> On 4/12/19 7:20 AM, Yury Kotov wrote:
> > Hi,
> > 
> > I've added 'inline-fd:' proto to simplify migration to/from fd.
> > I thought about modifying the existing 'fd:' proto but I'm not sure it's a
> > good idea to change it's contract.
> > 
> > Existing 'fd:' proto works with previously added fd by getfd or add-fd commands.
> > If client doesn't want to work with this fd before or after migration then it's
> > easier to send an fd with the migrate-* command. Also, client shouldn't maintain
> > this fd.
> 
> While the sentiment of making it easier by having fewer QMP commands
> might be worthwhile, I'm worried about whether this scales. Having just
> a limited set of commands that take an fd over SCM rights, and every
> other command wired to automagically work with existing named fds,
> scales a lot easier than having to teach individual commands how to take
> an fd inline.

Yeah I tend to agree - we use FD passing extensively across QMP/HMP
and all these commands are written to interact with "getfd". I think
it would be a step backwards to introduce a special case with
migration that doesn't use "getfd".

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 0/3] Add 'inline-fd:' protocol for migration

[Daniel P. Berrangé]

On Fri, Apr 12, 2019 at 12:13:51PM -0500, Eric Blake wrote:
> On 4/12/19 7:20 AM, Yury Kotov wrote:
> > Hi,
> > 
> > I've added 'inline-fd:' proto to simplify migration to/from fd.
> > I thought about modifying the existing 'fd:' proto but I'm not sure it's a
> > good idea to change it's contract.
> > 
> > Existing 'fd:' proto works with previously added fd by getfd or add-fd commands.
> > If client doesn't want to work with this fd before or after migration then it's
> > easier to send an fd with the migrate-* command. Also, client shouldn't maintain
> > this fd.
> 
> While the sentiment of making it easier by having fewer QMP commands
> might be worthwhile, I'm worried about whether this scales. Having just
> a limited set of commands that take an fd over SCM rights, and every
> other command wired to automagically work with existing named fds,
> scales a lot easier than having to teach individual commands how to take
> an fd inline.

Yeah I tend to agree - we use FD passing extensively across QMP/HMP
and all these commands are written to interact with "getfd". I think
it would be a step backwards to introduce a special case with
migration that doesn't use "getfd".

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 0/3] Add 'inline-fd:' protocol for migration

[Dr. David Alan Gilbert]

* Daniel P. Berrangé (berrange@redhat.com) wrote:
> On Fri, Apr 12, 2019 at 12:13:51PM -0500, Eric Blake wrote:
> > On 4/12/19 7:20 AM, Yury Kotov wrote:
> > > Hi,
> > > 
> > > I've added 'inline-fd:' proto to simplify migration to/from fd.
> > > I thought about modifying the existing 'fd:' proto but I'm not sure it's a
> > > good idea to change it's contract.
> > > 
> > > Existing 'fd:' proto works with previously added fd by getfd or add-fd commands.
> > > If client doesn't want to work with this fd before or after migration then it's
> > > easier to send an fd with the migrate-* command. Also, client shouldn't maintain
> > > this fd.
> > 
> > While the sentiment of making it easier by having fewer QMP commands
> > might be worthwhile, I'm worried about whether this scales. Having just
> > a limited set of commands that take an fd over SCM rights, and every
> > other command wired to automagically work with existing named fds,
> > scales a lot easier than having to teach individual commands how to take
> > an fd inline.
> 
> Yeah I tend to agree - we use FD passing extensively across QMP/HMP
> and all these commands are written to interact with "getfd". I think
> it would be a step backwards to introduce a special case with
> migration that doesn't use "getfd".

Yes, agreed - as you spotted passing fd's gets a bit hairy, and hence
it's best to keep it to a few places.

Lets figure out the right way of fixing the double-close you spotted
rather than adding new schemes.

Dave

> Regards,
> Daniel
> -- 
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 0/3] Add 'inline-fd:' protocol for migration

[Dr. David Alan Gilbert]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="UTF-8", Size: 1880 bytes --]

* Daniel P. Berrangé (berrange@redhat.com) wrote:
> On Fri, Apr 12, 2019 at 12:13:51PM -0500, Eric Blake wrote:
> > On 4/12/19 7:20 AM, Yury Kotov wrote:
> > > Hi,
> > > 
> > > I've added 'inline-fd:' proto to simplify migration to/from fd.
> > > I thought about modifying the existing 'fd:' proto but I'm not sure it's a
> > > good idea to change it's contract.
> > > 
> > > Existing 'fd:' proto works with previously added fd by getfd or add-fd commands.
> > > If client doesn't want to work with this fd before or after migration then it's
> > > easier to send an fd with the migrate-* command. Also, client shouldn't maintain
> > > this fd.
> > 
> > While the sentiment of making it easier by having fewer QMP commands
> > might be worthwhile, I'm worried about whether this scales. Having just
> > a limited set of commands that take an fd over SCM rights, and every
> > other command wired to automagically work with existing named fds,
> > scales a lot easier than having to teach individual commands how to take
> > an fd inline.
> 
> Yeah I tend to agree - we use FD passing extensively across QMP/HMP
> and all these commands are written to interact with "getfd". I think
> it would be a step backwards to introduce a special case with
> migration that doesn't use "getfd".

Yes, agreed - as you spotted passing fd's gets a bit hairy, and hence
it's best to keep it to a few places.

Lets figure out the right way of fixing the double-close you spotted
rather than adding new schemes.

Dave

> Regards,
> Daniel
> -- 
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK