All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL 00/28] migration: support for TLS
@ 2016-05-26  6:11 Amit Shah
  2016-05-26  6:11 ` [Qemu-devel] [PULL 01/28] s390: use FILE instead of QEMUFile for creating text file Amit Shah
                   ` (28 more replies)
  0 siblings, 29 replies; 36+ messages in thread
From: Amit Shah @ 2016-05-26  6:11 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Juan Quintela, Dr. David Alan Gilbert, Daniel P. Berrange,
	qemu list, Amit Shah

The following changes since commit 287db79df8af8e31f18e262feb5e05103a09e4d4:

  Merge remote-tracking branch 'remotes/ehabkost/tags/x86-pull-request' into staging (2016-05-24 13:06:33 +0100)

are available in the git repository at:

  https://git.kernel.org/pub/scm/virt/qemu/amit/migration.git tags/migration-2.7-2

for you to fetch changes up to 12992c16d9afd8a23a94a84ad532a1adedf9e511:

  migration: remove qemu_get_fd method from QEMUFile (2016-05-26 11:32:21 +0530)

----------------------------------------------------------------
migration: add TLS support to the migration data channel

This is a big refactoring of the migration backend code - moving away from
QEMUFile to the new QIOChannel framework introduced here.  This brings a
good level of abstraction and reduction of many lines of code.

This series also adds the ability for many backends (all except RDMA) to
use TLS for encrypting the migration data between the endpoints.

----------------------------------------------------------------


Daniel P. Berrange (28):
  s390: use FILE instead of QEMUFile for creating text file
  io: avoid double-free when closing QIOChannelBuffer
  migration: remove use of qemu_bufopen from vmstate tests
  migration: ensure qemu_fflush() always writes full data amount
  migration: split migration hooks out of QEMUFileOps
  migration: introduce set_blocking function in QEMUFileOps
  migration: force QEMUFile to blocking mode for outgoing migration
  migration: introduce a new QEMUFile impl based on QIOChannel
  migration: add helpers for creating QEMUFile from a QIOChannel
  migration: add reporting of errors for outgoing migration
  migration: convert post-copy to use QIOChannelBuffer
  migration: convert unix socket protocol to use QIOChannel
  migration: rename unix.c to socket.c
  migration: convert tcp socket protocol to use QIOChannel
  migration: convert fd socket protocol to use QIOChannel
  migration: convert exec socket protocol to use QIOChannel
  migration: convert RDMA to use QIOChannel interface
  migration: convert savevm to use QIOChannel for writing to files
  migration: delete QEMUFile buffer implementation
  migration: delete QEMUSizedBuffer struct
  migration: delete QEMUFile sockets implementation
  migration: delete QEMUFile stdio implementation
  migration: move definition of struct QEMUFile back into qemu-file.c
  migration: don't use an array for storing migrate parameters
  migration: define 'tls-creds' and 'tls-hostname' migration parameters
  migration: add support for encrypting data with TLS
  migration: remove support for non-iovec based write handlers
  migration: remove qemu_get_fd method from QEMUFile

 docs/migration.txt             |   4 +-
 hmp-commands.hx                |   2 +-
 hmp.c                          |  57 ++++-
 hw/s390x/s390-skeys.c          |  26 +--
 include/migration/migration.h  |  26 ++-
 include/migration/qemu-file.h  |  57 ++---
 include/qapi/error.h           |   2 +-
 include/qemu/typedefs.h        |   1 -
 include/sysemu/sysemu.h        |   2 +-
 io/channel-buffer.c            |   1 +
 migration/Makefile.objs        |   7 +-
 migration/exec.c               |  62 +++---
 migration/fd.c                 |  75 +++----
 migration/migration.c          | 157 +++++++++-----
 migration/qemu-file-buf.c      | 464 -----------------------------------------
 migration/qemu-file-channel.c  | 180 ++++++++++++++++
 migration/qemu-file-internal.h |  53 -----
 migration/qemu-file-stdio.c    | 196 -----------------
 migration/qemu-file-unix.c     | 323 ----------------------------
 migration/qemu-file.c          | 110 +++++-----
 migration/ram.c                |   6 +-
 migration/rdma.c               | 380 ++++++++++++++++++++++++---------
 migration/savevm.c             |  63 ++----
 migration/socket.c             | 183 ++++++++++++++++
 migration/tcp.c                | 102 ---------
 migration/tls.c                | 161 ++++++++++++++
 migration/unix.c               | 103 ---------
 qapi-schema.json               |  65 +++++-
 tests/Makefile                 |   6 +-
 tests/test-vmstate.c           |  55 ++---
 trace-events                   |  25 ++-
 util/error.c                   |   2 +-
 32 files changed, 1281 insertions(+), 1675 deletions(-)
 delete mode 100644 migration/qemu-file-buf.c
 create mode 100644 migration/qemu-file-channel.c
 delete mode 100644 migration/qemu-file-internal.h
 delete mode 100644 migration/qemu-file-stdio.c
 delete mode 100644 migration/qemu-file-unix.c
 create mode 100644 migration/socket.c
 delete mode 100644 migration/tcp.c
 create mode 100644 migration/tls.c
 delete mode 100644 migration/unix.c

-- 
2.5.5

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PULL 01/28] s390: use FILE instead of QEMUFile for creating text file
  2016-05-26  6:11 [Qemu-devel] [PULL 00/28] migration: support for TLS Amit Shah
@ 2016-05-26  6:11 ` Amit Shah
  2016-05-26  6:11 ` [Qemu-devel] [PULL 02/28] io: avoid double-free when closing QIOChannelBuffer Amit Shah
                   ` (27 subsequent siblings)
  28 siblings, 0 replies; 36+ messages in thread
From: Amit Shah @ 2016-05-26  6:11 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Juan Quintela, Dr. David Alan Gilbert, Daniel P. Berrange,
	qemu list, Amit Shah

From: "Daniel P. Berrange" <berrange@redhat.com>

The s390 skeys monitor command needs to write out a plain text
file. Currently it is using the QEMUFile class for this, but
work is ongoing to refactor QEMUFile and eliminate much code
related to it. The only feature qemu_fopen() gives over fopen()
is support for QEMU FD passing, but this can be achieved with
qemu_open() + fdopen() too. Switching to regular stdio FILE
APIs avoids the need to sprintf via an intermedia buffer which
slightly simplifies the code.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Message-Id: <1461751518-12128-2-git-send-email-berrange@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hw/s390x/s390-skeys.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c
index d772cfc..e2d4e1a 100644
--- a/hw/s390x/s390-skeys.c
+++ b/hw/s390x/s390-skeys.c
@@ -47,15 +47,11 @@ void s390_skeys_init(void)
     qdev_init_nofail(DEVICE(obj));
 }
 
-static void write_keys(QEMUFile *f, uint8_t *keys, uint64_t startgfn,
+static void write_keys(FILE *f, uint8_t *keys, uint64_t startgfn,
                        uint64_t count, Error **errp)
 {
     uint64_t curpage = startgfn;
     uint64_t maxpage = curpage + count - 1;
-    const char *fmt = "page=%03" PRIx64 ": key(%d) => ACC=%X, FP=%d, REF=%d,"
-                      " ch=%d, reserved=%d\n";
-    char buf[128];
-    int len;
 
     for (; curpage <= maxpage; curpage++) {
         uint8_t acc = (*keys & 0xF0) >> 4;
@@ -64,10 +60,9 @@ static void write_keys(QEMUFile *f, uint8_t *keys, uint64_t startgfn,
         int ch = (*keys & 0x02);
         int res = (*keys & 0x01);
 
-        len = snprintf(buf, sizeof(buf), fmt, curpage,
-                       *keys, acc, fp, ref, ch, res);
-        assert(len < sizeof(buf));
-        qemu_put_buffer(f, (uint8_t *)buf, len);
+        fprintf(f, "page=%03" PRIx64 ": key(%d) => ACC=%X, FP=%d, REF=%d,"
+                " ch=%d, reserved=%d\n",
+                curpage, *keys, acc, fp, ref, ch, res);
         keys++;
     }
 }
@@ -116,7 +111,8 @@ void qmp_dump_skeys(const char *filename, Error **errp)
     vaddr cur_gfn = 0;
     uint8_t *buf;
     int ret;
-    QEMUFile *f;
+    int fd;
+    FILE *f;
 
     /* Quick check to see if guest is using storage keys*/
     if (!skeyclass->skeys_enabled(ss)) {
@@ -125,8 +121,14 @@ void qmp_dump_skeys(const char *filename, Error **errp)
         return;
     }
 
-    f = qemu_fopen(filename, "wb");
+    fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC, 0600);
+    if (fd < 0) {
+        error_setg_file_open(errp, errno, filename);
+        return;
+    }
+    f = fdopen(fd, "wb");
     if (!f) {
+        close(fd);
         error_setg_file_open(errp, errno, filename);
         return;
     }
@@ -162,7 +164,7 @@ out_free:
     error_propagate(errp, lerr);
     g_free(buf);
 out:
-    qemu_fclose(f);
+    fclose(f);
 }
 
 static void qemu_s390_skeys_init(Object *obj)
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PULL 02/28] io: avoid double-free when closing QIOChannelBuffer
  2016-05-26  6:11 [Qemu-devel] [PULL 00/28] migration: support for TLS Amit Shah
  2016-05-26  6:11 ` [Qemu-devel] [PULL 01/28] s390: use FILE instead of QEMUFile for creating text file Amit Shah
@ 2016-05-26  6:11 ` Amit Shah
  2016-05-26  6:11 ` [Qemu-devel] [PULL 03/28] migration: remove use of qemu_bufopen from vmstate tests Amit Shah
                   ` (26 subsequent siblings)
  28 siblings, 0 replies; 36+ messages in thread
From: Amit Shah @ 2016-05-26  6:11 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Juan Quintela, Dr. David Alan Gilbert, Daniel P. Berrange,
	qemu list, Amit Shah

From: "Daniel P. Berrange" <berrange@redhat.com>

The QIOChannelBuffer's close implementation will free
the internal data buffer. It failed to reset the pointer
to NULL though, so when the object is later finalized
it will free it a second time with predictable crash.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Message-Id: <1461751518-12128-3-git-send-email-berrange@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 io/channel-buffer.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/io/channel-buffer.c b/io/channel-buffer.c
index 3e5117b..43d7959 100644
--- a/io/channel-buffer.c
+++ b/io/channel-buffer.c
@@ -140,6 +140,7 @@ static int qio_channel_buffer_close(QIOChannel *ioc,
     QIOChannelBuffer *bioc = QIO_CHANNEL_BUFFER(ioc);
 
     g_free(bioc->data);
+    bioc->data = NULL;
     bioc->capacity = bioc->usage = bioc->offset = 0;
 
     return 0;
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PULL 03/28] migration: remove use of qemu_bufopen from vmstate tests
  2016-05-26  6:11 [Qemu-devel] [PULL 00/28] migration: support for TLS Amit Shah
  2016-05-26  6:11 ` [Qemu-devel] [PULL 01/28] s390: use FILE instead of QEMUFile for creating text file Amit Shah
  2016-05-26  6:11 ` [Qemu-devel] [PULL 02/28] io: avoid double-free when closing QIOChannelBuffer Amit Shah
@ 2016-05-26  6:11 ` Amit Shah
  2016-05-26  6:11 ` [Qemu-devel] [PULL 04/28] migration: ensure qemu_fflush() always writes full data amount Amit Shah
                   ` (25 subsequent siblings)
  28 siblings, 0 replies; 36+ messages in thread
From: Amit Shah @ 2016-05-26  6:11 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Juan Quintela, Dr. David Alan Gilbert, Daniel P. Berrange,
	qemu list, Amit Shah

From: "Daniel P. Berrange" <berrange@redhat.com>

Some of the test-vmstate.c test cases use a temporary file
while others use a memory buffer. To facilitate the future
removal of the qemu_bufopen() function, convert all the tests
to use a temporary file.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Message-Id: <1461751518-12128-4-git-send-email-berrange@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 tests/Makefile       |  2 +-
 tests/test-vmstate.c | 44 +++++++++++++-------------------------------
 2 files changed, 14 insertions(+), 32 deletions(-)

diff --git a/tests/Makefile b/tests/Makefile
index 1bbd1ca..b5cb75e 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -438,7 +438,7 @@ tests/test-qdev-global-props$(EXESUF): tests/test-qdev-global-props.o \
 	hw/core/fw-path-provider.o \
 	$(test-qapi-obj-y)
 tests/test-vmstate$(EXESUF): tests/test-vmstate.o \
-	migration/vmstate.o migration/qemu-file.o migration/qemu-file-buf.o \
+	migration/vmstate.o migration/qemu-file.o \
         migration/qemu-file-unix.o migration/qjson.o \
 	$(test-qom-obj-y)
 tests/test-timed-average$(EXESUF): tests/test-timed-average.o qemu-timer.o \
diff --git a/tests/test-vmstate.c b/tests/test-vmstate.c
index 713d444..f337cf6 100644
--- a/tests/test-vmstate.c
+++ b/tests/test-vmstate.c
@@ -44,11 +44,6 @@ void yield_until_fd_readable(int fd)
     select(fd + 1, &fds, NULL, NULL, NULL);
 }
 
-/*
- * Some tests use 'open_test_file' to work on a real fd, some use
- * an in memory file (QEMUSizedBuffer+qemu_bufopen); we could pick one
- * but this way we test both.
- */
 
 /* Duplicate temp_fd and seek to the beginning of the file */
 static QEMUFile *open_test_file(bool write)
@@ -61,20 +56,6 @@ static QEMUFile *open_test_file(bool write)
     return qemu_fdopen(fd, write ? "wb" : "rb");
 }
 
-/*
- * Check that the contents of the memory-buffered file f match
- * the given size/data.
- */
-static void check_mem_file(QEMUFile *f, void *data, size_t size)
-{
-    uint8_t *result = g_malloc(size);
-    const QEMUSizedBuffer *qsb = qemu_buf_get(f);
-    g_assert_cmpint(qsb_get_length(qsb), ==, size);
-    g_assert_cmpint(qsb_get_buffer(qsb, 0, size, result), ==, size);
-    g_assert_cmpint(memcmp(result, data, size), ==, 0);
-    g_free(result);
-}
-
 #define SUCCESS(val) \
     g_assert_cmpint((val), ==, 0)
 
@@ -392,7 +373,7 @@ static const VMStateDescription vmstate_skipping = {
 
 static void test_save_noskip(void)
 {
-    QEMUFile *fsave = qemu_bufopen("w", NULL);
+    QEMUFile *fsave = open_test_file(true);
     TestStruct obj = { .a = 1, .b = 2, .c = 3, .d = 4, .e = 5, .f = 6,
                        .skip_c_e = false };
     vmstate_save_state(fsave, &vmstate_skipping, &obj, NULL);
@@ -406,13 +387,14 @@ static void test_save_noskip(void)
         0, 0, 0, 5,             /* e */
         0, 0, 0, 0, 0, 0, 0, 6, /* f */
     };
-    check_mem_file(fsave, expected, sizeof(expected));
+
     qemu_fclose(fsave);
+    compare_vmstate(expected, sizeof(expected));
 }
 
 static void test_save_skip(void)
 {
-    QEMUFile *fsave = qemu_bufopen("w", NULL);
+    QEMUFile *fsave = open_test_file(true);
     TestStruct obj = { .a = 1, .b = 2, .c = 3, .d = 4, .e = 5, .f = 6,
                        .skip_c_e = true };
     vmstate_save_state(fsave, &vmstate_skipping, &obj, NULL);
@@ -424,13 +406,14 @@ static void test_save_skip(void)
         0, 0, 0, 0, 0, 0, 0, 4, /* d */
         0, 0, 0, 0, 0, 0, 0, 6, /* f */
     };
-    check_mem_file(fsave, expected, sizeof(expected));
 
     qemu_fclose(fsave);
+    compare_vmstate(expected, sizeof(expected));
 }
 
 static void test_load_noskip(void)
 {
+    QEMUFile *fsave = open_test_file(true);
     uint8_t buf[] = {
         0, 0, 0, 10,             /* a */
         0, 0, 0, 20,             /* b */
@@ -440,10 +423,10 @@ static void test_load_noskip(void)
         0, 0, 0, 0, 0, 0, 0, 60, /* f */
         QEMU_VM_EOF, /* just to ensure we won't get EOF reported prematurely */
     };
+    qemu_put_buffer(fsave, buf, sizeof(buf));
+    qemu_fclose(fsave);
 
-    QEMUSizedBuffer *qsb = qsb_create(buf, sizeof(buf));
-    g_assert(qsb);
-    QEMUFile *loading = qemu_bufopen("r", qsb);
+    QEMUFile *loading = open_test_file(false);
     TestStruct obj = { .skip_c_e = false };
     vmstate_load_state(loading, &vmstate_skipping, &obj, 2);
     g_assert(!qemu_file_get_error(loading));
@@ -454,11 +437,11 @@ static void test_load_noskip(void)
     g_assert_cmpint(obj.e, ==, 50);
     g_assert_cmpint(obj.f, ==, 60);
     qemu_fclose(loading);
-    qsb_free(qsb);
 }
 
 static void test_load_skip(void)
 {
+    QEMUFile *fsave = open_test_file(true);
     uint8_t buf[] = {
         0, 0, 0, 10,             /* a */
         0, 0, 0, 20,             /* b */
@@ -466,10 +449,10 @@ static void test_load_skip(void)
         0, 0, 0, 0, 0, 0, 0, 60, /* f */
         QEMU_VM_EOF, /* just to ensure we won't get EOF reported prematurely */
     };
+    qemu_put_buffer(fsave, buf, sizeof(buf));
+    qemu_fclose(fsave);
 
-    QEMUSizedBuffer *qsb = qsb_create(buf, sizeof(buf));
-    g_assert(qsb);
-    QEMUFile *loading = qemu_bufopen("r", qsb);
+    QEMUFile *loading = open_test_file(false);
     TestStruct obj = { .skip_c_e = true, .c = 300, .e = 500 };
     vmstate_load_state(loading, &vmstate_skipping, &obj, 2);
     g_assert(!qemu_file_get_error(loading));
@@ -480,7 +463,6 @@ static void test_load_skip(void)
     g_assert_cmpint(obj.e, ==, 500);
     g_assert_cmpint(obj.f, ==, 60);
     qemu_fclose(loading);
-    qsb_free(qsb);
 }
 
 int main(int argc, char **argv)
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PULL 04/28] migration: ensure qemu_fflush() always writes full data amount
  2016-05-26  6:11 [Qemu-devel] [PULL 00/28] migration: support for TLS Amit Shah
                   ` (2 preceding siblings ...)
  2016-05-26  6:11 ` [Qemu-devel] [PULL 03/28] migration: remove use of qemu_bufopen from vmstate tests Amit Shah
@ 2016-05-26  6:11 ` Amit Shah
  2016-05-26  6:11 ` [Qemu-devel] [PULL 05/28] migration: split migration hooks out of QEMUFileOps Amit Shah
                   ` (24 subsequent siblings)
  28 siblings, 0 replies; 36+ messages in thread
From: Amit Shah @ 2016-05-26  6:11 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Juan Quintela, Dr. David Alan Gilbert, Daniel P. Berrange,
	qemu list, Amit Shah

From: "Daniel P. Berrange" <berrange@redhat.com>

The QEMUFile writev_buffer / put_buffer functions are expected
to write out the full set of requested data, blocking until
complete. The qemu_fflush() caller does not expect to deal with
partial writes. Clarify the function comments and add a sanity
check to the code to catch mistaken implementations.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Message-Id: <1461751518-12128-5-git-send-email-berrange@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 include/migration/qemu-file.h |  6 ++++--
 migration/qemu-file.c         | 16 ++++++++++++----
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
index 3f6b4ed..5909ff0 100644
--- a/include/migration/qemu-file.h
+++ b/include/migration/qemu-file.h
@@ -28,7 +28,8 @@
 
 /* This function writes a chunk of data to a file at the given position.
  * The pos argument can be ignored if the file is only being used for
- * streaming.  The handler should try to write all of the data it can.
+ * streaming.  The handler must write all of the data or return a negative
+ * errno value.
  */
 typedef ssize_t (QEMUFilePutBufferFunc)(void *opaque, const uint8_t *buf,
                                         int64_t pos, size_t size);
@@ -54,7 +55,8 @@ typedef int (QEMUFileCloseFunc)(void *opaque);
 typedef int (QEMUFileGetFD)(void *opaque);
 
 /*
- * This function writes an iovec to file.
+ * This function writes an iovec to file. The handler must write all
+ * of the data or return a negative errno value.
  */
 typedef ssize_t (QEMUFileWritevBufferFunc)(void *opaque, struct iovec *iov,
                                            int iovcnt, int64_t pos);
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 6f4a129..656db4a 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -108,11 +108,13 @@ bool qemu_file_is_writable(QEMUFile *f)
  * Flushes QEMUFile buffer
  *
  * If there is writev_buffer QEMUFileOps it uses it otherwise uses
- * put_buffer ops.
+ * put_buffer ops. This will flush all pending data. If data was
+ * only partially flushed, it will set an error state.
  */
 void qemu_fflush(QEMUFile *f)
 {
     ssize_t ret = 0;
+    ssize_t expect = 0;
 
     if (!qemu_file_is_writable(f)) {
         return;
@@ -120,21 +122,27 @@ void qemu_fflush(QEMUFile *f)
 
     if (f->ops->writev_buffer) {
         if (f->iovcnt > 0) {
+            expect = iov_size(f->iov, f->iovcnt);
             ret = f->ops->writev_buffer(f->opaque, f->iov, f->iovcnt, f->pos);
         }
     } else {
         if (f->buf_index > 0) {
+            expect = f->buf_index;
             ret = f->ops->put_buffer(f->opaque, f->buf, f->pos, f->buf_index);
         }
     }
+
     if (ret >= 0) {
         f->pos += ret;
     }
+    /* We expect the QEMUFile write impl to send the full
+     * data set we requested, so sanity check that.
+     */
+    if (ret != expect) {
+        qemu_file_set_error(f, ret < 0 ? ret : -EIO);
+    }
     f->buf_index = 0;
     f->iovcnt = 0;
-    if (ret < 0) {
-        qemu_file_set_error(f, ret);
-    }
 }
 
 void ram_control_before_iterate(QEMUFile *f, uint64_t flags)
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PULL 05/28] migration: split migration hooks out of QEMUFileOps
  2016-05-26  6:11 [Qemu-devel] [PULL 00/28] migration: support for TLS Amit Shah
                   ` (3 preceding siblings ...)
  2016-05-26  6:11 ` [Qemu-devel] [PULL 04/28] migration: ensure qemu_fflush() always writes full data amount Amit Shah
@ 2016-05-26  6:11 ` Amit Shah
  2016-05-26  6:11 ` [Qemu-devel] [PULL 06/28] migration: introduce set_blocking function in QEMUFileOps Amit Shah
                   ` (23 subsequent siblings)
  28 siblings, 0 replies; 36+ messages in thread
From: Amit Shah @ 2016-05-26  6:11 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Juan Quintela, Dr. David Alan Gilbert, Daniel P. Berrange,
	qemu list, Amit Shah

From: "Daniel P. Berrange" <berrange@redhat.com>

The QEMUFileOps struct contains the I/O subsystem callbacks
and the migration stage hooks. Split the hooks out into a
separate QEMUFileHooks struct to make it easier to refactor
the I/O side of QEMUFile without affecting the hooks.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Message-Id: <1461751518-12128-6-git-send-email-berrange@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 include/migration/qemu-file.h  | 10 +++++++---
 migration/qemu-file-internal.h |  1 +
 migration/qemu-file.c          | 24 +++++++++++++++---------
 migration/rdma.c               |  8 ++++++++
 4 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
index 5909ff0..1934a64 100644
--- a/include/migration/qemu-file.h
+++ b/include/migration/qemu-file.h
@@ -108,13 +108,16 @@ typedef struct QEMUFileOps {
     QEMUFileCloseFunc *close;
     QEMUFileGetFD *get_fd;
     QEMUFileWritevBufferFunc *writev_buffer;
+    QEMURetPathFunc *get_return_path;
+    QEMUFileShutdownFunc *shut_down;
+} QEMUFileOps;
+
+typedef struct QEMUFileHooks {
     QEMURamHookFunc *before_ram_iterate;
     QEMURamHookFunc *after_ram_iterate;
     QEMURamHookFunc *hook_ram_load;
     QEMURamSaveFunc *save_page;
-    QEMURetPathFunc *get_return_path;
-    QEMUFileShutdownFunc *shut_down;
-} QEMUFileOps;
+} QEMUFileHooks;
 
 struct QEMUSizedBuffer {
     struct iovec *iov;
@@ -129,6 +132,7 @@ QEMUFile *qemu_fdopen(int fd, const char *mode);
 QEMUFile *qemu_fopen_socket(int fd, const char *mode);
 QEMUFile *qemu_popen_cmd(const char *command, const char *mode);
 QEMUFile *qemu_bufopen(const char *mode, QEMUSizedBuffer *input);
+void qemu_file_set_hooks(QEMUFile *f, const QEMUFileHooks *hooks);
 int qemu_get_fd(QEMUFile *f);
 int qemu_fclose(QEMUFile *f);
 int64_t qemu_ftell(QEMUFile *f);
diff --git a/migration/qemu-file-internal.h b/migration/qemu-file-internal.h
index d95e853..8fdfa95 100644
--- a/migration/qemu-file-internal.h
+++ b/migration/qemu-file-internal.h
@@ -33,6 +33,7 @@
 
 struct QEMUFile {
     const QEMUFileOps *ops;
+    const QEMUFileHooks *hooks;
     void *opaque;
 
     int64_t bytes_xfer;
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 656db4a..b480b72 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -80,6 +80,12 @@ QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops)
     return f;
 }
 
+
+void qemu_file_set_hooks(QEMUFile *f, const QEMUFileHooks *hooks)
+{
+    f->hooks = hooks;
+}
+
 /*
  * Get last error for stream f
  *
@@ -149,8 +155,8 @@ void ram_control_before_iterate(QEMUFile *f, uint64_t flags)
 {
     int ret = 0;
 
-    if (f->ops->before_ram_iterate) {
-        ret = f->ops->before_ram_iterate(f, f->opaque, flags, NULL);
+    if (f->hooks && f->hooks->before_ram_iterate) {
+        ret = f->hooks->before_ram_iterate(f, f->opaque, flags, NULL);
         if (ret < 0) {
             qemu_file_set_error(f, ret);
         }
@@ -161,8 +167,8 @@ void ram_control_after_iterate(QEMUFile *f, uint64_t flags)
 {
     int ret = 0;
 
-    if (f->ops->after_ram_iterate) {
-        ret = f->ops->after_ram_iterate(f, f->opaque, flags, NULL);
+    if (f->hooks && f->hooks->after_ram_iterate) {
+        ret = f->hooks->after_ram_iterate(f, f->opaque, flags, NULL);
         if (ret < 0) {
             qemu_file_set_error(f, ret);
         }
@@ -173,8 +179,8 @@ void ram_control_load_hook(QEMUFile *f, uint64_t flags, void *data)
 {
     int ret = -EINVAL;
 
-    if (f->ops->hook_ram_load) {
-        ret = f->ops->hook_ram_load(f, f->opaque, flags, data);
+    if (f->hooks && f->hooks->hook_ram_load) {
+        ret = f->hooks->hook_ram_load(f, f->opaque, flags, data);
         if (ret < 0) {
             qemu_file_set_error(f, ret);
         }
@@ -193,9 +199,9 @@ size_t ram_control_save_page(QEMUFile *f, ram_addr_t block_offset,
                              ram_addr_t offset, size_t size,
                              uint64_t *bytes_sent)
 {
-    if (f->ops->save_page) {
-        int ret = f->ops->save_page(f, f->opaque, block_offset,
-                                    offset, size, bytes_sent);
+    if (f->hooks && f->hooks->save_page) {
+        int ret = f->hooks->save_page(f, f->opaque, block_offset,
+                                      offset, size, bytes_sent);
 
         if (ret != RAM_SAVE_CONTROL_DELAYED) {
             if (bytes_sent && *bytes_sent > 0) {
diff --git a/migration/rdma.c b/migration/rdma.c
index f6a9992..0d067a1 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -3380,12 +3380,18 @@ static const QEMUFileOps rdma_read_ops = {
     .get_buffer    = qemu_rdma_get_buffer,
     .get_fd        = qemu_rdma_get_fd,
     .close         = qemu_rdma_close,
+};
+
+static const QEMUFileHooks rdma_read_hooks = {
     .hook_ram_load = rdma_load_hook,
 };
 
 static const QEMUFileOps rdma_write_ops = {
     .put_buffer         = qemu_rdma_put_buffer,
     .close              = qemu_rdma_close,
+};
+
+static const QEMUFileHooks rdma_write_hooks = {
     .before_ram_iterate = qemu_rdma_registration_start,
     .after_ram_iterate  = qemu_rdma_registration_stop,
     .save_page          = qemu_rdma_save_page,
@@ -3404,8 +3410,10 @@ static void *qemu_fopen_rdma(RDMAContext *rdma, const char *mode)
 
     if (mode[0] == 'w') {
         r->file = qemu_fopen_ops(r, &rdma_write_ops);
+        qemu_file_set_hooks(r->file, &rdma_write_hooks);
     } else {
         r->file = qemu_fopen_ops(r, &rdma_read_ops);
+        qemu_file_set_hooks(r->file, &rdma_read_hooks);
     }
 
     return r->file;
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PULL 06/28] migration: introduce set_blocking function in QEMUFileOps
  2016-05-26  6:11 [Qemu-devel] [PULL 00/28] migration: support for TLS Amit Shah
                   ` (4 preceding siblings ...)
  2016-05-26  6:11 ` [Qemu-devel] [PULL 05/28] migration: split migration hooks out of QEMUFileOps Amit Shah
@ 2016-05-26  6:11 ` Amit Shah
  2016-05-26  6:11 ` [Qemu-devel] [PULL 07/28] migration: force QEMUFile to blocking mode for outgoing migration Amit Shah
                   ` (22 subsequent siblings)
  28 siblings, 0 replies; 36+ messages in thread
From: Amit Shah @ 2016-05-26  6:11 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Juan Quintela, Dr. David Alan Gilbert, Daniel P. Berrange,
	qemu list, Amit Shah

From: "Daniel P. Berrange" <berrange@redhat.com>

Remove the assumption that every QEMUFile implementation has
a file descriptor available by introducing a new function
in QEMUFileOps to change the blocking state of a QEMUFile.

If not set, it will fallback to the original code using
the get_fd method.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Message-Id: <1461751518-12128-7-git-send-email-berrange@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 include/migration/qemu-file.h |  5 +++++
 migration/migration.c         |  4 +---
 migration/qemu-file.c         | 10 +++++++---
 3 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
index 1934a64..2dea81f 100644
--- a/include/migration/qemu-file.h
+++ b/include/migration/qemu-file.h
@@ -54,6 +54,10 @@ typedef int (QEMUFileCloseFunc)(void *opaque);
  */
 typedef int (QEMUFileGetFD)(void *opaque);
 
