Re: [libvirt] [PATCH 25/27] fdstream: Suppress use of IO helper for sparse streams

On Thu, May 05, 2016 at 09:51:22AM -0600, Eric Blake wrote: > On 05/05/2016 09:32 AM, Daniel P. Berrange wrote: >> On Thu, Apr 28, 2016 at 12:05:12PM +0200, Michal Privoznik wrote: >>> This is kind of a hacky approach to the following problem, but so >>> far I am unable to come up with anything better. On some >>> occasions (esp. when dealing with regular files) libvirt_iohelper >>> is spawned to prefetch data for us. We will then have a pipe then >>> for reading the data from it. This does not fit in our sparse >>> stream implementation as one simply doesn't lseek() over a pipe. >>> Until this is resolved, let's suppress use of the IO helper and >>> read data from FD directly. >> >> This doesn't really fly - the problem is that with regular files, >> poll() on the FD will always return ready, even if the read or >> write will block in I/O. So by nomt using the iohelper this is >> going to cause our main loop to block on I/O for streams. > > The only real solution is to teach libvirt_iohelper to do structured > reads when requested. That is, you'll have to add a command-line flag > to libvirt_iohelper, which if present, says all of the output from > libvirt_iohelper will be structured as tuples of either > <type=data,length,bytes> or of <type=hole,length>. When used in this > mode, the client HAS to parse the tuples, rather than assuming that the > pipe can be read literally. So that means we also have to teach the > consumer of libvirt_iohelper how to read tuples off the pipe, at which > point it then knows whether to send a regular VIR_NET_STREAM or the > compact VIR_NET_STREAM_SKIP. Yeah, that doesn't sound too bad - its rather similar to the HTTP chunked encoding idea. It isn't much extra overhead to have a type + len field in the byte stream, as long as we put a sensible min size on the holes we transmit. eg don't send a hole that's less than 512 bytes in len.