Create quota before _data dir for volumes

This resolves an ordering issue that prevented quotas from being
applied. XFS quotas are applied recursively, but only for
subdirectories created after the quota is applied; if we create
`_data` before the quota, and then use `_data` for all data in
the volume, the quota will never be used by the volume.

Also, add a test that volume quotas are working as designed using
an XFS formatted loop device in the system tests. This should
prevent any further regressions on basic quota functionality,
such as quotas being shared between volumes.

Fixes containers#25368

Signed-off-by: Matt Heon <[email protected]>

Author: mheon

libpod/runtime_volume_common.go

@@ -168,16 +168,11 @@ func (r *Runtime) newVolume(ctx context.Context, noCreatePluginVolume bool, opti
 		if err := idtools.SafeChown(volPathRoot, volume.config.UID, volume.config.GID); err != nil {
 			return nil, fmt.Errorf("chowning volume directory %q to %d:%d: %w", volPathRoot, volume.config.UID, volume.config.GID, err)
 		}
-		fullVolPath := filepath.Join(volPathRoot, "_data")
-		if err := os.MkdirAll(fullVolPath, 0755); err != nil {
-			return nil, fmt.Errorf("creating volume directory %q: %w", fullVolPath, err)
-		}
-		if err := idtools.SafeChown(fullVolPath, volume.config.UID, volume.config.GID); err != nil {
-			return nil, fmt.Errorf("chowning volume directory %q to %d:%d: %w", fullVolPath, volume.config.UID, volume.config.GID, err)
-		}
-		if err := LabelVolumePath(fullVolPath, volume.config.MountLabel); err != nil {
-			return nil, err
-		}
+
+		// Setting quotas must happen *before* the _data inner directory
+		// is created, as the volume must be empty for the quota to be
+		// properly applied - if any subdirectories exist before the
+		// quota is applied, the quota will not be applied to them.
 		switch {
 		case volume.config.DisableQuota:
 			if volume.config.Size > 0 || volume.config.Inodes > 0 {
@@ -206,10 +201,20 @@ func (r *Runtime) newVolume(ctx context.Context, noCreatePluginVolume bool, opti
 			// subdirectory - so the quota ID assignment logic works
 			// properly.
 			if err := q.SetQuota(volPathRoot, quota); err != nil {
-				return nil, fmt.Errorf("failed to set size quota size=%d inodes=%d for volume directory %q: %w", volume.config.Size, volume.config.Inodes, fullVolPath, err)
+				return nil, fmt.Errorf("failed to set size quota size=%d inodes=%d for volume directory %q: %w", volume.config.Size, volume.config.Inodes, volPathRoot, err)
 			}
 		}
 
+		fullVolPath := filepath.Join(volPathRoot, "_data")
+		if err := os.MkdirAll(fullVolPath, 0755); err != nil {
+			return nil, fmt.Errorf("creating volume directory %q: %w", fullVolPath, err)
+		}
+		if err := idtools.SafeChown(fullVolPath, volume.config.UID, volume.config.GID); err != nil {
+			return nil, fmt.Errorf("chowning volume directory %q to %d:%d: %w", fullVolPath, volume.config.UID, volume.config.GID, err)
+		}
+		if err := LabelVolumePath(fullVolPath, volume.config.MountLabel); err != nil {
+			return nil, err
+		}
 		volume.config.MountPoint = fullVolPath
 	}
 

test/system/160-volumes.bats

@@ -619,4 +619,42 @@ EOF
     run_podman volume rm $volume_name --force
 }
 
+@test "podman volumes with XFS quotas" {
+    skip_if_rootless "Quotas are only possible with root"
+
+    loop=$PODMAN_TMPDIR/loopdevice
+    dd if=/dev/zero of=$(loopfile) bs=1M count=25
+    loop_dev=$(losetup -f)
+    losetup $loop_dev $loop
+    mkfs.xfs $loop_dev
+
+    safe_opts=$(podman_isolation_opts ${PODMAN_TMPDIR})
+    vol_path=$PODMAN_TMPDIR/volpath
+    safe_opts="$safe_opts --volumepath=$vol_path"
+    mount -t xfs -o defaults,pquota $loop_dev $vol_path
+
+    vol_one="testvol1"
+    run_podman $safe_opts volume create --opt o=size=2m $vol_one
+
+    vol_two="testvol2"
+    run_podman $safe_opts volume create --opt o=size=4m $vol_two
+
+    ctrname="testctr"
+    run_podman $safe_opts run --name=$ctrname -t -i -v $vol_one:/one -v $vol_two:/two $IMAGE top
+
+    run_podman $safe_opts exec $ctrname dd if=/dev/zero of=/one/onemb bs=1M count=1
+    run_podman 2 $safe_opts exec $ctrname dd if=/dev/zero of=/one/twomb bs=1M count=1
+    assert "$output" =~ "No space left on device"
+    run_podman $safe_opts exec $ctrname dd if=/dev/zero of=/two/threemb bs=1M count=3
+    run_podman 2 $safe_opts exec $ctrname dd if=/dev/zero of=/two/onemb bs=1M count=1
+    assert "$output" =~ "No space left on device"
+
+    run_podman rm -f -t 0 $ctrname
+    run_podman $safe_opts volume rm -af
+
+    umount $vol_path
+    losetup -d $loop_dev
+    rm -f $loop
+}
+
 # vim: filetype=sh