Re: [libvirt] [PATCH 05/11] Add a generic internal API for handling any FD based stream
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(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/