devmapper: more precise way of checking if device is activated

mxpv · mxpv · commit 7efda48c5395 · 2019-02-21T16:26:46.000-08:00
Signed-off-by: Maksym Pavlenko &lt;makpav@amazon.com&gt;
diff --git a/snapshots/devmapper/pool_device.go b/snapshots/devmapper/pool_device.go
@@ -188,9 +188,8 @@ func (p *PoolDevice) CreateSnapshotDevice(ctx context.Context, deviceName string
 		return errors.Wrapf(err, "failed to query device metadata for %q", deviceName)
 	}
 
-	isActivated := baseInfo.State == Activated
-
 	// Suspend thin device if it was activated previously
+	isActivated := p.IsActivated(baseInfo.Name)
 	if isActivated {
 		if err := p.transition(ctx, baseInfo.Name, Suspending, Suspended, func() error {
 			return dmsetup.SuspendDevice(baseInfo.Name)
@@ -245,13 +244,8 @@ func (p *PoolDevice) CreateSnapshotDevice(ctx context.Context, deviceName string
 
 // DeactivateDevice deactivates thin device
 func (p *PoolDevice) DeactivateDevice(ctx context.Context, deviceName string, deferred bool) error {
-	devicePath := dmsetup.GetFullDevicePath(deviceName)
-	if _, err := os.Stat(devicePath); err != nil {
-		if os.IsNotExist(err) {
-			return ErrNotFound
-		}
-
-		return err
+	if !p.IsActivated(deviceName) {
+		return nil
 	}
 
 	opts := []dmsetup.RemoveDeviceOpt{dmsetup.RemoveWithForce, dmsetup.RemoveWithRetries}
@@ -268,14 +262,29 @@ func (p *PoolDevice) DeactivateDevice(ctx context.Context, deviceName string, de
 	return nil
 }
 
+// IsActivated returns true if thin-device is activated and not suspended
+func (p *PoolDevice) IsActivated(deviceName string) bool {
+	infos, err := dmsetup.Info(deviceName)
+	if err != nil || len(infos) == 0 {
+		// Couldn't query device info, device not active
+		return false
+	}
+
+	if devInfo := infos[0]; devInfo.Suspended {
+		return false
+	}
+
+	return true
+}
+
 // RemoveDevice completely wipes out thin device from thin-pool and frees it's device ID
 func (p *PoolDevice) RemoveDevice(ctx context.Context, deviceName string) error {
 	info, err := p.metadata.GetDevice(ctx, deviceName)
 	if err != nil {
 		return errors.Wrapf(err, "can't query metadata for device %q", deviceName)
 	}
 
-	if err := p.DeactivateDevice(ctx, deviceName, true); err != nil && err != ErrNotFound {
+	if err := p.DeactivateDevice(ctx, deviceName, true); err != nil {
 		return err
 	}
 
@@ -305,7 +314,7 @@ func (p *PoolDevice) RemovePool(ctx context.Context) error {
 
 	// Deactivate devices if any
 	for _, name := range deviceNames {
-		if err := p.DeactivateDevice(ctx, name, true); err != nil && err != ErrNotFound {
+		if err := p.DeactivateDevice(ctx, name, true); err != nil {
 			result = multierror.Append(result, errors.Wrapf(err, "failed to remove %q", name))
 		}
 	}
diff --git a/snapshots/devmapper/pool_device_test.go b/snapshots/devmapper/pool_device_test.go
@@ -191,12 +191,13 @@ func testDeactivateThinDevice(t *testing.T, pool *PoolDevice) {
 	}
 
 	for _, deviceName := range deviceList {
+		assert.Assert(t, pool.IsActivated(deviceName))
+
 		err := pool.DeactivateDevice(context.Background(), deviceName, false)
 		assert.NilError(t, err, "failed to remove '%s'", deviceName)
-	}
 
-	err := pool.DeactivateDevice(context.Background(), "not-existing-device", false)
-	assert.Assert(t, err != nil, "should return an error if trying to remove not existing device")
+		assert.Assert(t, !pool.IsActivated(deviceName))
+	}
 }
 
 func testRemoveThinDevice(t *testing.T, pool *PoolDevice) {

Original file line number	Diff line number	Diff line change
`@@ -191,12 +191,13 @@ func testDeactivateThinDevice(t testing.T, pool PoolDevice) {`
`191`	`191`	`}`
`192`	`192`
`193`	`193`	`for _, deviceName := range deviceList {`
	`194`	`+ assert.Assert(t, pool.IsActivated(deviceName))`
	`195`	`+`
`194`	`196`	`err := pool.DeactivateDevice(context.Background(), deviceName, false)`
`195`	`197`	`assert.NilError(t, err, "failed to remove '%s'", deviceName)`
`196`		`- }`
`197`	`198`
`198`		`- err := pool.DeactivateDevice(context.Background(), "not-existing-device", false)`
`199`		`- assert.Assert(t, err != nil, "should return an error if trying to remove not existing device")`
	`199`	`+ assert.Assert(t, !pool.IsActivated(deviceName))`
	`200`	`+ }`
`200`	`201`	`}`
`201`	`202`
`202`	`203`	`func testRemoveThinDevice(t testing.T, pool PoolDevice) {`