+/* Called to change the blocking mode of the file
+ */
+typedef int (QEMUFileSetBlocking)(void *opaque, bool enabled);
+
 /*
  * This function writes an iovec to file. The handler must write all
  * of the data or return a negative errno value.
@@ -107,6 +111,7 @@ typedef struct QEMUFileOps {
     QEMUFileGetBufferFunc *get_buffer;
     QEMUFileCloseFunc *close;
     QEMUFileGetFD *get_fd;
+    QEMUFileSetBlocking *set_blocking;
     QEMUFileWritevBufferFunc *writev_buffer;
     QEMURetPathFunc *get_return_path;
     QEMUFileShutdownFunc *shut_down;
diff --git a/migration/migration.c b/migration/migration.c
index f5327e8..ac7790f 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -422,11 +422,9 @@ static void process_incoming_migration_co(void *opaque)
 void process_incoming_migration(QEMUFile *f)
 {
     Coroutine *co = qemu_coroutine_create(process_incoming_migration_co);
-    int fd = qemu_get_fd(f);
 
-    assert(fd != -1);
     migrate_decompress_threads_create();
-    qemu_set_nonblock(fd);
+    qemu_file_set_blocking(f, false);
     qemu_coroutine_enter(co, f);
 }
 
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index b480b72..2b25dec 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -684,9 +684,13 @@ size_t qemu_get_counted_string(QEMUFile *f, char buf[256])
  */
 void qemu_file_set_blocking(QEMUFile *f, bool block)
 {
-    if (block) {
-        qemu_set_block(qemu_get_fd(f));
+    if (f->ops->set_blocking) {
+        f->ops->set_blocking(f->opaque, block);
     } else {
-        qemu_set_nonblock(qemu_get_fd(f));
+        if (block) {
+            qemu_set_block(qemu_get_fd(f));
+        } else {
+            qemu_set_nonblock(qemu_get_fd(f));
+        }
     }
 }
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PULL 07/28] migration: force QEMUFile to blocking mode for outgoing migration
  2016-05-26  6:11 [Qemu-devel] [PULL 00/28] migration: support for TLS Amit Shah
                   ` (5 preceding siblings ...)
  2016-05-26  6:11 ` [Qemu-devel] [PULL 06/28] migration: introduce set_blocking function in QEMUFileOps Amit Shah
@ 2016-05-26  6:11 ` Amit Shah
  2016-05-26  6:11 ` [Qemu-devel] [PULL 08/28] migration: introduce a new QEMUFile impl based on QIOChannel Amit Shah
                   ` (21 subsequent siblings)
  28 siblings, 0 replies; 36+ messages in thread
From: Amit Shah @ 2016-05-26  6:11 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Juan Quintela, Dr. David Alan Gilbert, Daniel P. Berrange,
	qemu list, Amit Shah

From: "Daniel P. Berrange" <berrange@redhat.com>

Instead of relying on the default QEMUFile I/O blocking flag
state, explicitly turn on blocking I/O for outgoing migration
since it takes place in a background thread.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Message-Id: <1461751518-12128-8-git-send-email-berrange@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 migration/migration.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/migration/migration.c b/migration/migration.c
index ac7790f..c8d10ee 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1791,6 +1791,7 @@ void migrate_fd_connect(MigrationState *s)
     s->expected_downtime = max_downtime/1000000;
     s->cleanup_bh = qemu_bh_new(migrate_fd_cleanup, s);
 
+    qemu_file_set_blocking(s->to_dst_file, true);
     qemu_file_set_rate_limit(s->to_dst_file,
                              s->bandwidth_limit / XFER_LIMIT_RATIO);
 
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PULL 08/28] migration: introduce a new QEMUFile impl based on QIOChannel
  2016-05-26  6:11 [Qemu-devel] [PULL 00/28] migration: support for TLS Amit Shah
                   ` (6 preceding siblings ...)
  2016-05-26  6:11 ` [Qemu-devel] [PULL 07/28] migration: force QEMUFile to blocking mode for outgoing migration Amit Shah
@ 2016-05-26  6:11 ` Amit Shah
  2016-05-26  6:12 ` [Qemu-devel] [PULL 09/28] migration: add helpers for creating QEMUFile from a QIOChannel Amit Shah
                   ` (20 subsequent siblings)
  28 siblings, 0 replies; 36+ messages in thread
From: Amit Shah @ 2016-05-26  6:11 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Juan Quintela, Dr. David Alan Gilbert, Daniel P. Berrange,
	qemu list, Amit Shah

From: "Daniel P. Berrange" <berrange@redhat.com>

Introduce a new QEMUFile implementation that is based on
the QIOChannel objects. This impl is different from existing
impls in that there is no file descriptor that can be made
available, as some channels may be based on higher level
protocols such as TLS.

Although the QIOChannel based implementation can trivially
provide a bi-directional stream, initially we have separate
functions for opening input & output directions to fit with
the expectation of the current QEMUFile interface.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-Id: <1461751518-12128-9-git-send-email-berrange@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 include/migration/qemu-file.h |   4 +
 migration/Makefile.objs       |   1 +
 migration/qemu-file-channel.c | 180 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 185 insertions(+)
 create mode 100644 migration/qemu-file-channel.c

diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
index 2dea81f..0329ccc 100644
--- a/include/migration/qemu-file.h
+++ b/include/migration/qemu-file.h
@@ -23,7 +23,9 @@
  */
 #ifndef QEMU_FILE_H
 #define QEMU_FILE_H 1
+#include "qemu-common.h"
 #include "exec/cpu-common.h"
+#include "io/channel.h"
 
 
 /* This function writes a chunk of data to a file at the given position.
@@ -135,6 +137,8 @@ QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops);
 QEMUFile *qemu_fopen(const char *filename, const char *mode);
 QEMUFile *qemu_fdopen(int fd, const char *mode);
 QEMUFile *qemu_fopen_socket(int fd, const char *mode);
+QEMUFile *qemu_fopen_channel_input(QIOChannel *ioc);
+QEMUFile *qemu_fopen_channel_output(QIOChannel *ioc);
 QEMUFile *qemu_popen_cmd(const char *command, const char *mode);
 QEMUFile *qemu_bufopen(const char *mode, QEMUSizedBuffer *input);
 void qemu_file_set_hooks(QEMUFile *f, const QEMUFileHooks *hooks);
diff --git a/migration/Makefile.objs b/migration/Makefile.objs
index d25ff48..facf251 100644
--- a/migration/Makefile.objs
+++ b/migration/Makefile.objs
@@ -1,6 +1,7 @@
 common-obj-y += migration.o tcp.o
 common-obj-y += vmstate.o
 common-obj-y += qemu-file.o qemu-file-buf.o qemu-file-unix.o qemu-file-stdio.o
+common-obj-y += qemu-file-channel.o
 common-obj-y += xbzrle.o postcopy-ram.o
 common-obj-y += qjson.o
 
diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
new file mode 100644
index 0000000..45c13f1
--- /dev/null
+++ b/migration/qemu-file-channel.c
@@ -0,0 +1,180 @@
+/*
+ * QEMUFile backend for QIOChannel objects
+ *
+ * Copyright (c) 2015-2016 Red Hat, Inc
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "migration/qemu-file.h"
+#include "io/channel-socket.h"
+#include "qemu/iov.h"
+
+
+static ssize_t channel_writev_buffer(void *opaque,
+                                     struct iovec *iov,
+                                     int iovcnt,
+                                     int64_t pos)
+{
+    QIOChannel *ioc = QIO_CHANNEL(opaque);
+    ssize_t done = 0;
+    struct iovec *local_iov = g_new(struct iovec, iovcnt);
+    struct iovec *local_iov_head = local_iov;
+    unsigned int nlocal_iov = iovcnt;
+
+    nlocal_iov = iov_copy(local_iov, nlocal_iov,
+                          iov, iovcnt,
+                          0, iov_size(iov, iovcnt));
+
+    while (nlocal_iov > 0) {
+        ssize_t len;
+        len = qio_channel_writev(ioc, local_iov, nlocal_iov, NULL);
+        if (len == QIO_CHANNEL_ERR_BLOCK) {
+            qio_channel_wait(ioc, G_IO_OUT);
+            continue;
+        }
+        if (len < 0) {
+            /* XXX handle Error objects */
+            done = -EIO;
+            goto cleanup;
+        }
+
+        iov_discard_front(&local_iov, &nlocal_iov, len);
+        done += len;
+    }
+
+ cleanup:
+    g_free(local_iov_head);
+    return done;
+}
+
+
+static ssize_t channel_get_buffer(void *opaque,
+                                  uint8_t *buf,
+                                  int64_t pos,
+                                  size_t size)
+{
+    QIOChannel *ioc = QIO_CHANNEL(opaque);
+    ssize_t ret;
+
+    do {
+        ret = qio_channel_read(ioc, (char *)buf, size, NULL);
+        if (ret < 0) {
+            if (ret == QIO_CHANNEL_ERR_BLOCK) {
+                qio_channel_yield(ioc, G_IO_IN);
+            } else {
+                /* XXX handle Error * object */
+                return -EIO;
+            }
+        }
+    } while (ret == QIO_CHANNEL_ERR_BLOCK);
+
+    return ret;
+}
+
+
+static int channel_close(void *opaque)
+{
+    QIOChannel *ioc = QIO_CHANNEL(opaque);
+    qio_channel_close(ioc, NULL);
+    object_unref(OBJECT(ioc));
+    return 0;
+}
+
+
+static int channel_shutdown(void *opaque,
+                            bool rd,
+                            bool wr)
+{
+    QIOChannel *ioc = QIO_CHANNEL(opaque);
+
+    if (qio_channel_has_feature(ioc,
+                                QIO_CHANNEL_FEATURE_SHUTDOWN)) {
+        QIOChannelShutdown mode;
+        if (rd && wr) {
+            mode = QIO_CHANNEL_SHUTDOWN_BOTH;
+        } else if (rd) {
+            mode = QIO_CHANNEL_SHUTDOWN_READ;
+        } else {
+            mode = QIO_CHANNEL_SHUTDOWN_WRITE;
+        }
+        if (qio_channel_shutdown(ioc, mode, NULL) < 0) {
+            /* XXX handler Error * object */
+            return -EIO;
+        }
+    }
+    return 0;
+}
+
+
+static int channel_set_blocking(void *opaque,
+                                bool enabled)
+{
+    QIOChannel *ioc = QIO_CHANNEL(opaque);
+
+    if (qio_channel_set_blocking(ioc, enabled, NULL) < 0) {
+        return -1;
+    }
+    return 0;
+}
+
+static QEMUFile *channel_get_input_return_path(void *opaque)
+{
+    QIOChannel *ioc = QIO_CHANNEL(opaque);
+
+    return qemu_fopen_channel_output(ioc);
+}
+
+static QEMUFile *channel_get_output_return_path(void *opaque)
+{
+    QIOChannel *ioc = QIO_CHANNEL(opaque);
+
+    return qemu_fopen_channel_input(ioc);
+}
+
+static const QEMUFileOps channel_input_ops = {
+    .get_buffer = channel_get_buffer,
+    .close = channel_close,
+    .shut_down = channel_shutdown,
+    .set_blocking = channel_set_blocking,
+    .get_return_path = channel_get_input_return_path,
+};
+
+
+static const QEMUFileOps channel_output_ops = {
+    .writev_buffer = channel_writev_buffer,
+    .close = channel_close,
+    .shut_down = channel_shutdown,
+    .set_blocking = channel_set_blocking,
+    .get_return_path = channel_get_output_return_path,
+};
+
+
+QEMUFile *qemu_fopen_channel_input(QIOChannel *ioc)
+{
+    object_ref(OBJECT(ioc));
+    return qemu_fopen_ops(ioc, &channel_input_ops);
+}
+
+QEMUFile *qemu_fopen_channel_output(QIOChannel *ioc)
+{
+    object_ref(OBJECT(ioc));
+    return qemu_fopen_ops(ioc, &channel_output_ops);
+}
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PULL 09/28] migration: add helpers for creating QEMUFile from a QIOChannel
  2016-05-26  6:11 [Qemu-devel] [PULL 00/28] migration: support for TLS Amit Shah
                   ` (7 preceding siblings ...)
  2016-05-26  6:11 ` [Qemu-devel] [PULL 08/28] migration: introduce a new QEMUFile impl based on QIOChannel Amit Shah
@ 2016-05-26  6:12 ` Amit Shah
  2016-05-26  6:12 ` [Qemu-devel] [PULL 10/28] migration: add reporting of errors for outgoing migration Amit Shah
                   ` (19 subsequent siblings)
  28 siblings, 0 replies; 36+ messages in thread
From: Amit Shah @ 2016-05-26  6:12 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Juan Quintela, Dr. David Alan Gilbert, Daniel P. Berrange,
	qemu list, Amit Shah

From: "Daniel P. Berrange" <berrange@redhat.com>

Currently creating a QEMUFile instance from a QIOChannel is
quite simple only requiring a single call to
qemu_fopen_channel_input or  qemu_fopen_channel_output
depending on the end of migration connection.

When QEMU gains TLS support, however, there will need to be
a TLS negotiation done inbetween creation of the QIOChannel
and creation of the final QEMUFile. Introduce some helper
methods that will encapsulate this logic, isolating the
migration protocol drivers from knowledge about TLS.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Acked-by: Juan Quintela <quintela@redhat.com>
Message-Id: <1461751518-12128-10-git-send-email-berrange@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 include/migration/migration.h |  6 ++++++
 migration/migration.c         | 21 +++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/include/migration/migration.h b/include/migration/migration.h
index 9e36a97..87ad577 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -179,6 +179,12 @@ void process_incoming_migration(QEMUFile *f);
 
 void qemu_start_incoming_migration(const char *uri, Error **errp);
 
+void migration_set_incoming_channel(MigrationState *s,
+                                    QIOChannel *ioc);
+
+void migration_set_outgoing_channel(MigrationState *s,
+                                    QIOChannel *ioc);
+
 uint64_t migrate_max_downtime(void);
 
 void exec_start_incoming_migration(const char *host_port, Error **errp);
diff --git a/migration/migration.c b/migration/migration.c
index c8d10ee..c960e16 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -428,6 +428,27 @@ void process_incoming_migration(QEMUFile *f)
     qemu_coroutine_enter(co, f);
 }
 
+
+void migration_set_incoming_channel(MigrationState *s,
+                                    QIOChannel *ioc)
+{
+    QEMUFile *f = qemu_fopen_channel_input(ioc);
+
+    process_incoming_migration(f);
+}
+
+
+void migration_set_outgoing_channel(MigrationState *s,
+                                    QIOChannel *ioc)
+{
+    QEMUFile *f = qemu_fopen_channel_output(ioc);
+
+    s->to_dst_file = f;
+
+    migrate_fd_connect(s);
+}
+
+
 /*
  * Send a message on the return channel back to the source
  * of the migration.
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PULL 10/28] migration: add reporting of errors for outgoing migration
  2016-05-26  6:11 [Qemu-devel] [PULL 00/28] migration: support for TLS Amit Shah
                   ` (8 preceding siblings ...)
  2016-05-26  6:12 ` [Qemu-devel] [PULL 09/28] migration: add helpers for creating QEMUFile from a QIOChannel Amit Shah
@ 2016-05-26  6:12 ` Amit Shah
  2016-05-26 15:00   ` Eric Blake
  2016-06-06  8:38   ` Paolo Bonzini
  2016-05-26  6:12 ` [Qemu-devel] [PULL 11/28] migration: convert post-copy to use QIOChannelBuffer Amit Shah
                   ` (18 subsequent siblings)
  28 siblings, 2 replies; 36+ messages in thread
From: Amit Shah @ 2016-05-26  6:12 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Juan Quintela, Dr. David Alan Gilbert, Daniel P. Berrange,
	qemu list, Amit Shah

From: "Daniel P. Berrange" <berrange@redhat.com>

Currently if an application initiates an outgoing migration,
it may or may not, get an error reported back on failure. If
the error occurs synchronously to the 'migrate' command
execution, the client app will see the error message. This
is the case for DNS lookup failures. If the error occurs
asynchronously to the monitor command though, the error
will be thrown away and the client left guessing about
what went wrong. This is the case for failure to connect
to the TCP server (eg due to wrong port, or firewall
rules, or other similar errors).

In the future we'll be adding more scope for errors to
happen asynchronously with the TLS protocol handshake.
TLS errors are hard to diagnose even when they are well
reported, so discarding errors entirely will make it
impossible to debug TLS connection problems.

Management apps which do migration are already using
'query-migrate' / 'info migrate' to check up on progress
of background migration operations and to see their end
status. This is a fine place to also include the error
message when things go wrong.

This patch thus adds an 'error-desc' field to the
MigrationInfo struct, which will be populated when
the 'status' is set to 'failed':

(qemu) migrate -d tcp:localhost:9001
(qemu) info migrate
capabilities: xbzrle: off rdma-pin-all: off auto-converge: off zero-blocks: off compress: off events: off x-postcopy-ram: off
Migration status: failed (Error connecting to socket: Connection refused)
total time: 0 milliseconds

In the HMP, when doing non-detached migration, it is
also possible to display this error message directly
to the app.

(qemu) migrate tcp:localhost:9001
Error connecting to socket: Connection refused

Or with QMP

  {
    "execute": "query-migrate",
    "arguments": {}
  }
  {
    "return": {
      "status": "failed",
      "error-desc": "address resolution failed for myhost:9000: No address associated with hostname"
    }
  }

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Message-Id: <1461751518-12128-11-git-send-email-berrange@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hmp.c                         | 13 ++++++++++++-
 include/migration/migration.h |  5 ++++-
 include/qapi/error.h          |  2 +-
 migration/migration.c         | 15 ++++++++++++---
 migration/rdma.c              | 10 +++-------
 migration/tcp.c               |  2 +-
 migration/unix.c              |  2 +-
 qapi-schema.json              |  7 ++++++-
 trace-events                  |  2 +-
 util/error.c                  |  2 +-
 10 files changed, 42 insertions(+), 18 deletions(-)

diff --git a/hmp.c b/hmp.c
index 9f9bcf9..a464ca9 100644
--- a/hmp.c
+++ b/hmp.c
@@ -35,6 +35,7 @@
 #include "block/qapi.h"
 #include "qemu-io.h"
 #include "qemu/cutils.h"
+#include "qemu/error-report.h"
 
 #ifdef CONFIG_SPICE
 #include <spice/enums.h>
