[libvirt] [PATCH 05/11] Add a generic internal API for handling any FD based stream

[Daniel Veillard]

On Tue, Nov 02, 2010 at 05:49:09PM +0000, Daniel P. Berrange wrote:
> To avoid the need for duplicating implementations of virStream
> drivers, provide a generic implementation that can handle any
> FD based stream. This code is copied from the existing impl
> in the QEMU driver, with the locking moved into the stream
> impl, and addition of a read callback
> 
> The FD stream code will refuse to operate on regular files or
> block devices, since those can't report EAGAIN properly when
> they would block on I/O
> 
> * include/libvirt/virterror.h, include/libvirt/virterror.h: Add
>   VIR_FROM_STREAM error domain
> * src/qemu/qemu_driver.c: Remove code obsoleted by the new
>   generic streams driver.
> * src/fdstream.h, src/fdstream.c, src/fdstream.c,
>   src/libvirt_private.syms: Generic reusable FD based streams
[...]
> +
> +    virMutexLock(&fdst->lock);

  Somehow unrelated, but as I was going through the patch to check
mostly on error handling it appeard to me we kept all the virMutex
functions void, and in the pthread implementation we discard any
return value to pthread_mutex_lock/unlock ... I wonder if errors
should not be caught in those sub functions, and the erro stored in
the virMutex structure itself.

> +retry:
> +    ret = read(fdst->fd, bytes, nbytes);
> +    if (ret < 0) {
> +        if (errno == EAGAIN || errno == EWOULDBLOCK) {
> +            ret = -2;
> +        } else if (errno == EINTR) {
> +            goto retry;
> +        } else {
> +            ret = -1;
> +            virReportSystemError(errno, "%s",
> +                                 _("cannot read from stream"));
> +        }
> +    }

  I was wondering about reusing saferead/write but they have a different
semantic on blocking,

[...]
> +int virFDStreamOpen(virStreamPtr st,
> +                    int fd)
[...]
> +#if HAVE_SYS_UN_H
> +int virFDStreamConnectUNIX(virStreamPtr st,
> +                           const char *path,
> +                           bool abstract)
[...]
> +int virFDStreamOpenFile(virStreamPtr st,
> +                        const char *path,
> +                        int flags)

  I was wondering about remote socket, not needed right now but should
be doable too, right ?

ACK,

Daniel
-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/