[Qemu-devel] [PATCH v5 00/28] Convert migration to QIOChannel & support TLS

[Daniel P. Berrange]

This is an update of patches that were previously posted

  FYI: https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg00829.html
   v1: https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg01914.html
   v2: https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg03509.html
   v3: https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg06279.html

The primary goal of this series of patches is to support TLS on the
migration data channel. The bulk of the work in this series though,
is converting the various QEMUFile implementations to be based on the
new QIOChannel framework.

At the end of this current series there is just one remaining impl
of QEMUFileOps that is not based on QIOChannel - the one in savevm.c
that is using BlockDriverState. It would be possible to create a
QIOChannel wrapper around BlockDriverState too, at which point all
QEMUFile impls would be QIOChannel based. This would then let us
cut out the QEMUFileOps driver callbacks entirely and thus simply
code even more. This patch series is already too large, so I left
that for now.

The first 7 patches are some basic clean ups/fixes mostly to
the QEMUFile code

The 8th patch introduces the QIOChannel based QEMUFile impl
and the 9th adds helpers for using it to start migrations.

Patch 10 adds very long overdue support for reporting errors
during migration back to the management app, which is critical
for TLS otherwise it is impossible to debug any failures.

Patches 11-18 convert the various migration protocols to use
the QIOChannel based QEMUFile impl. In this refactoring the
TCP and UNIX implementations were able to be merged into a
generic sockets impl.

Patches 19-22 remove the now unused QEMUFile impls that do
not use QIOChanel
Patches 23 & 24 do some more cleanup

Patch 25 defines some new migration parameters which are used
to enable use of TLS

Patch 26 actually implements support for TLS with migration,
working with tcp, unix, fd and exec migration backend protocols.
Only RDMA is unsupported with TLS. The commit message shows the
example usage via the HMP

Patches 27 & 28 do some final cleanup.

Overall we have a net win of deleting ~350 lines of code,
despite adding more features, which is always nice.

I have been testing the various migration protocols, including
RDMA and appear to be still functional.

In terms of performance, I have tested TCP with TLS migration
enabled over a 10 Gig-E network interface.

With plain TCP we were able to reach 8500mbs (according to
'info migrate' stats).

With TCP and TLS enabled, we are only able to reach 1800 mbs.
IOW, we can max out 1 Gig-E NICs with TLS, but not 10 Gig-E
where we only reach 21% of potential plain text throughput.

The source host migration thread is only hitting 60% CPU
utilization, but the target host incoming migration thread
is hitting 100% CPU.

The source migration thread is dominated solely by GNUTLS
AES encryption functions as would be expected.

The target migration thread is dominated by the same GNUTLS
AES encryption functions, but also memcpy(). IIUC, the memcpy
is QEMU generic migration code copying RAM pages into place.

In talking with Dave Gilbert we thought it might be possible
to use two threads for incoming migration on the target host.
The first would be responsible for doing network I/O into
local buffers, including the TLS decryption. The second
would be responsible for processing the data. That way the
memcpy() of RAM would move into another thread, allowing the
first thread to spend 100% of its time doing TLS decryption.

If we assume the decryption + encryption take equal amounts
of time, then it ought to let us raise TLS throughput from
1800 mbs, to approx 3000 mbs. Still a good way off 8500mbs
from non-TLS migration, but a worth while improvement none
the less.

NB, these TLS migration results were on a CPU with native AES
instructionset support. CPUs with AES instructions would be
even worse performance.

Changed in v5:

  (Only patch 25 has changed since v3)

 - Resolve conflicts with removal of socket_errno() in
   git master
 - Fix crash in migrate_set_parameters HMP impl

Changed in v4:

 (Only patches 2, 8, 10 & 25 have changes since v3)

 - Expanded docs for new 'error_desc' field in query-migrate
 - Drop new HMP migrate_set_str_parameter command and just
   change migrate_set_parameter to accept a string instead
   of only int
 - Add 'get_return_path' impl for QIOChannel based QEMUFile
   to make post-copy work
 - Replace logic which tried to modify struct iovec elements
   in-replace, with iov_copy + iov_discard_front to avoid
   issue with niov == 0
 - Fix double-free in QIOChannelBuffer triggered by post-copy
 - Reset error_desc field in migrate_init so old errors don't
   persist when restarting a failed migrate
 - Keep the first reported migration error message instead of
   the last reported on.

Changed in v3:

 - Rebase to resolve conflicts with recent merged
   patches
 - Fix up include qemu/osdep.h in various new files

Changed in v2:

 - Switch to setting migration parameters for TLS instead
   of adding to the URI syntax
 - Support TLS over tcp, unix, fd, and socket protocols, not
   just tcp
 - Allow passing in a hostname override for x509 cert checks
 - Enable error reporting for outgoing migration problems
 - Fix inverted I/O direction in post-copy code
 - Use uint8_t / size_t in post-copy conversion instead of
   casting types
 - Merge unix and tcp driver implementations
 - Use tracepoints instead of DPRINTF
 - Use error_report for incoming migration problems
 - Fix broken logic in RDMA read conversion
 - Add missing I/O callback & set_blocking API callbacks
   for RMDA QIOChannel impl
 - Moved socket vs file FD detection to QIOChannel common
   code

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               |  63 +++---
 migration/fd.c                 |  76 +++----
 migration/migration.c          | 158 +++++++++-----
 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             | 182 ++++++++++++++++
 migration/tcp.c                | 102 ---------
 migration/tls.c                | 160 ++++++++++++++
 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, 1280 insertions(+), 1677 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.0