@@ -168,8 +169,15 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
     }
 
     if (info->has_status) {
-        monitor_printf(mon, "Migration status: %s\n",
+        monitor_printf(mon, "Migration status: %s",
                        MigrationStatus_lookup[info->status]);
+        if (info->status == MIGRATION_STATUS_FAILED &&
+            info->has_error_desc) {
+            monitor_printf(mon, " (%s)\n", info->error_desc);
+        } else {
+            monitor_printf(mon, "\n");
+        }
+
         monitor_printf(mon, "total time: %" PRIu64 " milliseconds\n",
                        info->total_time);
         if (info->has_expected_downtime) {
@@ -1533,6 +1541,9 @@ static void hmp_migrate_status_cb(void *opaque)
         if (status->is_block_migration) {
             monitor_printf(status->mon, "\n");
         }
+        if (info->has_error_desc) {
+            error_report("%s", info->error_desc);
+        }
         monitor_resume(status->mon);
         timer_del(status->timer);
         g_free(status);
diff --git a/include/migration/migration.h b/include/migration/migration.h
index 87ad577..d24c6ef 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -171,6 +171,9 @@ struct MigrationState
     QSIMPLEQ_HEAD(src_page_requests, MigrationSrcPageRequest) src_page_requests;
     /* The RAMBlock used in the last src_page_request */
     RAMBlock *last_req_rb;
+
+    /* The last error that occurred */
+    Error *error;
 };
 
 void migrate_set_state(int *state, int old_state, int new_state);
@@ -207,7 +210,7 @@ void rdma_start_outgoing_migration(void *opaque, const char *host_port, Error **
 
 void rdma_start_incoming_migration(const char *host_port, Error **errp);
 
-void migrate_fd_error(MigrationState *s);
+void migrate_fd_error(MigrationState *s, const Error *error);
 
 void migrate_fd_connect(MigrationState *s);
 
diff --git a/include/qapi/error.h b/include/qapi/error.h
index 11be232..0576659 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -134,7 +134,7 @@ typedef enum ErrorClass {
 /*
  * Get @err's human-readable error message.
  */
-const char *error_get_pretty(Error *err);
+const char *error_get_pretty(const Error *err);
 
 /*
  * Get @err's error class.
diff --git a/migration/migration.c b/migration/migration.c
index c960e16..1420ccc 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -691,6 +691,10 @@ MigrationInfo *qmp_query_migrate(Error **errp)
         break;
     case MIGRATION_STATUS_FAILED:
         info->has_status = true;
+        if (s->error) {
+            info->has_error_desc = true;
+            info->error_desc = g_strdup(error_get_pretty(s->error));
+        }
         break;
     case MIGRATION_STATUS_CANCELLED:
         info->has_status = true;
@@ -863,12 +867,15 @@ static void migrate_fd_cleanup(void *opaque)
     notifier_list_notify(&migration_state_notifiers, s);
 }
 
-void migrate_fd_error(MigrationState *s)
+void migrate_fd_error(MigrationState *s, const Error *error)
 {
-    trace_migrate_fd_error();
+    trace_migrate_fd_error(error ? error_get_pretty(error) : "");
     assert(s->to_dst_file == NULL);
     migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
                       MIGRATION_STATUS_FAILED);
+    if (!s->error) {
+        s->error = error_copy(error);
+    }
     notifier_list_notify(&migration_state_notifiers, s);
 }
 
@@ -967,6 +974,8 @@ MigrationState *migrate_init(const MigrationParams *params)
     s->postcopy_after_devices = false;
     s->migration_thread_running = false;
     s->last_req_rb = NULL;
+    error_free(s->error);
+    s->error = NULL;
 
     migrate_set_state(&s->state, MIGRATION_STATUS_NONE, MIGRATION_STATUS_SETUP);
 
@@ -1076,7 +1085,7 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
     }
 
     if (local_err) {
-        migrate_fd_error(s);
+        migrate_fd_error(s, local_err);
         error_propagate(errp, local_err);
         return;
     }
diff --git a/migration/rdma.c b/migration/rdma.c
index 0d067a1..f8578b9 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -3489,16 +3489,14 @@ void rdma_start_outgoing_migration(void *opaque,
                             const char *host_port, Error **errp)
 {
     MigrationState *s = opaque;
-    Error *local_err = NULL, **temp = &local_err;
-    RDMAContext *rdma = qemu_rdma_data_init(host_port, &local_err);
+    RDMAContext *rdma = qemu_rdma_data_init(host_port, errp);
     int ret = 0;
 
     if (rdma == NULL) {
-        ERROR(temp, "Failed to initialize RDMA data structures! %d", ret);
         goto err;
     }
 
-    ret = qemu_rdma_source_init(rdma, &local_err,
+    ret = qemu_rdma_source_init(rdma, errp,
         s->enabled_capabilities[MIGRATION_CAPABILITY_RDMA_PIN_ALL]);
 
     if (ret) {
@@ -3506,7 +3504,7 @@ void rdma_start_outgoing_migration(void *opaque,
     }
 
     trace_rdma_start_outgoing_migration_after_rdma_source_init();
-    ret = qemu_rdma_connect(rdma, &local_err);
+    ret = qemu_rdma_connect(rdma, errp);
 
     if (ret) {
         goto err;
@@ -3518,7 +3516,5 @@ void rdma_start_outgoing_migration(void *opaque,
     migrate_fd_connect(s);
     return;
 err:
-    error_propagate(errp, local_err);
     g_free(rdma);
-    migrate_fd_error(s);
 }
diff --git a/migration/tcp.c b/migration/tcp.c
index e1fa7f8..d0e0db9 100644
--- a/migration/tcp.c
+++ b/migration/tcp.c
@@ -40,7 +40,7 @@ static void tcp_wait_for_connect(int fd, Error *err, void *opaque)
     if (fd < 0) {
         DPRINTF("migrate connect error: %s\n", error_get_pretty(err));
         s->to_dst_file = NULL;
-        migrate_fd_error(s);
+        migrate_fd_error(s, err);
     } else {
         DPRINTF("migrate connect success\n");
         s->to_dst_file = qemu_fopen_socket(fd, "wb");
diff --git a/migration/unix.c b/migration/unix.c
index d9aac36..b3537fd 100644
--- a/migration/unix.c
+++ b/migration/unix.c
@@ -40,7 +40,7 @@ static void unix_wait_for_connect(int fd, Error *err, void *opaque)
     if (fd < 0) {
         DPRINTF("migrate connect error: %s\n", error_get_pretty(err));
         s->to_dst_file = NULL;
-        migrate_fd_error(s);
+        migrate_fd_error(s, err);
     } else {
         DPRINTF("migrate connect success\n");
         s->to_dst_file = qemu_fopen_socket(fd, "wb");
diff --git a/qapi-schema.json b/qapi-schema.json
index 9a322d1..e8c0353 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -484,6 +484,10 @@
 #        throttled during auto-converge. This is only present when auto-converge
 #        has started throttling guest cpus. (Since 2.7)
 #
+# @error-desc: #optional the human readable error description string, when
+#              @status is 'failed'. Clients should not attempt to parse the
+#              error strings. (Since 2.6)
+#
 # Since: 0.14.0
 ##
 { 'struct': 'MigrationInfo',
@@ -494,7 +498,8 @@
            '*expected-downtime': 'int',
            '*downtime': 'int',
            '*setup-time': 'int',
-           '*cpu-throttle-percentage': 'int'} }
+           '*cpu-throttle-percentage': 'int',
+           '*error-desc': 'str'} }
 
 ##
 # @query-migrate
diff --git a/trace-events b/trace-events
index b53c354..1ef4a9a 100644
--- a/trace-events
+++ b/trace-events
@@ -1481,7 +1481,7 @@ await_return_path_close_on_source_close(void) ""
 await_return_path_close_on_source_joining(void) ""
 migrate_set_state(int new_state) "new state %d"
 migrate_fd_cleanup(void) ""
-migrate_fd_error(void) ""
+migrate_fd_error(const char *error_desc) "error=%s"
 migrate_fd_cancel(void) ""
 migrate_handle_rp_req_pages(const char *rbname, size_t start, size_t len) "in %s at %zx len %zx"
 migrate_pending(uint64_t size, uint64_t max, uint64_t post, uint64_t nonpost) "pending size %" PRIu64 " max %" PRIu64 " (post=%" PRIu64 " nonpost=%" PRIu64 ")"
diff --git a/util/error.c b/util/error.c
index cae2511..9c40b1f 100644
--- a/util/error.c
+++ b/util/error.c
@@ -217,7 +217,7 @@ ErrorClass error_get_class(const Error *err)
     return err->err_class;
 }
 
-const char *error_get_pretty(Error *err)
+const char *error_get_pretty(const Error *err)
 {
     return err->msg;
 }
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PULL 11/28] migration: convert post-copy to use QIOChannelBuffer
  2016-05-26  6:11 [Qemu-devel] [PULL 00/28] migration: support for TLS Amit Shah
                   ` (9 preceding siblings ...)
  2016-05-26  6:12 ` [Qemu-devel] [PULL 10/28] migration: add reporting of errors for outgoing migration Amit Shah
@ 2016-05-26  6:12 ` Amit Shah
  2016-05-26  6:12 ` [Qemu-devel] [PULL 12/28] migration: convert unix socket protocol to use QIOChannel Amit Shah
                   ` (17 subsequent siblings)
  28 siblings, 0 replies; 36+ messages in thread
From: Amit Shah @ 2016-05-26  6:12 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Juan Quintela, Dr. David Alan Gilbert, Daniel P. Berrange,
	qemu list, Amit Shah

From: "Daniel P. Berrange" <berrange@redhat.com>

The post-copy code does some I/O to/from an intermediate
in-memory buffer rather than direct to the underlying
I/O channel. Switch this code to use QIOChannelBuffer
instead of QEMUSizedBuffer.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-Id: <1461751518-12128-12-git-send-email-berrange@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 docs/migration.txt      |  4 ++--
 include/sysemu/sysemu.h |  2 +-
 migration/migration.c   | 15 +++++++--------
 migration/savevm.c      | 47 ++++++++++++++++-------------------------------
 4 files changed, 26 insertions(+), 42 deletions(-)

diff --git a/docs/migration.txt b/docs/migration.txt
index 90209ab..6503c17 100644
--- a/docs/migration.txt
+++ b/docs/migration.txt
@@ -403,8 +403,8 @@ listen thread:                     --- page -- page -- page -- page -- page --
 
 On receipt of CMD_PACKAGED (1)
    All the data associated with the package - the ( ... ) section in the
-diagram - is read into memory (into a QEMUSizedBuffer), and the main thread
-recurses into qemu_loadvm_state_main to process the contents of the package (2)
+diagram - is read into memory, and the main thread recurses into
+qemu_loadvm_state_main to process the contents of the package (2)
 which contains commands (3,6) and devices (4...)
 
 On receipt of 'postcopy listen' - 3 -(i.e. the 1st command in the package)
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 618169c..9428141 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -119,7 +119,7 @@ void qemu_savevm_command_send(QEMUFile *f, enum qemu_vm_cmd command,
                               uint16_t len, uint8_t *data);
 void qemu_savevm_send_ping(QEMUFile *f, uint32_t value);
 void qemu_savevm_send_open_return_path(QEMUFile *f);
-int qemu_savevm_send_packaged(QEMUFile *f, const QEMUSizedBuffer *qsb);
+int qemu_savevm_send_packaged(QEMUFile *f, const uint8_t *buf, size_t len);
 void qemu_savevm_send_postcopy_advise(QEMUFile *f);
 void qemu_savevm_send_postcopy_listen(QEMUFile *f);
 void qemu_savevm_send_postcopy_run(QEMUFile *f);
diff --git a/migration/migration.c b/migration/migration.c
index 1420ccc..1ab9535 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -34,6 +34,7 @@
 #include "qom/cpu.h"
 #include "exec/memory.h"
 #include "exec/address-spaces.h"
+#include "io/channel-buffer.h"
 
 #define MAX_THROTTLE  (32 << 20)      /* Migration transfer speed throttling */
 
@@ -1457,7 +1458,8 @@ static int await_return_path_close_on_source(MigrationState *ms)
 static int postcopy_start(MigrationState *ms, bool *old_vm_running)
 {
     int ret;
-    const QEMUSizedBuffer *qsb;
+    QIOChannelBuffer *bioc;
+    QEMUFile *fb;
     int64_t time_at_stop = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
     migrate_set_state(&ms->state, MIGRATION_STATUS_ACTIVE,
                       MIGRATION_STATUS_POSTCOPY_ACTIVE);
@@ -1516,11 +1518,9 @@ static int postcopy_start(MigrationState *ms, bool *old_vm_running)
      * So we wrap the device state up in a package with a length at the start;
      * to do this we use a qemu_buf to hold the whole of the device state.
      */
-    QEMUFile *fb = qemu_bufopen("w", NULL);
-    if (!fb) {
-        error_report("Failed to create buffered file");
-        goto fail;
-    }
+    bioc = qio_channel_buffer_new(4096);
+    fb = qemu_fopen_channel_output(QIO_CHANNEL(bioc));
+    object_unref(OBJECT(bioc));
 
     /*
      * Make sure the receiver can get incoming pages before we send the rest
@@ -1534,10 +1534,9 @@ static int postcopy_start(MigrationState *ms, bool *old_vm_running)
     qemu_savevm_send_postcopy_run(fb);
 
     /* <><> end of stuff going into the package */
-    qsb = qemu_buf_get(fb);
 
     /* Now send that blob */
-    if (qemu_savevm_send_packaged(ms->to_dst_file, qsb)) {
+    if (qemu_savevm_send_packaged(ms->to_dst_file, bioc->data, bioc->usage)) {
         goto fail_closefb;
     }
     qemu_fclose(fb);
diff --git a/migration/savevm.c b/migration/savevm.c
index 65ce0c6..43031a0 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -51,6 +51,7 @@
 #include "block/snapshot.h"
 #include "block/qapi.h"
 #include "qemu/cutils.h"
+#include "io/channel-buffer.h"
 
 #ifndef ETH_P_RARP
 #define ETH_P_RARP 0x8035
@@ -760,10 +761,8 @@ void qemu_savevm_send_open_return_path(QEMUFile *f)
  *    0 on success
  *    -ve on error
  */
-int qemu_savevm_send_packaged(QEMUFile *f, const QEMUSizedBuffer *qsb)
+int qemu_savevm_send_packaged(QEMUFile *f, const uint8_t *buf, size_t len)
 {
-    size_t cur_iov;
-    size_t len = qsb_get_length(qsb);
     uint32_t tmp;
 
     if (len > MAX_VM_CMD_PACKAGED_SIZE) {
@@ -777,18 +776,7 @@ int qemu_savevm_send_packaged(QEMUFile *f, const QEMUSizedBuffer *qsb)
     trace_qemu_savevm_send_packaged();
     qemu_savevm_command_send(f, MIG_CMD_PACKAGED, 4, (uint8_t *)&tmp);
 
-    /* all the data follows (concatinating the iov's) */
-    for (cur_iov = 0; cur_iov < qsb->n_iov; cur_iov++) {
-        /* The iov entries are partially filled */
-        size_t towrite = MIN(qsb->iov[cur_iov].iov_len, len);
-        len -= towrite;
-
-        if (!towrite) {
-            break;
-        }
-
-        qemu_put_buffer(f, qsb->iov[cur_iov].iov_base, towrite);
-    }
+    qemu_put_buffer(f, buf, len);
 
     return 0;
 }
@@ -1578,39 +1566,36 @@ static int loadvm_postcopy_handle_run(MigrationIncomingState *mis)
 static int loadvm_handle_cmd_packaged(MigrationIncomingState *mis)
 {
     int ret;
-    uint8_t *buffer;
-    uint32_t length;
-    QEMUSizedBuffer *qsb;
+    size_t length;
+    QIOChannelBuffer *bioc;
 
     length = qemu_get_be32(mis->from_src_file);
     trace_loadvm_handle_cmd_packaged(length);
 
     if (length > MAX_VM_CMD_PACKAGED_SIZE) {
-        error_report("Unreasonably large packaged state: %u", length);
+        error_report("Unreasonably large packaged state: %zu", length);
         return -1;
     }
-    buffer = g_malloc0(length);
-    ret = qemu_get_buffer(mis->from_src_file, buffer, (int)length);
+
+    bioc = qio_channel_buffer_new(length);
+    ret = qemu_get_buffer(mis->from_src_file,
+                          bioc->data,
+                          length);
     if (ret != length) {
-        g_free(buffer);
-        error_report("CMD_PACKAGED: Buffer receive fail ret=%d length=%d",
+        object_unref(OBJECT(bioc));
+        error_report("CMD_PACKAGED: Buffer receive fail ret=%d length=%zu",
                      ret, length);
         return (ret < 0) ? ret : -EAGAIN;
     }
+    bioc->usage += length;
     trace_loadvm_handle_cmd_packaged_received(ret);
 
-    /* Setup a dummy QEMUFile that actually reads from the buffer */
-    qsb = qsb_create(buffer, length);
-    g_free(buffer); /* Because qsb_create copies */
-    if (!qsb) {
-        error_report("Unable to create qsb");
-    }
-    QEMUFile *packf = qemu_bufopen("r", qsb);
+    QEMUFile *packf = qemu_fopen_channel_input(QIO_CHANNEL(bioc));
 
     ret = qemu_loadvm_state_main(packf, mis);
     trace_loadvm_handle_cmd_packaged_main(ret);
     qemu_fclose(packf);
-    qsb_free(qsb);
+    object_unref(OBJECT(bioc));
 
     return ret;
 }
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PULL 12/28] migration: convert unix socket protocol to use QIOChannel
  2016-05-26  6:11 [Qemu-devel] [PULL 00/28] migration: support for TLS Amit Shah
                   ` (10 preceding siblings ...)
  2016-05-26  6:12 ` [Qemu-devel] [PULL 11/28] migration: convert post-copy to use QIOChannelBuffer Amit Shah
@ 2016-05-26  6:12 ` Amit Shah
  2016-05-26  6:12 ` [Qemu-devel] [PULL 13/28] migration: rename unix.c to socket.c Amit Shah
                   ` (16 subsequent siblings)
  28 siblings, 0 replies; 36+ messages in thread
From: Amit Shah @ 2016-05-26  6:12 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Juan Quintela, Dr. David Alan Gilbert, Daniel P. Berrange,
	qemu list, Amit Shah

From: "Daniel P. Berrange" <berrange@redhat.com>

Convert the unix socket migration protocol driver to use
QIOChannel and QEMUFileChannel, instead of plain sockets
APIs. It can be unconditionally built, since the socket
impl of QIOChannel will report a suitable error on platforms
where UNIX sockets are unavailable.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-Id: <1461751518-12128-13-git-send-email-berrange@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 migration/Makefile.objs |   4 +-
 migration/migration.c   |   4 ++
 migration/unix.c        | 120 ++++++++++++++++++++++++++++--------------------
 trace-events            |   5 ++
 4 files changed, 80 insertions(+), 53 deletions(-)

diff --git a/migration/Makefile.objs b/migration/Makefile.objs
index facf251..ad7fed9 100644
--- a/migration/Makefile.objs
+++ b/migration/Makefile.objs
@@ -1,4 +1,4 @@
-common-obj-y += migration.o tcp.o
+common-obj-y += migration.o tcp.o unix.o
 common-obj-y += vmstate.o
 common-obj-y += qemu-file.o qemu-file-buf.o qemu-file-unix.o qemu-file-stdio.o
 common-obj-y += qemu-file-channel.o
@@ -6,7 +6,7 @@ common-obj-y += xbzrle.o postcopy-ram.o
 common-obj-y += qjson.o
 
 common-obj-$(CONFIG_RDMA) += rdma.o
-common-obj-$(CONFIG_POSIX) += exec.o unix.o fd.o
+common-obj-$(CONFIG_POSIX) += exec.o fd.o
 
 common-obj-y += block.o
 
diff --git a/migration/migration.c b/migration/migration.c
index 1ab9535..a38da3a 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -314,8 +314,10 @@ void qemu_start_incoming_migration(const char *uri, Error **errp)
 #if !defined(WIN32)
     } else if (strstart(uri, "exec:", &p)) {
         exec_start_incoming_migration(p, errp);
+#endif
     } else if (strstart(uri, "unix:", &p)) {
         unix_start_incoming_migration(p, errp);
+#if !defined(WIN32)
     } else if (strstart(uri, "fd:", &p)) {
         fd_start_incoming_migration(p, errp);
 #endif
@@ -1072,8 +1074,10 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
 #if !defined(WIN32)
     } else if (strstart(uri, "exec:", &p)) {
         exec_start_outgoing_migration(s, p, &local_err);
+#endif
     } else if (strstart(uri, "unix:", &p)) {
         unix_start_outgoing_migration(s, p, &local_err);
+#if !defined(WIN32)
     } else if (strstart(uri, "fd:", &p)) {
         fd_start_outgoing_migration(s, p, &local_err);
 #endif
diff --git a/migration/unix.c b/migration/unix.c
index b3537fd..75205d4 100644
--- a/migration/unix.c
+++ b/migration/unix.c
@@ -1,10 +1,11 @@
 /*
  * QEMU live migration via Unix Domain Sockets
  *
- * Copyright Red Hat, Inc. 2009
+ * Copyright Red Hat, Inc. 2009-2016
  *
  * Authors:
  *  Chris Lalancette <clalance@redhat.com>
+ *  Daniel P. Berrange <berrange@redhat.com>
  *
  * This work is licensed under the terms of the GNU GPL, version 2.  See
  * the COPYING file in the top-level directory.
@@ -17,87 +18,104 @@
 
 #include "qemu-common.h"
 #include "qemu/error-report.h"
-#include "qemu/sockets.h"
-#include "qemu/main-loop.h"
+#include "qapi/error.h"
 #include "migration/migration.h"
 #include "migration/qemu-file.h"
-#include "block/block.h"
+#include "io/channel-socket.h"
+#include "trace.h"
 
-//#define DEBUG_MIGRATION_UNIX
 
-#ifdef DEBUG_MIGRATION_UNIX
-#define DPRINTF(fmt, ...) \
-    do { printf("migration-unix: " fmt, ## __VA_ARGS__); } while (0)
-#else
-#define DPRINTF(fmt, ...) \
-    do { } while (0)
-#endif
+static SocketAddress *unix_build_address(const char *path)
+{
+    SocketAddress *saddr;
+
+    saddr = g_new0(SocketAddress, 1);
+    saddr->type = SOCKET_ADDRESS_KIND_UNIX;
+    saddr->u.q_unix.data = g_new0(UnixSocketAddress, 1);
+    saddr->u.q_unix.data->path = g_strdup(path);
+
+    return saddr;
+}
 
-static void unix_wait_for_connect(int fd, Error *err, void *opaque)
+
+static void unix_outgoing_migration(Object *src,
+                                    Error *err,
+                                    gpointer opaque)
 {
     MigrationState *s = opaque;
+    QIOChannel *sioc = QIO_CHANNEL(src);
 
-    if (fd < 0) {
-        DPRINTF("migrate connect error: %s\n", error_get_pretty(err));
+    if (err) {
+        trace_migration_unix_outgoing_error(error_get_pretty(err));
         s->to_dst_file = NULL;
         migrate_fd_error(s, err);
     } else {
-        DPRINTF("migrate connect success\n");
-        s->to_dst_file = qemu_fopen_socket(fd, "wb");
-        migrate_fd_connect(s);
+        trace_migration_unix_outgoing_connected();
+        migration_set_outgoing_channel(s, sioc);
     }
+    object_unref(src);
 }
 
+
 void unix_start_outgoing_migration(MigrationState *s, const char *path, Error **errp)
 {
-    unix_nonblocking_connect(path, unix_wait_for_connect, s, errp);
+    SocketAddress *saddr = unix_build_address(path);
+    QIOChannelSocket *sioc;
+    sioc = qio_channel_socket_new();
+    qio_channel_socket_connect_async(sioc,
+                                     saddr,
+                                     unix_outgoing_migration,
+                                     s,
+                                     NULL);
+    qapi_free_SocketAddress(saddr);
 }
 
-static void unix_accept_incoming_migration(void *opaque)
+
+static gboolean unix_accept_incoming_migration(QIOChannel *ioc,
+                                               GIOCondition condition,
+                                               gpointer opaque)
 {
-    struct sockaddr_un addr;
-    socklen_t addrlen = sizeof(addr);
-    int s = (intptr_t)opaque;
-    QEMUFile *f;
-    int c, err;
-
-    do {
-        c = qemu_accept(s, (struct sockaddr *)&addr, &addrlen);
-        err = errno;
-    } while (c < 0 && err == EINTR);
-    qemu_set_fd_handler(s, NULL, NULL, NULL);
-    close(s);
-
-    DPRINTF("accepted migration\n");
-
-    if (c < 0) {
-        error_report("could not accept migration connection (%s)",
-                     strerror(err));
-        return;
-    }
+    QIOChannelSocket *sioc;
+    Error *err = NULL;
 
-    f = qemu_fopen_socket(c, "rb");
-    if (f == NULL) {
-        error_report("could not qemu_fopen socket");
+    sioc = qio_channel_socket_accept(QIO_CHANNEL_SOCKET(ioc),
+                                     &err);
+    if (!sioc) {
+        error_report("could not accept migration connection (%s)",
+                     error_get_pretty(err));
         goto out;
     }
 
-    process_incoming_migration(f);
-    return;
+    trace_migration_unix_incoming_accepted();
+
+    migration_set_incoming_channel(migrate_get_current(),
+                                   QIO_CHANNEL(sioc));
+    object_unref(OBJECT(sioc));
 
 out:
-    close(c);
+    /* Close listening socket as its no longer needed */
+    qio_channel_close(ioc, NULL);
+    return FALSE; /* unregister */
 }
 
+
 void unix_start_incoming_migration(const char *path, Error **errp)
 {
-    int s;
+    SocketAddress *saddr = unix_build_address(path);
+    QIOChannelSocket *listen_ioc;
 
-    s = unix_listen(path, NULL, 0, errp);
-    if (s < 0) {
+    listen_ioc = qio_channel_socket_new();
+    if (qio_channel_socket_listen_sync(listen_ioc, saddr, errp) < 0) {
+        object_unref(OBJECT(listen_ioc));
+        qapi_free_SocketAddress(saddr);
         return;
     }
 
-    qemu_set_fd_handler(s, unix_accept_incoming_migration, NULL,
-                        (void *)(intptr_t)s);
+    qio_channel_add_watch(QIO_CHANNEL(listen_ioc),
+                          G_IO_IN,
+                          unix_accept_incoming_migration,
+                          listen_ioc,
+                          (GDestroyNotify)object_unref);
+
+    qapi_free_SocketAddress(saddr);
 }
diff --git a/trace-events b/trace-events
index 1ef4a9a..df4530d 100644
--- a/trace-events
+++ b/trace-events
@@ -1595,6 +1595,11 @@ postcopy_ram_incoming_cleanup_entry(void) ""
 postcopy_ram_incoming_cleanup_exit(void) ""
 postcopy_ram_incoming_cleanup_join(void) ""
 
+# migration/unix.c
+migration_unix_incoming_accepted(void) ""
+migration_unix_outgoing_connected(void) ""
+migration_unix_outgoing_error(const char *err) "error=%s"
+
 # kvm-all.c
 kvm_ioctl(int type, void *arg) "type 0x%x, arg %p"
 kvm_vm_ioctl(int type, void *arg) "type 0x%x, arg %p"
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PULL 13/28] migration: rename unix.c to socket.c
  2016-05-26  6:11 [Qemu-devel] [PULL 00/28] migration: support for TLS Amit Shah
                   ` (11 preceding siblings ...)
  2016-05-26  6:12 ` [Qemu-devel] [PULL 12/28] migration: convert unix socket protocol to use QIOChannel Amit Shah
@ 2016-05-26  6:12 ` Amit Shah
  2016-05-26  6:12 ` [Qemu-devel] [PULL 14/28] migration: convert tcp socket protocol to use QIOChannel Amit Shah
                   ` (15 subsequent siblings)
  28 siblings, 0 replies; 36+ messages in thread
From: Amit Shah @ 2016-05-26  6:12 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Juan Quintela, Dr. David Alan Gilbert, Daniel P. Berrange,
	qemu list, Amit Shah

From: "Daniel P. Berrange" <berrange@redhat.com>

The unix.c file will be nearly the same as the tcp.c file,
only differing in the initial SocketAddress creation code.
Rename unix.c to socket.c and refactor it a little to
prepare for merging the TCP code.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-Id: <1461751518-12128-14-git-send-email-berrange@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 migration/Makefile.objs |   2 +-
 migration/socket.c      | 132 ++++++++++++++++++++++++++++++++++++++++++++++++
 migration/unix.c        | 121 --------------------------------------------
 trace-events            |   8 +--
 4 files changed, 137 insertions(+), 126 deletions(-)
 create mode 100644 migration/socket.c
 delete mode 100644 migration/unix.c

diff --git a/migration/Makefile.objs b/migration/Makefile.objs
index ad7fed9..b0b9550 100644
--- a/migration/Makefile.objs
+++ b/migration/Makefile.objs
@@ -1,4 +1,4 @@
-common-obj-y += migration.o tcp.o unix.o
+common-obj-y += migration.o tcp.o socket.o
 common-obj-y += vmstate.o
 common-obj-y += qemu-file.o qemu-file-buf.o qemu-file-unix.o qemu-file-stdio.o
 common-obj-y += qemu-file-channel.o
diff --git a/migration/socket.c b/migration/socket.c
new file mode 100644
index 0000000..a9911d6
--- /dev/null
+++ b/migration/socket.c
@@ -0,0 +1,132 @@
+/*
+ * QEMU live migration via Unix Domain Sockets
+ *
+ * Copyright Red Hat, Inc. 2009-2016
+ *
+ * Authors:
+ *  Chris Lalancette <clalance@redhat.com>
+ *  Daniel P. Berrange <berrange@redhat.com>
+ *
+ * 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 "qemu-common.h"
+#include "qemu/error-report.h"
+#include "qapi/error.h"
+#include "migration/migration.h"
+#include "migration/qemu-file.h"
+#include "io/channel-socket.h"
+#include "trace.h"
+
+
+static SocketAddress *unix_build_address(const char *path)
+{
+    SocketAddress *saddr;
+
+    saddr = g_new0(SocketAddress, 1);
+    saddr->type = SOCKET_ADDRESS_KIND_UNIX;
+    saddr->u.q_unix.data = g_new0(UnixSocketAddress, 1);
+    saddr->u.q_unix.data->path = g_strdup(path);
+
+    return saddr;
+}
+
+
+static void socket_outgoing_migration(Object *src,
+                                      Error *err,
+                                      gpointer opaque)
+{
+    MigrationState *s = opaque;
+    QIOChannel *sioc = QIO_CHANNEL(src);
+
+    if (err) {
+        trace_migration_socket_outgoing_error(error_get_pretty(err));
+        s->to_dst_file = NULL;
+        migrate_fd_error(s, err);
+    } else {
+        trace_migration_socket_outgoing_connected();
+        migration_set_outgoing_channel(s, sioc);
+    }
+    object_unref(src);
+}
+
+static void socket_start_outgoing_migration(MigrationState *s,
+                                            SocketAddress *saddr,
+                                            Error **errp)
+{
+    QIOChannelSocket *sioc = qio_channel_socket_new();
+    qio_channel_socket_connect_async(sioc,
+                                     saddr,
+                                     socket_outgoing_migration,
+                                     s,
+                                     NULL);
+    qapi_free_SocketAddress(saddr);
+}
+
+void unix_start_outgoing_migration(MigrationState *s,
+                                   const char *path,
+                                   Error **errp)
+{
+    SocketAddress *saddr = unix_build_address(path);
+    socket_start_outgoing_migration(s, saddr, errp);
+}
+
+
+static gboolean socket_accept_incoming_migration(QIOChannel *ioc,
+                                                 GIOCondition condition,
+                                                 gpointer opaque)
+{
+    QIOChannelSocket *sioc;
+    Error *err = NULL;
+
+    sioc = qio_channel_socket_accept(QIO_CHANNEL_SOCKET(ioc),
+                                     &err);
+    if (!sioc) {
+        error_report("could not accept migration connection (%s)",
+                     error_get_pretty(err));
+        goto out;
+    }
+
+    trace_migration_socket_incoming_accepted();
+
+    migration_set_incoming_channel(migrate_get_current(),
+                                   QIO_CHANNEL(sioc));
+    object_unref(OBJECT(sioc));
+
+out:
+    /* Close listening socket as its no longer needed */
+    qio_channel_close(ioc, NULL);
+    return FALSE; /* unregister */
+}
+
+
+static void socket_start_incoming_migration(SocketAddress *saddr,
+                                            Error **errp)
+{
+    QIOChannelSocket *listen_ioc = qio_channel_socket_new();
+
+    if (qio_channel_socket_listen_sync(listen_ioc, saddr, errp) < 0) {
+        object_unref(OBJECT(listen_ioc));
+        qapi_free_SocketAddress(saddr);
+        return;
+    }
+
+    qio_channel_add_watch(QIO_CHANNEL(listen_ioc),
+                          G_IO_IN,
+                          socket_accept_incoming_migration,
+                          listen_ioc,
+                          (GDestroyNotify)object_unref);
+    qapi_free_SocketAddress(saddr);
+}
+
+void unix_start_incoming_migration(const char *path, Error **errp)
+{
+    SocketAddress *saddr = unix_build_address(path);
+    socket_start_incoming_migration(saddr, errp);
+}
diff --git a/migration/unix.c b/migration/unix.c
deleted file mode 100644
index 75205d4..0000000
--- a/migration/unix.c
+++ /dev/null
@@ -1,121 +0,0 @@
-/*
- * QEMU live migration via Unix Domain Sockets
- *
- * Copyright Red Hat, Inc. 2009-2016
- *
- * Authors:
- *  Chris Lalancette <clalance@redhat.com>
- *  Daniel P. Berrange <berrange@redhat.com>
- *
- * 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 "qemu-common.h"
-#include "qemu/error-report.h"
-#include "qapi/error.h"
-#include "migration/migration.h"
-#include "migration/qemu-file.h"
-#include "io/channel-socket.h"
-#include "trace.h"
-
-
-static SocketAddress *unix_build_address(const char *path)
-{
-    SocketAddress *saddr;
-
-    saddr = g_new0(SocketAddress, 1);
-    saddr->type = SOCKET_ADDRESS_KIND_UNIX;
-    saddr->u.q_unix.data = g_new0(UnixSocketAddress, 1);
-    saddr->u.q_unix.data->path = g_strdup(path);
-
-    return saddr;
-}
-
-
-static void unix_outgoing_migration(Object *src,
-                                    Error *err,
-                                    gpointer opaque)
-{
-    MigrationState *s = opaque;
-    QIOChannel *sioc = QIO_CHANNEL(src);
-
-    if (err) {
-        trace_migration_unix_outgoing_error(error_get_pretty(err));
-        s->to_dst_file = NULL;
-        migrate_fd_error(s, err);
-    } else {
-        trace_migration_unix_outgoing_connected();
-        migration_set_outgoing_channel(s, sioc);
-    }
-    object_unref(src);
-}
-
-
-void unix_start_outgoing_migration(MigrationState *s, const char *path, Error **errp)
-{
-    SocketAddress *saddr = unix_build_address(path);
-    QIOChannelSocket *sioc;
-    sioc = qio_channel_socket_new();
-    qio_channel_socket_connect_async(sioc,
-                                     saddr,
-                                     unix_outgoing_migration,
-                                     s,
-                                     NULL);
-    qapi_free_SocketAddress(saddr);
-}
-
-
-static gboolean unix_accept_incoming_migration(QIOChannel *ioc,
-                                               GIOCondition condition,
-                                               gpointer opaque)
-{
-    QIOChannelSocket *sioc;
-    Error *err = NULL;
-
-    sioc = qio_channel_socket_accept(QIO_CHANNEL_SOCKET(ioc),
-                                     &err);
-    if (!sioc) {
-        error_report("could not accept migration connection (%s)",
-                     error_get_pretty(err));
-        goto out;
-    }
-
-    trace_migration_unix_incoming_accepted();
-
-    migration_set_incoming_channel(migrate_get_current(),
-                                   QIO_CHANNEL(sioc));
-    object_unref(OBJECT(sioc));
-
-out:
-    /* Close listening socket as its no longer needed */
-    qio_channel_close(ioc, NULL);
-    return FALSE; /* unregister */
-}
-
-
-void unix_start_incoming_migration(const char *path, Error **errp)
-{
-    SocketAddress *saddr = unix_build_address(path);
-    QIOChannelSocket *listen_ioc;
-
-    listen_ioc = qio_channel_socket_new();
-    if (qio_channel_socket_listen_sync(listen_ioc, saddr, errp) < 0) {
-        object_unref(OBJECT(listen_ioc));
-        qapi_free_SocketAddress(saddr);
-        return;
-    }
-
-    qio_channel_add_watch(QIO_CHANNEL(listen_ioc),
-                          G_IO_IN,
-                          unix_accept_incoming_migration,
-                          listen_ioc,
-                          (GDestroyNotify)object_unref);
-
-    qapi_free_SocketAddress(saddr);
-}
diff --git a/trace-events b/trace-events
index df4530d..62f634d 100644
--- a/trace-events
+++ b/trace-events
@@ -1595,10 +1595,10 @@ postcopy_ram_incoming_cleanup_entry(void) ""
 postcopy_ram_incoming_cleanup_exit(void) ""
 postcopy_ram_incoming_cleanup_join(void) ""
 
-# migration/unix.c
-migration_unix_incoming_accepted(void) ""
-migration_unix_outgoing_connected(void) ""
-migration_unix_outgoing_error(const char *err) "error=%s"
+# migration/socket.c
+migration_socket_incoming_accepted(void) ""
+migration_socket_outgoing_connected(void) ""
+migration_socket_outgoing_error(const char *err) "error=%s"
 
 # kvm-all.c
 kvm_ioctl(int type, void *arg) "type 0x%x, arg %p"
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PULL 14/28] migration: convert tcp socket protocol to use QIOChannel
  2016-05-26  6:11 [Qemu-devel] [PULL 00/28] migration: support for TLS Amit Shah
                   ` (12 preceding siblings ...)
  2016-05-26  6:12 ` [Qemu-devel] [PULL 13/28] migration: rename unix.c to socket.c Amit Shah
@ 2016-05-26  6:12 ` Amit Shah
  2016-05-26  6:12 ` [Qemu-devel] [PULL 15/28] migration: convert fd " Amit Shah
                   ` (14 subsequent siblings)
  28 siblings, 0 replies; 36+ messages in thread
From: Amit Shah @ 2016-05-26  6:12 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Juan Quintela, Dr. David Alan Gilbert, Daniel P. Berrange,
	qemu list, Amit Shah

From: "Daniel P. Berrange" <berrange@redhat.com>

Drop the current TCP socket migration driver and extend
the new generic socket driver to cope with the TCP address
format

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-Id: <1461751518-12128-15-git-send-email-berrange@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 migration/Makefile.objs |   2 +-
 migration/socket.c      |  31 +++++++++++++++
 migration/tcp.c         | 102 ------------------------------------------------
 3 files changed, 32 insertions(+), 103 deletions(-)
 delete mode 100644 migration/tcp.c

diff --git a/migration/Makefile.objs b/migration/Makefile.objs
index b0b9550..4221861 100644
--- a/migration/Makefile.objs
+++ b/migration/Makefile.objs
@@ -1,4 +1,4 @@
-common-obj-y += migration.o tcp.o socket.o
+common-obj-y += migration.o socket.o
 common-obj-y += vmstate.o
 common-obj-y += qemu-file.o qemu-file-buf.o qemu-file-unix.o qemu-file-stdio.o
 common-obj-y += qemu-file-channel.o
diff --git a/migration/socket.c b/migration/socket.c
index a9911d6..25457ea 100644
--- a/migration/socket.c
+++ b/migration/socket.c
@@ -25,6 +25,23 @@
 #include "trace.h"
 
 
+static SocketAddress *tcp_build_address(const char *host_port, Error **errp)
+{
+    InetSocketAddress *iaddr = inet_parse(host_port, errp);
+    SocketAddress *saddr;
+
+    if (!iaddr) {
+        return NULL;
+    }
+
+    saddr = g_new0(SocketAddress, 1);
+    saddr->type = SOCKET_ADDRESS_KIND_INET;
+    saddr->u.inet.data = iaddr;
+
+    return saddr;
+}
+
+
 static SocketAddress *unix_build_address(const char *path)
 {
     SocketAddress *saddr;
@@ -69,6 +86,14 @@ static void socket_start_outgoing_migration(MigrationState *s,
     qapi_free_SocketAddress(saddr);
 }
 
+void tcp_start_outgoing_migration(MigrationState *s,
+                                  const char *host_port,
+                                  Error **errp)
+{
+    SocketAddress *saddr = tcp_build_address(host_port, errp);
+    socket_start_outgoing_migration(s, saddr, errp);
+}
+
 void unix_start_outgoing_migration(MigrationState *s,
                                    const char *path,
                                    Error **errp)
@@ -125,6 +150,12 @@ static void socket_start_incoming_migration(SocketAddress *saddr,
     qapi_free_SocketAddress(saddr);
 }
 
+void tcp_start_incoming_migration(const char *host_port, Error **errp)
+{
+    SocketAddress *saddr = tcp_build_address(host_port, errp);
+    socket_start_incoming_migration(saddr, errp);
+}
+
 void unix_start_incoming_migration(const char *path, Error **errp)
 {
     SocketAddress *saddr = unix_build_address(path);
diff --git a/migration/tcp.c b/migration/tcp.c
deleted file mode 100644
index d0e0db9..0000000
--- a/migration/tcp.c
+++ /dev/null
@@ -1,102 +0,0 @@
-/*
- * QEMU live migration
- *
- * Copyright IBM, Corp. 2008
- *
- * Authors:
- *  Anthony Liguori   <aliguori@us.ibm.com>
- *
- * 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 "qemu-common.h"
-#include "qemu/error-report.h"
-#include "qemu/sockets.h"
-#include "migration/migration.h"
-#include "migration/qemu-file.h"
-#include "block/block.h"
-#include "qemu/main-loop.h"
-
-//#define DEBUG_MIGRATION_TCP
-
-#ifdef DEBUG_MIGRATION_TCP
-#define DPRINTF(fmt, ...) \
-    do { printf("migration-tcp: " fmt, ## __VA_ARGS__); } while (0)
-#else
-#define DPRINTF(fmt, ...) \
-    do { } while (0)
-#endif
-
-static void tcp_wait_for_connect(int fd, Error *err, void *opaque)
-{
-    MigrationState *s = opaque;
-
-    if (fd < 0) {
-        DPRINTF("migrate connect error: %s\n", error_get_pretty(err));
-        s->to_dst_file = NULL;
-        migrate_fd_error(s, err);
-    } else {
-        DPRINTF("migrate connect success\n");
-        s->to_dst_file = qemu_fopen_socket(fd, "wb");
-        migrate_fd_connect(s);
-    }
-}
-
-void tcp_start_outgoing_migration(MigrationState *s, const char *host_port, Error **errp)
-{
-    inet_nonblocking_connect(host_port, tcp_wait_for_connect, s, errp);
-}
-
-static void tcp_accept_incoming_migration(void *opaque)
-{
-    struct sockaddr_in addr;
-    socklen_t addrlen = sizeof(addr);
-    int s = (intptr_t)opaque;
-    QEMUFile *f;
-    int c;
-
-    do {
-        c = qemu_accept(s, (struct sockaddr *)&addr, &addrlen);
-    } while (c < 0 && errno == EINTR);
-    qemu_set_fd_handler(s, NULL, NULL, NULL);
-    closesocket(s);
-
-    DPRINTF("accepted migration\n");
-
-    if (c < 0) {
-        error_report("could not accept migration connection (%s)",
-                     strerror(errno));
-        return;
-    }
-
-    f = qemu_fopen_socket(c, "rb");
-    if (f == NULL) {
-        error_report("could not qemu_fopen socket");
-        goto out;
-    }
-
-    process_incoming_migration(f);
-    return;
-
-out:
-    closesocket(c);
-}
-
-void tcp_start_incoming_migration(const char *host_port, Error **errp)
-{
-    int s;
-
-    s = inet_listen(host_port, NULL, 256, SOCK_STREAM, 0, errp);
-    if (s < 0) {
-        return;
-    }
-
-    qemu_set_fd_handler(s, tcp_accept_incoming_migration, NULL,
-                        (void *)(intptr_t)s);
-}
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PULL 15/28] migration: convert fd socket protocol to use QIOChannel
  2016-05-26  6:11 [Qemu-devel] [PULL 00/28] migration: support for TLS Amit Shah
                   ` (13 preceding siblings ...)
  2016-05-26  6:12 ` [Qemu-devel] [PULL 14/28] migration: convert tcp socket protocol to use QIOChannel Amit Shah
@ 2016-05-26  6:12 ` Amit Shah
  2016-05-26  6:12 ` [Qemu-devel] [PULL 16/28] migration: convert exec " Amit Shah
                   ` (13 subsequent siblings)
  28 siblings, 0 replies; 36+ messages in thread
From: Amit Shah @ 2016-05-26  6:12 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Juan Quintela, Dr. David Alan Gilbert, Daniel P. Berrange,
	qemu list, Amit Shah

From: "Daniel P. Berrange" <berrange@redhat.com>

Convert the fd socket migration protocol driver to use
QIOChannel and QEMUFileChannel, instead of plain sockets
APIs. It can be unconditionally built because the
QIOChannel APIs it uses will take care to report suitable
error messages if needed.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-Id: <1461751518-12128-16-git-send-email-berrange@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 migration/Makefile.objs |  4 +--
 migration/fd.c          | 75 +++++++++++++++++++------------------------------
 migration/migration.c   |  4 ---
 trace-events            |  4 +++
 4 files changed, 35 insertions(+), 52 deletions(-)

diff --git a/migration/Makefile.objs b/migration/Makefile.objs
index 4221861..2e5040c 100644
--- a/migration/Makefile.objs
+++ b/migration/Makefile.objs
@@ -1,4 +1,4 @@
-common-obj-y += migration.o socket.o
+common-obj-y += migration.o socket.o fd.o
 common-obj-y += vmstate.o
 common-obj-y += qemu-file.o qemu-file-buf.o qemu-file-unix.o qemu-file-stdio.o
 common-obj-y += qemu-file-channel.o
@@ -6,7 +6,7 @@ common-obj-y += xbzrle.o postcopy-ram.o
 common-obj-y += qjson.o
 
 common-obj-$(CONFIG_RDMA) += rdma.o
-common-obj-$(CONFIG_POSIX) += exec.o fd.o
+common-obj-$(CONFIG_POSIX) += exec.o
 
 common-obj-y += block.o
 
diff --git a/migration/fd.c b/migration/fd.c
index 3d788bb..60a75b8 100644
--- a/migration/fd.c
+++ b/migration/fd.c
@@ -1,10 +1,11 @@
 /*
  * QEMU live migration via generic fd
  *
- * Copyright Red Hat, Inc. 2009
+ * Copyright Red Hat, Inc. 2009-2016
  *
  * Authors:
  *  Chris Lalancette <clalance@redhat.com>
+ *  Daniel P. Berrange <berrange@redhat.com>
  *
  * This work is licensed under the terms of the GNU GPL, version 2.  See
  * the COPYING file in the top-level directory.
@@ -16,75 +17,57 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu-common.h"
-#include "qemu/main-loop.h"
-#include "qemu/sockets.h"
 #include "migration/migration.h"
 #include "monitor/monitor.h"
-#include "migration/qemu-file.h"
-#include "block/block.h"
+#include "io/channel-util.h"
+#include "trace.h"
 
-//#define DEBUG_MIGRATION_FD
-
-#ifdef DEBUG_MIGRATION_FD
-#define DPRINTF(fmt, ...) \
-    do { printf("migration-fd: " fmt, ## __VA_ARGS__); } while (0)
-#else
-#define DPRINTF(fmt, ...) \
-    do { } while (0)
-#endif
-
-static bool fd_is_socket(int fd)
-{
-    struct stat stat;
-    int ret = fstat(fd, &stat);
-    if (ret == -1) {
-        /* When in doubt say no */
-        return false;
-    }
-    return S_ISSOCK(stat.st_mode);
-}
 
 void fd_start_outgoing_migration(MigrationState *s, const char *fdname, Error **errp)
 {
+    QIOChannel *ioc;
     int fd = monitor_get_fd(cur_mon, fdname, errp);
     if (fd == -1) {
         return;
     }
 
-    if (fd_is_socket(fd)) {
-        s->to_dst_file = qemu_fopen_socket(fd, "wb");
-    } else {
-        s->to_dst_file = qemu_fdopen(fd, "wb");
+    trace_migration_fd_outgoing(fd);
+    ioc = qio_channel_new_fd(fd, errp);
+    if (!ioc) {
+        close(fd);
+        return;
     }
 
-    migrate_fd_connect(s);
+    migration_set_outgoing_channel(s, ioc);
+    object_unref(OBJECT(ioc));
 }
 
-static void fd_accept_incoming_migration(void *opaque)
+static gboolean fd_accept_incoming_migration(QIOChannel *ioc,
+                                             GIOCondition condition,
+                                             gpointer opaque)
 {
-    QEMUFile *f = opaque;
-
-    qemu_set_fd_handler(qemu_get_fd(f), NULL, NULL, NULL);
-    process_incoming_migration(f);
+    migration_set_incoming_channel(migrate_get_current(), ioc);
+    object_unref(OBJECT(ioc));
+    return FALSE; /* unregister */
 }
 
 void fd_start_incoming_migration(const char *infd, Error **errp)
 {
+    QIOChannel *ioc;
     int fd;
-    QEMUFile *f;
-
-    DPRINTF("Attempting to start an incoming migration via fd\n");
 
     fd = strtol(infd, NULL, 0);
-    if (fd_is_socket(fd)) {
-        f = qemu_fopen_socket(fd, "rb");
-    } else {
-        f = qemu_fdopen(fd, "rb");
-    }
-    if(f == NULL) {
-        error_setg_errno(errp, errno, "failed to open the source descriptor");
+    trace_migration_fd_incoming(fd);
+
+    ioc = qio_channel_new_fd(fd, errp);
+    if (!ioc) {
+        close(fd);
         return;
     }
 
-    qemu_set_fd_handler(fd, fd_accept_incoming_migration, NULL, f);
+    qio_channel_add_watch(ioc,
+                          G_IO_IN,
+                          fd_accept_incoming_migration,
+                          NULL,
+                          NULL);
 }
diff --git a/migration/migration.c b/migration/migration.c
index a38da3a..8decc7d 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -317,10 +317,8 @@ void qemu_start_incoming_migration(const char *uri, Error **errp)
 #endif
     } else if (strstart(uri, "unix:", &p)) {
         unix_start_incoming_migration(p, errp);
-#if !defined(WIN32)
     } else if (strstart(uri, "fd:", &p)) {
         fd_start_incoming_migration(p, errp);
-#endif
     } else {
         error_setg(errp, "unknown migration protocol: %s", uri);
     }
@@ -1077,10 +1075,8 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
 #endif
     } else if (strstart(uri, "unix:", &p)) {
         unix_start_outgoing_migration(s, p, &local_err);
-#if !defined(WIN32)
     } else if (strstart(uri, "fd:", &p)) {
         fd_start_outgoing_migration(s, p, &local_err);
-#endif
     } else {
         error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "uri",
                    "a valid migration protocol");
diff --git a/trace-events b/trace-events
index 62f634d..e9945e7 100644
--- a/trace-events
+++ b/trace-events
@@ -1595,6 +1595,10 @@ postcopy_ram_incoming_cleanup_entry(void) ""
 postcopy_ram_incoming_cleanup_exit(void) ""
 postcopy_ram_incoming_cleanup_join(void) ""
 
+# migration/fd.c
+migration_fd_outgoing(int fd) "fd=%d"
+migration_fd_incoming(int fd) "fd=%d"
+
 # migration/socket.c
 migration_socket_incoming_accepted(void) ""
 migration_socket_outgoing_connected(void) ""
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PULL 16/28] migration: convert exec socket protocol to use QIOChannel
  2016-05-26  6:11 [Qemu-devel] [PULL 00/28] migration: support for TLS Amit Shah
                   ` (14 preceding siblings ...)
  2016-05-26  6:12 ` [Qemu-devel] [PULL 15/28] migration: convert fd " Amit Shah
@ 2016-05-26  6:12 ` Amit Shah
  2016-05-26  6:12 ` [Qemu-devel] [PULL 17/28] migration: convert RDMA to use QIOChannel interface Amit Shah
                   ` (12 subsequent siblings)
  28 siblings, 0 replies; 36+ messages in thread
From: Amit Shah @ 2016-05-26  6:12 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Juan Quintela, Dr. David Alan Gilbert, Daniel P. Berrange,
	qemu list, Amit Shah

From: "Daniel P. Berrange" <berrange@redhat.com>

Convert the exec socket migration protocol driver to use
QIOChannel and QEMUFileChannel, instead of the stdio
popen APIs. It can be unconditionally built because the
QIOChannelCommand class can report suitable error messages
on platforms which can't fork processes.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-Id: <1461751518-12128-17-git-send-email-berrange@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 migration/Makefile.objs |  3 +--
 migration/exec.c        | 62 +++++++++++++++++++++++++------------------------
 migration/migration.c   |  4 ----
 trace-events            |  4 ++++
 4 files changed, 37 insertions(+), 36 deletions(-)

diff --git a/migration/Makefile.objs b/migration/Makefile.objs
index 2e5040c..c73fb8a 100644
--- a/migration/Makefile.objs
+++ b/migration/Makefile.objs
@@ -1,4 +1,4 @@
-common-obj-y += migration.o socket.o fd.o
+common-obj-y += migration.o socket.o fd.o exec.o
 common-obj-y += vmstate.o
 common-obj-y += qemu-file.o qemu-file-buf.o qemu-file-unix.o qemu-file-stdio.o
 common-obj-y += qemu-file-channel.o
@@ -6,7 +6,6 @@ common-obj-y += xbzrle.o postcopy-ram.o
 common-obj-y += qjson.o
 
 common-obj-$(CONFIG_RDMA) += rdma.o
-common-obj-$(CONFIG_POSIX) += exec.o
 
 common-obj-y += block.o
 
diff --git a/migration/exec.c b/migration/exec.c
index 5594209..c825e27 100644
--- a/migration/exec.c
+++ b/migration/exec.c
@@ -3,10 +3,12 @@
  *
  * Copyright IBM, Corp. 2008
  * Copyright Dell MessageOne 2008
+ * Copyright Red Hat, Inc. 2015-2016
  *
  * Authors:
  *  Anthony Liguori   <aliguori@us.ibm.com>
  *  Charles Duffy     <charles_duffy@messageone.com>
+ *  Daniel P. Berrange <berrange@redhat.com>
  *
  * This work is licensed under the terms of the GNU GPL, version 2.  See
  * the COPYING file in the top-level directory.
@@ -18,53 +20,53 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu-common.h"
-#include "qemu/sockets.h"
-#include "qemu/main-loop.h"
 #include "migration/migration.h"
-#include "migration/qemu-file.h"
-#include "block/block.h"
-#include <sys/wait.h>
+#include "io/channel-command.h"
+#include "trace.h"
 
-//#define DEBUG_MIGRATION_EXEC
-
-#ifdef DEBUG_MIGRATION_EXEC
-#define DPRINTF(fmt, ...) \
-    do { printf("migration-exec: " fmt, ## __VA_ARGS__); } while (0)
-#else
-#define DPRINTF(fmt, ...) \
-    do { } while (0)
-#endif
 
 void exec_start_outgoing_migration(MigrationState *s, const char *command, Error **errp)
 {
-    s->to_dst_file = qemu_popen_cmd(command, "w");
-    if (s->to_dst_file == NULL) {
-        error_setg_errno(errp, errno, "failed to popen the migration target");
+    QIOChannel *ioc;
+    const char *argv[] = { "/bin/sh", "-c", command, NULL };
+
+    trace_migration_exec_outgoing(command);
+    ioc = QIO_CHANNEL(qio_channel_command_new_spawn(argv,
+                                                    O_WRONLY,
+                                                    errp));
+    if (!ioc) {
         return;
     }
 
-    migrate_fd_connect(s);
+    migration_set_outgoing_channel(s, ioc);
+    object_unref(OBJECT(ioc));
 }
 
-static void exec_accept_incoming_migration(void *opaque)
+static gboolean exec_accept_incoming_migration(QIOChannel *ioc,
+                                               GIOCondition condition,
+                                               gpointer opaque)
 {
-    QEMUFile *f = opaque;
-
-    qemu_set_fd_handler(qemu_get_fd(f), NULL, NULL, NULL);
-    process_incoming_migration(f);
+    migration_set_incoming_channel(migrate_get_current(), ioc);
+    object_unref(OBJECT(ioc));
+    return FALSE; /* unregister */
 }
 
 void exec_start_incoming_migration(const char *command, Error **errp)
 {
-    QEMUFile *f;
+    QIOChannel *ioc;
+    const char *argv[] = { "/bin/sh", "-c", command, NULL };
 
-    DPRINTF("Attempting to start an incoming migration\n");
-    f = qemu_popen_cmd(command, "r");
-    if(f == NULL) {
-        error_setg_errno(errp, errno, "failed to popen the migration source");
+    trace_migration_exec_incoming(command);
+    ioc = QIO_CHANNEL(qio_channel_command_new_spawn(argv,
+                                                    O_RDONLY,
+                                                    errp));
+    if (!ioc) {
         return;
     }
 
-    qemu_set_fd_handler(qemu_get_fd(f), exec_accept_incoming_migration, NULL,
-                        f);
+    qio_channel_add_watch(ioc,
+                          G_IO_IN,
+                          exec_accept_incoming_migration,
+                          NULL,
+                          NULL);
 }
diff --git a/migration/migration.c b/migration/migration.c
index 8decc7d..695384b 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -311,10 +311,8 @@ void qemu_start_incoming_migration(const char *uri, Error **errp)
     } else if (strstart(uri, "rdma:", &p)) {
         rdma_start_incoming_migration(p, errp);
 #endif
-#if !defined(WIN32)
     } else if (strstart(uri, "exec:", &p)) {
         exec_start_incoming_migration(p, errp);
-#endif
     } else if (strstart(uri, "unix:", &p)) {
         unix_start_incoming_migration(p, errp);
     } else if (strstart(uri, "fd:", &p)) {
@@ -1069,10 +1067,8 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
     } else if (strstart(uri, "rdma:", &p)) {
         rdma_start_outgoing_migration(s, p, &local_err);
 #endif
-#if !defined(WIN32)
     } else if (strstart(uri, "exec:", &p)) {
         exec_start_outgoing_migration(s, p, &local_err);
-#endif
     } else if (strstart(uri, "unix:", &p)) {
         unix_start_outgoing_migration(s, p, &local_err);
     } else if (strstart(uri, "fd:", &p)) {
diff --git a/trace-events b/trace-events
index e9945e7..1584b0a 100644
--- a/trace-events
+++ b/trace-events
@@ -1595,6 +1595,10 @@ postcopy_ram_incoming_cleanup_entry(void) ""
 postcopy_ram_incoming_cleanup_exit(void) ""
 postcopy_ram_incoming_cleanup_join(void) ""
 
+# migration/exec.c
+migration_exec_outgoing(const char *cmd) "cmd=%s"
+migration_exec_incoming(const char *cmd) "cmd=%s"
+
 # migration/fd.c
 migration_fd_outgoing(int fd) "fd=%d"
 migration_fd_incoming(int fd) "fd=%d"
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PULL 17/28] migration: convert RDMA to use QIOChannel interface
  2016-05-26  6:11 [Qemu-devel] [PULL 00/28] migration: support for TLS Amit Shah
                   ` (15 preceding siblings ...)
  2016-05-26  6:12 ` [Qemu-devel] [PULL 16/28] migration: convert exec " Amit Shah
@ 2016-05-26  6:12 ` Amit Shah
  2016-05-26  6:12 ` [Qemu-devel] [PULL 18/28] migration: convert savevm to use QIOChannel for writing to files Amit Shah
                   ` (11 subsequent siblings)
  28 siblings, 0 replies; 36+ messages in thread
From: Amit Shah @ 2016-05-26  6:12 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Juan Quintela, Dr. David Alan Gilbert, Daniel P. Berrange,
	qemu list, Amit Shah

From: "Daniel P. Berrange" <berrange@redhat.com>

This converts the RDMA code to provide a subclass of QIOChannel
that uses RDMA for the data transport.

This implementation of RDMA does not correctly handle non-blocking
mode. Reads might block if there was not already some pending data
and writes will block until all data is sent. This flawed behaviour
was already present in the existing impl, so appears to not be a
critical problem at this time. It should be on the list of things
to fix in the future though.

The RDMA code would be much better off it it could be split up in
a generic RDMA layer, a QIOChannel impl based on RMDA, and then
the RMDA migration glue. This is left as a future exercise for
the brave.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-Id: <1461751518-12128-18-git-send-email-berrange@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 migration/rdma.c | 374 ++++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 275 insertions(+), 99 deletions(-)

diff --git a/migration/rdma.c b/migration/rdma.c
index f8578b9..51bafc7 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -2,10 +2,12 @@
  * RDMA protocol and interfaces
  *
  * Copyright IBM, Corp. 2010-2013
+ * Copyright Red Hat, Inc. 2015-2016
  *
  * Authors:
  *  Michael R. Hines <mrhines@us.ibm.com>
  *  Jiuxing Liu <jl@us.ibm.com>
+ *  Daniel P. Berrange <berrange@redhat.com>
  *
  * This work is licensed under the terms of the GNU GPL, version 2 or
  * later.  See the COPYING file in the top-level directory.
@@ -374,14 +376,20 @@ typedef struct RDMAContext {
     GHashTable *blockmap;
 } RDMAContext;
 
-/*
- * Interface to the rest of the migration call stack.
- */
-typedef struct QEMUFileRDMA {
+#define TYPE_QIO_CHANNEL_RDMA "qio-channel-rdma"
+#define QIO_CHANNEL_RDMA(obj)                                     \
+    OBJECT_CHECK(QIOChannelRDMA, (obj), TYPE_QIO_CHANNEL_RDMA)
+
+typedef struct QIOChannelRDMA QIOChannelRDMA;
+
+
+struct QIOChannelRDMA {
+    QIOChannel parent;
     RDMAContext *rdma;
+    QEMUFile *file;
     size_t len;
-    void *file;
-} QEMUFileRDMA;
+    bool blocking; /* XXX we don't actually honour this yet */
+};
 
 /*
  * Main structure for IB Send/Recv control messages.
@@ -2518,15 +2526,19 @@ static void *qemu_rdma_data_init(const char *host_port, Error **errp)
  * SEND messages for control only.
  * VM's ram is handled with regular RDMA messages.
  */
-static ssize_t qemu_rdma_put_buffer(void *opaque, const uint8_t *buf,
-                                    int64_t pos, size_t size)
-{
-    QEMUFileRDMA *r = opaque;
-    QEMUFile *f = r->file;
-    RDMAContext *rdma = r->rdma;
-    size_t remaining = size;
-    uint8_t * data = (void *) buf;
+static ssize_t qio_channel_rdma_writev(QIOChannel *ioc,
+                                       const struct iovec *iov,
+                                       size_t niov,
+                                       int *fds,
+                                       size_t nfds,
+                                       Error **errp)
+{
+    QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(ioc);
+    QEMUFile *f = rioc->file;
+    RDMAContext *rdma = rioc->rdma;
     int ret;
+    ssize_t done = 0;
+    size_t i;
 
     CHECK_ERROR_STATE();
 
@@ -2540,27 +2552,31 @@ static ssize_t qemu_rdma_put_buffer(void *opaque, const uint8_t *buf,
         return ret;
     }
 
-    while (remaining) {
-        RDMAControlHeader head;
+    for (i = 0; i < niov; i++) {
+        size_t remaining = iov[i].iov_len;
+        uint8_t * data = (void *)iov[i].iov_base;
+        while (remaining) {
+            RDMAControlHeader head;
 
-        r->len = MIN(remaining, RDMA_SEND_INCREMENT);
-        remaining -= r->len;
+            rioc->len = MIN(remaining, RDMA_SEND_INCREMENT);
+            remaining -= rioc->len;
 
-        /* Guaranteed to fit due to RDMA_SEND_INCREMENT MIN above */
-        head.len = (uint32_t)r->len;
-        head.type = RDMA_CONTROL_QEMU_FILE;
+            head.len = rioc->len;
+            head.type = RDMA_CONTROL_QEMU_FILE;
 
-        ret = qemu_rdma_exchange_send(rdma, &head, data, NULL, NULL, NULL);
+            ret = qemu_rdma_exchange_send(rdma, &head, data, NULL, NULL, NULL);
 
-        if (ret < 0) {
-            rdma->error_state = ret;
-            return ret;
-        }
+            if (ret < 0) {
+                rdma->error_state = ret;
+                return ret;
+            }
 
-        data += r->len;
+            data += rioc->len;
+            done += rioc->len;
+        }
     }
 
-    return size;
+    return done;
 }
 
 static size_t qemu_rdma_fill(RDMAContext *rdma, uint8_t *buf,
@@ -2585,41 +2601,74 @@ static size_t qemu_rdma_fill(RDMAContext *rdma, uint8_t *buf,
  * RDMA links don't use bytestreams, so we have to
  * return bytes to QEMUFile opportunistically.
  */
-static ssize_t qemu_rdma_get_buffer(void *opaque, uint8_t *buf,
-                                    int64_t pos, size_t size)
-{
-    QEMUFileRDMA *r = opaque;
-    RDMAContext *rdma = r->rdma;
+static ssize_t qio_channel_rdma_readv(QIOChannel *ioc,
+                                      const struct iovec *iov,
+                                      size_t niov,
+                                      int **fds,
+                                      size_t *nfds,
+                                      Error **errp)
+{
+    QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(ioc);
+    RDMAContext *rdma = rioc->rdma;
     RDMAControlHeader head;
     int ret = 0;
+    ssize_t i;
+    size_t done = 0;
 
     CHECK_ERROR_STATE();
 
-    /*
-     * First, we hold on to the last SEND message we
-     * were given and dish out the bytes until we run
-     * out of bytes.
-     */
-    r->len = qemu_rdma_fill(r->rdma, buf, size, 0);
-    if (r->len) {
-        return r->len;
-    }
+    for (i = 0; i < niov; i++) {
+        size_t want = iov[i].iov_len;
+        uint8_t *data = (void *)iov[i].iov_base;
 
-    /*
-     * Once we run out, we block and wait for another
-     * SEND message to arrive.
-     */
-    ret = qemu_rdma_exchange_recv(rdma, &head, RDMA_CONTROL_QEMU_FILE);
+        /*
+         * First, we hold on to the last SEND message we
+         * were given and dish out the bytes until we run
+         * out of bytes.
+         */
+        ret = qemu_rdma_fill(rioc->rdma, data, want, 0);
+        done += ret;
+        want -= ret;
+        /* Got what we needed, so go to next iovec */
+        if (want == 0) {
+            continue;
+        }
 
-    if (ret < 0) {
-        rdma->error_state = ret;
-        return ret;
-    }
+        /* If we got any data so far, then don't wait
+         * for more, just return what we have */
+        if (done > 0) {
+            break;
+        }
 
-    /*
-     * SEND was received with new bytes, now try again.
-     */
-    return qemu_rdma_fill(r->rdma, buf, size, 0);
+
+        /* We've got nothing at all, so lets wait for
+         * more to arrive
+         */
+        ret = qemu_rdma_exchange_recv(rdma, &head, RDMA_CONTROL_QEMU_FILE);
+
+        if (ret < 0) {
+            rdma->error_state = ret;
+            return ret;
+        }
+
+        /*
+         * SEND was received with new bytes, now try again.
+         */
+        ret = qemu_rdma_fill(rioc->rdma, data, want, 0);
+        done += ret;
+        want -= ret;
+
+        /* Still didn't get enough, so lets just return */
+        if (want) {
+            if (done == 0) {
+                return QIO_CHANNEL_ERR_BLOCK;
+            } else {
+                break;
+            }
+        }
+    }
+    rioc->len = done;
+    return rioc->len;
 }
 
 /*
@@ -2646,15 +2695,122 @@ static int qemu_rdma_drain_cq(QEMUFile *f, RDMAContext *rdma)
     return 0;
 }
 
-static int qemu_rdma_close(void *opaque)
+
+static int qio_channel_rdma_set_blocking(QIOChannel *ioc,
+                                         bool blocking,
+                                         Error **errp)
+{
+    QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(ioc);
+    /* XXX we should make readv/writev actually honour this :-) */
+    rioc->blocking = blocking;
+    return 0;
+}
+
+
+typedef struct QIOChannelRDMASource QIOChannelRDMASource;
+struct QIOChannelRDMASource {
+    GSource parent;
+    QIOChannelRDMA *rioc;
+    GIOCondition condition;
+};
+
+static gboolean
+qio_channel_rdma_source_prepare(GSource *source,
+                                gint *timeout)
+{
+    QIOChannelRDMASource *rsource = (QIOChannelRDMASource *)source;
+    RDMAContext *rdma = rsource->rioc->rdma;
+    GIOCondition cond = 0;
+    *timeout = -1;
+
+    if (rdma->wr_data[0].control_len) {
+        cond |= G_IO_IN;
+    }
+    cond |= G_IO_OUT;
+
+    return cond & rsource->condition;
+}
+
+static gboolean
+qio_channel_rdma_source_check(GSource *source)
+{
+    QIOChannelRDMASource *rsource = (QIOChannelRDMASource *)source;
+    RDMAContext *rdma = rsource->rioc->rdma;
+    GIOCondition cond = 0;
+
+    if (rdma->wr_data[0].control_len) {
+        cond |= G_IO_IN;
+    }
+    cond |= G_IO_OUT;
+
+    return cond & rsource->condition;
+}
+
+static gboolean
+qio_channel_rdma_source_dispatch(GSource *source,
+                                 GSourceFunc callback,
+                                 gpointer user_data)
+{
+    QIOChannelFunc func = (QIOChannelFunc)callback;
+    QIOChannelRDMASource *rsource = (QIOChannelRDMASource *)source;
+    RDMAContext *rdma = rsource->rioc->rdma;
+    GIOCondition cond = 0;
+
+    if (rdma->wr_data[0].control_len) {
+        cond |= G_IO_IN;
+    }
+    cond |= G_IO_OUT;
+
+    return (*func)(QIO_CHANNEL(rsource->rioc),
+                   (cond & rsource->condition),
+                   user_data);
+}
+
+static void
+qio_channel_rdma_source_finalize(GSource *source)
+{
+    QIOChannelRDMASource *ssource = (QIOChannelRDMASource *)source;
+
+    object_unref(OBJECT(ssource->rioc));
+}
+
+GSourceFuncs qio_channel_rdma_source_funcs = {
+    qio_channel_rdma_source_prepare,
+    qio_channel_rdma_source_check,
+    qio_channel_rdma_source_dispatch,
+    qio_channel_rdma_source_finalize
+};
+
+static GSource *qio_channel_rdma_create_watch(QIOChannel *ioc,
+                                              GIOCondition condition)
 {
+    QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(ioc);
+    QIOChannelRDMASource *ssource;
+    GSource *source;
+
+    source = g_source_new(&qio_channel_rdma_source_funcs,
+                          sizeof(QIOChannelRDMASource));
+    ssource = (QIOChannelRDMASource *)source;
+
+    ssource->rioc = rioc;
+    object_ref(OBJECT(rioc));
+
+    ssource->condition = condition;
+
+    return source;
+}
+
+
+static int qio_channel_rdma_close(QIOChannel *ioc,
+                                  Error **errp)
+{
+    QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(ioc);
     trace_qemu_rdma_close();
-    QEMUFileRDMA *r = opaque;
-    if (r->rdma) {
-        qemu_rdma_cleanup(r->rdma);
-        g_free(r->rdma);
+    if (rioc->rdma) {
+        qemu_rdma_cleanup(rioc->rdma);
+        g_free(rioc->rdma);
+        rioc->rdma = NULL;
     }
-    g_free(r);
     return 0;
 }
 
@@ -2696,8 +2852,8 @@ static size_t qemu_rdma_save_page(QEMUFile *f, void *opaque,
                                   ram_addr_t block_offset, ram_addr_t offset,
                                   size_t size, uint64_t *bytes_sent)
 {
-    QEMUFileRDMA *rfile = opaque;
-    RDMAContext *rdma = rfile->rdma;
+    QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(opaque);
+    RDMAContext *rdma = rioc->rdma;
     int ret;
 
     CHECK_ERROR_STATE();
@@ -2951,8 +3107,8 @@ static int qemu_rdma_registration_handle(QEMUFile *f, void *opaque)
                              };
     RDMAControlHeader blocks = { .type = RDMA_CONTROL_RAM_BLOCKS_RESULT,
                                  .repeat = 1 };
-    QEMUFileRDMA *rfile = opaque;
-    RDMAContext *rdma = rfile->rdma;
+    QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(opaque);
+    RDMAContext *rdma = rioc->rdma;
     RDMALocalBlocks *local = &rdma->local_ram_blocks;
     RDMAControlHeader head;
     RDMARegister *reg, *registers;
@@ -3207,9 +3363,10 @@ out:
  * We've already built our local RAMBlock list, but not yet sent the list to
  * the source.
  */
-static int rdma_block_notification_handle(QEMUFileRDMA *rfile, const char *name)
+static int
+rdma_block_notification_handle(QIOChannelRDMA *rioc, const char *name)
 {
-    RDMAContext *rdma = rfile->rdma;
+    RDMAContext *rdma = rioc->rdma;
     int curr;
     int found = -1;
 
@@ -3251,8 +3408,8 @@ static int rdma_load_hook(QEMUFile *f, void *opaque, uint64_t flags, void *data)
 static int qemu_rdma_registration_start(QEMUFile *f, void *opaque,
                                         uint64_t flags, void *data)
 {
-    QEMUFileRDMA *rfile = opaque;
-    RDMAContext *rdma = rfile->rdma;
+    QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(opaque);
+    RDMAContext *rdma = rioc->rdma;
 
     CHECK_ERROR_STATE();
 
@@ -3271,8 +3428,8 @@ static int qemu_rdma_registration_stop(QEMUFile *f, void *opaque,
                                        uint64_t flags, void *data)
 {
     Error *local_err = NULL, **errp = &local_err;
-    QEMUFileRDMA *rfile = opaque;
-    RDMAContext *rdma = rfile->rdma;
+    QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(opaque);
+    RDMAContext *rdma = rioc->rdma;
     RDMAControlHeader head = { .len = 0, .repeat = 1 };
     int ret = 0;
 
@@ -3368,55 +3525,74 @@ err:
     return ret;
 }
 
-static int qemu_rdma_get_fd(void *opaque)
-{
-    QEMUFileRDMA *rfile = opaque;
-    RDMAContext *rdma = rfile->rdma;
-
-    return rdma->comp_channel->fd;
-}
-
-static const QEMUFileOps rdma_read_ops = {
-    .get_buffer    = qemu_rdma_get_buffer,
-    .get_fd        = qemu_rdma_get_fd,
-    .close         = qemu_rdma_close,
-};
-
 static const QEMUFileHooks rdma_read_hooks = {
     .hook_ram_load = rdma_load_hook,
 };
 
-static const QEMUFileOps rdma_write_ops = {
-    .put_buffer         = qemu_rdma_put_buffer,
-    .close              = qemu_rdma_close,
-};
-
 static const QEMUFileHooks rdma_write_hooks = {
     .before_ram_iterate = qemu_rdma_registration_start,
     .after_ram_iterate  = qemu_rdma_registration_stop,
     .save_page          = qemu_rdma_save_page,
 };
 
-static void *qemu_fopen_rdma(RDMAContext *rdma, const char *mode)
+
+static void qio_channel_rdma_finalize(Object *obj)
+{
+    QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(obj);
+    if (rioc->rdma) {
+        qemu_rdma_cleanup(rioc->rdma);
+        g_free(rioc->rdma);
+        rioc->rdma = NULL;
+    }
+}
+
+static void qio_channel_rdma_class_init(ObjectClass *klass,
+                                        void *class_data G_GNUC_UNUSED)
+{
+    QIOChannelClass *ioc_klass = QIO_CHANNEL_CLASS(klass);
+
+    ioc_klass->io_writev = qio_channel_rdma_writev;
+    ioc_klass->io_readv = qio_channel_rdma_readv;
+    ioc_klass->io_set_blocking = qio_channel_rdma_set_blocking;
+    ioc_klass->io_close = qio_channel_rdma_close;
+    ioc_klass->io_create_watch = qio_channel_rdma_create_watch;
+}
+
+static const TypeInfo qio_channel_rdma_info = {
+    .parent = TYPE_QIO_CHANNEL,
+    .name = TYPE_QIO_CHANNEL_RDMA,
+    .instance_size = sizeof(QIOChannelRDMA),
+    .instance_finalize = qio_channel_rdma_finalize,
+    .class_init = qio_channel_rdma_class_init,
+};
+
+static void qio_channel_rdma_register_types(void)
+{
+    type_register_static(&qio_channel_rdma_info);
+}
+
+type_init(qio_channel_rdma_register_types);
+
+static QEMUFile *qemu_fopen_rdma(RDMAContext *rdma, const char *mode)
 {
-    QEMUFileRDMA *r;
+    QIOChannelRDMA *rioc;
 
     if (qemu_file_mode_is_not_valid(mode)) {
         return NULL;
     }
 
-    r = g_new0(QEMUFileRDMA, 1);
-    r->rdma = rdma;
+    rioc = QIO_CHANNEL_RDMA(object_new(TYPE_QIO_CHANNEL_RDMA));
+    rioc->rdma = rdma;
 
     if (mode[0] == 'w') {
-        r->file = qemu_fopen_ops(r, &rdma_write_ops);
-        qemu_file_set_hooks(r->file, &rdma_write_hooks);
+        rioc->file = qemu_fopen_channel_output(QIO_CHANNEL(rioc));
+        qemu_file_set_hooks(rioc->file, &rdma_write_hooks);
     } else {
-        r->file = qemu_fopen_ops(r, &rdma_read_ops);
-        qemu_file_set_hooks(r->file, &rdma_read_hooks);
+        rioc->file = qemu_fopen_channel_input(QIO_CHANNEL(rioc));
+        qemu_file_set_hooks(rioc->file, &rdma_read_hooks);
     }
 
-    return r->file;
+    return rioc->file;
 }
 
 static void rdma_accept_incoming_migration(void *opaque)
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PULL 18/28] migration: convert savevm to use QIOChannel for writing to files
  2016-05-26  6:11 [Qemu-devel] [PULL 00/28] migration: support for TLS Amit Shah
                   ` (16 preceding siblings ...)
  2016-05-26  6:12 ` [Qemu-devel] [PULL 17/28] migration: convert RDMA to use QIOChannel interface Amit Shah
@ 2016-05-26  6:12 ` Amit Shah
  2016-05-26  6:12 ` [Qemu-devel] [PULL 19/28] migration: delete QEMUFile buffer implementation Amit Shah
                   ` (10 subsequent siblings)
  28 siblings, 0 replies; 36+ messages in thread
From: Amit Shah @ 2016-05-26  6:12 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Juan Quintela, Dr. David Alan Gilbert, Daniel P. Berrange,
	qemu list, Amit Shah

From: "Daniel P. Berrange" <berrange@redhat.com>

Convert the exec savevm code to use QIOChannel and QEMUFileChannel,
instead of the stdio APIs.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-Id: <1461751518-12128-19-git-send-email-berrange@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 migration/savevm.c   |  8 +++++---
 tests/Makefile       |  4 ++--
 tests/test-vmstate.c | 11 ++++++++++-
 3 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index 43031a0..2bd3452 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -52,6 +52,7 @@
 #include "block/qapi.h"
 #include "qemu/cutils.h"
 #include "io/channel-buffer.h"
+#include "io/channel-file.h"
 
 #ifndef ETH_P_RARP
 #define ETH_P_RARP 0x8035
@@ -2046,6 +2047,7 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
 void qmp_xen_save_devices_state(const char *filename, Error **errp)
 {
     QEMUFile *f;
+    QIOChannelFile *ioc;
     int saved_vm_running;
     int ret;
 
@@ -2053,11 +2055,11 @@ void qmp_xen_save_devices_state(const char *filename, Error **errp)
     vm_stop(RUN_STATE_SAVE_VM);
     global_state_store_running();
 
-    f = qemu_fopen(filename, "wb");
-    if (!f) {
-        error_setg_file_open(errp, errno, filename);
+    ioc = qio_channel_file_new_path(filename, O_WRONLY | O_CREAT, 0660, errp);
+    if (!ioc) {
         goto the_end;
     }
+    f = qemu_fopen_channel_output(QIO_CHANNEL(ioc));
     ret = qemu_save_device_state(f);
     qemu_fclose(f);
     if (ret < 0) {
diff --git a/tests/Makefile b/tests/Makefile
index b5cb75e..8397b96 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -439,8 +439,8 @@ tests/test-qdev-global-props$(EXESUF): tests/test-qdev-global-props.o \
 	$(test-qapi-obj-y)
 tests/test-vmstate$(EXESUF): tests/test-vmstate.o \
 	migration/vmstate.o migration/qemu-file.o \
-        migration/qemu-file-unix.o migration/qjson.o \
-	$(test-qom-obj-y)
+        migration/qemu-file-channel.o migration/qjson.o \
+	$(test-io-obj-y)
 tests/test-timed-average$(EXESUF): tests/test-timed-average.o qemu-timer.o \
 	$(test-util-obj-y)
 tests/test-base64$(EXESUF): tests/test-base64.o \
diff --git a/tests/test-vmstate.c b/tests/test-vmstate.c
index f337cf6..d19b16a 100644
--- a/tests/test-vmstate.c
+++ b/tests/test-vmstate.c
@@ -29,6 +29,7 @@
 #include "migration/migration.h"
 #include "migration/vmstate.h"
 #include "qemu/coroutine.h"
+#include "io/channel-file.h"
 
 static char temp_file[] = "/tmp/vmst.test.XXXXXX";
 static int temp_fd;
@@ -49,11 +50,17 @@ void yield_until_fd_readable(int fd)
 static QEMUFile *open_test_file(bool write)
 {
     int fd = dup(temp_fd);
+    QIOChannel *ioc;
     lseek(fd, 0, SEEK_SET);
     if (write) {
         g_assert_cmpint(ftruncate(fd, 0), ==, 0);
     }
-    return qemu_fdopen(fd, write ? "wb" : "rb");
+    ioc = QIO_CHANNEL(qio_channel_file_new_fd(fd));
+    if (write) {
+        return qemu_fopen_channel_output(ioc);
+    } else {
+        return qemu_fopen_channel_input(ioc);
+    }
 }
 
 #define SUCCESS(val) \
@@ -469,6 +476,8 @@ int main(int argc, char **argv)
 {
     temp_fd = mkstemp(temp_file);
 
+    module_call_init(MODULE_INIT_QOM);
+
     g_test_init(&argc, &argv, NULL);
     g_test_add_func("/vmstate/simple/primitive", test_simple_primitive);
     g_test_add_func("/vmstate/versioned/load/v1", test_load_v1);
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PULL 19/28] migration: delete QEMUFile buffer implementation
  2016-05-26  6:11 [Qemu-devel] [PULL 00/28] migration: support for TLS Amit Shah
                   ` (17 preceding siblings ...)
  2016-05-26  6:12 ` [Qemu-devel] [PULL 18/28] migration: convert savevm to use QIOChannel for writing to files Amit Shah
@ 2016-05-26  6:12 ` Amit Shah
  2016-05-26  6:12 ` [Qemu-devel] [PULL 20/28] migration: delete QEMUSizedBuffer struct Amit Shah
                   ` (9 subsequent siblings)
  28 siblings, 0 replies; 36+ messages in thread
From: Amit Shah @ 2016-05-26  6:12 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Juan Quintela, Dr. David Alan Gilbert, Daniel P. Berrange,
	qemu list, Amit Shah

From: "Daniel P. Berrange" <berrange@redhat.com>

The qemu_bufopen() method is no longer used, so the memory
buffer based QEMUFile backend can be deleted entirely.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-Id: <1461751518-12128-20-git-send-email-berrange@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 include/migration/qemu-file.h |  6 ---
 migration/qemu-file-buf.c     | 96 -------------------------------------------
 2 files changed, 102 deletions(-)

diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
index 0329ccc..6618d19 100644
--- a/include/migration/qemu-file.h
+++ b/include/migration/qemu-file.h
@@ -140,7 +140,6 @@ QEMUFile *qemu_fopen_socket(int fd, const char *mode);
 QEMUFile *qemu_fopen_channel_input(QIOChannel *ioc);
 QEMUFile *qemu_fopen_channel_output(QIOChannel *ioc);
 QEMUFile *qemu_popen_cmd(const char *command, const char *mode);
-QEMUFile *qemu_bufopen(const char *mode, QEMUSizedBuffer *input);
 void qemu_file_set_hooks(QEMUFile *f, const QEMUFileHooks *hooks);
 int qemu_get_fd(QEMUFile *f);
 int qemu_fclose(QEMUFile *f);
@@ -166,11 +165,6 @@ ssize_t qsb_write_at(QEMUSizedBuffer *qsb, const uint8_t *buf,
                      off_t pos, size_t count);
 
 
-/*
- * For use on files opened with qemu_bufopen
- */
-const QEMUSizedBuffer *qemu_buf_get(QEMUFile *f);
-
 static inline void qemu_put_ubyte(QEMUFile *f, unsigned int v)
 {
     qemu_put_byte(f, (int)v);
diff --git a/migration/qemu-file-buf.c b/migration/qemu-file-buf.c
index 7b8e78e..668ab35 100644
--- a/migration/qemu-file-buf.c
+++ b/migration/qemu-file-buf.c
@@ -366,99 +366,3 @@ ssize_t qsb_write_at(QEMUSizedBuffer *qsb, const uint8_t *source,
 
     return count;
 }
-
-typedef struct QEMUBuffer {
-    QEMUSizedBuffer *qsb;
-    QEMUFile *file;
-    bool qsb_allocated;
-} QEMUBuffer;
-
-static ssize_t buf_get_buffer(void *opaque, uint8_t *buf, int64_t pos,
-                              size_t size)
-{
-    QEMUBuffer *s = opaque;
-    ssize_t len = qsb_get_length(s->qsb) - pos;
-
-    if (len <= 0) {
-        return 0;
-    }
-
-    if (len > size) {
-        len = size;
-    }
-    return qsb_get_buffer(s->qsb, pos, len, buf);
-}
-
-static ssize_t buf_put_buffer(void *opaque, const uint8_t *buf,
-                              int64_t pos, size_t size)
-{
-    QEMUBuffer *s = opaque;
-
-    return qsb_write_at(s->qsb, buf, pos, size);
-}
-
-static int buf_close(void *opaque)
-{
-    QEMUBuffer *s = opaque;
-
-    if (s->qsb_allocated) {
-        qsb_free(s->qsb);
-    }
-
-    g_free(s);
-
-    return 0;
-}
-
-const QEMUSizedBuffer *qemu_buf_get(QEMUFile *f)
-{
-    QEMUBuffer *p;
-
-    qemu_fflush(f);
-
-    p = f->opaque;
-
-    return p->qsb;
-}
-
-static const QEMUFileOps buf_read_ops = {
-    .get_buffer = buf_get_buffer,
-    .close =      buf_close,
-};
-
-static const QEMUFileOps buf_write_ops = {
-    .put_buffer = buf_put_buffer,
-    .close =      buf_close,
-};
-
-QEMUFile *qemu_bufopen(const char *mode, QEMUSizedBuffer *input)
-{
-    QEMUBuffer *s;
-
-    if (mode == NULL || (mode[0] != 'r' && mode[0] != 'w') ||
-        mode[1] != '\0') {
-        error_report("qemu_bufopen: Argument validity check failed");
-        return NULL;
-    }
-
-    s = g_new0(QEMUBuffer, 1);
-    s->qsb = input;
-
-    if (s->qsb == NULL) {
-        s->qsb = qsb_create(NULL, 0);
-        s->qsb_allocated = true;
-    }
-    if (!s->qsb) {
-        g_free(s);
-        error_report("qemu_bufopen: qsb_create failed");
-        return NULL;
-    }
-
-
-    if (mode[0] == 'r') {
-        s->file = qemu_fopen_ops(s, &buf_read_ops);
-    } else {
-        s->file = qemu_fopen_ops(s, &buf_write_ops);
-    }
-    return s->file;
-}
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PULL 20/28] migration: delete QEMUSizedBuffer struct
  2016-05-26  6:11 [Qemu-devel] [PULL 00/28] migration: support for TLS Amit Shah
                   ` (18 preceding siblings ...)
  2016-05-26  6:12 ` [Qemu-devel] [PULL 19/28] migration: delete QEMUFile buffer implementation Amit Shah
@ 2016-05-26  6:12 ` Amit Shah
  2016-05-26  6:12 ` [Qemu-devel] [PULL 21/28] migration: delete QEMUFile sockets implementation Amit Shah
                   ` (8 subsequent siblings)
  28 siblings, 0 replies; 36+ messages in thread
From: Amit Shah @ 2016-05-26  6:12 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Juan Quintela, Dr. David Alan Gilbert, Daniel P. Berrange,
	qemu list, Amit Shah

From: "Daniel P. Berrange" <berrange@redhat.com>

Now that we don't have have a buffer based QemuFile
implementation, the QEMUSizedBuffer code is also
unused and can be deleted. A simpler buffer class
also exists in util/buffer.c which other code can
used as needed.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-Id: <1461751518-12128-21-git-send-email-berrange@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 include/migration/qemu-file.h |  16 --
 include/qemu/typedefs.h       |   1 -
 migration/Makefile.objs       |   2 +-
 migration/qemu-file-buf.c     | 368 ------------------------------------------
 4 files changed, 1 insertion(+), 386 deletions(-)
 delete mode 100644 migration/qemu-file-buf.c

diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
index 6618d19..edaf598 100644
--- a/include/migration/qemu-file.h
+++ b/include/migration/qemu-file.h
@@ -126,13 +126,6 @@ typedef struct QEMUFileHooks {
     QEMURamSaveFunc *save_page;
 } QEMUFileHooks;
 
-struct QEMUSizedBuffer {
-    struct iovec *iov;
-    size_t n_iov;
-    size_t size; /* total allocated size in all iov's */
-    size_t used; /* number of used bytes */
-};
-
 QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops);
 QEMUFile *qemu_fopen(const char *filename, const char *mode);
 QEMUFile *qemu_fdopen(int fd, const char *mode);
@@ -155,15 +148,6 @@ void qemu_put_buffer_async(QEMUFile *f, const uint8_t *buf, size_t size);
 bool qemu_file_mode_is_not_valid(const char *mode);
 bool qemu_file_is_writable(QEMUFile *f);
 
-QEMUSizedBuffer *qsb_create(const uint8_t *buffer, size_t len);
-void qsb_free(QEMUSizedBuffer *);
-size_t qsb_set_length(QEMUSizedBuffer *qsb, size_t length);
-size_t qsb_get_length(const QEMUSizedBuffer *qsb);
-ssize_t qsb_get_buffer(const QEMUSizedBuffer *, off_t start, size_t count,
-                       uint8_t *buf);
-ssize_t qsb_write_at(QEMUSizedBuffer *qsb, const uint8_t *buf,
-                     off_t pos, size_t count);
-
 
 static inline void qemu_put_ubyte(QEMUFile *f, unsigned int v)
 {
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index 1dcf6f5..b113fcf 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -82,7 +82,6 @@ typedef struct QemuOpt QemuOpt;
 typedef struct QemuOpts QemuOpts;
 typedef struct QemuOptsList QemuOptsList;
 typedef struct QEMUSGList QEMUSGList;
-typedef struct QEMUSizedBuffer QEMUSizedBuffer;
 typedef struct QEMUTimer QEMUTimer;
 typedef struct QEMUTimerListGroup QEMUTimerListGroup;
 typedef struct QObject QObject;
diff --git a/migration/Makefile.objs b/migration/Makefile.objs
index c73fb8a..9e977a4 100644
--- a/migration/Makefile.objs
+++ b/migration/Makefile.objs
@@ -1,6 +1,6 @@
 common-obj-y += migration.o socket.o fd.o exec.o
 common-obj-y += vmstate.o
-common-obj-y += qemu-file.o qemu-file-buf.o qemu-file-unix.o qemu-file-stdio.o
+common-obj-y += qemu-file.o qemu-file-unix.o qemu-file-stdio.o
 common-obj-y += qemu-file-channel.o
 common-obj-y += xbzrle.o postcopy-ram.o
 common-obj-y += qjson.o
diff --git a/migration/qemu-file-buf.c b/migration/qemu-file-buf.c
deleted file mode 100644
index 668ab35..0000000
--- a/migration/qemu-file-buf.c
+++ /dev/null
@@ -1,368 +0,0 @@
-/*
- * QEMU System Emulator
- *
- * Copyright (c) 2003-2008 Fabrice Bellard
- * Copyright (c) 2014 IBM Corp.
- *
- * Authors:
- *  Stefan Berger <stefanb@linux.vnet.ibm.com>
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
- */
-#include "qemu/osdep.h"
-#include "qemu-common.h"
-#include "qemu/error-report.h"
-#include "qemu/iov.h"
-#include "qemu/sockets.h"
-#include "qemu/coroutine.h"
-#include "migration/migration.h"
-#include "migration/qemu-file.h"
-#include "migration/qemu-file-internal.h"
-#include "trace.h"
-
-#define QSB_CHUNK_SIZE      (1 << 10)
-#define QSB_MAX_CHUNK_SIZE  (16 * QSB_CHUNK_SIZE)
-
-/**
- * Create a QEMUSizedBuffer
- * This type of buffer uses scatter-gather lists internally and
- * can grow to any size. Any data array in the scatter-gather list
- * can hold different amount of bytes.
- *
- * @buffer: Optional buffer to copy into the QSB
- * @len: size of initial buffer; if @buffer is given, buffer must
- *       hold at least len bytes
- *
- * Returns a pointer to a QEMUSizedBuffer or NULL on allocation failure
- */
-QEMUSizedBuffer *qsb_create(const uint8_t *buffer, size_t len)
-{
-    QEMUSizedBuffer *qsb;
-    size_t alloc_len, num_chunks, i, to_copy;
-    size_t chunk_size = (len > QSB_MAX_CHUNK_SIZE)
-                        ? QSB_MAX_CHUNK_SIZE
-                        : QSB_CHUNK_SIZE;
-
-    num_chunks = DIV_ROUND_UP(len ? len : QSB_CHUNK_SIZE, chunk_size);
-    alloc_len = num_chunks * chunk_size;
-
-    qsb = g_try_new0(QEMUSizedBuffer, 1);
-    if (!qsb) {
-        return NULL;
-    }
-
-    qsb->iov = g_try_new0(struct iovec, num_chunks);
-    if (!qsb->iov) {
-        g_free(qsb);
-        return NULL;
-    }
-
-    qsb->n_iov = num_chunks;
-
-    for (i = 0; i < num_chunks; i++) {
-        qsb->iov[i].iov_base = g_try_malloc0(chunk_size);
-        if (!qsb->iov[i].iov_base) {
-            /* qsb_free is safe since g_free can cope with NULL */
-            qsb_free(qsb);
-            return NULL;
-        }
-
-        qsb->iov[i].iov_len = chunk_size;
-        if (buffer) {
-            to_copy = (len - qsb->used) > chunk_size
-                      ? chunk_size : (len - qsb->used);
-            memcpy(qsb->iov[i].iov_base, &buffer[qsb->used], to_copy);
-            qsb->used += to_copy;
-        }
-    }
-
-    qsb->size = alloc_len;
-
-    return qsb;
-}
-
-/**
- * Free the QEMUSizedBuffer
- *
- * @qsb: The QEMUSizedBuffer to free
- */
-void qsb_free(QEMUSizedBuffer *qsb)
-{
-    size_t i;
-
-    if (!qsb) {
-        return;
-    }
-
-    for (i = 0; i < qsb->n_iov; i++) {
-        g_free(qsb->iov[i].iov_base);
-    }
-    g_free(qsb->iov);
-    g_free(qsb);
-}
-
-/**
- * Get the number of used bytes in the QEMUSizedBuffer
- *
- * @qsb: A QEMUSizedBuffer
- *
- * Returns the number of bytes currently used in this buffer
- */
-size_t qsb_get_length(const QEMUSizedBuffer *qsb)
-{
-    return qsb->used;
-}
-
-/**
- * Set the length of the buffer; the primary usage of this
- * function is to truncate the number of used bytes in the buffer.
- * The size will not be extended beyond the current number of
- * allocated bytes in the QEMUSizedBuffer.
- *
- * @qsb: A QEMUSizedBuffer
- * @new_len: The new length of bytes in the buffer
- *
- * Returns the number of bytes the buffer was truncated or extended
- * to.
- */
-size_t qsb_set_length(QEMUSizedBuffer *qsb, size_t new_len)
-{
-    if (new_len <= qsb->size) {
-        qsb->used = new_len;
-    } else {
-        qsb->used = qsb->size;
-    }
-    return qsb->used;
-}
-
-/**
- * Get the iovec that holds the data for a given position @pos.
- *
- * @qsb: A QEMUSizedBuffer
- * @pos: The index of a byte in the buffer
- * @d_off: Pointer to an offset that this function will indicate
- *         at what position within the returned iovec the byte
- *         is to be found
- *
- * Returns the index of the iovec that holds the byte at the given
- * index @pos in the byte stream; a negative number if the iovec
- * for the given position @pos does not exist.
- */
-static ssize_t qsb_get_iovec(const QEMUSizedBuffer *qsb,
-                             off_t pos, off_t *d_off)
-{
-    ssize_t i;
-    off_t curr = 0;
-
-    if (pos > qsb->used) {
-        return -1;
-    }
-
-    for (i = 0; i < qsb->n_iov; i++) {
-        if (curr + qsb->iov[i].iov_len > pos) {
-            *d_off = pos - curr;
-            return i;
-        }
-        curr += qsb->iov[i].iov_len;
-    }
-    return -1;
-}
-
-/*
- * Convert the QEMUSizedBuffer into a flat buffer.
- *
- * Note: If at all possible, try to avoid this function since it
- *       may unnecessarily copy memory around.
- *
- * @qsb: pointer to QEMUSizedBuffer
- * @start: offset to start at
- * @count: number of bytes to copy
- * @buf: a pointer to a buffer to write into (at least @count bytes)
- *
- * Returns the number of bytes copied into the output buffer
- */
-ssize_t qsb_get_buffer(const QEMUSizedBuffer *qsb, off_t start,
-                       size_t count, uint8_t *buffer)
-{
-    const struct iovec *iov;
-    size_t to_copy, all_copy;
-    ssize_t index;
-    off_t s_off;
-    off_t d_off = 0;
-    char *s;
-
-    if (start > qsb->used) {
-        return 0;
-    }
-
-    all_copy = qsb->used - start;
-    if (all_copy > count) {
-        all_copy = count;
-    } else {
-        count = all_copy;
-    }
-
-    index = qsb_get_iovec(qsb, start, &s_off);
-    if (index < 0) {
-        return 0;
-    }
-
-    while (all_copy > 0) {
-        iov = &qsb->iov[index];
-
-        s = iov->iov_base;
-
-        to_copy = iov->iov_len - s_off;
-        if (to_copy > all_copy) {
-            to_copy = all_copy;
-        }
-        memcpy(&buffer[d_off], &s[s_off], to_copy);
-
-        d_off += to_copy;
-        all_copy -= to_copy;
-
-        s_off = 0;
-        index++;
-    }
-
-    return count;
-}
-
-/**
- * Grow the QEMUSizedBuffer to the given size and allocate
- * memory for it.
- *
- * @qsb: A QEMUSizedBuffer
- * @new_size: The new size of the buffer
- *
- * Return:
- *    a negative error code in case of memory allocation failure
- * or
- *    the new size of the buffer. The returned size may be greater or equal
- *    to @new_size.
- */
-static ssize_t qsb_grow(QEMUSizedBuffer *qsb, size_t new_size)
-{
-    size_t needed_chunks, i;
-
-    if (qsb->size < new_size) {
-        struct iovec *new_iov;
-        size_t size_diff = new_size - qsb->size;
-        size_t chunk_size = (size_diff > QSB_MAX_CHUNK_SIZE)
-                             ? QSB_MAX_CHUNK_SIZE : QSB_CHUNK_SIZE;
-
-        needed_chunks = DIV_ROUND_UP(size_diff, chunk_size);
-
-        new_iov = g_try_new(struct iovec, qsb->n_iov + needed_chunks);
-        if (new_iov == NULL) {
-            return -ENOMEM;
-        }
-
-        /* Allocate new chunks as needed into new_iov */
-        for (i = qsb->n_iov; i < qsb->n_iov + needed_chunks; i++) {
-            new_iov[i].iov_base = g_try_malloc0(chunk_size);
-            new_iov[i].iov_len = chunk_size;
-            if (!new_iov[i].iov_base) {
-                size_t j;
-
-                /* Free previously allocated new chunks */
-                for (j = qsb->n_iov; j < i; j++) {
-                    g_free(new_iov[j].iov_base);
-                }
-                g_free(new_iov);
-
-                return -ENOMEM;
-            }
-        }
-
-        /*
-         * Now we can't get any allocation errors, copy over to new iov
-         * and switch.
-         */
-        for (i = 0; i < qsb->n_iov; i++) {
-            new_iov[i] = qsb->iov[i];
-        }
-
-        qsb->n_iov += needed_chunks;
-        g_free(qsb->iov);
-        qsb->iov = new_iov;
-        qsb->size += (needed_chunks * chunk_size);
-    }
-
-    return qsb->size;
-}
-
-/**
- * Write into the QEMUSizedBuffer at a given position and a given
- * number of bytes. This function will automatically grow the
- * QEMUSizedBuffer.
- *
- * @qsb: A QEMUSizedBuffer
- * @source: A byte array to copy data from
- * @pos: The position within the @qsb to write data to
- * @size: The number of bytes to copy into the @qsb
- *
- * Returns @size or a negative error code in case of memory allocation failure,
- *           or with an invalid 'pos'
- */
-ssize_t qsb_write_at(QEMUSizedBuffer *qsb, const uint8_t *source,
-                     off_t pos, size_t count)
-{
-    ssize_t rc = qsb_grow(qsb, pos + count);
-    size_t to_copy;
-    size_t all_copy = count;
-    const struct iovec *iov;
-    ssize_t index;
-    char *dest;
-    off_t d_off, s_off = 0;
-
-    if (rc < 0) {
-        return rc;
-    }
-
-    if (pos + count > qsb->used) {
-        qsb->used = pos + count;
-    }
-
-    index = qsb_get_iovec(qsb, pos, &d_off);
-    if (index < 0) {
-        return -EINVAL;
-    }
-
-    while (all_copy > 0) {
-        iov = &qsb->iov[index];
-
-        dest = iov->iov_base;
-
-        to_copy = iov->iov_len - d_off;
-        if (to_copy > all_copy) {
-            to_copy = all_copy;
-        }
-
-        memcpy(&dest[d_off], &source[s_off], to_copy);
-
-        s_off += to_copy;
-        all_copy -= to_copy;
-
-        d_off = 0;
-        index++;
-    }
-
-    return count;
-}
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PULL 21/28] migration: delete QEMUFile sockets implementation
  2016-05-26  6:11 [Qemu-devel] [PULL 00/28] migration: support for TLS Amit Shah
                   ` (19 preceding siblings ...)
  2016-05-26  6:12 ` [Qemu-devel] [PULL 20/28] migration: delete QEMUSizedBuffer struct Amit Shah
@ 2016-05-26  6:12 ` Amit Shah
  2016-05-26  6:12 ` [Qemu-devel] [PULL 22/28] migration: delete QEMUFile stdio implementation Amit Shah
                   ` (7 subsequent siblings)
  28 siblings, 0 replies; 36+ messages in thread
From: Amit Shah @ 2016-05-26  6:12 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Juan Quintela, Dr. David Alan Gilbert, Daniel P. Berrange,
	qemu list, Amit Shah

From: "Daniel P. Berrange" <berrange@redhat.com>

Now that the tcp, unix and fd migration backends have converted
to use the QIOChannel based QEMUFile, there is no user remaining
for the sockets based QEMUFile impl and it can be deleted.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-Id: <1461751518-12128-22-git-send-email-berrange@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 include/migration/qemu-file.h |   2 -
 migration/Makefile.objs       |   2 +-
 migration/qemu-file-unix.c    | 323 ------------------------------------------
 3 files changed, 1 insertion(+), 326 deletions(-)
 delete mode 100644 migration/qemu-file-unix.c

diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
index edaf598..ba5fe08 100644
--- a/include/migration/qemu-file.h
+++ b/include/migration/qemu-file.h
@@ -128,8 +128,6 @@ typedef struct QEMUFileHooks {
 
 QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops);
 QEMUFile *qemu_fopen(const char *filename, const char *mode);
-QEMUFile *qemu_fdopen(int fd, const char *mode);
-QEMUFile *qemu_fopen_socket(int fd, const char *mode);
 QEMUFile *qemu_fopen_channel_input(QIOChannel *ioc);
 QEMUFile *qemu_fopen_channel_output(QIOChannel *ioc);
 QEMUFile *qemu_popen_cmd(const char *command, const char *mode);
diff --git a/migration/Makefile.objs b/migration/Makefile.objs
index 9e977a4..5260048 100644
--- a/migration/Makefile.objs
+++ b/migration/Makefile.objs
@@ -1,6 +1,6 @@
 common-obj-y += migration.o socket.o fd.o exec.o
 common-obj-y += vmstate.o
-common-obj-y += qemu-file.o qemu-file-unix.o qemu-file-stdio.o
+common-obj-y += qemu-file.o qemu-file-stdio.o
 common-obj-y += qemu-file-channel.o
 common-obj-y += xbzrle.o postcopy-ram.o
 common-obj-y += qjson.o
diff --git a/migration/qemu-file-unix.c b/migration/qemu-file-unix.c
deleted file mode 100644
index 4474e18..0000000
--- a/migration/qemu-file-unix.c
+++ /dev/null
@@ -1,323 +0,0 @@
-/*
- * QEMU System Emulator
- *
- * Copyright (c) 2003-2008 Fabrice Bellard
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
- */
-#include "qemu/osdep.h"
-#include "qemu-common.h"
-#include "qemu/error-report.h"
-#include "qemu/iov.h"
-#include "qemu/sockets.h"
-#include "qemu/coroutine.h"
-#include "migration/qemu-file.h"
-#include "migration/qemu-file-internal.h"
-
-typedef struct QEMUFileSocket {
-    int fd;
-    QEMUFile *file;
-} QEMUFileSocket;
-
-static ssize_t socket_writev_buffer(void *opaque, struct iovec *iov, int iovcnt,
-                                    int64_t pos)
-{
-    QEMUFileSocket *s = opaque;
-    ssize_t len;
-    ssize_t size = iov_size(iov, iovcnt);
-    ssize_t offset = 0;
-    int     err;
-
-    while (size > 0) {
-        len = iov_send(s->fd, iov, iovcnt, offset, size);
-
-        if (len > 0) {
-            size -= len;
-            offset += len;
-        }
-
-        if (size > 0) {
-            if (errno != EAGAIN && errno != EWOULDBLOCK) {
-                error_report("socket_writev_buffer: Got err=%d for (%zu/%zu)",
-                             errno, (size_t)size, (size_t)len);
-                /*
-                 * If I've already sent some but only just got the error, I
-                 * could return the amount validly sent so far and wait for the
-                 * next call to report the error, but I'd rather flag the error
-                 * immediately.
-                 */
-                return -errno;
-            }
-
-            /* Emulate blocking */
-            GPollFD pfd;
-
-            pfd.fd = s->fd;
-            pfd.events = G_IO_OUT | G_IO_ERR;
-            pfd.revents = 0;
-            TFR(err = g_poll(&pfd, 1, -1 /* no timeout */));
-            /* Errors other than EINTR intentionally ignored */
-        }
-     }
-
-    return offset;
-}
-
-static int socket_get_fd(void *opaque)
-{
-    QEMUFileSocket *s = opaque;
-
-    return s->fd;
-}
-
-static ssize_t socket_get_buffer(void *opaque, uint8_t *buf, int64_t pos,
-                                 size_t size)
-{
-    QEMUFileSocket *s = opaque;
-    ssize_t len;
-
-    for (;;) {
-        len = qemu_recv(s->fd, buf, size, 0);
-        if (len != -1) {
-            break;
-        }
-        if (errno == EAGAIN) {
-            yield_until_fd_readable(s->fd);
-        } else if (errno != EINTR) {
-            break;
-        }
-    }
-
-    if (len == -1) {
-        len = -errno;
-    }
-    return len;
-}
-
-static int socket_close(void *opaque)
-{
-    QEMUFileSocket *s = opaque;
-    closesocket(s->fd);
-    g_free(s);
-    return 0;
-}
-
-static int socket_shutdown(void *opaque, bool rd, bool wr)
-{
-    QEMUFileSocket *s = opaque;
-
-    if (shutdown(s->fd, rd ? (wr ? SHUT_RDWR : SHUT_RD) : SHUT_WR)) {
-        return -errno;
-    } else {
-        return 0;
-    }
-}
-
-static int socket_return_close(void *opaque)
-{
-    QEMUFileSocket *s = opaque;
-    /*
-     * Note: We don't close the socket, that should be done by the forward
-     * path.
-     */
-    g_free(s);
-    return 0;
-}
-
-static const QEMUFileOps socket_return_read_ops = {
-    .get_fd          = socket_get_fd,
-    .get_buffer      = socket_get_buffer,
-    .close           = socket_return_close,
-    .shut_down       = socket_shutdown,
-};
-
-static const QEMUFileOps socket_return_write_ops = {
-    .get_fd          = socket_get_fd,
-    .writev_buffer   = socket_writev_buffer,
-    .close           = socket_return_close,
-    .shut_down       = socket_shutdown,
-};
-
-/*
- * Give a QEMUFile* off the same socket but data in the opposite
- * direction.
- */
-static QEMUFile *socket_get_return_path(void *opaque)
-{
-    QEMUFileSocket *forward = opaque;
-    QEMUFileSocket *reverse;
-
-    if (qemu_file_get_error(forward->file)) {
-        /* If the forward file is in error, don't try and open a return */
-        return NULL;
-    }
-
-    reverse = g_malloc0(sizeof(QEMUFileSocket));
-    reverse->fd = forward->fd;
-    /* I don't think there's a better way to tell which direction 'this' is */
-    if (forward->file->ops->get_buffer != NULL) {
-        /* being called from the read side, so we need to be able to write */
-        return qemu_fopen_ops(reverse, &socket_return_write_ops);
-    } else {
-        return qemu_fopen_ops(reverse, &socket_return_read_ops);
-    }
-}
-
-static ssize_t unix_writev_buffer(void *opaque, struct iovec *iov, int iovcnt,
-                                  int64_t pos)
-{
-    QEMUFileSocket *s = opaque;
-    ssize_t len, offset;
-    ssize_t size = iov_size(iov, iovcnt);
-    ssize_t total = 0;
-
-    assert(iovcnt > 0);
-    offset = 0;
-    while (size > 0) {
-        /* Find the next start position; skip all full-sized vector elements  */
-        while (offset >= iov[0].iov_len) {
-            offset -= iov[0].iov_len;
-            iov++, iovcnt--;
-        }
-
-        /* skip `offset' bytes from the (now) first element, undo it on exit */
-        assert(iovcnt > 0);
-        iov[0].iov_base += offset;
-        iov[0].iov_len -= offset;
-
-        do {
-            len = writev(s->fd, iov, iovcnt);
-        } while (len == -1 && errno == EINTR);
-        if (len == -1) {
-            return -errno;
-        }
-
-        /* Undo the changes above */
-        iov[0].iov_base -= offset;
-        iov[0].iov_len += offset;
-
-        /* Prepare for the next iteration */
-        offset += len;
-        total += len;
-        size -= len;
-    }
-
-    return total;
-}
-
-static ssize_t unix_get_buffer(void *opaque, uint8_t *buf, int64_t pos,
-                              size_t size)
-{
-    QEMUFileSocket *s = opaque;
-    ssize_t len;
-
-    for (;;) {
-        len = read(s->fd, buf, size);
-        if (len != -1) {
-            break;
-        }
-        if (errno == EAGAIN) {
-            yield_until_fd_readable(s->fd);
-        } else if (errno != EINTR) {
-            break;
-        }
-    }
-
-    if (len == -1) {
-        len = -errno;
-    }
-    return len;
-}
-
-static int unix_close(void *opaque)
-{
-    QEMUFileSocket *s = opaque;
-    close(s->fd);
-    g_free(s);
-    return 0;
-}
-
-static const QEMUFileOps unix_read_ops = {
-    .get_fd =     socket_get_fd,
-    .get_buffer = unix_get_buffer,
-    .close =      unix_close
-};
-
-static const QEMUFileOps unix_write_ops = {
-    .get_fd =     socket_get_fd,
-    .writev_buffer = unix_writev_buffer,
-    .close =      unix_close
-};
-
-QEMUFile *qemu_fdopen(int fd, const char *mode)
-{
-    QEMUFileSocket *s;
-
-    if (mode == NULL ||
-        (mode[0] != 'r' && mode[0] != 'w') ||
-        mode[1] != 'b' || mode[2] != 0) {
-        fprintf(stderr, "qemu_fdopen: Argument validity check failed\n");
-        return NULL;
-    }
-
-    s = g_new0(QEMUFileSocket, 1);
-    s->fd = fd;
-
-    if (mode[0] == 'r') {
-        s->file = qemu_fopen_ops(s, &unix_read_ops);
-    } else {
-        s->file = qemu_fopen_ops(s, &unix_write_ops);
-    }
-    return s->file;
-}
-
-static const QEMUFileOps socket_read_ops = {
-    .get_fd          = socket_get_fd,
-    .get_buffer      = socket_get_buffer,
-    .close           = socket_close,
-    .shut_down       = socket_shutdown,
-    .get_return_path = socket_get_return_path
-};
-
-static const QEMUFileOps socket_write_ops = {
-    .get_fd          = socket_get_fd,
-    .writev_buffer   = socket_writev_buffer,
-    .close           = socket_close,
-    .shut_down       = socket_shutdown,
-    .get_return_path = socket_get_return_path
-};
-
-QEMUFile *qemu_fopen_socket(int fd, const char *mode)
-{
-    QEMUFileSocket *s;
-
-    if (qemu_file_mode_is_not_valid(mode)) {
-        return NULL;
-    }
-
-    s = g_new0(QEMUFileSocket, 1);
-    s->fd = fd;
-    if (mode[0] == 'w') {
-        qemu_set_block(s->fd);
-        s->file = qemu_fopen_ops(s, &socket_write_ops);
-    } else {
-        s->file = qemu_fopen_ops(s, &socket_read_ops);
-    }
-    return s->file;
-}
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PULL 22/28] migration: delete QEMUFile stdio implementation
  2016-05-26  6:11 [Qemu-devel] [PULL 00/28] migration: support for TLS Amit Shah
                   ` (20 preceding siblings ...)
  2016-05-26  6:12 ` [Qemu-devel] [PULL 21/28] migration: delete QEMUFile sockets implementation Amit Shah
@ 2016-05-26  6:12 ` Amit Shah
  2016-05-26  6:12 ` [Qemu-devel] [PULL 23/28] migration: move definition of struct QEMUFile back into qemu-file.c Amit Shah
                   ` (6 subsequent siblings)
  28 siblings, 0 replies; 36+ messages in thread
From: Amit Shah @ 2016-05-26  6:12 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Juan Quintela, Dr. David Alan Gilbert, Daniel P. Berrange,
	qemu list, Amit Shah

From: "Daniel P. Berrange" <berrange@redhat.com>

Now that the exec migration backend and savevm have converted
to use the QIOChannel based QEMUFile, there is no user remaining
for the stdio based QEMUFile impl and it can be deleted.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-Id: <1461751518-12128-23-git-send-email-berrange@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 include/migration/qemu-file.h |   2 -
 migration/Makefile.objs       |   2 +-
 migration/qemu-file-stdio.c   | 196 ------------------------------------------
 3 files changed, 1 insertion(+), 199 deletions(-)
 delete mode 100644 migration/qemu-file-stdio.c

diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
index ba5fe08..43eba9b 100644
--- a/include/migration/qemu-file.h
+++ b/include/migration/qemu-file.h
@@ -127,10 +127,8 @@ typedef struct QEMUFileHooks {
 } QEMUFileHooks;
 
 QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops);
-QEMUFile *qemu_fopen(const char *filename, const char *mode);
 QEMUFile *qemu_fopen_channel_input(QIOChannel *ioc);
 QEMUFile *qemu_fopen_channel_output(QIOChannel *ioc);
-QEMUFile *qemu_popen_cmd(const char *command, const char *mode);
 void qemu_file_set_hooks(QEMUFile *f, const QEMUFileHooks *hooks);
 int qemu_get_fd(QEMUFile *f);
 int qemu_fclose(QEMUFile *f);
diff --git a/migration/Makefile.objs b/migration/Makefile.objs
index 5260048..7e1bec2 100644
--- a/migration/Makefile.objs
+++ b/migration/Makefile.objs
@@ -1,6 +1,6 @@
 common-obj-y += migration.o socket.o fd.o exec.o
 common-obj-y += vmstate.o
-common-obj-y += qemu-file.o qemu-file-stdio.o
+common-obj-y += qemu-file.o
 common-obj-y += qemu-file-channel.o
 common-obj-y += xbzrle.o postcopy-ram.o
 common-obj-y += qjson.o
diff --git a/migration/qemu-file-stdio.c b/migration/qemu-file-stdio.c
deleted file mode 100644
index f402e8f..0000000
--- a/migration/qemu-file-stdio.c
+++ /dev/null
@@ -1,196 +0,0 @@
-/*
- * QEMU System Emulator
- *
- * Copyright (c) 2003-2008 Fabrice Bellard
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
- */
-#include "qemu/osdep.h"
-#include "qemu-common.h"
-#include "qemu/coroutine.h"
-#include "migration/qemu-file.h"
-
-typedef struct QEMUFileStdio {
-    FILE *stdio_file;
-    QEMUFile *file;
-} QEMUFileStdio;
-
-static int stdio_get_fd(void *opaque)
-{
-    QEMUFileStdio *s = opaque;
-
-    return fileno(s->stdio_file);
-}
-
-static ssize_t stdio_put_buffer(void *opaque, const uint8_t *buf, int64_t pos,
-                                size_t size)
-{
-    QEMUFileStdio *s = opaque;
-    size_t res;
-
-    res = fwrite(buf, 1, size, s->stdio_file);
-
-    if (res != size) {
-        return -errno;
-    }
-    return res;
-}
-
-static ssize_t stdio_get_buffer(void *opaque, uint8_t *buf, int64_t pos,
-                                size_t size)
-{
-    QEMUFileStdio *s = opaque;
-    FILE *fp = s->stdio_file;
-    ssize_t bytes;
-
-    for (;;) {
-        clearerr(fp);
-        bytes = fread(buf, 1, size, fp);
-        if (bytes != 0 || !ferror(fp)) {
-            break;
-        }
-        if (errno == EAGAIN) {
-            yield_until_fd_readable(fileno(fp));
-        } else if (errno != EINTR) {
-            break;
-        }
-    }
-    return bytes;
-}
-
-static int stdio_pclose(void *opaque)
-{
-    QEMUFileStdio *s = opaque;
-    int ret;
-    ret = pclose(s->stdio_file);
-    if (ret == -1) {
-        ret = -errno;
-    } else if (!WIFEXITED(ret) || WEXITSTATUS(ret) != 0) {
-        /* close succeeded, but non-zero exit code: */
-        ret = -EIO; /* fake errno value */
-    }
-    g_free(s);
-    return ret;
-}
-
-static int stdio_fclose(void *opaque)
-{
-    QEMUFileStdio *s = opaque;
-    int ret = 0;
-
-    if (qemu_file_is_writable(s->file)) {
-        int fd = fileno(s->stdio_file);
-        struct stat st;
-
-        ret = fstat(fd, &st);
-        if (ret == 0 && S_ISREG(st.st_mode)) {
-            /*
-             * If the file handle is a regular file make sure the
-             * data is flushed to disk before signaling success.
-             */
-            ret = fsync(fd);
-            if (ret != 0) {
-                ret = -errno;
-                return ret;
-            }
-        }
-    }
-    if (fclose(s->stdio_file) == EOF) {
-        ret = -errno;
-    }
-    g_free(s);
-    return ret;
-}
-
-static const QEMUFileOps stdio_pipe_read_ops = {
-    .get_fd =     stdio_get_fd,
-    .get_buffer = stdio_get_buffer,
-    .close =      stdio_pclose
-};
-
-static const QEMUFileOps stdio_pipe_write_ops = {
-    .get_fd =     stdio_get_fd,
-    .put_buffer = stdio_put_buffer,
-    .close =      stdio_pclose
-};
-
-QEMUFile *qemu_popen_cmd(const char *command, const char *mode)
-{
-    FILE *stdio_file;
-    QEMUFileStdio *s;
-
-    if (mode == NULL || (mode[0] != 'r' && mode[0] != 'w') || mode[1] != 0) {
-        fprintf(stderr, "qemu_popen: Argument validity check failed\n");
-        return NULL;
-    }
-
-    stdio_file = popen(command, mode);
-    if (stdio_file == NULL) {
-        return NULL;
-    }
-
-    s = g_new0(QEMUFileStdio, 1);
-
-    s->stdio_file = stdio_file;
-
-    if (mode[0] == 'r') {
-        s->file = qemu_fopen_ops(s, &stdio_pipe_read_ops);
-    } else {
-        s->file = qemu_fopen_ops(s, &stdio_pipe_write_ops);
-    }
-    return s->file;
-}
-
-static const QEMUFileOps stdio_file_read_ops = {
-    .get_fd =     stdio_get_fd,
-    .get_buffer = stdio_get_buffer,
-    .close =      stdio_fclose
-};
-
-static const QEMUFileOps stdio_file_write_ops = {
-    .get_fd =     stdio_get_fd,
-    .put_buffer = stdio_put_buffer,
-    .close =      stdio_fclose
-};
-
-QEMUFile *qemu_fopen(const char *filename, const char *mode)
-{
-    QEMUFileStdio *s;
-
-    if (qemu_file_mode_is_not_valid(mode)) {
-        return NULL;
-    }
-
-    s = g_new0(QEMUFileStdio, 1);
-
-    s->stdio_file = fopen(filename, mode);
-    if (!s->stdio_file) {
-        goto fail;
-    }
-
-    if (mode[0] == 'w') {
-        s->file = qemu_fopen_ops(s, &stdio_file_write_ops);
-    } else {
-        s->file = qemu_fopen_ops(s, &stdio_file_read_ops);
-    }
-    return s->file;
-fail:
-    g_free(s);
-    return NULL;
-}
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PULL 23/28] migration: move definition of struct QEMUFile back into qemu-file.c
  2016-05-26  6:11 [Qemu-devel] [PULL 00/28] migration: support for TLS Amit Shah
                   ` (21 preceding siblings ...)
  2016-05-26  6:12 ` [Qemu-devel] [PULL 22/28] migration: delete QEMUFile stdio implementation Amit Shah
@ 2016-05-26  6:12 ` Amit Shah
  2016-05-26  6:12 ` [Qemu-devel] [PULL 24/28] migration: don't use an array for storing migrate parameters Amit Shah
                   ` (5 subsequent siblings)
  28 siblings, 0 replies; 36+ messages in thread
From: Amit Shah @ 2016-05-26  6:12 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Juan Quintela, Dr. David Alan Gilbert, Daniel P. Berrange,
	qemu list, Amit Shah

From: "Daniel P. Berrange" <berrange@redhat.com>

Now that the memory buffer based QEMUFile impl is gone, there
is no need for any backend to be accessing internals of the
QEMUFile struct, so it can be moved back into qemu-file.c

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-Id: <1461751518-12128-24-git-send-email-berrange@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 migration/qemu-file-internal.h | 54 ------------------------------------------
 migration/qemu-file.c          | 24 ++++++++++++++++++-
 2 files changed, 23 insertions(+), 55 deletions(-)
 delete mode 100644 migration/qemu-file-internal.h

diff --git a/migration/qemu-file-internal.h b/migration/qemu-file-internal.h
deleted file mode 100644
index 8fdfa95..0000000
--- a/migration/qemu-file-internal.h
+++ /dev/null
@@ -1,54 +0,0 @@
-/*
- * QEMU System Emulator
- *
- * Copyright (c) 2003-2008 Fabrice Bellard
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
- */
-
-#ifndef QEMU_FILE_INTERNAL_H
-#define QEMU_FILE_INTERNAL_H 1
-
-#include "qemu-common.h"
-#include "qemu/iov.h"
-
-#define IO_BUF_SIZE 32768
-#define MAX_IOV_SIZE MIN(IOV_MAX, 64)
-
-struct QEMUFile {
-    const QEMUFileOps *ops;
-    const QEMUFileHooks *hooks;
-    void *opaque;
-
-    int64_t bytes_xfer;
-    int64_t xfer_limit;
-
-    int64_t pos; /* start of buffer when writing, end of buffer
-                    when reading */
-    int buf_index;
-    int buf_size; /* 0 when writing */
-    uint8_t buf[IO_BUF_SIZE];
-
-    struct iovec iov[MAX_IOV_SIZE];
-    unsigned int iovcnt;
-
-    int last_error;
-};
-
-#endif
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 2b25dec..cf743d1 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -30,9 +30,31 @@
 #include "qemu/coroutine.h"
 #include "migration/migration.h"
 #include "migration/qemu-file.h"
-#include "migration/qemu-file-internal.h"
 #include "trace.h"
 
+#define IO_BUF_SIZE 32768
+#define MAX_IOV_SIZE MIN(IOV_MAX, 64)
+
+struct QEMUFile {
+    const QEMUFileOps *ops;
+    const QEMUFileHooks *hooks;
+    void *opaque;
+
+    int64_t bytes_xfer;
+    int64_t xfer_limit;
+
+    int64_t pos; /* start of buffer when writing, end of buffer
+                    when reading */
+    int buf_index;
+    int buf_size; /* 0 when writing */
+    uint8_t buf[IO_BUF_SIZE];
+
+    struct iovec iov[MAX_IOV_SIZE];
+    unsigned int iovcnt;
+
+    int last_error;
+};
+
 /*
  * Stop a file from being read/written - not all backing files can do this
  * typically only sockets can.
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PULL 24/28] migration: don't use an array for storing migrate parameters
  2016-05-26  6:11 [Qemu-devel] [PULL 00/28] migration: support for TLS Amit Shah
                   ` (22 preceding siblings ...)
  2016-05-26  6:12 ` [Qemu-devel] [PULL 23/28] migration: move definition of struct QEMUFile back into qemu-file.c Amit Shah
@ 2016-05-26  6:12 ` Amit Shah
  2016-05-26  6:12 ` [Qemu-devel] [PULL 25/28] migration: define 'tls-creds' and 'tls-hostname' migration parameters Amit Shah
                   ` (4 subsequent siblings)
  28 siblings, 0 replies; 36+ messages in thread
From: Amit Shah @ 2016-05-26  6:12 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Juan Quintela, Dr. David Alan Gilbert, Daniel P. Berrange,
	qemu list, Amit Shah

From: "Daniel P. Berrange" <berrange@redhat.com>

The MigrateState struct uses an array for storing migration
parameters. This presumes that all future parameters will
be integers too, which is not going to be the case. There
is no functional reason why an array is used, if anything
it makes the code less clear. The QAPI schema already
defines a struct - MigrationParameters - capable of storing
all the individual parameters, so just use that instead of
an array.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-Id: <1461751518-12128-25-git-send-email-berrange@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 include/migration/migration.h |  5 +++-
 migration/migration.c         | 55 ++++++++++++++++++-------------------------
 migration/ram.c               |  6 ++---
 3 files changed, 29 insertions(+), 37 deletions(-)

diff --git a/include/migration/migration.h b/include/migration/migration.h
index d24c6ef..74105a1 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -135,9 +135,12 @@ struct MigrationState
     QemuThread thread;
     QEMUBH *cleanup_bh;
     QEMUFile *to_dst_file;
-    int parameters[MIGRATION_PARAMETER__MAX];
+
+    /* New style params from 'migrate-set-parameters' */
+    MigrationParameters parameters;
 
     int state;
+    /* Old style params from 'migrate' command */
     MigrationParams params;
 
     /* State related to return path */
diff --git a/migration/migration.c b/migration/migration.c
index 695384b..27064af 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -82,16 +82,13 @@ MigrationState *migrate_get_current(void)
         .bandwidth_limit = MAX_THROTTLE,
         .xbzrle_cache_size = DEFAULT_MIGRATE_CACHE_SIZE,
         .mbps = -1,
-        .parameters[MIGRATION_PARAMETER_COMPRESS_LEVEL] =
-                DEFAULT_MIGRATE_COMPRESS_LEVEL,
-        .parameters[MIGRATION_PARAMETER_COMPRESS_THREADS] =
-                DEFAULT_MIGRATE_COMPRESS_THREAD_COUNT,
-        .parameters[MIGRATION_PARAMETER_DECOMPRESS_THREADS] =
-                DEFAULT_MIGRATE_DECOMPRESS_THREAD_COUNT,
-        .parameters[MIGRATION_PARAMETER_CPU_THROTTLE_INITIAL] =
-                DEFAULT_MIGRATE_CPU_THROTTLE_INITIAL,
-        .parameters[MIGRATION_PARAMETER_CPU_THROTTLE_INCREMENT] =
-                DEFAULT_MIGRATE_CPU_THROTTLE_INCREMENT,
+        .parameters = {
+            .compress_level = DEFAULT_MIGRATE_COMPRESS_LEVEL,
+            .compress_threads = DEFAULT_MIGRATE_COMPRESS_THREAD_COUNT,
+            .decompress_threads = DEFAULT_MIGRATE_DECOMPRESS_THREAD_COUNT,
+            .cpu_throttle_initial = DEFAULT_MIGRATE_CPU_THROTTLE_INITIAL,
+            .cpu_throttle_increment = DEFAULT_MIGRATE_CPU_THROTTLE_INCREMENT,
+        },
     };
 
     if (!once) {
@@ -534,15 +531,11 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
     MigrationState *s = migrate_get_current();
 
     params = g_malloc0(sizeof(*params));
-    params->compress_level = s->parameters[MIGRATION_PARAMETER_COMPRESS_LEVEL];
-    params->compress_threads =
-            s->parameters[MIGRATION_PARAMETER_COMPRESS_THREADS];
-    params->decompress_threads =
-            s->parameters[MIGRATION_PARAMETER_DECOMPRESS_THREADS];
-    params->cpu_throttle_initial =
-            s->parameters[MIGRATION_PARAMETER_CPU_THROTTLE_INITIAL];
-    params->cpu_throttle_increment =
-            s->parameters[MIGRATION_PARAMETER_CPU_THROTTLE_INCREMENT];
+    params->compress_level = s->parameters.compress_level;
+    params->compress_threads = s->parameters.compress_threads;
+    params->decompress_threads = s->parameters.decompress_threads;
+    params->cpu_throttle_initial = s->parameters.cpu_throttle_initial;
+    params->cpu_throttle_increment = s->parameters.cpu_throttle_increment;
 
     return params;
 }
@@ -743,7 +736,8 @@ void qmp_migrate_set_parameters(bool has_compress_level,
                                 bool has_cpu_throttle_initial,
                                 int64_t cpu_throttle_initial,
                                 bool has_cpu_throttle_increment,
-                                int64_t cpu_throttle_increment, Error **errp)
+                                int64_t cpu_throttle_increment,
+                                Error **errp)
 {
     MigrationState *s = migrate_get_current();
 
@@ -780,26 +774,23 @@ void qmp_migrate_set_parameters(bool has_compress_level,
     }
 
     if (has_compress_level) {
-        s->parameters[MIGRATION_PARAMETER_COMPRESS_LEVEL] = compress_level;
+        s->parameters.compress_level = compress_level;
     }
     if (has_compress_threads) {
-        s->parameters[MIGRATION_PARAMETER_COMPRESS_THREADS] = compress_threads;
+        s->parameters.compress_threads = compress_threads;
     }
     if (has_decompress_threads) {
-        s->parameters[MIGRATION_PARAMETER_DECOMPRESS_THREADS] =
-                                                    decompress_threads;
+        s->parameters.decompress_threads = decompress_threads;
     }
     if (has_cpu_throttle_initial) {
-        s->parameters[MIGRATION_PARAMETER_CPU_THROTTLE_INITIAL] =
-                                                    cpu_throttle_initial;
+        s->parameters.cpu_throttle_initial = cpu_throttle_initial;
     }
-
     if (has_cpu_throttle_increment) {
-        s->parameters[MIGRATION_PARAMETER_CPU_THROTTLE_INCREMENT] =
-                                                    cpu_throttle_increment;
+        s->parameters.cpu_throttle_increment = cpu_throttle_increment;
     }
 }
 
+
 void qmp_migrate_start_postcopy(Error **errp)
 {
     MigrationState *s = migrate_get_current();
@@ -1195,7 +1186,7 @@ int migrate_compress_level(void)
 
     s = migrate_get_current();
 
-    return s->parameters[MIGRATION_PARAMETER_COMPRESS_LEVEL];
+    return s->parameters.compress_level;
 }
 
 int migrate_compress_threads(void)
@@ -1204,7 +1195,7 @@ int migrate_compress_threads(void)
 
     s = migrate_get_current();
 
-    return s->parameters[MIGRATION_PARAMETER_COMPRESS_THREADS];
+    return s->parameters.compress_threads;
 }
 
 int migrate_decompress_threads(void)
@@ -1213,7 +1204,7 @@ int migrate_decompress_threads(void)
 
     s = migrate_get_current();
 
-    return s->parameters[MIGRATION_PARAMETER_DECOMPRESS_THREADS];
+    return s->parameters.decompress_threads;
 }
 
 bool migrate_use_events(void)
diff --git a/migration/ram.c b/migration/ram.c
index 54e2151..844ea46 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -429,10 +429,8 @@ static size_t save_page_header(QEMUFile *f, RAMBlock *block, ram_addr_t offset)
 static void mig_throttle_guest_down(void)
 {
     MigrationState *s = migrate_get_current();
-    uint64_t pct_initial =
-            s->parameters[MIGRATION_PARAMETER_CPU_THROTTLE_INITIAL];
-    uint64_t pct_icrement =
-            s->parameters[MIGRATION_PARAMETER_CPU_THROTTLE_INCREMENT];
+    uint64_t pct_initial = s->parameters.cpu_throttle_initial;
+    uint64_t pct_icrement = s->parameters.cpu_throttle_increment;
 
     /* We have not started throttling yet. Let's start it. */
     if (!cpu_throttle_active()) {
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PULL 25/28] migration: define 'tls-creds' and 'tls-hostname' migration parameters
  2016-05-26  6:11 [Qemu-devel] [PULL 00/28] migration: support for TLS Amit Shah
                   ` (23 preceding siblings ...)
  2016-05-26  6:12 ` [Qemu-devel] [PULL 24/28] migration: don't use an array for storing migrate parameters Amit Shah
@ 2016-05-26  6:12 ` Amit Shah
  2016-05-26 15:05   ` Eric Blake
  2016-05-26  6:12 ` [Qemu-devel] [PULL 26/28] migration: add support for encrypting data with TLS Amit Shah
                   ` (3 subsequent siblings)
  28 siblings, 1 reply; 36+ messages in thread
From: Amit Shah @ 2016-05-26  6:12 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Juan Quintela, Dr. David Alan Gilbert, Daniel P. Berrange,
	qemu list, Amit Shah

From: "Daniel P. Berrange" <berrange@redhat.com>

Define two new migration parameters to be used with TLS encryption.
The 'tls-creds' parameter provides the ID of an instance of the
'tls-creds' object type, or rather a subclass such as 'tls-creds-x509'.
Providing these credentials will enable use of TLS on the migration
data stream.

If using x509 certificates, together with a migration URI that does
not include a hostname, the 'tls-hostname' parameter provides the
hostname to use when verifying the server's x509 certificate. This
allows TLS to be used in combination with fd: and exec: protocols
where a TCP connection is established by a 3rd party outside of
QEMU.

NB, this requires changing the migrate_set_parameter method in the
HMP to accept a 's' (string) value instead of 'i' (integer). This
is backwards compatible, because the parsing of strings allows the
quotes to be optional, thus any integer is also a valid string.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-Id: <1461751518-12128-26-git-send-email-berrange@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hmp-commands.hx       |  2 +-
 hmp.c                 | 44 ++++++++++++++++++++++++++++++++------
 migration/migration.c | 14 +++++++++++++
 qapi-schema.json      | 58 ++++++++++++++++++++++++++++++++++++++++++++++++---
 4 files changed, 108 insertions(+), 10 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 4f4f60a..98b4b1a 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1008,7 +1008,7 @@ ETEXI
 
     {
         .name       = "migrate_set_parameter",
-        .args_type  = "parameter:s,value:i",
+        .args_type  = "parameter:s,value:s",
         .params     = "parameter value",
         .help       = "Set the parameter for migration",
         .mhandler.cmd = hmp_migrate_set_parameter,
diff --git a/hmp.c b/hmp.c
index a464ca9..a4b1d3d 100644
--- a/hmp.c
+++ b/hmp.c
@@ -294,6 +294,12 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict)
         monitor_printf(mon, " %s: %" PRId64,
             MigrationParameter_lookup[MIGRATION_PARAMETER_CPU_THROTTLE_INCREMENT],
             params->cpu_throttle_increment);
+        monitor_printf(mon, " %s: '%s'",
+            MigrationParameter_lookup[MIGRATION_PARAMETER_TLS_CREDS],
+            params->tls_creds ? : "");
+        monitor_printf(mon, " %s: '%s'",
+            MigrationParameter_lookup[MIGRATION_PARAMETER_TLS_HOSTNAME],
+            params->tls_hostname ? : "");
         monitor_printf(mon, "\n");
     }
 
@@ -1243,13 +1249,17 @@ void hmp_migrate_set_capability(Monitor *mon, const QDict *qdict)
 void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
 {
     const char *param = qdict_get_str(qdict, "parameter");
-    int value = qdict_get_int(qdict, "value");
+    const char *valuestr = qdict_get_str(qdict, "value");
+    long valueint = 0;
     Error *err = NULL;
     bool has_compress_level = false;
     bool has_compress_threads = false;
     bool has_decompress_threads = false;
     bool has_cpu_throttle_initial = false;
     bool has_cpu_throttle_increment = false;
+    bool has_tls_creds = false;
+    bool has_tls_hostname = false;
+    bool use_int_value = false;
     int i;
 
     for (i = 0; i < MIGRATION_PARAMETER__MAX; i++) {
@@ -1257,25 +1267,46 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
             switch (i) {
             case MIGRATION_PARAMETER_COMPRESS_LEVEL:
                 has_compress_level = true;
+                use_int_value = true;
                 break;
             case MIGRATION_PARAMETER_COMPRESS_THREADS:
                 has_compress_threads = true;
+                use_int_value = true;
                 break;
             case MIGRATION_PARAMETER_DECOMPRESS_THREADS:
                 has_decompress_threads = true;
+                use_int_value = true;
                 break;
             case MIGRATION_PARAMETER_CPU_THROTTLE_INITIAL:
                 has_cpu_throttle_initial = true;
+                use_int_value = true;
                 break;
             case MIGRATION_PARAMETER_CPU_THROTTLE_INCREMENT:
                 has_cpu_throttle_increment = true;
                 break;
+            case MIGRATION_PARAMETER_TLS_CREDS:
+                has_tls_creds = true;
+                break;
+            case MIGRATION_PARAMETER_TLS_HOSTNAME:
+                has_tls_hostname = true;
+                break;
+            }
+
+            if (use_int_value) {
+                if (qemu_strtol(valuestr, NULL, 10, &valueint) < 0) {
+                    error_setg(&err, "Unable to parse '%s' as an int",
+                               valuestr);
+                    goto cleanup;
+                }
             }
-            qmp_migrate_set_parameters(has_compress_level, value,
-                                       has_compress_threads, value,
-                                       has_decompress_threads, value,
-                                       has_cpu_throttle_initial, value,
-                                       has_cpu_throttle_increment, value,
+
+            qmp_migrate_set_parameters(has_compress_level, valueint,
+                                       has_compress_threads, valueint,
+                                       has_decompress_threads, valueint,
+                                       has_cpu_throttle_initial, valueint,
+                                       has_cpu_throttle_increment, valueint,
+                                       has_tls_creds, valuestr,
+                                       has_tls_hostname, valuestr,
                                        &err);
             break;
         }
@@ -1285,6 +1316,7 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
         error_setg(&err, QERR_INVALID_PARAMETER, param);
     }
 
+ cleanup:
     if (err) {
         error_report_err(err);
     }
diff --git a/migration/migration.c b/migration/migration.c
index 27064af..9b037ab 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -536,6 +536,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
     params->decompress_threads = s->parameters.decompress_threads;
     params->cpu_throttle_initial = s->parameters.cpu_throttle_initial;
     params->cpu_throttle_increment = s->parameters.cpu_throttle_increment;
+    params->tls_creds = g_strdup(s->parameters.tls_creds);
+    params->tls_hostname = g_strdup(s->parameters.tls_hostname);
 
     return params;
 }
@@ -737,6 +739,10 @@ void qmp_migrate_set_parameters(bool has_compress_level,
                                 int64_t cpu_throttle_initial,
                                 bool has_cpu_throttle_increment,
                                 int64_t cpu_throttle_increment,
+                                bool has_tls_creds,
+                                const char *tls_creds,
+                                bool has_tls_hostname,
+                                const char *tls_hostname,
                                 Error **errp)
 {
     MigrationState *s = migrate_get_current();
@@ -788,6 +794,14 @@ void qmp_migrate_set_parameters(bool has_compress_level,
     if (has_cpu_throttle_increment) {
         s->parameters.cpu_throttle_increment = cpu_throttle_increment;
     }
+    if (has_tls_creds) {
+        g_free(s->parameters.tls_creds);
+        s->parameters.tls_creds = g_strdup(tls_creds);
+    }
+    if (has_tls_hostname) {
+        g_free(s->parameters.tls_hostname);
+        s->parameters.tls_hostname = g_strdup(tls_hostname);
+    }
 }
 
 
diff --git a/qapi-schema.json b/qapi-schema.json
index e8c0353..8483bdf 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -617,11 +617,28 @@
 # @cpu-throttle-increment: throttle percentage increase each time
 #                          auto-converge detects that migration is not making
 #                          progress. The default value is 10. (Since 2.7)
+#
+# @tls-creds: ID of the 'tls-creds' object that provides credentials for
+#             establishing a TLS connection over the migration data channel.
+#             On the outgoing side of the migration, the credentials must
+#             be for a 'client' endpoint, while for the incoming side the
+#             credentials must be for a 'server' endpoint. Setting this
+#             will enable TLS for all migrations. The default is unset,
+#             resulting in unsecured migration at the QEMU level. (Since 2.7)
+#
+# @tls-hostname: hostname of the target host for the migration. This is
+#                required when using x509 based TLS credentials and the
+#                migration URI does not already include a hostname. For
+#                example if using fd: or exec: based migration, the
+#                hostname must be provided so that the server's x509
+#                certificate identity canbe validated. (Since 2.7)
+#
 # Since: 2.4
 ##
 { 'enum': 'MigrationParameter',
   'data': ['compress-level', 'compress-threads', 'decompress-threads',
-           'cpu-throttle-initial', 'cpu-throttle-increment'] }
+           'cpu-throttle-initial', 'cpu-throttle-increment',
+           'tls-creds', 'tls-hostname'] }
 
 #
 # @migrate-set-parameters
@@ -641,6 +658,22 @@
 # @cpu-throttle-increment: throttle percentage increase each time
 #                          auto-converge detects that migration is not making
 #                          progress. The default value is 10. (Since 2.7)
+#
+# @tls-creds: ID of the 'tls-creds' object that provides credentials for
+#             establishing a TLS connection over the migration data channel.
+#             On the outgoing side of the migration, the credentials must
+#             be for a 'client' endpoint, while for the incoming side the
+#             credentials must be for a 'server' endpoint. Setting this
+#             will enable TLS for all migrations. The default is unset,
+#             resulting in unsecured migration at the QEMU level. (Since 2.7)
+#
+# @tls-hostname: hostname of the target host for the migration. This is
+#                required when using x509 based TLS credentials and the
+#                migration URI does not already include a hostname. For
+#                example if using fd: or exec: based migration, the
+#                hostname must be provided so that the server's x509
+#                certificate identity canbe validated. (Since 2.7)
+#
 # Since: 2.4
 ##
 { 'command': 'migrate-set-parameters',
@@ -648,7 +681,9 @@
             '*compress-threads': 'int',
             '*decompress-threads': 'int',
             '*cpu-throttle-initial': 'int',
-            '*cpu-throttle-increment': 'int'} }
+            '*cpu-throttle-increment': 'int',
+            '*tls-creds': 'str',
+            '*tls-hostname': 'str'} }
 
 #
 # @MigrationParameters
