Re: [PATCH v2 15/17] virfdstream: Emulate skip for block devices

[Peter Krempa]

On Tue, Jul 07, 2020 at 21:46:33 +0200, Michal Privoznik wrote:
> This is similar to one of previous patches.
> 
> When receiving stream (on virStorageVolUpload() and subsequent
> virStreamSparseSendAll()) we may receive a hole. If the volume we
> are saving the incoming data into is a regular file we just
> lseek() and ftruncate() to create the hole. But this won't work
> if the file is a block device. If that is the case we must write
> zeroes so that any subsequent reader reads nothing just zeroes
> (just like they would from a hole in a regular file).
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1852528
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/util/virfdstream.c | 59 +++---
>  1 file changed, 44 insertions(+), 15 deletions(-)

Reviewed-by: Peter Krempa 

[PATCH v2 15/17] virfdstream: Emulate skip for block devices

[Michal Privoznik]

This is similar to one of previous patches.

When receiving stream (on virStorageVolUpload() and subsequent
virStreamSparseSendAll()) we may receive a hole. If the volume we
are saving the incoming data into is a regular file we just
lseek() and ftruncate() to create the hole. But this won't work
if the file is a block device. If that is the case we must write
zeroes so that any subsequent reader reads nothing just zeroes
(just like they would from a hole in a regular file).

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1852528

Signed-off-by: Michal Privoznik 
---
 src/util/virfdstream.c | 59 +++---
 1 file changed, 44 insertions(+), 15 deletions(-)

diff --git a/src/util/virfdstream.c b/src/util/virfdstream.c
index 44b4855549..40825a8811 100644
--- a/src/util/virfdstream.c
+++ b/src/util/virfdstream.c
@@ -505,6 +505,7 @@ virFDStreamThreadDoRead(virFDStreamDataPtr fdst,
 static ssize_t
 virFDStreamThreadDoWrite(virFDStreamDataPtr fdst,
  bool sparse,
+ bool isBlock,
  const int fdin,
  const int fdout,
  const char *fdinname,
@@ -512,7 +513,6 @@ virFDStreamThreadDoWrite(virFDStreamDataPtr fdst,
 {
 ssize_t got = 0;
 virFDStreamMsgPtr msg = fdst->msg;
-off_t off;
 bool pop = false;
 
 switch (msg->type) {
@@ -540,19 +540,48 @@ virFDStreamThreadDoWrite(virFDStreamDataPtr fdst,
 }
 
 got = msg->stream.hole.len;
-off = lseek(fdout, got, SEEK_CUR);
-if (off == (off_t) -1) {
-virReportSystemError(errno,
- _("unable to seek in %s"),
- fdoutname);
-return -1;
-}
-
-if (ftruncate(fdout, off) < 0) {
-virReportSystemError(errno,
- _("unable to truncate %s"),
- fdoutname);
-return -1;
+if (isBlock) {
+g_autofree char * buf = NULL;
+const size_t buflen = 1 * 1024 * 1024; /* 1MiB */
+size_t toWrite = got;
+
+/* While for files it's enough to lseek() and ftruncate() to create
+ * a hole which would emulate zeroes on read(), for block devices
+ * we have to write zeroes to read() zeroes. And we have to write
+ * @got bytes of zeroes. Do that in smaller chunks though.*/
+
+buf = g_new0(char, buflen);
+
+while (toWrite) {
+size_t count = MIN(toWrite, buflen);
+ssize_t r;
+
+if ((r = safewrite(fdout, buf, count)) < 0) {
+virReportSystemError(errno,
+ _("Unable to write %s"),
+ fdoutname);
+return -1;
+}
+
+toWrite -= r;
+}
+} else {
+off_t off;
+
+off = lseek(fdout, got, SEEK_CUR);
+if (off == (off_t) -1) {
+virReportSystemError(errno,
+ _("unable to seek in %s"),
+ fdoutname);
+return -1;
+}
+
+if (ftruncate(fdout, off) < 0) {
+virReportSystemError(errno,
+ _("unable to truncate %s"),
+ fdoutname);
+return -1;
+}
 }
 
 pop = true;
@@ -618,7 +647,7 @@ virFDStreamThread(void *opaque)
   length, total,
   , buflen);
 else
-got = virFDStreamThreadDoWrite(fdst, sparse,
+got = virFDStreamThreadDoWrite(fdst, sparse, isBlock,
fdin, fdout,
fdinname, fdoutname);
 
-- 
2.26.2