From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58792) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zob1C-0003ay-SN for qemu-devel@nongnu.org; Tue, 20 Oct 2015 13:48:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zob17-0003L0-Rp for qemu-devel@nongnu.org; Tue, 20 Oct 2015 13:48:14 -0400 Received: from mx1.redhat.com ([209.132.183.28]:32975) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zob17-0003Kv-Ik for qemu-devel@nongnu.org; Tue, 20 Oct 2015 13:48:09 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (Postfix) with ESMTPS id 02631A2C35 for ; Tue, 20 Oct 2015 17:48:09 +0000 (UTC) References: <1444648509-29179-1-git-send-email-berrange@redhat.com> <1444648509-29179-9-git-send-email-berrange@redhat.com> From: Eric Blake Message-ID: <56267E53.1050708@redhat.com> Date: Tue, 20 Oct 2015 11:48:03 -0600 MIME-Version: 1.0 In-Reply-To: <1444648509-29179-9-git-send-email-berrange@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="TOp3jUogK6mmIFqbfsRWDr2F2XdreOk3a" Subject: Re: [Qemu-devel] [PATCH v2 08/16] io: add abstract QIOChannel classes List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" , qemu-devel@nongnu.org Cc: Paolo Bonzini , "Dr. David Alan Gilbert" This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --TOp3jUogK6mmIFqbfsRWDr2F2XdreOk3a Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable 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. >=20 > 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. >=20 > Signed-off-by: Daniel P. Berrange > --- > 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 >=20 > +++ 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 */ =2E.. > + > + /* 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); =2E..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 byte= s > + * > + * 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 =3D 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 pas= sing"); > + 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 =3D QIO_CHANNEL_GET_CLASS(ioc); > + return klass->io_writev(ioc, iov, niov, fds, nfds, errp); =2E..but rely on the callbacks to filter fds here? --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --TOp3jUogK6mmIFqbfsRWDr2F2XdreOk3a Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJWJn5TAAoJEKeha0olJ0NqlFkH/1Km4yZTjEA+O3s7s87Q59Jz YZq0E5u3Qoawxu/yWfYnFD/S+NlatNbEBJqUMkBIOXIy3VFMGvEsGuVA5QNKJhHO hEQmIO6LsKaUK2piQB3tH2E5xKUKa8imV2UgJpFWFglCzdzDdI46ki7SM6ZboWdN D9B3Ps9DVO0Nm9PmNCSHDWPECXVirk6qA0/tTtk6F2ClwI6yB7t91StZwC73aQB8 NRynSjLvu3NKhPJMfBV/n49ke4KhR+OmG1XKcP+blyNBAf6zbnLIDycftMs6mP/x Gyx2/bN9xO+DXazEXbOrUdFVAwl63xFP+HOmaFQ6tj0aREAK3ATLV3u6O7gS//M= =iZk6 -----END PGP SIGNATURE----- --TOp3jUogK6mmIFqbfsRWDr2F2XdreOk3a--