From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56163) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yj8ZD-00022J-D4 for qemu-devel@nongnu.org; Fri, 17 Apr 2015 11:52:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Yj8Z9-0004Ja-Mf for qemu-devel@nongnu.org; Fri, 17 Apr 2015 11:52:31 -0400 Received: from mx1.redhat.com ([209.132.183.28]:2096) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yj8Z9-0004JJ-GI for qemu-devel@nongnu.org; Fri, 17 Apr 2015 11:52:27 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (Postfix) with ESMTPS id 1BF308E3EF for ; Fri, 17 Apr 2015 15:52:27 +0000 (UTC) Date: Fri, 17 Apr 2015 16:52:22 +0100 From: "Daniel P. Berrange" Message-ID: <20150417155222.GK23619@redhat.com> References: <1429280557-8887-1-git-send-email-berrange@redhat.com> <1429280557-8887-24-git-send-email-berrange@redhat.com> <55312689.3070202@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <55312689.3070202@redhat.com> Subject: Re: [Qemu-devel] [PATCH v1 RFC 23/34] io: add QIOChannelSocket class Reply-To: "Daniel P. Berrange" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org, Stefan Hajnoczi , Gerd Hoffmann On Fri, Apr 17, 2015 at 05:28:09PM +0200, Paolo Bonzini wrote: > > > On 17/04/2015 16:22, Daniel P. Berrange wrote: > > Implement a QIOChannel subclass that supports sockets I/O > > > > TBD check errno handling of windows port & fix watch impl > > > > Signed-off-by: Daniel P. Berrange > > --- > > include/io/channel-socket.h | 168 +++++++++++++ > > io/Makefile.objs | 1 + > > io/channel-socket.c | 572 ++++++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 741 insertions(+) > > create mode 100644 include/io/channel-socket.h > > create mode 100644 io/channel-socket.c > > > > diff --git a/include/io/channel-socket.h b/include/io/channel-socket.h > > new file mode 100644 > > index 0000000..b95349b > > --- /dev/null > > +++ b/include/io/channel-socket.h > > +/** > > + * qio_channel_socket_get_local_addr_string: > > + * @ioc: the socket channel object > > + * @hostname: pointer to be filled with hostname string > > + * @servicename: pointer to be filled with servicename string > > + * @family: pointer to be filled with network address family > > + * @errp: pointer to an uninitialized error object > > + * > > + * Get the string representation of the local socket > > + * address. The address information will be stored in > > + * the @hostname, @servicename and @family parameters. > > + * The @hostname and @servicename strings will be > > + * allocated to the size required and should be free > > + * with g_free() when no longer required > > + * > > + * Returns: 0 on success, -1 on error > > + */ > > +int > > +qio_channel_socket_get_local_addr_string(QIOChannelSocket *ioc, > > + char **hostname, > > + char **servicename, > > + NetworkAddressFamily *family, > > + Error **errp); > > + > > +/** > > + * qio_channel_socket_get_remote_addr_string: > > + * @ioc: the socket channel object > > + * @hostname: pointer to be filled with hostname string > > + * @servicename: pointer to be filled with servicename string > > + * @family: pointer to be filled with network address family > > + * @errp: pointer to an uninitialized error object > > + * > > + * Get the string representation of the remote socket > > + * address. The address information will be stored in > > + * the @hostname, @servicename and @family parameters. > > + * The @hostname and @servicename strings will be > > + * allocated to the size required and should be free > > + * with g_free() when no longer required > > + * > > + * Returns: 0 on success, -1 on error > > + */ > > +int > > +qio_channel_socket_get_remote_addr_string(QIOChannelSocket *ioc, > > + char **hostname, > > + char **servicename, > > + NetworkAddressFamily *family, > > + Error **errp); > > Would it be possible to change these to use a SocketAddress* type? Yeah, that looks like it is a viable option. > > + > > +/** > > + * qio_channel_socket_set_nodelay: > > + * @ioc: the socket channel object > > + * @enabled: the new flag state > > + * > > + * Set the state of the NODELAY socket flag. If the > > The function name is okay, but please write TCP_NODELAY in the > documentation, or talk about Nagle's algorithm instead of mentioning the > flag. Sure will do. > > + * @enabled parameter is true, then NODELAY will be > > + * set and data will be transmitted immediately. If > > + * @enabled is false, then data may be temporarily > > + * held for transmission to enable writes to be > > + * coallesced. > > + */ > > +void > > +qio_channel_socket_set_nodelay(QIOChannelSocket *ioc, > > + bool enabled); > > + > > +/** > > + * qio_channel_socket_accept: > > + * @ioc: the socket channel object > > + * @errp: pointer to an uninitialized error object > > + * > > + * If the socket represents a server, then this accepts > > + * a new client connection. The returned channel will > > + * represent the connected client socket. > > + * > > + * Returns: the new client channel, or NULL on error > > + */ > > +QIOChannelSocket * > > +qio_channel_socket_accept(QIOChannelSocket *ioc, > > + Error **errp); > > Does it make sense for a passive socket to be a QIOChannelSocket? We > have already a pretty decent API in util/qemu-sockets.c, and > QIOChannelSocket will become more similar to qemu-sockets if you switch > to SocketAddress. Perhaps this function can just take a file descriptor? I was somewhat undecided about that really - One of my todos is to see about better integrating with qemu-sockets for the connection facilities, so will consider this problem too. 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 :|