Re: [libvirt] [PATCH 06/38] virfdstream: Use messages instead of pipe
On Thu, Apr 13, 2017 at 03:31:14PM +0200, Michal Privoznik wrote:
One big downside of using the pipe to transfer the data is that
we can really transfer just bare data. No metadata can be carried
through unless some formatted messages are introduced. That would
be quite painful to achieve so let's use a message queue. It's
fairly easy to exchange info between threads now that iohelper is
no longer used.
I'm not seeing how this works correctly with the event loop.
@@ -752,8 +1014,6 @@ virFDStreamOpenFileInternal(virStreamPtr st,
if ((st->flags & VIR_STREAM_NONBLOCK) &&
((!S_ISCHR(sb.st_mode) &&
!S_ISFIFO(sb.st_mode)) || forceIOHelper)) {
- int fds[2] = { -1, -1 };
-
if ((oflags & O_ACCMODE) == O_RDWR) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("%s: Cannot request read and write flags
together"),
@@ -761,12 +1021,6 @@ virFDStreamOpenFileInternal(virStreamPtr st,
goto error;
}
- if (pipe(fds) < 0) {
- virReportSystemError(errno, "%s",
- _("Unable to create pipe"));
- goto error;
- }
Here we previously created the pipe....
@@ -775,18 +1029,14 @@ virFDStreamOpenFileInternal(virStreamPtr st,
if ((oflags & O_ACCMODE) == O_RDONLY) {
threadData->fdin = fd;
- threadData->fdout = fds[1];
- if (VIR_STRDUP(threadData->fdinname, path) < 0 ||
- VIR_STRDUP(threadData->fdoutname, "pipe") < 0)
+ threadData->fdout = -1;
+ if (VIR_STRDUP(threadData->fdinname, path) < 0)
goto error;
- fd = fds[0];
And here we set 'fd' to be the pipe
} else {
- threadData->fdin = fds[0];
+ threadData->fdin = -1;
threadData->fdout = fd;
- if (VIR_STRDUP(threadData->fdinname, "pipe") < 0 ||
- VIR_STRDUP(threadData->fdoutname, path) < 0)
+ if (VIR_STRDUP(threadData->fdoutname, path) < 0)
goto error;
- fd = fds[1];
Likewise here
}
}
...now here 'fd' is passed to virFDStreamOpenInternal() and is the thing
that the event loop watches are registered against by virFDStreamAddCallback
With this change 'fd' is the actual plain file the thread is reading to/from,
so the callbacks are being registered against the plain file, not the pipe.
poll/select on POSIX always reports plain files as readable/writable even
when they would block. So with this change we're just going to busy loop
in the main event thread even when we'll block, which defeats the whole
purpose of having a iohelper and/or thread.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|