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