[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 08/16] io: add abstract QIOChannel classes
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v2 08/16] io: add abstract QIOChannel classes |
Date: |
Tue, 20 Oct 2015 11:48:03 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
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 <address@hidden>
> ---
> 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
- Re: [Qemu-devel] [PATCH v2 04/16] ui: convert VNC startup code to use SocketAddress, (continued)
[Qemu-devel] [PATCH v2 01/16] sockets: add helpers for creating SocketAddress from a socket, Daniel P. Berrange, 2015/10/12
[Qemu-devel] [PATCH v2 06/16] coroutine: move into libqemuutil.a library, Daniel P. Berrange, 2015/10/12
[Qemu-devel] [PATCH v2 07/16] util: pull Buffer code out of VNC module, Daniel P. Berrange, 2015/10/12
[Qemu-devel] [PATCH v2 08/16] io: add abstract QIOChannel classes, Daniel P. Berrange, 2015/10/12
- Re: [Qemu-devel] [PATCH v2 08/16] io: add abstract QIOChannel classes,
Eric Blake <=
[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