Browse Source

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 <cpuguy83@gmail.com>
Brian Goff 9 years ago
parent
commit
f9befce2d3
2 changed files with 43 additions and 2 deletions
  1. 11 2
      daemon/graphdriver/btrfs/btrfs.go
  2. 32 0
      daemon/graphdriver/btrfs/btrfs_test.go

+ 11 - 2
daemon/graphdriver/btrfs/btrfs.go

@@ -188,20 +188,29 @@ func subvolDelete(dirpath, name string) error {
 		return err
 		return err
 	}
 	}
 	defer closeDir(dir)
 	defer closeDir(dir)
+	fullPath := path.Join(dirpath, name)
 
 
 	var args C.struct_btrfs_ioctl_vol_args
 	var args C.struct_btrfs_ioctl_vol_args
 
 
 	// walk the btrfs subvolumes
 	// walk the btrfs subvolumes
 	walkSubvolumes := func(p string, f os.FileInfo, err error) error {
 	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
 		// we want to check children only so skip itself
 		// it will be removed after the filepath walk anyways
 		// 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)
 			sv, err := isSubvolume(p)
 			if err != nil {
 			if err != nil {
 				return fmt.Errorf("Failed to test if %s is a btrfs subvolume: %v", p, err)
 				return fmt.Errorf("Failed to test if %s is a btrfs subvolume: %v", p, err)
 			}
 			}
 			if sv {
 			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)
 					return fmt.Errorf("Failed to destroy btrfs child subvolume (%s) of parent (%s): %v", p, dirpath, err)
 				}
 				}
 			}
 			}

+ 32 - 0
daemon/graphdriver/btrfs/btrfs_test.go

@@ -3,6 +3,8 @@
 package btrfs
 package btrfs
 
 
 import (
 import (
+	"os"
+	"path"
 	"testing"
 	"testing"
 
 
 	"github.com/docker/docker/daemon/graphdriver/graphtest"
 	"github.com/docker/docker/daemon/graphdriver/graphtest"
@@ -26,6 +28,36 @@ func TestBtrfsCreateSnap(t *testing.T) {
 	graphtest.DriverTestCreateSnap(t, "btrfs")
 	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) {
 func TestBtrfsTeardown(t *testing.T) {
 	graphtest.PutDriver(t)
 	graphtest.PutDriver(t)
 }
 }