Merge pull request #18506 from nalind/so-much-diffsize

libpod/Container.rootFsSize(): use recorded image sizes

libpod/container_internal.go

@@ -55,10 +55,11 @@ const (
 	preCheckpointDir  = "pre-checkpoint"
 )
 
-// rootFsSize gets the size of the container's root filesystem
+// rootFsSize gets the size of the container, which can be divided notionally
-// A container FS is split into two parts.  The first is the top layer, a
+// into two parts.  The first is the part of its size that can be directly
-// mutable layer, and the rest is the RootFS: the set of immutable layers
+// attributed to its base image, if it has one.  The second is the set of
-// that make up the image on which the container is based.
+// changes that the container has had made relative to that base image.  Both
+// parts include some ancillary data, and we count that, too.
 func (c *Container) rootFsSize() (int64, error) {
 	if c.config.Rootfs != "" {
 		return 0, nil
@@ -72,58 +73,33 @@ func (c *Container) rootFsSize() (int64, error) {
 		return 0, err
 	}
 
-	// Ignore the size of the top layer.   The top layer is a mutable RW layer
-	// and is not considered a part of the rootfs
-	rwLayer, err := c.runtime.store.Layer(container.LayerID)
-	if err != nil {
-		return 0, err
-	}
-	layer, err := c.runtime.store.Layer(rwLayer.Parent)
-	if err != nil {
-		return 0, err
-	}
-
 	size := int64(0)
-	for layer.Parent != "" {
+	if container.ImageID != "" {
-		layerSize, err := c.runtime.store.DiffSize(layer.Parent, layer.ID)
+		size, err = c.runtime.store.ImageSize(container.ImageID)
-		if err != nil {
-			return size, fmt.Errorf("getting diffsize of layer %q and its parent %q: %w", layer.ID, layer.Parent, err)
-		}
-		size += layerSize
-		layer, err = c.runtime.store.Layer(layer.Parent)
 		if err != nil {
 			return 0, err
 		}
 	}
-	// Get the size of the last layer.  Has to be outside of the loop
+
-	// because the parent of the last layer is "", and lstore.Get("")
+	layerSize, err := c.runtime.store.ContainerSize(c.ID())
-	// will return an error.
+
-	layerSize, err := c.runtime.store.DiffSize(layer.Parent, layer.ID)
 	return size + layerSize, err
 }
 
-// rwSize gets the size of the mutable top layer of the container.
+// rwSize gets the combined size of the writeable layer and any ancillary data
+// for a given container.
 func (c *Container) rwSize() (int64, error) {
 	if c.config.Rootfs != "" {
 		size, err := util.SizeOfPath(c.config.Rootfs)
 		return int64(size), err
 	}
 
-	container, err := c.runtime.store.Container(c.ID())
+	layerSize, err := c.runtime.store.ContainerSize(c.ID())
 	if err != nil {
 		return 0, err
 	}
 
-	// The top layer of a container is
+	return layerSize, nil
-	// the only readable/writeable layer, all others are immutable.
-	rwLayer, err := c.runtime.store.Layer(container.LayerID)
-	if err != nil {
-		return 0, err
-	}
-
-	// Get the size of the top layer by calculating the size of the diff
-	// between the layer and its parent.
-	return c.runtime.store.DiffSize(rwLayer.Parent, rwLayer.ID)
 }
 
 // bundlePath returns the path to the container's root filesystem - where the OCI spec will be

libpod/volume.go

@@ -6,7 +6,7 @@ import (
 	"github.com/containers/podman/v4/libpod/define"
 	"github.com/containers/podman/v4/libpod/lock"
 	"github.com/containers/podman/v4/libpod/plugin"
-	"github.com/containers/podman/v4/pkg/util"
+	"github.com/containers/storage/pkg/directory"
 )
 
 // Volume is a libpod named volume.
@@ -109,7 +109,8 @@ func (v *Volume) Name() string {
 
 // Returns the size on disk of volume
 func (v *Volume) Size() (uint64, error) {
-	return util.SizeOfPath(v.config.MountPoint)
+	size, err := directory.Size(v.config.MountPoint)
+	return uint64(size), err
 }
 
 // Driver retrieves the volume's driver.

pkg/domain/infra/abi/system.go

@@ -18,6 +18,7 @@ import (
 	"github.com/containers/podman/v4/pkg/util"
 	"github.com/containers/podman/v4/utils"
 	"github.com/containers/storage"
+	"github.com/containers/storage/pkg/directory"
 	"github.com/containers/storage/pkg/unshare"
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/pflag"
@@ -280,7 +281,7 @@ func (ic *ContainerEngine) SystemDf(ctx context.Context, options entities.System
 		dfImages = append(dfImages, &report)
 	}
 
-	// Get Containers and iterate them
+	// Get containers and iterate over them
 	cons, err := ic.Libpod.GetAllContainers()
 	if err != nil {
 		return nil, err
@@ -322,7 +323,7 @@ func (ic *ContainerEngine) SystemDf(ctx context.Context, options entities.System
 		dfContainers = append(dfContainers, &report)
 	}
 
-	//	Get volumes and iterate them
+	// Get volumes and iterate over them
 	vols, err := ic.Libpod.GetAllVolumes()
 	if err != nil {
 		return nil, err
@@ -330,7 +331,7 @@ func (ic *ContainerEngine) SystemDf(ctx context.Context, options entities.System
 
 	dfVolumes := make([]*entities.SystemDfVolumeReport, 0, len(vols))
 	for _, v := range vols {
-		var reclaimableSize uint64
+		var reclaimableSize int64
 		mountPoint, err := v.MountPoint()
 		if err != nil {
 			return nil, err
@@ -341,7 +342,7 @@ func (ic *ContainerEngine) SystemDf(ctx context.Context, options entities.System
 			// TODO: fix this.
 			continue
 		}
-		volSize, err := util.SizeOfPath(mountPoint)
+		volSize, err := directory.Size(mountPoint)
 		if err != nil {
 			return nil, err
 		}
@@ -355,8 +356,8 @@ func (ic *ContainerEngine) SystemDf(ctx context.Context, options entities.System
 		report := entities.SystemDfVolumeReport{
 			VolumeName:      v.Name(),
 			Links:           len(inUse),
-			Size:            int64(volSize),
+			Size:            volSize,
-			ReclaimableSize: int64(reclaimableSize),
+			ReclaimableSize: reclaimableSize,
 		}
 		dfVolumes = append(dfVolumes, &report)
 	}

pkg/util/utils.go

@@ -3,7 +3,6 @@ package util
 import (
 	"errors"
 	"fmt"
-	"io/fs"
 	"math"
 	"os"
 	"os/user"
@@ -26,6 +25,7 @@ import (
 	"github.com/containers/podman/v4/pkg/namespaces"
 	"github.com/containers/podman/v4/pkg/rootless"
 	"github.com/containers/podman/v4/pkg/signal"
+	"github.com/containers/storage/pkg/directory"
 	"github.com/containers/storage/pkg/idtools"
 	stypes "github.com/containers/storage/types"
 	securejoin "github.com/cyphar/filepath-securejoin"
@@ -618,19 +618,10 @@ func LookupUser(name string) (*user.User, error) {
 
 // SizeOfPath determines the file usage of a given path. it was called volumeSize in v1
 // and now is made to be generic and take a path instead of a libpod volume
+// Deprecated: use github.com/containers/storage/pkg/directory.Size() instead.
 func SizeOfPath(path string) (uint64, error) {
-	var size uint64
+	size, err := directory.Size(path)
-	err := filepath.WalkDir(path, func(path string, d fs.DirEntry, err error) error {
+	return uint64(size), err
-		if err == nil && !d.IsDir() {
-			info, err := d.Info()
-			if err != nil {
-				return err
-			}
-			size += uint64(info.Size())
-		}
-		return err
-	})
-	return size, err
 }
 
 // EncryptConfig translates encryptionKeys into a EncriptionsConfig structure

test/system/320-system-df.bats

@@ -55,18 +55,23 @@ function teardown() {
 Type           | Images    | Containers | Local Volumes
 Total          |         1 |          2 |             0
 Active         |         1 |          1 |             0
-RawSize        | ~12...... |          0 |             0
+RawSize        | ~12...... |         !0 |             0
-RawReclaimable |         0 |          0 |             0
+RawReclaimable |         0 |         !0 |             0
+Reclaimable    |   ~\(0%\) |   ~\(50%\) |       ~\(0%\)
 TotalCount     |         1 |          2 |             0
-Size           |   ~12.*MB |         0B |            0B
+Size           |   ~12.*MB |        !0B |            0B
 '
     while read -a fields; do
         for i in 0 1 2;do
             expect="${fields[$((i+1))]}"
             actual=$(jq -r ".[$i].${fields[0]}" <<<"$results")
 
-            # Do exact-match check, unless the expect term starts with ~
+            # Do exact-match check, unless the expect term starts with ~ or !
             op='='
+            if [[ "$expect" =~ ^\! ]]; then
+                op='!='
+                expect=${expect##\!}
+            fi
             if [[ "$expect" =~ ^~ ]]; then
                 op='=~'
                 expect=${expect##\~}