[PATCH v2 13/17] virfdstream: Allow sparse stream vol-download

[Michal Privoznik]

On 8/20/20 1:57 PM, Peter Krempa wrote:
> On Tue, Jul 07, 2020 at 21:46:31 +0200, Michal Privoznik wrote:
>> When handling sparse stream, a thread is executed. This thread
>> runs a read() or write() loop (depending what API is called; in
>> this case it's virStorageVolDownload() and  this the thread run
>> read() loop). The read() is handled in virFDStreamThreadDoRead()
>> which is then data/hole section aware, meaning it uses
>> virFileInData() to detect data and hole sections and sends
>> TYPE_DATA or TYPE_HOLE virStream messages accordingly.
>>
>> However, virFileInData() does not work with block devices. Simply
>> because block devices don't have data and hole sections. But we
>> can use new virFileInDataDetectZeroes() which is block device
>> friendly for that.
>>
>> Partially resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1852528
>>
>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>> ---
>>   src/util/virfdstream.c | 15 ++++++++++++---
>>   1 file changed, 12 insertions(+), 3 deletions(-)
> 
> IMO this goes against the semantics of the _SPARSE_STREAM flag. A block
> device by definition is not sparse, so there are no holes to send.
> 
> What you've implemented is a way to sparsify a block device, but that
> IMO should not be considered by default when a block device is used.
> If a file is not sparse, the previous code doesn't actually transmit
> holes either.
> 
> If you want to achieve sparsification on the source side of the
> transmission, this IMO needs an explicit flag to opt-in and then we
> should sparsify also regular files using the same algorithm.
> 

Fair enough. So how about I'll send v3 where:

a) in the first patches I make our stream read/write functions handle 
block devices for _SPARSE_STREAM without any zero block detection. Only 
thing that will happen is that if the source is a sparse regular file 
and thus the stream receiver gets a HOLE packet and it is writing the 
data into a block device it will have to emulate the hole by writing 
block of zeroes. However, if the stream source is a block device then no 
HOLE shall ever be sent.

b) in next patches I'll introduce _DETECT_ZEROES flag (and possibly make 
it require _SPARSE_STREAM too) which will handle the case where the 
stream source is a block device with zero blocks, at which point it will 
try to detect them and be allowed to send HOLE down the stream.

Does this sound reasonable?

BTW: I've pushed the first N patches which were just switching FDStream 
to glib.

Michal