On Mon, Nov 25, 2013 at 01:45:54PM -0700, Eric Blake wrote: > On 11/25/2013 08:59 AM, Daniel P. Berrange wrote: > > On Fri, Nov 22, 2013 at 08:20:30PM -0700, Eric Blake wrote: > >> Putting together pieces from previous patches, it is now possible > >> for 'virsh vol-dumpxml --pool gluster volname' to report metadata > >> about a qcow2 file stored on gluster. The backing file is still > >> treated as raw; to fix that, more patches are needed to make the > >> storage backing chain analysis recursive rather than halting at > >> a network protocol name, but that work will not need any further > >> calls into libgfapi so much as just reusing this code, and that > >> should be the only code outside of the storage driver that needs > >> any help from libgfapi. Any additional use of libgfapi within > >> libvirt should only be needed for implementing storage pool APIs > >> such as volume creation or resizing, where backing chain analysis > >> should be unaffected. > >> > > >> + while (maxlen) { > >> + ssize_t r = glfs_read(fd, s, maxlen, 0); > >> + if (r < 0 && errno == EINTR) > >> + continue; > >> + if (r < 0) { > >> + VIR_FREE(*buf); > >> + virReportSystemError(errno, _("unable to read '%s'"), name); > >> + return r; > >> + } > > > > Further down you're requesting O_NONBLOCK, and here you are > > not handling EAGAIN explicitly. Is is desirable that we turn > > EAGAIN into a fatal error, or should we remove the O_NONBLOCK > > flag ? > > Hmm. I was copying from directory pools, which also use O_NONBLOCK then > call into virFileReadHeaderFD. So that code needs to be fixed (separate > patch). For gluster, I think it's easiest to just drop O_NONBLOCK from > the code (I just verified that attempts to use 'mkfifo' inside a gluster > volume fail with EACCES); for directory pools you DO want to open > O_NONBLOCK (otherwise opening a fifo for read would hang waiting for a > writer), then use virSetNonBlock() after verifying file type but before > reading the header (since we already reject fifos as unable to be used > as a storage volume). Fortunately with local directory based pools, the only effect that O_NONBLOCK has is to prevent you blocking on fifos. If you use the O_NONBLOCK flag in conjunction with plain files, you'll still block waiting for I/O, to the annoyance of programmers everywhere. 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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list