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@xxxxxxxxxx>
---
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
