devmapper: deferred remove can break consistency

Eric Ren · renzhen.rz · commit 37373c174809 · 2019-07-24T19:13:39.000+08:00
dmsetup remove with "--deferred" option may remove
device in kernel latter than the time when its device
info has been removed in database, causing the "file
exists" error as previous commit.

This error really happens in our extreme testing:
power off server and disk disappear while k8s is
creating/deleting pods.

The fix reveals a testcase bug in
"TestSnapshotterSuite/RemoveIntermediateSnapshot",
for block device snapshot, we should umount the
snapshot before removing it.

Signed-off-by: Eric Ren &lt;renzhen@linux.alibaba.com&gt;
diff --git a/snapshots/devmapper/pool_device.go b/snapshots/devmapper/pool_device.go
@@ -324,7 +324,7 @@ func (p *PoolDevice) RemoveDevice(ctx context.Context, deviceName string) error
 		return errors.Wrapf(err, "can't query metadata for device %q", deviceName)
 	}
 
-	if err := p.DeactivateDevice(ctx, deviceName, true, true); err != nil {
+	if err := p.DeactivateDevice(ctx, deviceName, false, true); err != nil {
 		return err
 	}
 
diff --git a/snapshots/testsuite/testsuite.go b/snapshots/testsuite/testsuite.go
@@ -501,7 +501,12 @@ func checkRemoveIntermediateSnapshot(ctx context.Context, t *testing.T, snapshot
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer testutil.Unmount(t, base)
+	needUmount := true
+	defer func() {
+		if needUmount {
+			testutil.Unmount(t, base)
+		}
+	}()
 
 	committedBase := filepath.Join(work, "committed-base")
 	if err = snapshotter.Commit(ctx, committedBase, base, opt); err != nil {
@@ -540,6 +545,10 @@ func checkRemoveIntermediateSnapshot(ctx context.Context, t *testing.T, snapshot
 	if err != nil {
 		t.Fatal(err)
 	}
+
+	testutil.Unmount(t, base)
+	needUmount = false
+
 	err = snapshotter.Remove(ctx, committedBase)
 	if err != nil {
 		t.Fatal(err)

Original file line number	Diff line number	Diff line change
`@@ -324,7 +324,7 @@ func (p *PoolDevice) RemoveDevice(ctx context.Context, deviceName string) error`
`324`	`324`	`return errors.Wrapf(err, "can't query metadata for device %q", deviceName)`
`325`	`325`	`}`
`326`	`326`
`327`		`- if err := p.DeactivateDevice(ctx, deviceName, true, true); err != nil {`
	`327`	`+ if err := p.DeactivateDevice(ctx, deviceName, false, true); err != nil {`
`328`	`328`	`return err`
`329`	`329`	`}`
`330`	`330`