From 341415cdcb10cd17342d69830aa008c127da578d Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Tue, 15 Dec 2015 15:16:06 -0500 Subject: [PATCH] Fix btrfs recursive btrfs subvol delete Really fixing 2 things: 1. Panic when any error is detected while walking the btrfs graph dir on removal due to no error check. 2. Nested subvolumes weren't actually being removed due to passing in the wrong path On point 2, for a path detected as a nested subvolume, we were calling `subvolDelete("/path/to/subvol", "subvol")`, where the last part of the path was duplicated due to a logic error, and as such actually causing point #1 since `subvolDelete` joins the two arguemtns, and `/path/to/subvol/subvol` (the joined version) doesn't exist. Also adds a test for nested subvol delete. Signed-off-by: Brian Goff Upstream-commit: f9befce2d38614de3dfa474bc0f2e1b9937a8ca2 Component: engine --- .../engine/daemon/graphdriver/btrfs/btrfs.go | 13 ++++++-- .../daemon/graphdriver/btrfs/btrfs_test.go | 32 +++++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/components/engine/daemon/graphdriver/btrfs/btrfs.go b/components/engine/daemon/graphdriver/btrfs/btrfs.go index eaf5498407..400978d8af 100644 --- a/components/engine/daemon/graphdriver/btrfs/btrfs.go +++ b/components/engine/daemon/graphdriver/btrfs/btrfs.go @@ -188,20 +188,29 @@ func subvolDelete(dirpath, name string) error { return err } defer closeDir(dir) + fullPath := path.Join(dirpath, name) var args C.struct_btrfs_ioctl_vol_args // walk the btrfs subvolumes walkSubvolumes := func(p string, f os.FileInfo, err error) error { + if err != nil { + if os.IsNotExist(err) && p != fullPath { + // missing most likely because the path was a subvolume that got removed in the previous iteration + // since it's gone anyway, we don't care + return nil + } + return fmt.Errorf("error walking subvolumes: %v", err) + } // we want to check children only so skip itself // it will be removed after the filepath walk anyways - if f.IsDir() && p != path.Join(dirpath, name) { + if f.IsDir() && p != fullPath { sv, err := isSubvolume(p) if err != nil { return fmt.Errorf("Failed to test if %s is a btrfs subvolume: %v", p, err) } if sv { - if err := subvolDelete(p, f.Name()); err != nil { + if err := subvolDelete(path.Dir(p), f.Name()); err != nil { return fmt.Errorf("Failed to destroy btrfs child subvolume (%s) of parent (%s): %v", p, dirpath, err) } } diff --git a/components/engine/daemon/graphdriver/btrfs/btrfs_test.go b/components/engine/daemon/graphdriver/btrfs/btrfs_test.go index 32a8bf1741..7494218d61 100644 --- a/components/engine/daemon/graphdriver/btrfs/btrfs_test.go +++ b/components/engine/daemon/graphdriver/btrfs/btrfs_test.go @@ -3,6 +3,8 @@ package btrfs import ( + "os" + "path" "testing" "github.com/docker/docker/daemon/graphdriver/graphtest" @@ -26,6 +28,36 @@ func TestBtrfsCreateSnap(t *testing.T) { graphtest.DriverTestCreateSnap(t, "btrfs") } +func TestBtrfsSubvolDelete(t *testing.T) { + d := graphtest.GetDriver(t, "btrfs") + if err := d.Create("test", "", ""); err != nil { + t.Fatal(err) + } + defer graphtest.PutDriver(t) + + dir, err := d.Get("test", "") + if err != nil { + t.Fatal(err) + } + defer d.Put("test") + + if err := subvolCreate(dir, "subvoltest"); err != nil { + t.Fatal(err) + } + + if _, err := os.Stat(path.Join(dir, "subvoltest")); err != nil { + t.Fatal(err) + } + + if err := d.Remove("test"); err != nil { + t.Fatal(err) + } + + if _, err := os.Stat(path.Join(dir, "subvoltest")); !os.IsNotExist(err) { + t.Fatalf("expected not exist error on nested subvol, got: %v", err) + } +} + func TestBtrfsTeardown(t *testing.T) { graphtest.PutDriver(t) }