On 10/12/2015 05:15 AM, Daniel P. Berrange wrote: > Start the new generic I/O channel framework by defining a > QIOChannel abstract base class. This is designed to feel > similar to GLib's GIOChannel, but with the addition of > support for using iovecs, qemu error reporting, file > descriptor passing, coroutine integration and use of > the QOM framework for easier sub-classing. > > The intention is that anywhere in QEMU that almost > anywhere that deals with sockets will use this new I/O > infrastructure, so that it becomes trivial to then layer > in support for TLS encryption. This will at least include > the VNC server, char device backend and migration code. > > Signed-off-by: Daniel P. Berrange <berra...@redhat.com> > --- > MAINTAINERS | 7 + > Makefile | 2 + > Makefile.objs | 5 + > Makefile.target | 2 + > include/io/channel.h | 506 > +++++++++++++++++++++++++++++++++++++++++++++++++++ > io/Makefile.objs | 1 + > io/channel.c | 283 ++++++++++++++++++++++++++++ > 7 files changed, 806 insertions(+) > create mode 100644 include/io/channel.h > create mode 100644 io/Makefile.objs > create mode 100644 io/channel.c >
> +++ b/include/io/channel.h
> @@ -0,0 +1,506 @@
> +struct QIOChannel {
> + Object parent;
> + int features; /* bitmask of QIOChannelFeatures */
Would an unsigned type make bit manipulations any less likely to avoid
clang sanitizer warnings under future extensions of new bits?
> +};
> +
> +/**
> + * QIOChannelClass:
> + *
> + * This class defines the contract that all subclasses
> + * must follow to provide specific channel implementations.
> + * All the callbacks are mandatory with the exception of
> + * io_has_feature, which defaults to returning false.
And yet...
> + *
> + * Consult the corresponding public API docs for a description
> + * of the semantics of each callback
> + */
> +struct QIOChannelClass {
> + ObjectClass parent;
> +
> + /* Mandatory callbacks */
...
> +
> + /* Optional callbacks */
> + int (*io_shutdown)(QIOChannel *ioc,
> + QIOChannelShutdown how,
> + Error **errp);
> + void (*io_set_cork)(QIOChannel *ioc,
> + bool enabled);
> + void (*io_set_delay)(QIOChannel *ioc,
> + bool enabled);
> + off64_t (*io_seek)(QIOChannel *ioc,
> + off64_t offset,
> + int whence,
> + Error **errp);
...there are four optional callbacks listed, and no mention of a
callback named io_has_feature. Sounds like docs need an update.
> +
> +/**
> + * qio_channel_has_feature:
> + * @ioc: the channel object
> + * @feature: the feature to check support of
> + *
> + * Determine whether the channel implementation supports
> + * the optional feature named in @feature.
> + *
> + * Returns: true if supported, false otherwise.
> + */
> +bool qio_channel_has_feature(QIOChannel *ioc,
> + QIOChannelFeature feature);
Can this be used to check multiple features at once, or are we content
that checking two features requires two calls?
> +/**
> + * qio_channel_seek:
> + * @ioc: the channel object
> + * @offset: the position to seek to, relative to @whence
> + * @whence: one of the POSIX SEEK_* constants
Including SEEK_HOLE/SEEK_DATA?
> + * @errp: pointer to an uninitialized error object
> + *
> + * Moves the current I/O position within the channel
> + * @ioc, to be @offset. The value of @offset is
> + * interpreted relative to @whence:
> + *
> + * SEEK_SET - the position is set to @offset bytes
> + * SEEK_CUR - the position is moved by @offset bytes
> + * SEEK_END - the position is set to end of the file plus @offset bytes
> + *
> + * Not all implementations will support this facility,
> + * so may report an error. To avoid errors, the
> + * caller may check for the feature flag
> + * QIO_CHANNEL_FEATURE_SEEKABLE prior to calling
> + * this method.
I don't see that constant defined; stale docs?
> +++ b/io/channel.c
> @@ -0,0 +1,283 @@
> +ssize_t qio_channel_readv_full(QIOChannel *ioc,
> + const struct iovec *iov,
> + size_t niov,
> + int **fds,
> + size_t *nfds,
> + Error **errp)
> +{
> + QIOChannelClass *klass = QIO_CHANNEL_GET_CLASS(ioc);
> +
> + if ((fds || nfds) &&
> + !(ioc->features & (1 << QIO_CHANNEL_FEATURE_FD_PASS))) {
> + error_setg_errno(errp, EINVAL,
> + "Channel does not support file descriptor passing");
> + return -1;
> + }
Why pre-filter fds here...
> +
> + return klass->io_readv(ioc, iov, niov, fds, nfds, errp);
> +}
> +
> +
> +ssize_t qio_channel_writev_full(QIOChannel *ioc,
> + const struct iovec *iov,
> + size_t niov,
> + int *fds,
> + size_t nfds,
> + Error **errp)
> +{
> + QIOChannelClass *klass = QIO_CHANNEL_GET_CLASS(ioc);
> + return klass->io_writev(ioc, iov, niov, fds, nfds, errp);
...but rely on the callbacks to filter fds here?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