@@ -667,6 +702,21 @@
 #                          auto-converge detects that migration is not making
 #                          progress. The default value is 10. (Since 2.7)
 #
+# @tls-creds: ID of the 'tls-creds' object that provides credentials for
+#             establishing a TLS connection over the migration data channel.
+#             On the outgoing side of the migration, the credentials must
+#             be for a 'client' endpoint, while for the incoming side the
+#             credentials must be for a 'server' endpoint. Setting this
+#             will enable TLS for all migrations. The default is unset,
+#             resulting in unsecured migration at the QEMU level. (Since 2.6)
+#
+# @tls-hostname: hostname of the target host for the migration. This is
+#                required when using x509 based TLS credentials and the
+#                migration URI does not already include a hostname. For
+#                example if using fd: or exec: based migration, the
+#                hostname must be provided so that the server's x509
+#                certificate identity canbe validated. (Since 2.6)
+#
 # Since: 2.4
 ##
 { 'struct': 'MigrationParameters',
@@ -674,7 +724,9 @@
             'compress-threads': 'int',
             'decompress-threads': 'int',
             'cpu-throttle-initial': 'int',
-            'cpu-throttle-increment': 'int'} }
+            'cpu-throttle-increment': 'int',
+            'tls-creds': 'str',
+            'tls-hostname': 'str'} }
 ##
 # @query-migrate-parameters
 #
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PULL 26/28] migration: add support for encrypting data with TLS
  2016-05-26  6:11 [Qemu-devel] [PULL 00/28] migration: support for TLS Amit Shah
                   ` (24 preceding siblings ...)
  2016-05-26  6:12 ` [Qemu-devel] [PULL 25/28] migration: define 'tls-creds' and 'tls-hostname' migration parameters Amit Shah
