[libvirt] [PATCH 03/12] Introduce a generic object for using network sockets

[Eric Blake]

On 03/18/2011 12:54 PM, Daniel P. Berrange wrote:
> Introduces a simple wrapper around the raw POSIX sockets APIs
> and name resolution APIs. Allows for easy creation of client
> and server sockets with correct usage of name resolution APIs
> for protocol agnostic socket setup.
> 
> It can listen for UNIX and TCP stream sockets.
> 
> It can connect to UNIX, TCP streams directly, or indirectly
> to UNIX sockets via an SSH tunnel or external command
> 
> * src/Makefile.am: Add to libvirt-net-rpc.la
> * src/rpc/virnetsocket.c, src/rpc/virnetsocket.h: Generic
>   sockets APIs
> ---
>  .x-sc_avoid_write      |    1 +
>  cfg.mk                 |    1 +
>  configure.ac           |    2 +-
>  po/POTFILES.in         |    1 +
>  src/Makefile.am        |    3 +-
>  src/rpc/virnetsocket.c |  835 ++++++++++++++++++++++++++++++++++++++++++++++++
>  src/rpc/virnetsocket.h |  107 ++++++
>  7 files changed, 948 insertions(+), 2 deletions(-)
>  create mode 100644 src/rpc/virnetsocket.c
>  create mode 100644 src/rpc/virnetsocket.h
> 
> diff --git a/.x-sc_avoid_write b/.x-sc_avoid_write
> index f6fc1b2..303c940 100644
> --- a/.x-sc_avoid_write
> +++ b/.x-sc_avoid_write
> @@ -1,6 +1,7 @@
>  ^src/libvirt\.c$
>  ^src/fdstream\.c$
>  ^src/qemu/qemu_monitor\.c$
> +^src/rpc/virnetsocket\.c$

I'd rather not change this file, but instead apply my (unreviewed) patch
first, and then you merely edit cfg.mk to add this to the list of exempt
files:
https://www.redhat.com/archives/libvir-list/2011-March/msg00770.html

But if your patch goes in first, I can adapt.

> +
> +    oldmask = umask(~mask);
> +
> +    if (bind(fd, &addr.data.sa, addr.len) < 0) {
> +        virReportSystemError(errno,
> +                             _("Failed to bind socket to '%s'"),
> +                             path);
> +        goto error;
> +    }
> +    umask(oldmask);

Is it worth a comment why umask() is safe here?  You already mentioned
on list that it is because this setup function is only called during
single-threaded startup code.

> +
> +int virNetSocketNewListenTCP(const char *nodename,
> +                             const char *service,
> +                             virNetSocketPtr **addrs,
> +                             size_t *naddrs);

ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
ATTRIBUTE_NONNULL(4)

Many other functions in this header can likewise have various parameters
marked up.

ACK with those nits addressed.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20110321/1fcec269/attachment-0001.sig>