storage: fix tar split writer to actually write the full file

The compressor must be closed before we write the bytes. However overall
I am not sure why we did write all bytes fully into memory first. So
chnage it to directly write to a file but still use a buffer for that to
avoid many small writes.

Signed-off-by: Paul Holzinger <[email protected]>

Author: Luap99

storage/layers.go

@@ -31,6 +31,7 @@ import (
 	"go.podman.io/storage/pkg/ioutils"
 	"go.podman.io/storage/pkg/lockfile"
 	"go.podman.io/storage/pkg/mount"
+	"go.podman.io/storage/pkg/pools"
 	"go.podman.io/storage/pkg/stringid"
 	"go.podman.io/storage/pkg/system"
 	"go.podman.io/storage/pkg/tarlog"
@@ -2452,95 +2453,90 @@ func (r *layerStore) ApplyDiff(to string, diff io.Reader) (size int64, err error
 	return r.applyDiffWithOptions(layer, nil, diff)
 }
 
-type diffApplyFunc func(id string, parent string, options drivers.ApplyDiffOpts, tsBytes *bytes.Buffer) error
+type diffApplyFunc func(id string, parent string, options drivers.ApplyDiffOpts) error
 
 func (r *layerStore) applyDiffUnlocked(dd drivers.ApplyDiffStaging, layer *Layer, layerOptions *LayerOptions, diff io.Reader) ([]tempdir.CleanupTempDirFunc, func() error, int64, error) {
 	var (
 		size           int64
 		cleanFunctions []tempdir.CleanupTempDirFunc
-		applyDiff      func() error
+		commit         tempdir.CommitFunc
 	)
 
-	f := func(id string, parent string, options drivers.ApplyDiffOpts, tsBytes *bytes.Buffer) error {
+	f := func(id string, parent string, options drivers.ApplyDiffOpts) error {
 		var (
 			err     error
 			cleanup tempdir.CleanupTempDirFunc
-			commit  tempdir.CommitFunc
 		)
 		cleanup, commit, size, err = dd.StartStagingDiffToApply(layer.ID, layer.Parent, options)
 		cleanFunctions = append(cleanFunctions, cleanup)
+		return err
+	}
+
+	td, err := tempdir.NewTempDir(filepath.Join(r.layerdir, tempDirPath))
+	cleanFunctions = append(cleanFunctions, td.Cleanup)
+	if err != nil {
+		return cleanFunctions, nil, 0, err
+	}
+
+	sa, err := td.StageFileAddition(func(path string) error {
+		tsFile, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY, 0o600)
 		if err != nil {
 			return err
 		}
+		defer tsFile.Close()
+		return r.applyDiffWithOptionsInner(layer, layerOptions, diff, f, tsFile)
+	})
+	if err != nil {
+		return cleanFunctions, nil, 0, err
+	}
 
-		td, err := tempdir.NewTempDir(filepath.Join(r.layerdir, tempDirPath))
-		cleanFunctions = append(cleanFunctions, td.Cleanup)
+	applyDiff := func() error {
+		err := dd.CommitStagedLayer(layer.ID, commit)
 		if err != nil {
-			return err
+			return fmt.Errorf("commit temporary layer: %w", err)
 		}
 
-		sa, err := td.StageFileAddition(func(path string) error {
-			return os.WriteFile(path, tsBytes.Bytes(), 0o600)
-		})
-		cleanFunctions = append(cleanFunctions, td.Cleanup)
-		if err != nil {
+		if err := os.MkdirAll(filepath.Dir(r.tspath(layer.ID)), 0o700); err != nil {
 			return err
 		}
 
-		applyDiff = func() error {
-			err := dd.CommitStagedLayer(layer.ID, commit)
-			if err != nil {
-				return fmt.Errorf("commit temporary layer: %w", err)
-			}
-
-			if err := os.MkdirAll(filepath.Dir(r.tspath(layer.ID)), 0o700); err != nil {
-				return err
-			}
-
-			err = sa.Commit(r.tspath(layer.ID))
-			if err != nil {
-				return fmt.Errorf("commit tar split file: %w", err)
-			}
-			return nil
+		err = sa.Commit(r.tspath(layer.ID))
+		if err != nil {
+			return fmt.Errorf("commit tar split file: %w", err)
 		}
 		return nil
 	}
 
-	err := r.applyDiffWithOptionsInner(layer, layerOptions, diff, f)
-	if err != nil {
-		return cleanFunctions, nil, 0, err
-	}
-
 	return cleanFunctions, applyDiff, size, nil
 }
 
 // Requires startWriting.
 func (r *layerStore) applyDiffWithOptions(layer *Layer, layerOptions *LayerOptions, diff io.Reader) (int64, error) {
 	var size int64
-	f := func(id string, parent string, options drivers.ApplyDiffOpts, tsBytes *bytes.Buffer) error {
+	f := func(id string, parent string, options drivers.ApplyDiffOpts) error {
 		var err error
 		size, err = r.driver.ApplyDiff(layer.ID, layer.Parent, options)
-		if err != nil {
-			return err
-		}
-		if err := os.MkdirAll(filepath.Dir(r.tspath(layer.ID)), 0o700); err != nil {
-			return err
-		}
-		if err := ioutils.AtomicWriteFile(r.tspath(layer.ID), tsBytes.Bytes(), 0o600); err != nil {
-			return err
-		}
-		return nil
+		return err
 	}
 
-	err := r.applyDiffWithOptionsInner(layer, layerOptions, diff, f)
+	if err := os.MkdirAll(filepath.Dir(r.tspath(layer.ID)), 0o700); err != nil {
+		return -1, err
+	}
+	tsFile, err := os.OpenFile(r.tspath(layer.ID), os.O_CREATE|os.O_WRONLY, 0o600)
+	if err != nil {
+		return -1, err
+	}
+	defer tsFile.Close()
+
+	err = r.applyDiffWithOptionsInner(layer, layerOptions, diff, f, tsFile)
 	if err != nil {
 		return 0, err
 	}
 
 	return size, r.saveFor(layer)
 }
 
-func (r *layerStore) applyDiffWithOptionsInner(layer *Layer, layerOptions *LayerOptions, diff io.Reader, applyFunc diffApplyFunc) (err error) {
+func (r *layerStore) applyDiffWithOptionsInner(layer *Layer, layerOptions *LayerOptions, diff io.Reader, applyFunc diffApplyFunc, tsFile *os.File) (err error) {
 	if !r.lockfile.IsReadWrite() {
 		return fmt.Errorf("not allowed to modify layer contents at %q: %w", r.layerdir, ErrStoreIsReadOnly)
 	}
@@ -2578,13 +2574,14 @@ func (r *layerStore) applyDiffWithOptionsInner(layer *Layer, layerOptions *Layer
 	compressedCounter := ioutils.NewWriteCounter(compressedWriter)
 	defragmented = io.TeeReader(defragmented, compressedCounter)
 
-	tsdata := bytes.Buffer{}
+	tsdata := pools.BufioWriter32KPool.Get(tsFile)
+	defer tsdata.Flush()
 	uidLog := make(map[uint32]struct{})
 	gidLog := make(map[uint32]struct{})
 	var uncompressedCounter *ioutils.WriteCounter
 
 	err = func() error { // A scope for defer
-		compressor, err := pgzip.NewWriterLevel(&tsdata, pgzip.BestSpeed)
+		compressor, err := pgzip.NewWriterLevel(tsdata, pgzip.BestSpeed)
 		if err != nil {
 			return err
 		}
@@ -2622,7 +2619,7 @@ func (r *layerStore) applyDiffWithOptionsInner(layer *Layer, layerOptions *Layer
 			Mappings:   r.layerMappings(layer),
 			MountLabel: layer.MountLabel,
 		}
-		return applyFunc(layer.ID, layer.Parent, options, &tsdata)
+		return applyFunc(layer.ID, layer.Parent, options)
 	}()
 	if err != nil {
 		return err