[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 00/16] Introduce I/O channels framework
From: |
Daniel P. Berrange |
Subject: |
Re: [Qemu-devel] [PATCH v2 00/16] Introduce I/O channels framework |
Date: |
Mon, 19 Oct 2015 14:47:50 +0100 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
Ping, does anyone feel like doing a review of this series, if not I'll
just do a pull request as-is...
Daniel
On Mon, Oct 12, 2015 at 12:14:53PM +0100, 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
>
> In this series, I have provided only the general I/O channels
> framework, which will ultimately be used by VNC, chardev and
> migration code, whicih 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, and command
> 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. The memory buffer impl
> also lacks unit tests for now, simply due to not getting
> around to it yet.
>
> This series merely introduces all the new framework. To keep
> it an acceptable length for review, it doesn't include any of
> the patches which actually use the new APIs. If reviewers want
> to see real world usage of these APIs, they can refer to my
> previous RFC series.
>
> https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg00829.html
>
> In particular
>
> - VNC conversion to QIOChannelSocket
> https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg00851.html
>
> - VNC conversion to QIOChannelTLS
> https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg00845.html
>
> - VNC conversion to QIOChannelWebsock
> https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg00842.html
>
> - Chardev conversion to QIOChannelSocket
> https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg00849.html
>
> - Chardev addition of TLS support
> https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg00848.html
>
> - All the migration ones, but most interesting addition of TLS
> https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg00871.html
>
> Finally I've listed myself as maintainer for the new io/ and
> include/io/ directory prefixes.
>
>
> Changed in v2:
>
> - Put the Buffer APIs extracted from VNC code into util/ instead
> of io/ directories (Paolo)
>
> - Added Kevin & Stefan to MAINTAINERS for coroutine code (Paolo)
>
> - 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 (16):
> sockets: add helpers for creating SocketAddress from a socket
> sockets: move qapi_copy_SocketAddress into qemu-sockets.c
> sockets: allow port to be NULL when listening on IP address
> ui: convert VNC startup code to use SocketAddress
> osdep: add qemu_fork() wrapper for safely handling signals
> coroutine: move into libqemuutil.a library
> util: pull Buffer code out of VNC module
> 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 | 20 +
> Makefile | 2 +
> Makefile.objs | 9 +-
> Makefile.target | 2 +
> block.c | 2 +-
> block/qcow2.h | 2 +-
> block/vdi.c | 2 +-
> block/write-threshold.c | 2 +-
> blockjob.c | 2 +-
> configure | 11 +
> hw/9pfs/codir.c | 2 +-
> hw/9pfs/cofile.c | 2 +-
> hw/9pfs/cofs.c | 2 +-
> hw/9pfs/coxattr.c | 2 +-
> hw/9pfs/virtio-9p-coth.c | 2 +-
> hw/9pfs/virtio-9p-coth.h | 2 +-
> hw/9pfs/virtio-9p.h | 2 +-
> include/block/block.h | 2 +-
> include/block/block_int.h | 2 +-
> include/io/channel-buffer.h | 59 ++
> 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 | 506 +++++++++++
> include/io/task.h | 256 ++++++
> include/qemu/buffer.h | 118 +++
> include/{block => qemu}/coroutine.h | 0
> include/{block => qemu}/coroutine_int.h | 2 +-
> include/qemu/osdep.h | 16 +
> include/qemu/sockets.h | 34 +
> io/Makefile.objs | 9 +
> io/channel-buffer.c | 255 ++++++
> io/channel-command.c | 369 ++++++++
> io/channel-file.c | 237 +++++
> io/channel-socket.c | 737 ++++++++++++++++
> io/channel-tls.c | 405 +++++++++
> io/channel-watch.c | 200 +++++
> io/channel-websock.c | 975
> +++++++++++++++++++++
> io/channel.c | 283 ++++++
> io/task.c | 159 ++++
> migration/qemu-file-buf.c | 2 +-
> migration/qemu-file-stdio.c | 2 +-
> migration/qemu-file-unix.c | 2 +-
> migration/qemu-file.c | 2 +-
> migration/rdma.c | 2 +-
> nbd.c | 2 +-
> qemu-char.c | 25 -
> scripts/create_config | 9 +
> tests/.gitignore | 7 +
> tests/Makefile | 16 +
> tests/io-channel-helpers.c | 247 ++++++
> tests/io-channel-helpers.h | 33 +
> tests/test-coroutine.c | 4 +-
> tests/test-io-channel-command.c | 122 +++
> tests/test-io-channel-file.c | 91 ++
> tests/test-io-channel-socket.c | 367 ++++++++
> tests/test-io-channel-tls.c | 335 +++++++
> tests/test-io-task.c | 276 ++++++
> tests/test-vmstate.c | 2 +-
> thread-pool.c | 2 +-
> trace-events | 56 ++
> ui/vnc.c | 204 ++---
> ui/vnc.h | 16 +-
> util/Makefile.objs | 4 +
> util/buffer.c | 65 ++
> coroutine-gthread.c => util/coroutine-gthread.c | 2 +-
> .../coroutine-sigaltstack.c | 2 +-
> coroutine-ucontext.c => util/coroutine-ucontext.c | 2 +-
> coroutine-win32.c => util/coroutine-win32.c | 2 +-
> util/oslib-posix.c | 71 ++
> util/oslib-win32.c | 9 +
> qemu-coroutine-io.c => util/qemu-coroutine-io.c | 2 +-
> .../qemu-coroutine-lock.c | 4 +-
> .../qemu-coroutine-sleep.c | 2 +-
> qemu-coroutine.c => util/qemu-coroutine.c | 4 +-
> util/qemu-sockets.c | 158 +++-
> 79 files changed, 7396 insertions(+), 197 deletions(-)
> 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 include/qemu/buffer.h
> rename include/{block => qemu}/coroutine.h (100%)
> rename include/{block => qemu}/coroutine_int.h (98%)
> 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-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
> create mode 100644 util/buffer.c
> rename coroutine-gthread.c => util/coroutine-gthread.c (99%)
> rename coroutine-sigaltstack.c => util/coroutine-sigaltstack.c (99%)
> rename coroutine-ucontext.c => util/coroutine-ucontext.c (99%)
> rename coroutine-win32.c => util/coroutine-win32.c (98%)
> rename qemu-coroutine-io.c => util/qemu-coroutine-io.c (99%)
> rename qemu-coroutine-lock.c => util/qemu-coroutine-lock.c (98%)
> rename qemu-coroutine-sleep.c => util/qemu-coroutine-sleep.c (96%)
> rename qemu-coroutine.c => util/qemu-coroutine.c (98%)
>
> --
> 2.4.3
>
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 :|
- [Qemu-devel] [PATCH v2 08/16] io: add abstract QIOChannel classes, (continued)
- [Qemu-devel] [PATCH v2 08/16] io: add abstract QIOChannel classes, Daniel P. Berrange, 2015/10/12
- [Qemu-devel] [PATCH v2 09/16] io: add helper module for creating watches on FDs, Daniel P. Berrange, 2015/10/12
- [Qemu-devel] [PATCH v2 12/16] io: add QIOChannelFile class, Daniel P. Berrange, 2015/10/12
- [Qemu-devel] [PATCH v2 13/16] io: add QIOChannelTLS class, Daniel P. Berrange, 2015/10/12
- [Qemu-devel] [PATCH v2 14/16] io: add QIOChannelWebsock class, Daniel P. Berrange, 2015/10/12
- [Qemu-devel] [PATCH v2 16/16] io: add QIOChannelBuffer class, Daniel P. Berrange, 2015/10/12
- [Qemu-devel] [PATCH v2 15/16] io: add QIOChannelCommand class, Daniel P. Berrange, 2015/10/12
- [Qemu-devel] [PATCH v2 05/16] osdep: add qemu_fork() wrapper for safely handling signals, Daniel P. Berrange, 2015/10/15
- [Qemu-devel] [PATCH v2 11/16] io: add QIOChannelSocket class, Daniel P. Berrange, 2015/10/15
- [Qemu-devel] [PATCH v2 10/16] io: add QIOTask class for async operations, Daniel P. Berrange, 2015/10/15
- Re: [Qemu-devel] [PATCH v2 00/16] Introduce I/O channels framework,
Daniel P. Berrange <=