Re: [Qemu-devel] [PATCH v3 0/9] Introduce I/O channels framework

["Daniel P. Berrange" <berrange@redhat.com>]

Ping, unless anyone wants to review this further I'll send a pull
request for it once 2.6 opens up, so I can focus on getting the
other dependant patch series reviewed & merged in 2.6 too.

Regards,
Daniel

On Wed, Nov 18, 2015 at 06:42:43PM +0000, Daniel P. Berrange wrote:
> This series of patches is a followup submission for review of another
> chunk of my large series supporting TLS across chardevs, VNC and
> migration, previously shown here:
> 
>  RFC: https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg00829.html
>   v1: https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg04853.html
>   v2: https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg03439.html
> 
> In this series, I have provided only the general I/O channels
> framework, which will ultimately be used by VNC, chardev and
> migration code, which currently work as follows:
> 
>  - VNC: uses raw sockets APIs directly, layering in TLS, SASL
>    and websockets where needed. This has resulted in what should
>    be fairly generic code for TLS and websockets being entangled
>    with the rest of the VNC server code.
> 
>  - Chardevs: uses GLib's GIOChannel framework. This only provides
>    implementations for sockets and files. Its API lacks support for
>    using struct iovec for read/writes, file descriptor passing,
>    misc sockets ioctls/fcntls. While you can work around these
>    problems by accessing the underling file descriptor directly,
>    this breaks the encapsulation, requiring callers to know about
>    specific implementations. It also does not have integration
>    with QEMU Error reporting framework. So while the GIOChannel
>    is extensible, extending it would result in throwing away
>    pretty much the entire of the existing implementations
> 
>  - Migration: uses QEMUFile framework. The provides an abstract
>    interface for I/O channels used during migration. It has
>    impls for sockets, files, memory buffers & commands. While
>    it has struct iovec support for writes, it does not have
>    the same for reads. It also doesn't support file descriptor
>    passing or control of the sockets ioctls/fcntls. It also
>    does not have any explicit event loop integration, expecting
>    the callers to directly access the underling file desctriptor
>    and setup events on that. This limits its utility in cases
>    where you need channels which are not backed by file
>    descriptors. It has no integration with QEMU Error object
>    for error reporting, has fixed semantics wrt writes
>    ie must always do a full write, no partial writes, and
>    most strangely forces either input or output mode, but
>    refuses to allow both, so no bi-directional channels!
> 
> Out of all of these, the migration code is probably closest
> to what is needed, but is still a good way off from being a
> generic framework that be can reused outside of the migration
> code.
> 
> There is also the GLib GIO library which provides a generic
> framework, but we can't depend on that due to the minimum
> GLib requirement. It also has various missing pieces such as
> file descriptor passing, and no support for struct iovec
> either.
> 
> Hence, this series was born, which tries to take the best of
> the designs for the GIOChannel, QIO and QEMUFile code to
> provide QIOChannel. Right from the start this new framework
> is explicitly isolated from any other QEMU subsystem, so its
> implementation will not get mixed up with specific use cases.
> 
> The QIOChannel abstract base class defines the overall API
> contract
> 
>  - qio_channel_{write,writev,write_full} for writing data. The
>    underling impl always uses struct iovec, but non-iov
>    APIs are provided as simple wrappers for convenience
> 
>  - qio_channel_{read,readv,read_full} for reading data. The
>    underling impl always uses struct iovec, but non-iov
>    APIs are provided as simple wrappers for convenience
> 
>  - qio_channel_has_feature to determine which optional
>    features the channel supports - eg file descriptor
>    passing, nagle, etc
> 
>  - qio_channel_set_{blocking,delay,cork} for various fcntl
>    and ioctl controls
> 
>  - qio_channel_{close,shutdown} for closing the I/O stream
>    or individual channels
> 
>  - qio_channel_seek for random access channels
> 
>  - qio_channel_{add,create}_watch for integration into the
>    main loop for event notifications
> 
>  - qio_channel_wait for blocking of execution pending an
>    event notification
> 
>  - qio_channel_yield for switching coroutine until an
>    event notification
> 
> All the APIs support Error ** object arguments where needed.
> The object is built using QOM, in order to get reference
> counting and sub-classing with type checking. They are *not*
> user creatable/visible objects though - these are internal
> infrastructure, so we will be free to adapt APIs/objects at
> will over time.
> 
> In this series there are a variety of implementations. Some
> of them provide an actual base layer data transport, while
> others provide a data translation layer:
> 
> In this series there are a variety of implementations. Some
> of them provide an actual base layer data transport, while
> others provide a data translation layer:
> 
>  - QIOChannelSocket - for socket based I/O, IPv4, IPV6 & UNIX,
>                       both stream and datagram.
>  - QIOChannelFile - any non-socket file, plain file, char dev,
>                     fifo, pipe, etc
>  - QIOChannelTLS - data translation layer to apply TLS protocol
>  - QIOChannelWebsock - data translation layer to apply the
>                        websockets server protocol
>  - QIOChannelCommand - spawn & communicate with a command via
>                        pipes
>  - QIOChannelBuffer - a simple in-memory buffer
> 
> These 6 implementations are sufficient to support the current
> needs of the VNC server, chardev network backends, and all
> the various migration protocols, except RDMA (which is part
> of the larger RFC series I've posted previously).
> 
> The sockets implementation in particular solves a long standing
> limitation of the qemu-sockets.c API by providing a truely
> asynchronous API for establishing listener sockets / connections.
> The current code is only async for the socket establishment,
> but still blocks the caller for DNS resolution.
> 
> I have not attempted to make the socket implementation support
> multiple listener sockets in a single object. I looked at this
> but it would significantly complicate the QIOChannelSocket
> implementation. I intend to suggest an alternative approach
> for that problem at a later date, whereby we define a
> QIONetworkService object, which provides the infrastructure
> for managing multiple QIOChannelSocket listeners, and clients,
> which was resuable for any QEMU network service.
> 
> There are unit tests for the socket, file, tls, command and
> buffer channel implementations. I didn't create unit test for
> the websock impl, as it needs a websock client to usefully
> test it which doesn't exist in QEMU yet.
> 
> This series merely introduces all the new framework. I will be
> sending 2 further series immediately after this, one which
> converts the chardev backends, and one which converts the VNC
> backend. Assuming review is acceptable, I would expect to send
> a PULL request for this series myself, and then send the other
> chardev & VNC patches via their respective maintainers, hence
> why I split the series.
> 
> Finally I've listed myself as maintainer for the new io/ and
> include/io/ directory prefixes.
> 
> Changed in v3:
> 
>  - Resolve merge conflicts with Eric's QAPI code which caused
>    rename of SocketAddress struct fields.
>  - Added further unit test coverage of QIOChanneBuffer class
>  - Fixed misc memory leaks in code and unit tests
> 
> Changed in v2:
> 
>  - Remove feature checks for TCP_NODELAY and TCP_CORK and
>     treat them as hints which cannot fail (Paolo)
>  - Push feature checking for FD passing up into QIOChannel
>     base class (Paolo)
>  - Use more constants in websockets code instead of magic
>     values (David Gilbert)
> 
> Daniel P. Berrange (9):
>   io: add abstract QIOChannel classes
>   io: add helper module for creating watches on FDs
>   io: add QIOTask class for async operations
>   io: add QIOChannelSocket class
>   io: add QIOChannelFile class
>   io: add QIOChannelTLS class
>   io: add QIOChannelWebsock class
>   io: add QIOChannelCommand class
>   io: add QIOChannelBuffer class
> 
>  MAINTAINERS                     |   7 +
>  Makefile                        |   2 +
>  Makefile.objs                   |   5 +
>  Makefile.target                 |   2 +
>  configure                       |  11 +
>  include/io/channel-buffer.h     |  60 +++
>  include/io/channel-command.h    |  91 ++++
>  include/io/channel-file.h       |  93 ++++
>  include/io/channel-socket.h     | 244 ++++++++++
>  include/io/channel-tls.h        | 142 ++++++
>  include/io/channel-watch.h      |  72 +++
>  include/io/channel-websock.h    | 108 +++++
>  include/io/channel.h            | 503 +++++++++++++++++++++
>  include/io/task.h               | 256 +++++++++++
>  include/qemu/sockets.h          |  19 +
>  io/Makefile.objs                |   9 +
>  io/channel-buffer.c             | 261 +++++++++++
>  io/channel-command.c            | 369 +++++++++++++++
>  io/channel-file.c               | 237 ++++++++++
>  io/channel-socket.c             | 753 +++++++++++++++++++++++++++++++
>  io/channel-tls.c                | 405 +++++++++++++++++
>  io/channel-watch.c              | 200 +++++++++
>  io/channel-websock.c            | 975 ++++++++++++++++++++++++++++++++++++++++
>  io/channel.c                    | 291 ++++++++++++
>  io/task.c                       | 159 +++++++
>  scripts/create_config           |   9 +
>  tests/.gitignore                |   8 +
>  tests/Makefile                  |  19 +
>  tests/io-channel-helpers.c      | 246 ++++++++++
>  tests/io-channel-helpers.h      |  42 ++
>  tests/test-io-channel-buffer.c  |  50 +++
>  tests/test-io-channel-command.c | 129 ++++++
>  tests/test-io-channel-file.c    | 100 +++++
>  tests/test-io-channel-socket.c  | 399 ++++++++++++++++
>  tests/test-io-channel-tls.c     | 342 ++++++++++++++
>  tests/test-io-task.c            | 276 ++++++++++++
>  trace-events                    |  56 +++
>  util/qemu-sockets.c             |   2 +-
>  38 files changed, 6951 insertions(+), 1 deletion(-)
>  create mode 100644 include/io/channel-buffer.h
>  create mode 100644 include/io/channel-command.h
>  create mode 100644 include/io/channel-file.h
>  create mode 100644 include/io/channel-socket.h
>  create mode 100644 include/io/channel-tls.h
>  create mode 100644 include/io/channel-watch.h
>  create mode 100644 include/io/channel-websock.h
>  create mode 100644 include/io/channel.h
>  create mode 100644 include/io/task.h
>  create mode 100644 io/Makefile.objs
>  create mode 100644 io/channel-buffer.c
>  create mode 100644 io/channel-command.c
>  create mode 100644 io/channel-file.c
>  create mode 100644 io/channel-socket.c
>  create mode 100644 io/channel-tls.c
>  create mode 100644 io/channel-watch.c
>  create mode 100644 io/channel-websock.c
>  create mode 100644 io/channel.c
>  create mode 100644 io/task.c
>  create mode 100644 tests/io-channel-helpers.c
>  create mode 100644 tests/io-channel-helpers.h
>  create mode 100644 tests/test-io-channel-buffer.c
>  create mode 100644 tests/test-io-channel-command.c
>  create mode 100644 tests/test-io-channel-file.c
>  create mode 100644 tests/test-io-channel-socket.c
>  create mode 100644 tests/test-io-channel-tls.c
>  create mode 100644 tests/test-io-task.c
> 
> -- 
> 2.5.0
> 

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 :|