From 607722362d78cd05e81170333ad1373f594a9b6a Mon Sep 17 00:00:00 2001 From: obarisk Date: Thu, 5 Feb 2026 22:25:21 +0800 Subject: [PATCH 1/4] fix: delete subvolume will automatically delete the associated qgroup Signed-off-by: obarisk --- storage/drivers/btrfs/btrfs.go | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/storage/drivers/btrfs/btrfs.go b/storage/drivers/btrfs/btrfs.go index aba898ed55..c6893af096 100644 --- a/storage/drivers/btrfs/btrfs.go +++ b/storage/drivers/btrfs/btrfs.go @@ -31,7 +31,6 @@ import ( "github.com/docker/go-units" "github.com/opencontainers/selinux/go-selinux/label" - "github.com/sirupsen/logrus" graphdriver "go.podman.io/storage/drivers" "go.podman.io/storage/internal/tempdir" "go.podman.io/storage/pkg/directory" @@ -279,21 +278,6 @@ func subvolDelete(dirpath, name string, quotaEnabled bool) error { return fmt.Errorf("recursively walking subvolumes for %s failed: %w", dirpath, err) } - if quotaEnabled { - if qgroupid, err := subvolLookupQgroup(fullPath); err == nil { - var args C.struct_btrfs_ioctl_qgroup_create_args - args.qgroupid = C.__u64(qgroupid) - - _, _, errno := unix.Syscall(unix.SYS_IOCTL, getDirFd(dir), C.BTRFS_IOC_QGROUP_CREATE, - uintptr(unsafe.Pointer(&args))) - if errno != 0 { - logrus.Errorf("Failed to delete btrfs qgroup %v for %s: %v", qgroupid, fullPath, errno.Error()) - } - } else { - logrus.Errorf("Failed to lookup btrfs qgroup for %s: %v", fullPath, err.Error()) - } - } - // all subvolumes have been removed // now remove the one originally passed in for i, c := range []byte(name) { From 3c3023f0cdd973978920ad2de0880b00be0887c0 Mon Sep 17 00:00:00 2001 From: obarisk Date: Wed, 11 Feb 2026 11:31:00 +0800 Subject: [PATCH 2/4] chore: tests Signed-off-by: obarisk --- storage/drivers/btrfs/btrfs_test.go | 126 ++++++++++++++++++++++++++++ 1 file changed, 126 insertions(+) diff --git a/storage/drivers/btrfs/btrfs_test.go b/storage/drivers/btrfs/btrfs_test.go index 0e88054f69..87ffbd0d4e 100644 --- a/storage/drivers/btrfs/btrfs_test.go +++ b/storage/drivers/btrfs/btrfs_test.go @@ -3,8 +3,12 @@ package btrfs import ( + "bytes" + "fmt" "os" + "os/exec" "path" + "strings" "testing" graphdriver "go.podman.io/storage/drivers" @@ -77,3 +81,125 @@ func TestBtrfsListLayers(t *testing.T) { func TestBtrfsTeardown(t *testing.T) { graphtest.PutDriver(t) } + +// dependencies: btrfs-progs, libbtrfs-dev +// permission: root, loop device +func TestSubVolDelete(t *testing.T) { + if os.Getuid() != 0 { + t.Skip("test requires root") + } + + // Helpers + + runCmd := func(name string, arg ...string) error { + cmd := exec.Command(name, arg...) + var stderr bytes.Buffer + cmd.Stderr = &stderr + if err := cmd.Run(); err != nil { + return fmt.Errorf("command '%s %s' failed: %v, stderr: %s", name, strings.Join(arg, " "), err, stderr.String()) + } + return nil + } + + qgroupShow := func(mountPath string) string { + cmd := exec.Command("btrfs", "qgroup", "show", mountPath) + var out bytes.Buffer + cmd.Stdout = &out + cmd.Run() + return out.String() + } + + // parepare btrfs using loop device + baseDir, err := os.MkdirTemp("/mnt", "btrfs-test-") + if err != nil { + t.Fatalf("Failed to create base temp dir: %v", err) + } + defer os.RemoveAll(baseDir) + + mnt := path.Join(baseDir, "mountpoint") + if err := os.Mkdir(mnt, 755); err != nil { + t.Fatalf("Failed to create mountpoint dir: %v", err) + } + + blockFile := path.Join(baseDir, "btrfs.img") + if err := runCmd("dd", "if=/dev/zero", "of="+blockFile, "bs=1M", "count=120", "status=none"); err != nil { + t.Skipf("Failed to create block file: %v", err) + } + + if err := runCmd("mkfs.btrfs", "-f", blockFile); err != nil { + t.Skipf("Failed to format to btrfs: %v", err) + } + + findLoopCmd := exec.Command("losetup", "-f") + var loopDevBytes bytes.Buffer + findLoopCmd.Stdout = &loopDevBytes + if err := findLoopCmd.Run(); err != nil { + t.Skipf("Failed to find a loop device with 'losetup -f': %v", err) + } + loopDev := strings.TrimSpace(loopDevBytes.String()) + if loopDev == "" { + t.Skip("No available loop devices found") + } + + if err := runCmd("losetup", loopDev, blockFile); err != nil { + t.Skipf("Failed to setup loop device with 'losetup %s %s': %v", loopDev, blockFile, err) + } + defer runCmd("losetup", "-d", loopDev) + + if err := runCmd("mount", loopDev, mnt); err != nil { + t.Skipf("Failed to mount loop device %s: %v", loopDev, err) + } + defer runCmd("umount", mnt) + + d := &Driver{home: mnt} + if err := d.enableQuota(); err != nil { + t.Skipf("Failed to enable qgroup using API: %v", err) + } + + t.Run("subVolDelete", func(t *testing.T) { + subvolName := "subvol1" + subvolPath := path.Join(mnt, subvolName) + if err := subvolCreate(mnt, subvolName); err != nil { + t.Fatalf("Failed to create subvolume using API: %v", err) + } + + if err := d.subvolRescanQuota(); err != nil { + t.Fatalf("Failed to rescan quota using API: %v", err) + } + + qtreeid, err := subvolLookupQgroup(subvolPath) + if err != nil { + t.Fatalf("Failed to lookup qgroup for subvolume using API: %v", err) + } + qgroupID := fmt.Sprintf("0/%d", qtreeid) + t.Logf("subvolume %s has qgroup ID %s", subvolPath, qgroupID) + + if !strings.Contains(qgroupShow(mnt), qgroupID) { + t.Fatalf("qgroup %s was not created for subvolume %s", qgroupID, subvolPath) + } + + if err := subvolDelete(mnt, subvolName, true); err != nil { + t.Fatalf("Failed to delete subvolume using API: %v", err) + } + + if err := d.subvolRescanQuota(); err != nil { + t.Fatalf("Failed to rescan quota after delete using API: %v", err) + } + + qgroupInfo := qgroupShow(mnt) + t.Logf("Current qgroup info:\n%s", qgroupInfo) + + if strings.Contains(qgroupInfo, qgroupID) && !strings.Contains(qgroupInfo, "under deletion") { + t.Fatalf("qgroup %s was not marked as 'under deletion' for subvolume %s", qgroupID, subvolPath) + } + + runCmd("btrfs", "qgroup", "clear-stale", mnt) + + qgroupInfo = qgroupShow(mnt) + t.Logf("Current qgroup info after clearing stale:\n%s", qgroupInfo) + + if strings.Contains(qgroupInfo, qgroupID) { + t.Fatalf("qgroup %s was not deleted for subvolume %s", qgroupID, subvolPath) + } + }) +} From 225e20a38507cd02ef5dbb0cc461cd11601e297e Mon Sep 17 00:00:00 2001 From: obarisk Date: Wed, 11 Feb 2026 11:38:42 +0800 Subject: [PATCH 3/4] chore Signed-off-by: obarisk --- storage/drivers/btrfs/btrfs_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/storage/drivers/btrfs/btrfs_test.go b/storage/drivers/btrfs/btrfs_test.go index 87ffbd0d4e..07fe7e42fb 100644 --- a/storage/drivers/btrfs/btrfs_test.go +++ b/storage/drivers/btrfs/btrfs_test.go @@ -90,7 +90,6 @@ func TestSubVolDelete(t *testing.T) { } // Helpers - runCmd := func(name string, arg ...string) error { cmd := exec.Command(name, arg...) var stderr bytes.Buffer @@ -100,7 +99,6 @@ func TestSubVolDelete(t *testing.T) { } return nil } - qgroupShow := func(mountPath string) string { cmd := exec.Command("btrfs", "qgroup", "show", mountPath) var out bytes.Buffer @@ -117,7 +115,7 @@ func TestSubVolDelete(t *testing.T) { defer os.RemoveAll(baseDir) mnt := path.Join(baseDir, "mountpoint") - if err := os.Mkdir(mnt, 755); err != nil { + if err := os.Mkdir(mnt, 0o755); err != nil { t.Fatalf("Failed to create mountpoint dir: %v", err) } From 6c5931723e218ecd8127092f25343e17196ef607 Mon Sep 17 00:00:00 2001 From: obarisk Date: Thu, 12 Feb 2026 23:26:52 +0800 Subject: [PATCH 4/4] chore: seperate test cant using race Signed-off-by: obarisk --- storage/drivers/btrfs/btrfs_no_race_test.go | 133 ++++++++++++++++++++ storage/drivers/btrfs/btrfs_test.go | 124 ------------------ 2 files changed, 133 insertions(+), 124 deletions(-) create mode 100644 storage/drivers/btrfs/btrfs_no_race_test.go diff --git a/storage/drivers/btrfs/btrfs_no_race_test.go b/storage/drivers/btrfs/btrfs_no_race_test.go new file mode 100644 index 0000000000..d3c1216226 --- /dev/null +++ b/storage/drivers/btrfs/btrfs_no_race_test.go @@ -0,0 +1,133 @@ +//go:build !race + +package btrfs + +import ( + "bytes" + "fmt" + "os" + "os/exec" + "path" + "strings" + "testing" +) + +// dependencies: btrfs-progs, libbtrfs-dev +// permission: root, loop device +func TestSubVolDelete(t *testing.T) { + if os.Getuid() != 0 { + t.Skip("test requires root") + } + + // Helpers + runCmd := func(name string, arg ...string) error { + cmd := exec.Command(name, arg...) + var stderr bytes.Buffer + cmd.Stderr = &stderr + if err := cmd.Run(); err != nil { + return fmt.Errorf("command '%s %s' failed: %v, stderr: %s", name, strings.Join(arg, " "), err, stderr.String()) + } + return nil + } + qgroupShow := func(mountPath string) string { + cmd := exec.Command("btrfs", "qgroup", "show", mountPath) + var out bytes.Buffer + cmd.Stdout = &out + cmd.Run() + return out.String() + } + + // parepare btrfs using loop device + baseDir, err := os.MkdirTemp("/mnt", "btrfs-test-") + if err != nil { + t.Fatalf("Failed to create base temp dir: %v", err) + } + defer os.RemoveAll(baseDir) + + mnt := path.Join(baseDir, "mountpoint") + if err := os.Mkdir(mnt, 0o755); err != nil { + t.Fatalf("Failed to create mountpoint dir: %v", err) + } + + blockFile := path.Join(baseDir, "btrfs.img") + if err := runCmd("dd", "if=/dev/zero", "of="+blockFile, "bs=1M", "count=120", "status=none"); err != nil { + t.Skipf("Failed to create block file: %v", err) + } + + if err := runCmd("mkfs.btrfs", "-f", blockFile); err != nil { + t.Skipf("Failed to format to btrfs: %v", err) + } + + findLoopCmd := exec.Command("losetup", "-f") + var loopDevBytes bytes.Buffer + findLoopCmd.Stdout = &loopDevBytes + if err := findLoopCmd.Run(); err != nil { + t.Skipf("Failed to find a loop device with 'losetup -f': %v", err) + } + loopDev := strings.TrimSpace(loopDevBytes.String()) + if loopDev == "" { + t.Skip("No available loop devices found") + } + + if err := runCmd("losetup", loopDev, blockFile); err != nil { + t.Skipf("Failed to setup loop device with 'losetup %s %s': %v", loopDev, blockFile, err) + } + defer runCmd("losetup", "-d", loopDev) + + if err := runCmd("mount", loopDev, mnt); err != nil { + t.Skipf("Failed to mount loop device %s: %v", loopDev, err) + } + defer runCmd("umount", mnt) + + d := &Driver{home: mnt} + if err := d.enableQuota(); err != nil { + t.Skipf("Failed to enable qgroup using API: %v", err) + } + + t.Run("subVolDelete", func(t *testing.T) { + subvolName := "subvol1" + subvolPath := path.Join(mnt, subvolName) + if err := subvolCreate(mnt, subvolName); err != nil { + t.Fatalf("Failed to create subvolume using API: %v", err) + } + + if err := d.subvolRescanQuota(); err != nil { + t.Fatalf("Failed to rescan quota using API: %v", err) + } + + qtreeid, err := subvolLookupQgroup(subvolPath) + if err != nil { + t.Fatalf("Failed to lookup qgroup for subvolume using API: %v", err) + } + qgroupID := fmt.Sprintf("0/%d", qtreeid) + t.Logf("subvolume %s has qgroup ID %s", subvolPath, qgroupID) + + if !strings.Contains(qgroupShow(mnt), qgroupID) { + t.Fatalf("qgroup %s was not created for subvolume %s", qgroupID, subvolPath) + } + + if err := subvolDelete(mnt, subvolName, true); err != nil { + t.Fatalf("Failed to delete subvolume using API: %v", err) + } + + if err := d.subvolRescanQuota(); err != nil { + t.Fatalf("Failed to rescan quota after delete using API: %v", err) + } + + qgroupInfo := qgroupShow(mnt) + t.Logf("Current qgroup info:\n%s", qgroupInfo) + + if strings.Contains(qgroupInfo, qgroupID) && !strings.Contains(qgroupInfo, "under deletion") { + t.Fatalf("qgroup %s was not marked as 'under deletion' for subvolume %s", qgroupID, subvolPath) + } + + runCmd("btrfs", "qgroup", "clear-stale", mnt) + + qgroupInfo = qgroupShow(mnt) + t.Logf("Current qgroup info after clearing stale:\n%s", qgroupInfo) + + if strings.Contains(qgroupInfo, qgroupID) { + t.Fatalf("qgroup %s was not deleted for subvolume %s", qgroupID, subvolPath) + } + }) +} diff --git a/storage/drivers/btrfs/btrfs_test.go b/storage/drivers/btrfs/btrfs_test.go index 07fe7e42fb..0e88054f69 100644 --- a/storage/drivers/btrfs/btrfs_test.go +++ b/storage/drivers/btrfs/btrfs_test.go @@ -3,12 +3,8 @@ package btrfs import ( - "bytes" - "fmt" "os" - "os/exec" "path" - "strings" "testing" graphdriver "go.podman.io/storage/drivers" @@ -81,123 +77,3 @@ func TestBtrfsListLayers(t *testing.T) { func TestBtrfsTeardown(t *testing.T) { graphtest.PutDriver(t) } - -// dependencies: btrfs-progs, libbtrfs-dev -// permission: root, loop device -func TestSubVolDelete(t *testing.T) { - if os.Getuid() != 0 { - t.Skip("test requires root") - } - - // Helpers - runCmd := func(name string, arg ...string) error { - cmd := exec.Command(name, arg...) - var stderr bytes.Buffer - cmd.Stderr = &stderr - if err := cmd.Run(); err != nil { - return fmt.Errorf("command '%s %s' failed: %v, stderr: %s", name, strings.Join(arg, " "), err, stderr.String()) - } - return nil - } - qgroupShow := func(mountPath string) string { - cmd := exec.Command("btrfs", "qgroup", "show", mountPath) - var out bytes.Buffer - cmd.Stdout = &out - cmd.Run() - return out.String() - } - - // parepare btrfs using loop device - baseDir, err := os.MkdirTemp("/mnt", "btrfs-test-") - if err != nil { - t.Fatalf("Failed to create base temp dir: %v", err) - } - defer os.RemoveAll(baseDir) - - mnt := path.Join(baseDir, "mountpoint") - if err := os.Mkdir(mnt, 0o755); err != nil { - t.Fatalf("Failed to create mountpoint dir: %v", err) - } - - blockFile := path.Join(baseDir, "btrfs.img") - if err := runCmd("dd", "if=/dev/zero", "of="+blockFile, "bs=1M", "count=120", "status=none"); err != nil { - t.Skipf("Failed to create block file: %v", err) - } - - if err := runCmd("mkfs.btrfs", "-f", blockFile); err != nil { - t.Skipf("Failed to format to btrfs: %v", err) - } - - findLoopCmd := exec.Command("losetup", "-f") - var loopDevBytes bytes.Buffer - findLoopCmd.Stdout = &loopDevBytes - if err := findLoopCmd.Run(); err != nil { - t.Skipf("Failed to find a loop device with 'losetup -f': %v", err) - } - loopDev := strings.TrimSpace(loopDevBytes.String()) - if loopDev == "" { - t.Skip("No available loop devices found") - } - - if err := runCmd("losetup", loopDev, blockFile); err != nil { - t.Skipf("Failed to setup loop device with 'losetup %s %s': %v", loopDev, blockFile, err) - } - defer runCmd("losetup", "-d", loopDev) - - if err := runCmd("mount", loopDev, mnt); err != nil { - t.Skipf("Failed to mount loop device %s: %v", loopDev, err) - } - defer runCmd("umount", mnt) - - d := &Driver{home: mnt} - if err := d.enableQuota(); err != nil { - t.Skipf("Failed to enable qgroup using API: %v", err) - } - - t.Run("subVolDelete", func(t *testing.T) { - subvolName := "subvol1" - subvolPath := path.Join(mnt, subvolName) - if err := subvolCreate(mnt, subvolName); err != nil { - t.Fatalf("Failed to create subvolume using API: %v", err) - } - - if err := d.subvolRescanQuota(); err != nil { - t.Fatalf("Failed to rescan quota using API: %v", err) - } - - qtreeid, err := subvolLookupQgroup(subvolPath) - if err != nil { - t.Fatalf("Failed to lookup qgroup for subvolume using API: %v", err) - } - qgroupID := fmt.Sprintf("0/%d", qtreeid) - t.Logf("subvolume %s has qgroup ID %s", subvolPath, qgroupID) - - if !strings.Contains(qgroupShow(mnt), qgroupID) { - t.Fatalf("qgroup %s was not created for subvolume %s", qgroupID, subvolPath) - } - - if err := subvolDelete(mnt, subvolName, true); err != nil { - t.Fatalf("Failed to delete subvolume using API: %v", err) - } - - if err := d.subvolRescanQuota(); err != nil { - t.Fatalf("Failed to rescan quota after delete using API: %v", err) - } - - qgroupInfo := qgroupShow(mnt) - t.Logf("Current qgroup info:\n%s", qgroupInfo) - - if strings.Contains(qgroupInfo, qgroupID) && !strings.Contains(qgroupInfo, "under deletion") { - t.Fatalf("qgroup %s was not marked as 'under deletion' for subvolume %s", qgroupID, subvolPath) - } - - runCmd("btrfs", "qgroup", "clear-stale", mnt) - - qgroupInfo = qgroupShow(mnt) - t.Logf("Current qgroup info after clearing stale:\n%s", qgroupInfo) - - if strings.Contains(qgroupInfo, qgroupID) { - t.Fatalf("qgroup %s was not deleted for subvolume %s", qgroupID, subvolPath) - } - }) -}