@ 2016-05-26  6:12 ` Amit Shah
  2016-05-26  6:12 ` [Qemu-devel] [PULL 27/28] migration: remove support for non-iovec based write handlers Amit Shah
                   ` (2 subsequent siblings)
  28 siblings, 0 replies; 36+ messages in thread
From: Amit Shah @ 2016-05-26  6:12 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Juan Quintela, Dr. David Alan Gilbert, Daniel P. Berrange,
	qemu list, Amit Shah

From: "Daniel P. Berrange" <berrange@redhat.com>

This extends the migration_set_incoming_channel and
migration_set_outgoing_channel methods so that they
will automatically wrap the QIOChannel in a
QIOChannelTLS instance if TLS credentials are configured
in the migration parameters.

This allows TLS to work for tcp, unix, fd and exec
migration protocols. It does not (currently) work for
RDMA since it does not use these APIs, but it is
unlikely that TLS would be desired with RDMA anyway
since it would degrade the performance to that seen
with TCP defeating the purpose of using RDMA.

On the target host, QEMU would be launched with a set
of TLS credentials for a server endpoint

 $ qemu-system-x86_64 -monitor stdio -incoming defer \
    -object tls-creds-x509,dir=/home/berrange/security/qemutls,endpoint=server,id=tls0 \
    ...other args...

To enable incoming TLS migration 2 monitor commands are
then used

  (qemu) migrate_set_str_parameter tls-creds tls0
  (qemu) migrate_incoming tcp:myhostname:9000

On the source host, QEMU is launched in a similar
manner but using client endpoint credentials

 $ qemu-system-x86_64 -monitor stdio \
    -object tls-creds-x509,dir=/home/berrange/security/qemutls,endpoint=client,id=tls0 \
    ...other args...

To enable outgoing TLS migration 2 monitor commands are
then used

  (qemu) migrate_set_str_parameter tls-creds tls0
  (qemu) migrate tcp:otherhostname:9000

Thanks to earlier improvements to error reporting,
TLS errors can be seen 'info migrate' when doing a
detached migration. For example:

  (qemu) info migrate
  capabilities: xbzrle: off rdma-pin-all: off auto-converge: off zero-blocks: off compress: off events: off x-postcopy-ram: off
  Migration status: failed
  total time: 0 milliseconds
  error description: TLS handshake failed: The TLS connection was non-properly terminated.

Or

  (qemu) info migrate
  capabilities: xbzrle: off rdma-pin-all: off auto-converge: off zero-blocks: off compress: off events: off x-postcopy-ram: off
  Migration status: failed
  total time: 0 milliseconds
  error description: Certificate does not match the hostname localhost

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-Id: <1461751518-12128-27-git-send-email-berrange@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 include/migration/migration.h |  12 +++-
 migration/Makefile.objs       |   1 +
 migration/exec.c              |   2 +-
 migration/fd.c                |   2 +-
 migration/migration.c         |  40 +++++++++--
 migration/socket.c            |  34 +++++++--
 migration/tls.c               | 161 ++++++++++++++++++++++++++++++++++++++++++
 trace-events                  |  12 +++-
 8 files changed, 247 insertions(+), 17 deletions(-)
 create mode 100644 migration/tls.c

diff --git a/include/migration/migration.h b/include/migration/migration.h
index 74105a1..13b12b7 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -188,8 +188,18 @@ void qemu_start_incoming_migration(const char *uri, Error **errp);
 void migration_set_incoming_channel(MigrationState *s,
                                     QIOChannel *ioc);
 
+void migration_tls_set_incoming_channel(MigrationState *s,
+                                        QIOChannel *ioc,
+                                        Error **errp);
+
 void migration_set_outgoing_channel(MigrationState *s,
-                                    QIOChannel *ioc);
+                                    QIOChannel *ioc,
+                                    const char *hostname);
+
+void migration_tls_set_outgoing_channel(MigrationState *s,
+                                        QIOChannel *ioc,
+                                        const char *hostname,
+                                        Error **errp);
 
 uint64_t migrate_max_downtime(void);
 
diff --git a/migration/Makefile.objs b/migration/Makefile.objs
index 7e1bec2..30ad945 100644
--- a/migration/Makefile.objs
+++ b/migration/Makefile.objs
@@ -1,4 +1,5 @@
 common-obj-y += migration.o socket.o fd.o exec.o
+common-obj-y += tls.o
 common-obj-y += vmstate.o
 common-obj-y += qemu-file.o
 common-obj-y += qemu-file-channel.o
diff --git a/migration/exec.c b/migration/exec.c
index c825e27..1515cc3 100644
--- a/migration/exec.c
+++ b/migration/exec.c
@@ -38,7 +38,7 @@ void exec_start_outgoing_migration(MigrationState *s, const char *command, Error
         return;
     }
 
-    migration_set_outgoing_channel(s, ioc);
+    migration_set_outgoing_channel(s, ioc, NULL);
     object_unref(OBJECT(ioc));
 }
 
diff --git a/migration/fd.c b/migration/fd.c
index 60a75b8..fc5c9ee 100644
--- a/migration/fd.c
+++ b/migration/fd.c
@@ -38,7 +38,7 @@ void fd_start_outgoing_migration(MigrationState *s, const char *fdname, Error **
         return;
     }
 
-    migration_set_outgoing_channel(s, ioc);
+    migration_set_outgoing_channel(s, ioc, NULL);
     object_unref(OBJECT(ioc));
 }
 
diff --git a/migration/migration.c b/migration/migration.c
index 9b037ab..7ecbade 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -35,6 +35,7 @@
 #include "exec/memory.h"
 #include "exec/address-spaces.h"
 #include "io/channel-buffer.h"
+#include "io/channel-tls.h"
 
 #define MAX_THROTTLE  (32 << 20)      /* Migration transfer speed throttling */
 
@@ -428,20 +429,47 @@ void process_incoming_migration(QEMUFile *f)
 void migration_set_incoming_channel(MigrationState *s,
                                     QIOChannel *ioc)
 {
-    QEMUFile *f = qemu_fopen_channel_input(ioc);
+    trace_migration_set_incoming_channel(
+        ioc, object_get_typename(OBJECT(ioc)));
 
-    process_incoming_migration(f);
+    if (s->parameters.tls_creds &&
+        !object_dynamic_cast(OBJECT(ioc),
+                             TYPE_QIO_CHANNEL_TLS)) {
+        Error *local_err = NULL;
+        migration_tls_set_incoming_channel(s, ioc, &local_err);
+        if (local_err) {
+            error_report_err(local_err);
+        }
+    } else {
+        QEMUFile *f = qemu_fopen_channel_input(ioc);
+        process_incoming_migration(f);
+    }
 }
 
 
 void migration_set_outgoing_channel(MigrationState *s,
-                                    QIOChannel *ioc)
+                                    QIOChannel *ioc,
+                                    const char *hostname)
 {
-    QEMUFile *f = qemu_fopen_channel_output(ioc);
+    trace_migration_set_outgoing_channel(
+        ioc, object_get_typename(OBJECT(ioc)), hostname);
 
-    s->to_dst_file = f;
+    if (s->parameters.tls_creds &&
+        !object_dynamic_cast(OBJECT(ioc),
+                             TYPE_QIO_CHANNEL_TLS)) {
+        Error *local_err = NULL;
+        migration_tls_set_outgoing_channel(s, ioc, hostname, &local_err);
+        if (local_err) {
+            migrate_fd_error(s, local_err);
+            error_free(local_err);
+        }
+    } else {
+        QEMUFile *f = qemu_fopen_channel_output(ioc);
+
+        s->to_dst_file = f;
 
-    migrate_fd_connect(s);
+        migrate_fd_connect(s);
+    }
 }
 
 
diff --git a/migration/socket.c b/migration/socket.c
index 25457ea..977a8d3 100644
--- a/migration/socket.c
+++ b/migration/socket.c
@@ -55,20 +55,35 @@ static SocketAddress *unix_build_address(const char *path)
 }
 
 
+struct SocketConnectData {
+    MigrationState *s;
+    char *hostname;
+};
+
+static void socket_connect_data_free(void *opaque)
+{
+    struct SocketConnectData *data = opaque;
+    if (!data) {
+        return;
+    }
+    g_free(data->hostname);
+    g_free(data);
+}
+
 static void socket_outgoing_migration(Object *src,
                                       Error *err,
                                       gpointer opaque)
 {
-    MigrationState *s = opaque;
+    struct SocketConnectData *data = opaque;
     QIOChannel *sioc = QIO_CHANNEL(src);
 
     if (err) {
         trace_migration_socket_outgoing_error(error_get_pretty(err));
-        s->to_dst_file = NULL;
-        migrate_fd_error(s, err);
+        data->s->to_dst_file = NULL;
+        migrate_fd_error(data->s, err);
     } else {
-        trace_migration_socket_outgoing_connected();
-        migration_set_outgoing_channel(s, sioc);
+        trace_migration_socket_outgoing_connected(data->hostname);
+        migration_set_outgoing_channel(data->s, sioc, data->hostname);
     }
     object_unref(src);
 }
@@ -78,11 +93,16 @@ static void socket_start_outgoing_migration(MigrationState *s,
                                             Error **errp)
 {
     QIOChannelSocket *sioc = qio_channel_socket_new();
+    struct SocketConnectData *data = g_new0(struct SocketConnectData, 1);
+    data->s = s;
+    if (saddr->type == SOCKET_ADDRESS_KIND_INET) {
+        data->hostname = g_strdup(saddr->u.inet.data->host);
+    }
     qio_channel_socket_connect_async(sioc,
                                      saddr,
                                      socket_outgoing_migration,
-                                     s,
-                                     NULL);
+                                     data,
+                                     socket_connect_data_free);
     qapi_free_SocketAddress(saddr);
 }
 
diff --git a/migration/tls.c b/migration/tls.c
new file mode 100644
index 0000000..75f959f
--- /dev/null
+++ b/migration/tls.c
@@ -0,0 +1,161 @@
+/*
+ * QEMU migration TLS support
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "migration/migration.h"
+#include "io/channel-tls.h"
+#include "crypto/tlscreds.h"
+#include "qemu/error-report.h"
+#include "qapi/error.h"
+#include "trace.h"
+
+static QCryptoTLSCreds *
+migration_tls_get_creds(MigrationState *s,
+                        QCryptoTLSCredsEndpoint endpoint,
+                        Error **errp)
+{
+    Object *creds;
+    QCryptoTLSCreds *ret;
+
+    creds = object_resolve_path_component(
+        object_get_objects_root(), s->parameters.tls_creds);
+    if (!creds) {
+        error_setg(errp, "No TLS credentials with id '%s'",
+                   s->parameters.tls_creds);
+        return NULL;
+    }
+    ret = (QCryptoTLSCreds *)object_dynamic_cast(
+        creds, TYPE_QCRYPTO_TLS_CREDS);
+    if (!ret) {
+        error_setg(errp, "Object with id '%s' is not TLS credentials",
+                   s->parameters.tls_creds);
+        return NULL;
+    }
+    if (ret->endpoint != endpoint) {
+        error_setg(errp,
+                   "Expected TLS credentials for a %s endpoint",
+                   endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_CLIENT ?
+                   "client" : "server");
+        return NULL;
+    }
+
+    object_ref(OBJECT(ret));
+    return ret;
+}
+
+
+static void migration_tls_incoming_handshake(Object *src,
+                                             Error *err,
+                                             gpointer opaque)
+{
+    QIOChannel *ioc = QIO_CHANNEL(src);
+
+    if (err) {
+        trace_migration_tls_incoming_handshake_error(error_get_pretty(err));
+        error_report("%s", error_get_pretty(err));
+    } else {
+        trace_migration_tls_incoming_handshake_complete();
+        migration_set_incoming_channel(migrate_get_current(), ioc);
+    }
+    object_unref(OBJECT(ioc));
+}
+
+void migration_tls_set_incoming_channel(MigrationState *s,
+                                        QIOChannel *ioc,
+                                        Error **errp)
+{
+    QCryptoTLSCreds *creds;
+    QIOChannelTLS *tioc;
+
+    creds = migration_tls_get_creds(
+        s, QCRYPTO_TLS_CREDS_ENDPOINT_SERVER, errp);
+    if (!creds) {
+        return;
+    }
+
+    tioc = qio_channel_tls_new_server(
+        ioc, creds,
+        NULL, /* XXX pass ACL name */
+        errp);
+    if (!tioc) {
+        return;
+    }
+
+    trace_migration_tls_incoming_handshake_start();
+    qio_channel_tls_handshake(tioc,
+                              migration_tls_incoming_handshake,
+                              NULL,
+                              NULL);
+}
+
+
+static void migration_tls_outgoing_handshake(Object *src,
+                                             Error *err,
+                                             gpointer opaque)
+{
+    MigrationState *s = opaque;
+    QIOChannel *ioc = QIO_CHANNEL(src);
+
+    if (err) {
+        trace_migration_tls_outgoing_handshake_error(error_get_pretty(err));
+        s->to_dst_file = NULL;
+        migrate_fd_error(s, err);
+    } else {
+        trace_migration_tls_outgoing_handshake_complete();
+        migration_set_outgoing_channel(s, ioc, NULL);
+    }
+    object_unref(OBJECT(ioc));
+}
+
+
+void migration_tls_set_outgoing_channel(MigrationState *s,
+                                        QIOChannel *ioc,
+                                        const char *hostname,
+                                        Error **errp)
+{
+    QCryptoTLSCreds *creds;
+    QIOChannelTLS *tioc;
+
+    creds = migration_tls_get_creds(
+        s, QCRYPTO_TLS_CREDS_ENDPOINT_CLIENT, errp);
+    if (!creds) {
+        return;
+    }
+
+    if (s->parameters.tls_hostname) {
+        hostname = s->parameters.tls_hostname;
+    }
+    if (!hostname) {
+        error_setg(errp, "No hostname available for TLS");
+        return;
+    }
+
+    tioc = qio_channel_tls_new_client(
+        ioc, creds, hostname, errp);
+    if (!tioc) {
+        return;
+    }
+
+    trace_migration_tls_outgoing_handshake_start(hostname);
+    qio_channel_tls_handshake(tioc,
+                              migration_tls_outgoing_handshake,
+                              s,
+                              NULL);
+}
diff --git a/trace-events b/trace-events
index 1584b0a..c276075 100644
--- a/trace-events
+++ b/trace-events
@@ -1511,6 +1511,8 @@ migrate_state_too_big(void) ""
 migrate_transferred(uint64_t tranferred, uint64_t time_spent, double bandwidth, uint64_t size) "transferred %" PRIu64 " time_spent %" PRIu64 " bandwidth %g max_size %" PRId64
 process_incoming_migration_co_end(int ret, int ps) "ret=%d postcopy-state=%d"
 process_incoming_migration_co_postcopy_end_main(void) ""
+migration_set_incoming_channel(void *ioc, const char *ioctype) "ioc=%p ioctype=%s"
+migration_set_outgoing_channel(void *ioc, const char *ioctype, const char *hostname)  "ioc=%p ioctype=%s hostname=%s"
 
 # migration/rdma.c
 qemu_rdma_accept_incoming_migration(void) ""
@@ -1605,9 +1607,17 @@ migration_fd_incoming(int fd) "fd=%d"
 
 # migration/socket.c
 migration_socket_incoming_accepted(void) ""
-migration_socket_outgoing_connected(void) ""
+migration_socket_outgoing_connected(const char *hostname) "hostname=%s"
 migration_socket_outgoing_error(const char *err) "error=%s"
 
+# migration/tls.c
+migration_tls_outgoing_handshake_start(const char *hostname) "hostname=%s"
+migration_tls_outgoing_handshake_error(const char *err) "err=%s"
+migration_tls_outgoing_handshake_complete(void) ""
+migration_tls_incoming_handshake_start(void) ""
+migration_tls_incoming_handshake_error(const char *err) "err=%s"
+migration_tls_incoming_handshake_complete(void) ""
+
 # kvm-all.c
 kvm_ioctl(int type, void *arg) "type 0x%x, arg %p"
 kvm_vm_ioctl(int type, void *arg) "type 0x%x, arg %p"
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PULL 27/28] migration: remove support for non-iovec based write handlers
  2016-05-26  6:11 [Qemu-devel] [PULL 00/28] migration: support for TLS Amit Shah
                   ` (25 preceding siblings ...)
  2016-05-26  6:12 ` [Qemu-devel] [PULL 26/28] migration: add support for encrypting data with TLS Amit Shah
@ 2016-05-26  6:12 ` Amit Shah
  2016-05-26  6:12 ` [Qemu-devel] [PULL 28/28] migration: remove qemu_get_fd method from QEMUFile Amit Shah
  2016-05-26 16:29 ` [Qemu-devel] [PULL 00/28] migration: support for TLS Peter Maydell
  28 siblings, 0 replies; 36+ messages in thread
