From 4a24c29809f8a924d1dcbc47a4dbe5ff05ba9158 Mon Sep 17 00:00:00 2001
From: Nir Soffer <nsoffer@redhat.com>
Date: Mon, 14 Oct 2024 22:09:45 +0300
Subject: [PATCH] Fix nativeimgutil.ConvertToRaw()

- Truncate before copying the data to eliminate the seeks during the
  copy. This also provides a hint to the file system that can minimize
  allocations and fragmentation of the file.

- Avoid unneeded seeks during copy using WriteAt. This does not improve
  performance since practically all time is spent on reading from the
  source image.

- Fix zero detection to handle short reads. Previously we would compare
  the entire buffer which can contain non-zero bytes from the previous
  read.

- Fix write to handle short reads. Previously we would write the entire
  buffer including data from previous read, corrupting the image.

- Fix error message for failed write. Looks like it was copied from the
  read branch.

Signed-off-by: Nir Soffer <nsoffer@redhat.com>
---
 pkg/nativeimgutil/nativeimgutil.go | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/pkg/nativeimgutil/nativeimgutil.go b/pkg/nativeimgutil/nativeimgutil.go
index 7db33c163d8..68ad213ff82 100644
--- a/pkg/nativeimgutil/nativeimgutil.go
+++ b/pkg/nativeimgutil/nativeimgutil.go
@@ -68,6 +68,13 @@ func ConvertToRaw(source, dest string, size *int64, allowSourceWithBackingFile b
 	defer os.RemoveAll(destTmp)
 	defer destTmpF.Close()
 
+	// Truncating before copy eliminates the seeks during copy and provide a
+	// hint to the file system that may minimize allocations and fragmanation
+	// of the file.
+	if err := MakeSparse(destTmpF, srcImg.Size()); err != nil {
+		return err
+	}
+
 	// Copy
 	srcImgR := io.NewSectionReader(srcImg, 0, srcImg.Size())
 	bar, err := progressbar.New(srcImg.Size())
@@ -124,8 +131,8 @@ func convertRawToRaw(source, dest string, size *int64) error {
 
 func copySparse(w *os.File, r io.Reader, bufSize int64) (int64, error) {
 	var (
-		n              int64
-		eof, hasWrites bool
+		n   int64
+		eof bool
 	)
 
 	zeroBuf := make([]byte, bufSize)
@@ -139,20 +146,15 @@ func copySparse(w *os.File, r io.Reader, bufSize int64) (int64, error) {
 			}
 		}
 		// TODO: qcow2reader should have a method to notify whether buf is zero
-		if bytes.Equal(buf, zeroBuf) {
-			if _, sErr := w.Seek(int64(rN), io.SeekCurrent); sErr != nil {
-				return n, fmt.Errorf("failed seek: %w", sErr)
-			}
-			// no need to ftruncate here
+		if bytes.Equal(buf[:rN], zeroBuf[:rN]) {
 			n += int64(rN)
 		} else {
-			hasWrites = true
-			wN, wErr := w.Write(buf)
+			wN, wErr := w.WriteAt(buf[:rN], n)
 			if wN > 0 {
 				n += int64(wN)
 			}
 			if wErr != nil {
-				return n, fmt.Errorf("failed to read: %w", wErr)
+				return n, fmt.Errorf("failed to write: %w", wErr)
 			}
 			if wN != rN {
 				return n, fmt.Errorf("read %d, but wrote %d bytes", rN, wN)
@@ -160,10 +162,6 @@ func copySparse(w *os.File, r io.Reader, bufSize int64) (int64, error) {
 		}
 	}
 
-	// Ftruncate must be run if the file contains only zeros
-	if !hasWrites {
-		return n, MakeSparse(w, n)
-	}
 	return n, nil
 }