From: Amit Shah @ 2016-05-26  6:12 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Juan Quintela, Dr. David Alan Gilbert, Daniel P. Berrange,
	qemu list, Amit Shah

From: "Daniel P. Berrange" <berrange@redhat.com>

All the remaining QEMUFile implementations provide an iovec
based write handler, so the put_buffer callback can be removed
to simplify the code.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-Id: <1461751518-12128-28-git-send-email-berrange@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 include/migration/qemu-file.h |  9 ---------
 migration/qemu-file.c         | 36 ++++++++----------------------------
 migration/savevm.c            |  8 --------
 3 files changed, 8 insertions(+), 45 deletions(-)

diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
index 43eba9b..36af5f4 100644
--- a/include/migration/qemu-file.h
+++ b/include/migration/qemu-file.h
@@ -28,14 +28,6 @@
 #include "io/channel.h"
 
 
-/* This function writes a chunk of data to a file at the given position.
- * The pos argument can be ignored if the file is only being used for
- * streaming.  The handler must write all of the data or return a negative
- * errno value.
- */
-typedef ssize_t (QEMUFilePutBufferFunc)(void *opaque, const uint8_t *buf,
-                                        int64_t pos, size_t size);
-
 /* Read a chunk of data from a file at the given position.  The pos argument
  * can be ignored if the file is only be used for streaming.  The number of
  * bytes actually read should be returned.
@@ -109,7 +101,6 @@ typedef QEMUFile *(QEMURetPathFunc)(void *opaque);
 typedef int (QEMUFileShutdownFunc)(void *opaque, bool rd, bool wr);
 
 typedef struct QEMUFileOps {
-    QEMUFilePutBufferFunc *put_buffer;
     QEMUFileGetBufferFunc *get_buffer;
     QEMUFileCloseFunc *close;
     QEMUFileGetFD *get_fd;
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index cf743d1..6790040 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -129,7 +129,7 @@ void qemu_file_set_error(QEMUFile *f, int ret)
 
 bool qemu_file_is_writable(QEMUFile *f)
 {
-    return f->ops->writev_buffer || f->ops->put_buffer;
+    return f->ops->writev_buffer;
 }
 
 /**
@@ -148,16 +148,9 @@ void qemu_fflush(QEMUFile *f)
         return;
     }
 
-    if (f->ops->writev_buffer) {
-        if (f->iovcnt > 0) {
-            expect = iov_size(f->iov, f->iovcnt);
-            ret = f->ops->writev_buffer(f->opaque, f->iov, f->iovcnt, f->pos);
-        }
-    } else {
-        if (f->buf_index > 0) {
-            expect = f->buf_index;
-            ret = f->ops->put_buffer(f->opaque, f->buf, f->pos, f->buf_index);
-        }
+    if (f->iovcnt > 0) {
+        expect = iov_size(f->iov, f->iovcnt);
+        ret = f->ops->writev_buffer(f->opaque, f->iov, f->iovcnt, f->pos);
     }
 
     if (ret >= 0) {
@@ -337,11 +330,6 @@ static void add_to_iovec(QEMUFile *f, const uint8_t *buf, size_t size)
 
 void qemu_put_buffer_async(QEMUFile *f, const uint8_t *buf, size_t size)
 {
-    if (!f->ops->writev_buffer) {
-        qemu_put_buffer(f, buf, size);
-        return;
-    }
-
     if (f->last_error) {
         return;
     }
@@ -365,9 +353,7 @@ void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, size_t size)
         }
         memcpy(f->buf + f->buf_index, buf, l);
         f->bytes_xfer += l;
-        if (f->ops->writev_buffer) {
-            add_to_iovec(f, f->buf + f->buf_index, l);
-        }
+        add_to_iovec(f, f->buf + f->buf_index, l);
         f->buf_index += l;
         if (f->buf_index == IO_BUF_SIZE) {
             qemu_fflush(f);
@@ -388,9 +374,7 @@ void qemu_put_byte(QEMUFile *f, int v)
 
     f->buf[f->buf_index] = v;
     f->bytes_xfer++;
-    if (f->ops->writev_buffer) {
-        add_to_iovec(f, f->buf + f->buf_index, 1);
-    }
+    add_to_iovec(f, f->buf + f->buf_index, 1);
     f->buf_index++;
     if (f->buf_index == IO_BUF_SIZE) {
         qemu_fflush(f);
@@ -554,12 +538,8 @@ int64_t qemu_ftell_fast(QEMUFile *f)
     int64_t ret = f->pos;
     int i;
 
-    if (f->ops->writev_buffer) {
-        for (i = 0; i < f->iovcnt; i++) {
-            ret += f->iov[i].iov_len;
-        }
-    } else {
-        ret += f->buf_index;
+    for (i = 0; i < f->iovcnt; i++) {
+        ret += f->iov[i].iov_len;
     }
 
     return ret;
diff --git a/migration/savevm.c b/migration/savevm.c
index 2bd3452..6c21231 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -160,13 +160,6 @@ static ssize_t block_writev_buffer(void *opaque, struct iovec *iov, int iovcnt,
     return qiov.size;
 }
 
-static ssize_t block_put_buffer(void *opaque, const uint8_t *buf,
-                                int64_t pos, size_t size)
-{
-    bdrv_save_vmstate(opaque, buf, pos, size);
-    return size;
-}
-
 static ssize_t block_get_buffer(void *opaque, uint8_t *buf, int64_t pos,
                                 size_t size)
 {
@@ -184,7 +177,6 @@ static const QEMUFileOps bdrv_read_ops = {
 };
 
 static const QEMUFileOps bdrv_write_ops = {
-    .put_buffer     = block_put_buffer,
     .writev_buffer  = block_writev_buffer,
     .close          = bdrv_fclose
 };
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [Qemu-devel] [PULL 28/28] migration: remove qemu_get_fd method from QEMUFile
  2016-05-26  6:11 [Qemu-devel] [PULL 00/28] migration: support for TLS Amit Shah
                   ` (26 preceding siblings ...)
  2016-05-26  6:12 ` [Qemu-devel] [PULL 27/28] migration: remove support for non-iovec based write handlers Amit Shah
@ 2016-05-26  6:12 ` Amit Shah
  2016-05-26 16:29 ` [Qemu-devel] [PULL 00/28] migration: support for TLS Peter Maydell
  28 siblings, 0 replies; 36+ messages in thread
From: Amit Shah @ 2016-05-26  6:12 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Juan Quintela, Dr. David Alan Gilbert, Daniel P. Berrange,
	qemu list, Amit Shah

From: "Daniel P. Berrange" <berrange@redhat.com>

Now that there is a set_blocking callback in QEMUFileOps,
and all users needing non-blocking support have been
converted to QIOChannel, there is no longer any codepath
requiring the qemu_get_fd() method for QEMUFile. Remove it
to avoid further code being introduced with an expectation
of direct file handle access.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-Id: <1461751518-12128-29-git-send-email-berrange@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 include/migration/qemu-file.h |  1 -
 migration/qemu-file.c         | 14 --------------
 2 files changed, 15 deletions(-)

diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
index 36af5f4..2409a98 100644
--- a/include/migration/qemu-file.h
+++ b/include/migration/qemu-file.h
@@ -103,7 +103,6 @@ typedef int (QEMUFileShutdownFunc)(void *opaque, bool rd, bool wr);
 typedef struct QEMUFileOps {
     QEMUFileGetBufferFunc *get_buffer;
     QEMUFileCloseFunc *close;
-    QEMUFileGetFD *get_fd;
     QEMUFileSetBlocking *set_blocking;
     QEMUFileWritevBufferFunc *writev_buffer;
     QEMURetPathFunc *get_return_path;
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 6790040..8aea1c7 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -268,14 +268,6 @@ static ssize_t qemu_fill_buffer(QEMUFile *f)
     return len;
 }
 
-int qemu_get_fd(QEMUFile *f)
-{
-    if (f->ops->get_fd) {
-        return f->ops->get_fd(f->opaque);
-    }
-    return -1;
-}
-
 void qemu_update_position(QEMUFile *f, size_t size)
 {
     f->pos += size;
@@ -688,11 +680,5 @@ void qemu_file_set_blocking(QEMUFile *f, bool block)
 {
     if (f->ops->set_blocking) {
         f->ops->set_blocking(f->opaque, block);
-    } else {
-        if (block) {
-            qemu_set_block(qemu_get_fd(f));
-        } else {
-            qemu_set_nonblock(qemu_get_fd(f));
-        }
     }
 }
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PULL 10/28] migration: add reporting of errors for outgoing migration
  2016-05-26  6:12 ` [Qemu-devel] [PULL 10/28] migration: add reporting of errors for outgoing migration Amit Shah
@ 2016-05-26 15:00   ` Eric Blake
  2016-05-31 15:16     ` Daniel P. Berrange
  2016-06-06  8:38   ` Paolo Bonzini
  1 sibling, 1 reply; 36+ messages in thread
From: Eric Blake @ 2016-05-26 15:00 UTC (permalink / raw)
  To: Amit Shah, Peter Maydell; +Cc: qemu list, Dr. David Alan Gilbert, Juan Quintela

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

On 05/26/2016 12:12 AM, Amit Shah wrote:
> From: "Daniel P. Berrange" <berrange@redhat.com>
> 
> Currently if an application initiates an outgoing migration,
> it may or may not, get an error reported back on failure. If
> the error occurs synchronously to the 'migrate' command
> execution, the client app will see the error message. This
> is the case for DNS lookup failures. If the error occurs
> asynchronously to the monitor command though, the error
> will be thrown away and the client left guessing about
> what went wrong. This is the case for failure to connect
> to the TCP server (eg due to wrong port, or firewall
> rules, or other similar errors).
> 

> +++ b/qapi-schema.json
> @@ -484,6 +484,10 @@
>  #        throttled during auto-converge. This is only present when auto-converge
>  #        has started throttling guest cpus. (Since 2.7)
>  #
> +# @error-desc: #optional the human readable error description string, when
> +#              @status is 'failed'. Clients should not attempt to parse the
> +#              error strings. (Since 2.6)

Since this is already in a pull request, we may need a followup patch to
fix that to be 2.7.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


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

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PULL 25/28] migration: define 'tls-creds' and 'tls-hostname' migration parameters
  2016-05-26  6:12 ` [Qemu-devel] [PULL 25/28] migration: define 'tls-creds' and 'tls-hostname' migration parameters Amit Shah
@ 2016-05-26 15:05   ` Eric Blake
  2016-05-27 10:02     ` Amit Shah
  0 siblings, 1 reply; 36+ messages in thread
From: Eric Blake @ 2016-05-26 15:05 UTC (permalink / raw)
  To: Amit Shah, Peter Maydell; +Cc: qemu list, Dr. David Alan Gilbert, Juan Quintela

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

On 05/26/2016 12:12 AM, Amit Shah wrote:
> From: "Daniel P. Berrange" <berrange@redhat.com>
> 
> Define two new migration parameters to be used with TLS encryption.
> The 'tls-creds' parameter provides the ID of an instance of the
> 'tls-creds' object type, or rather a subclass such as 'tls-creds-x509'.
> Providing these credentials will enable use of TLS on the migration
> data stream.
> 

> +++ b/qapi-schema.json

> +# @tls-hostname: hostname of the target host for the migration. This is
> +#                required when using x509 based TLS credentials and the
> +#                migration URI does not already include a hostname. For
> +#                example if using fd: or exec: based migration, the
> +#                hostname must be provided so that the server's x509
> +#                certificate identity canbe validated. (Since 2.7)

s/canbe/can be/


> +#
> +# @tls-hostname: hostname of the target host for the migration. This is
> +#                required when using x509 based TLS credentials and the
> +#                migration URI does not already include a hostname. For
> +#                example if using fd: or exec: based migration, the
> +#                hostname must be provided so that the server's x509
> +#                certificate identity canbe validated. (Since 2.7)

and again

> @@ -667,6 +702,21 @@
>  #                          auto-converge detects that migration is not making
>  #                          progress. The default value is 10. (Since 2.7)
>  #
> +# @tls-creds: ID of the 'tls-creds' object that provides credentials for
> +#             establishing a TLS connection over the migration data channel.
> +#             On the outgoing side of the migration, the credentials must
> +#             be for a 'client' endpoint, while for the incoming side the
> +#             credentials must be for a 'server' endpoint. Setting this
> +#             will enable TLS for all migrations. The default is unset,
> +#             resulting in unsecured migration at the QEMU level. (Since 2.6)

Missed a swap to call out 2.7

> +#
> +# @tls-hostname: hostname of the target host for the migration. This is
> +#                required when using x509 based TLS credentials and the
> +#                migration URI does not already include a hostname. For
> +#                example if using fd: or exec: based migration, the
> +#                hostname must be provided so that the server's x509
> +#                certificate identity canbe validated. (Since 2.6)

can be, 2.7

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


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

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PULL 00/28] migration: support for TLS
  2016-05-26  6:11 [Qemu-devel] [PULL 00/28] migration: support for TLS Amit Shah
                   ` (27 preceding siblings ...)
  2016-05-26  6:12 ` [Qemu-devel] [PULL 28/28] migration: remove qemu_get_fd method from QEMUFile Amit Shah
@ 2016-05-26 16:29 ` Peter Maydell
  28 siblings, 0 replies; 36+ messages in thread
From: Peter Maydell @ 2016-05-26 16:29 UTC (permalink / raw)
  To: Amit Shah
  Cc: Juan Quintela, Dr. David Alan Gilbert, Daniel P. Berrange, qemu list

On 26 May 2016 at 07:11, Amit Shah <amit.shah@redhat.com> wrote:
> The following changes since commit 287db79df8af8e31f18e262feb5e05103a09e4d4:
>
>   Merge remote-tracking branch 'remotes/ehabkost/tags/x86-pull-request' into staging (2016-05-24 13:06:33 +0100)
>
> are available in the git repository at:
>
>   https://git.kernel.org/pub/scm/virt/qemu/amit/migration.git tags/migration-2.7-2
>
> for you to fetch changes up to 12992c16d9afd8a23a94a84ad532a1adedf9e511:
>
>   migration: remove qemu_get_fd method from QEMUFile (2016-05-26 11:32:21 +0530)
>
> ----------------------------------------------------------------
> migration: add TLS support to the migration data channel
>
> This is a big refactoring of the migration backend code - moving away from
> QEMUFile to the new QIOChannel framework introduced here.  This brings a
> good level of abstraction and reduction of many lines of code.
>
> This series also adds the ability for many backends (all except RDMA) to
> use TLS for encrypting the migration data between the endpoints.
>
> ----------------------------------------------------------------
>

Applied, thanks.

-- PMM

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PULL 25/28] migration: define 'tls-creds' and 'tls-hostname' migration parameters
  2016-05-26 15:05   ` Eric Blake
@ 2016-05-27 10:02     ` Amit Shah
  2016-05-31  9:22       ` Daniel P. Berrange
  0 siblings, 1 reply; 36+ messages in thread
From: Amit Shah @ 2016-05-27 10:02 UTC (permalink / raw)
  To: Eric Blake
  Cc: Peter Maydell, qemu list, Dr. David Alan Gilbert, Juan Quintela

On (Thu) 26 May 2016 [09:05:37], Eric Blake wrote:

> > @@ -667,6 +702,21 @@
> >  #                          auto-converge detects that migration is not making
> >  #                          progress. The default value is 10. (Since 2.7)
> >  #
> > +# @tls-creds: ID of the 'tls-creds' object that provides credentials for
> > +#             establishing a TLS connection over the migration data channel.
> > +#             On the outgoing side of the migration, the credentials must
> > +#             be for a 'client' endpoint, while for the incoming side the
> > +#             credentials must be for a 'server' endpoint. Setting this
> > +#             will enable TLS for all migrations. The default is unset,
> > +#             resulting in unsecured migration at the QEMU level. (Since 2.6)
> 
> Missed a swap to call out 2.7

Ugh, I did miss a few didn't I?

Dan, can you please follow up with the fixes Eric sugests?

Thanks,



		Amit

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PULL 25/28] migration: define 'tls-creds' and 'tls-hostname' migration parameters
  2016-05-27 10:02     ` Amit Shah
@ 2016-05-31  9:22       ` Daniel P. Berrange
  0 siblings, 0 replies; 36+ messages in thread
From: Daniel P. Berrange @ 2016-05-31  9:22 UTC (permalink / raw)
  To: Amit Shah
  Cc: Eric Blake, Peter Maydell, Juan Quintela, qemu list,
	Dr. David Alan Gilbert

On Fri, May 27, 2016 at 03:32:14PM +0530, Amit Shah wrote:
> On (Thu) 26 May 2016 [09:05:37], Eric Blake wrote:
> 
> > > @@ -667,6 +702,21 @@
> > >  #                          auto-converge detects that migration is not making
> > >  #                          progress. The default value is 10. (Since 2.7)
> > >  #
> > > +# @tls-creds: ID of the 'tls-creds' object that provides credentials for
> > > +#             establishing a TLS connection over the migration data channel.
> > > +#             On the outgoing side of the migration, the credentials must
> > > +#             be for a 'client' endpoint, while for the incoming side the
> > > +#             credentials must be for a 'server' endpoint. Setting this
> > > +#             will enable TLS for all migrations. The default is unset,
> > > +#             resulting in unsecured migration at the QEMU level. (Since 2.6)
> > 
> > Missed a swap to call out 2.7
> 
> Ugh, I did miss a few didn't I?
> 
> Dan, can you please follow up with the fixes Eric sugests?

Yes, will do.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PULL 10/28] migration: add reporting of errors for outgoing migration
  2016-05-26 15:00   ` Eric Blake
@ 2016-05-31 15:16     ` Daniel P. Berrange
  0 siblings, 0 replies; 36+ messages in thread
From: Daniel P. Berrange @ 2016-05-31 15:16 UTC (permalink / raw)
  To: Eric Blake
  Cc: Amit Shah, Peter Maydell, Juan Quintela, qemu list,
	Dr. David Alan Gilbert

On Thu, May 26, 2016 at 09:00:12AM -0600, Eric Blake wrote:
> On 05/26/2016 12:12 AM, Amit Shah wrote:
> > From: "Daniel P. Berrange" <berrange@redhat.com>
> > 
> > Currently if an application initiates an outgoing migration,
> > it may or may not, get an error reported back on failure. If
> > the error occurs synchronously to the 'migrate' command
> > execution, the client app will see the error message. This
> > is the case for DNS lookup failures. If the error occurs
> > asynchronously to the monitor command though, the error
> > will be thrown away and the client left guessing about
> > what went wrong. This is the case for failure to connect
> > to the TCP server (eg due to wrong port, or firewall
> > rules, or other similar errors).
> > 
> 
> > +++ b/qapi-schema.json
> > @@ -484,6 +484,10 @@
> >  #        throttled during auto-converge. This is only present when auto-converge
> >  #        has started throttling guest cpus. (Since 2.7)
> >  #
> > +# @error-desc: #optional the human readable error description string, when
> > +#              @status is 'failed'. Clients should not attempt to parse the
> > +#              error strings. (Since 2.6)
> 
> Since this is already in a pull request, we may need a followup patch to
> fix that to be 2.7.

Yep, taking care of that now.


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [Qemu-devel] [PULL 10/28] migration: add reporting of errors for outgoing migration
  2016-05-26  6:12 ` [Qemu-devel] [PULL 10/28] migration: add reporting of errors for outgoing migration Amit Shah
  2016-05-26 15:00   ` Eric Blake
@ 2016-06-06  8:38   ` Paolo Bonzini
  1 sibling, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2016-06-06  8:38 UTC (permalink / raw)
  To: Amit Shah, Peter Maydell; +Cc: qemu list, Dr. David Alan Gilbert, Juan Quintela

On 26/05/2016 08:12, Amit Shah wrote:
> @@ -863,12 +867,15 @@ static void migrate_fd_cleanup(void *opaque)
>      notifier_list_notify(&migration_state_notifiers, s);
>  }
>  
> -void migrate_fd_error(MigrationState *s)
> +void migrate_fd_error(MigrationState *s, const Error *error)
>  {
> -    trace_migrate_fd_error();
> +    trace_migrate_fd_error(error ? error_get_pretty(error) : "");

This check on error == NULL is unnecessary, the only caller passes a
non-NULL error.

Paolo

^ permalink raw reply	[flat|nested] 36+ messages in thread

end of thread, other threads:[~2016-06-06  8:38 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-26  6:11 [Qemu-devel] [PULL 00/28] migration: support for TLS Amit Shah
2016-05-26  6:11 ` [Qemu-devel] [PULL 01/28] s390: use FILE instead of QEMUFile for creating text file Amit Shah
2016-05-26  6:11 ` [Qemu-devel] [PULL 02/28] io: avoid double-free when closing QIOChannelBuffer Amit Shah
2016-05-26  6:11 ` [Qemu-devel] [PULL 03/28] migration: remove use of qemu_bufopen from vmstate tests Amit Shah
2016-05-26  6:11 ` [Qemu-devel] [PULL 04/28] migration: ensure qemu_fflush() always writes full data amount Amit Shah
2016-05-26  6:11 ` [Qemu-devel] [PULL 05/28] migration: split migration hooks out of QEMUFileOps Amit Shah
2016-05-26  6:11 ` [Qemu-devel] [PULL 06/28] migration: introduce set_blocking function in QEMUFileOps Amit Shah
2016-05-26  6:11 ` [Qemu-devel] [PULL 07/28] migration: force QEMUFile to blocking mode for outgoing migration Amit Shah
2016-05-26  6:11 ` [Qemu-devel] [PULL 08/28] migration: introduce a new QEMUFile impl based on QIOChannel Amit Shah
2016-05-26  6:12 ` [Qemu-devel] [PULL 09/28] migration: add helpers for creating QEMUFile from a QIOChannel Amit Shah
2016-05-26  6:12 ` [Qemu-devel] [PULL 10/28] migration: add reporting of errors for outgoing migration Amit Shah
2016-05-26 15:00   ` Eric Blake
2016-05-31 15:16     ` Daniel P. Berrange
2016-06-06  8:38   ` Paolo Bonzini
2016-05-26  6:12 ` [Qemu-devel] [PULL 11/28] migration: convert post-copy to use QIOChannelBuffer Amit Shah
2016-05-26  6:12 ` [Qemu-devel] [PULL 12/28] migration: convert unix socket protocol to use QIOChannel Amit Shah
2016-05-26  6:12 ` [Qemu-devel] [PULL 13/28] migration: rename unix.c to socket.c Amit Shah
2016-05-26  6:12 ` [Qemu-devel] [PULL 14/28] migration: convert tcp socket protocol to use QIOChannel Amit Shah
2016-05-26  6:12 ` [Qemu-devel] [PULL 15/28] migration: convert fd " Amit Shah
2016-05-26  6:12 ` [Qemu-devel] [PULL 16/28] migration: convert exec " Amit Shah
2016-05-26  6:12 ` [Qemu-devel] [PULL 17/28] migration: convert RDMA to use QIOChannel interface Amit Shah
2016-05-26  6:12 ` [Qemu-devel] [PULL 18/28] migration: convert savevm to use QIOChannel for writing to files Amit Shah
2016-05-26  6:12 ` [Qemu-devel] [PULL 19/28] migration: delete QEMUFile buffer implementation Amit Shah
2016-05-26  6:12 ` [Qemu-devel] [PULL 20/28] migration: delete QEMUSizedBuffer struct Amit Shah
2016-05-26  6:12 ` [Qemu-devel] [PULL 21/28] migration: delete QEMUFile sockets implementation Amit Shah
2016-05-26  6:12 ` [Qemu-devel] [PULL 22/28] migration: delete QEMUFile stdio implementation Amit Shah
2016-05-26  6:12 ` [Qemu-devel] [PULL 23/28] migration: move definition of struct QEMUFile back into qemu-file.c Amit Shah
2016-05-26  6:12 ` [Qemu-devel] [PULL 24/28] migration: don't use an array for storing migrate parameters Amit Shah
2016-05-26  6:12 ` [Qemu-devel] [PULL 25/28] migration: define 'tls-creds' and 'tls-hostname' migration parameters Amit Shah
2016-05-26 15:05   ` Eric Blake
2016-05-27 10:02     ` Amit Shah
2016-05-31  9:22       ` Daniel P. Berrange
2016-05-26  6:12 ` [Qemu-devel] [PULL 26/28] migration: add support for encrypting data with TLS Amit Shah
2016-05-26  6:12 ` [Qemu-devel] [PULL 27/28] migration: remove support for non-iovec based write handlers Amit Shah
2016-05-26  6:12 ` [Qemu-devel] [PULL 28/28] migration: remove qemu_get_fd method from QEMUFile Amit Shah
2016-05-26 16:29 ` [Qemu-devel] [PULL 00/28] migration: support for TLS Peter Maydell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.