Repository: firecracker-microvm/firecracker-containerd
Pull request #71 (2019-01-17 22:08:58)

Manage device states, rework device removal
*Issue #, if available:*
https://github.com/firecracker-microvm/firecracker-containerd/issues/61

*Description of changes:*
This PR adds device states instead of transactions and reworks device removal. 

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.



Patch:
diff --git a/snapshotter/devmapper/device_info.go b/snapshotter/devmapper/device_info.go
new file mode 100644
index 0000000..0818479
--- /dev/null
+++ b/snapshotter/devmapper/device_info.go
@@ -0,0 +1,101 @@
+// Copyright 2018 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License"). You may
+// not use this file except in compliance with the License. A copy of the
+// License is located at
+//
+//	http://aws.amazon.com/apache2.0/
+//
+// or in the "license" file accompanying this file. This file is distributed
+// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+// express or implied. See the License for the specific language governing
+// permissions and limitations under the License.
+
+package devmapper
+
+import (
+	"fmt"
+)
+
+const (
+	maxDeviceID = 0xffffff // Device IDs are 24-bit numbers
+)
+
+// DeviceState represents current devmapper device state reflected in meta store
+type DeviceState int
+
+const (
+	// Unknown means that device just allocated and no operations were performed
+	Unknown DeviceState = iota
+	// Creating means that device is going to be created
+	Creating
+	// Created means that devices successfully created
+	Created
+	// Activating means that device is going to be activated
+	Activating
+	// Activated means that device successfully activated
+	Activated
+	// Suspending means that device is going to be suspended
+	Suspending
+	// Suspended means that device successfully suspended
+	Suspended
+	// Resuming means that device is going to be resumed from suspended state
+	Resuming
+	// Resumed means that device successfully resumed
+	Resumed
+	// Deactivating means that device is going to be deactivated
+	Deactivating
+	// Deactivated means that device successfully deactivated
+	Deactivated
+	// Removing means that device is going to be removed
+	Removing
+	// Removed means that device successfully removed but not yet deleted from meta store
+	Removed
+)
+
+func (s DeviceState) String() string {
+	switch s {
+	case Creating:
+		return "Creating"
+	case Created:
+		return "Created"
+	case Activating:
+		return "Activating"
+	case Activated:
+		return "Activated"
+	case Suspending:
+		return "Suspending"
+	case Suspended:
+		return "Suspended"
+	case Resuming:
+		return "Resuming"
+	case Resumed:
+		return "Resumed"
+	case Deactivating:
+		return "Deactivating"
+	case Deactivated:
+		return "Deactivated"
+	case Removing:
+		return "Removing"
+	case Removed:
+		return "Removed"
+	default:
+		return fmt.Sprintf("unknown %d", s)
+	}
+}
+
+// DeviceInfo represents metadata for thin device within thin-pool
+type DeviceInfo struct {
+	// DeviceID is a 24-bit number assigned to a device within thin-pool device
+	DeviceID uint32 `json:"device_id"`
+	// Size is a thin device size
+	Size uint64 `json:"size"`
+	// Name is a device name to be used in /dev/mapper/
+	Name string `json:"name"`
+	// ParentName is a name of parent device (if snapshot)
+	ParentName string `json:"parent_name"`
+	// State represents current device state
+	State DeviceState `json:"state"`
+	// Error details if device state change failed
+	Error string `json:"error"`
+}
diff --git a/snapshotter/devmapper/metadata.go b/snapshotter/devmapper/metadata.go
index 9be3037..057e763 100644
--- a/snapshotter/devmapper/metadata.go
+++ b/snapshotter/devmapper/metadata.go
@@ -23,35 +23,15 @@ import (
 	"go.etcd.io/bbolt"
 )
 
-// DeviceInfo represents metadata for thin device within thin-pool
-type DeviceInfo struct {
-	// DeviceID is a 24-bit number assigned to a device within thin-pool device
-	DeviceID uint32 `json:"device_id"`
-	// Size is a thin device size
-	Size uint64 `json:"size"`
-	// Name is a device name to be used in /dev/mapper/
-	Name string `json:"name"`
-	// ParentName is a name of parent device (if snapshot)
-	ParentName string `json:"parent_name"`
-	// IsActivated indicates whether thin device was actived
-	IsActivated bool `json:"is_active"`
-}
-
 type (
-	// DeviceIDCallback is a callback used for device ID acquisition
-	DeviceIDCallback func(deviceID uint32) error
 	// DeviceInfoCallback is a callback used for device updates
 	DeviceInfoCallback func(deviceInfo *DeviceInfo) error
 )
 
-const (
-	maxDeviceID = 0xffffff // Device IDs are 24-bit numbers
-)
-
-type deviceState byte
+type deviceIDState byte
 
 const (
-	deviceFree deviceState = iota
+	deviceFree deviceIDState = iota
 	deviceTaken
 )
 
@@ -106,10 +86,7 @@ func (m *PoolMetadata) ensureDatabaseInitialized() error {
 }
 
 // AddDevice saves device info to database.
-// The callback should be used to indicate whether device allocation was successful or not.
-// An error returned from the callback will rollback the ID assignment transaction in the database and
-// free it for future use.
-func (m *PoolMetadata) AddDevice(ctx context.Context, info *DeviceInfo, fn DeviceIDCallback) error {
+func (m *PoolMetadata) AddDevice(ctx context.Context, info *DeviceInfo) error {
 	return m.db.Update(func(tx *bolt.Tx) error {
 		devicesBucket := tx.Bucket(devicesBucketName)
 
@@ -124,10 +101,6 @@ func (m *PoolMetadata) AddDevice(ctx context.Context, info *DeviceInfo, fn Devic
 			return err
 		}
 
-		if err := fn(deviceID); err != nil {
-			return err
-		}
-
 		info.DeviceID = deviceID
 
 		return putObject(devicesBucket, info.Name, info, false)
@@ -136,8 +109,8 @@ func (m *PoolMetadata) AddDevice(ctx context.Context, info *DeviceInfo, fn Devic
 
 // getNextDeviceID finds the next free device ID by taking a cursor
 // through the deviceIDBucketName bucket and finding the next sequentially
-// unassigned ID.  Device ID state is marked by a byte deviceFree or
-// deviceTaken.  Low device IDs will be reused sooner.
+// unassigned ID. Device ID state is marked by a byte deviceFree or
+// deviceTaken. Low device IDs will be reused sooner.
 func getNextDeviceID(tx *bolt.Tx) (uint32, error) {
 	bucket := tx.Bucket(deviceIDBucketName)
 	cursor := bucket.Cursor()
@@ -183,7 +156,7 @@ func getNextDeviceID(tx *bolt.Tx) (uint32, error) {
 }
 
 // markDeviceID marks a device as deviceFree or deviceTaken
-func markDeviceID(tx *bolt.Tx, deviceID uint32, state deviceState) error {
+func markDeviceID(tx *bolt.Tx, deviceID uint32, state deviceIDState) error {
 	var (
 		bucket = tx.Bucket(deviceIDBucketName)
 		key    = strconv.FormatUint(uint64(deviceID), 10)
@@ -248,9 +221,7 @@ func (m *PoolMetadata) GetDevice(ctx context.Context, name string) (*DeviceInfo,
 }
 
 // RemoveDevice removes device info from store.
-// The callback should be used to indicate whether device removal was successful or not.
-// An error returned from the callback will rollback the remove transaction in the database.
-func (m *PoolMetadata) RemoveDevice(ctx context.Context, name string, fn DeviceInfoCallback) error {
+func (m *PoolMetadata) RemoveDevice(ctx context.Context, name string) error {
 	return m.db.Update(func(tx *bolt.Tx) error {
 		var (
 			device = &DeviceInfo{}
@@ -269,7 +240,7 @@ func (m *PoolMetadata) RemoveDevice(ctx context.Context, name string, fn DeviceI
 			return err
 		}
 
-		return fn(device)
+		return nil
 	})
 }
 
diff --git a/snapshotter/devmapper/pool_device.go b/snapshotter/devmapper/pool_device.go
index 4b31ea5..8e77885 100644
--- a/snapshotter/devmapper/pool_device.go
+++ b/snapshotter/devmapper/pool_device.go
@@ -73,87 +73,190 @@ func NewPoolDevice(ctx context.Context, config *Config) (*PoolDevice, error) {
 	}, nil
 }
 
+// transition invokes 'updateStateFn' callback to perform devmapper operation and reflects device state changes/errors in meta store.
+// 'tryingState' will be set before invoking callback. If callback succeeded 'successState' will be set, otherwise
+// error details will be recorded in meta store.
+func (p *PoolDevice) transition(ctx context.Context, deviceName string, tryingState DeviceState, successState DeviceState, updateStateFn func() error) error {
+	// Set device to trying state
+	uerr := p.metadata.UpdateDevice(ctx, deviceName, func(deviceInfo *DeviceInfo) error {
+		deviceInfo.State = tryingState
+		return nil
+	})
+
+	if uerr != nil {
+		return errors.Wrapf(uerr, "failed to set device %q state to %q", deviceName, tryingState)
+	}
+
+	var result *multierror.Error
+
+	// Invoke devmapper operation
+	err := updateStateFn()
+
+	if err != nil {
+		result = multierror.Append(result, err)
+	}
+
+	// If operation succeeded transition to success state, otherwise save error details
+	uerr = p.metadata.UpdateDevice(ctx, deviceName, func(deviceInfo *DeviceInfo) error {
+		if err == nil {
+			deviceInfo.State = successState
+			deviceInfo.Error = ""
+		} else {
+			deviceInfo.Error = err.Error()
+		}
+		return nil
+	})
+
+	if uerr != nil {
+		result = multierror.Append(result, uerr)
+	}
+
+	return result.ErrorOrNil()
+}
+
 // CreateThinDevice creates new devmapper thin-device with given name and size.
 // Device ID for thin-device will be allocated from metadata store.
 // If allocation successful, device will be activated with /dev/mapper/<deviceName>
 func (p *PoolDevice) CreateThinDevice(ctx context.Context, deviceName string, virtualSizeBytes uint64) error {
-	deviceInfo := &DeviceInfo{
-		Name: deviceName,
-		Size: virtualSizeBytes,
+	info := &DeviceInfo{
+		Name:  deviceName,
+		Size:  virtualSizeBytes,
+		State: Unknown,
 	}
 
-	// Create thin device and save metadata
-	err := p.metadata.AddDevice(ctx, deviceInfo, func(devID uint32) error {
-		return dmsetup.CreateDevice(p.poolName, devID)
-	})
+	// Save initial device metadata and allocate new device ID from store
+	if err := p.metadata.AddDevice(ctx, info); err != nil {
+		return errors.Wrapf(err, "failed to save initial metadata for new thin device %q", deviceName)
+	}
 
-	if err != nil {
-		return err
+	// Create thin device
+	if err := p.transition(ctx, deviceName, Creating, Created, func() error {
+		return dmsetup.CreateDevice(p.poolName, info.DeviceID)
+	}); err != nil {
+		return errors.Wrapf(err, "failed to create new thin device %q (dev: %d)", info.Name, info.DeviceID)
 	}
 
 	// Activate thin device
-	err = p.metadata.UpdateDevice(ctx, deviceName, func(info *DeviceInfo) error {
-		info.IsActivated = true
+	if err := p.transition(ctx, deviceName, Activating, Activated, func() error {
 		return dmsetup.ActivateDevice(p.poolName, info.Name, info.DeviceID, info.Size, "")
-	})
+	}); err != nil {
+		return errors.Wrapf(err, "failed to activate new thin device %q (dev: %d)", info.Name, info.DeviceID)
+	}
 
-	return err
+	return nil
 }
 
 // CreateSnapshotDevice creates and activates new thin-device from parent thin-device (makes snapshot)
 func (p *PoolDevice) CreateSnapshotDevice(ctx context.Context, deviceName string, snapshotName string, virtualSizeBytes uint64) error {
-	baseDeviceInfo, err := p.metadata.GetDevice(ctx, deviceName)
+	baseInfo, err := p.metadata.GetDevice(ctx, deviceName)
 	if err != nil {
-		return err
+		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 := baseDeviceInfo.IsActivated
 	if isActivated {
-		if err := dmsetup.SuspendDevice(deviceName); err != nil {
-			return errors.Wrapf(err, "failed to suspend device %q", deviceName)
+		if err := p.transition(ctx, baseInfo.Name, Suspending, Suspended, func() error {
+			return dmsetup.SuspendDevice(baseInfo.Name)
+		}); err != nil {
+			return errors.Wrapf(err, "failed to suspend device %q", baseInfo.Name)
 		}
 	}
 
-	snapshotDeviceInfo := &DeviceInfo{
+	snapInfo := &DeviceInfo{
 		Name:       snapshotName,
 		Size:       virtualSizeBytes,
 		ParentName: deviceName,
+		State:      Unknown,
 	}
 
-	err = p.metadata.AddDevice(ctx, snapshotDeviceInfo, func(devID uint32) error {
-		return dmsetup.CreateSnapshot(p.poolName, devID, baseDeviceInfo.DeviceID)
-	})
+	// Save snapshot metadata and allocate new device ID
+	if err := p.metadata.AddDevice(ctx, snapInfo); err != nil {
+		return errors.Wrapf(err, "failed to save initial metadata for snapshot %q", snapshotName)
+	}
 
-	if err != nil {
-		return err
+	// Create thin device snapshot
+	if err := p.transition(ctx, snapInfo.Name, Creating, Created, func() error {
+		return dmsetup.CreateSnapshot(p.poolName, snapInfo.DeviceID, baseInfo.DeviceID)
+	}); err != nil {
+		return errors.Wrapf(err,
+			"failed to create snapshot %q (dev: %d) from %q (dev: %d, activated: %t)",
+			snapInfo.Name,
+			snapInfo.DeviceID,
+			baseInfo.Name,
+			baseInfo.DeviceID,
+			isActivated)
 	}
 
 	if isActivated {
-		if err := dmsetup.ResumeDevice(deviceName); err != nil {
+		// Resume base thin-device
+		if err := p.transition(ctx, baseInfo.Name, Resuming, Resumed, func() error {
+			return dmsetup.ResumeDevice(baseInfo.Name)
+		}); err != nil {
 			return errors.Wrapf(err, "failed to resume device %q", deviceName)
 		}
 	}
 
-	err = p.metadata.UpdateDevice(ctx, snapshotName, func(info *DeviceInfo) error {
-		info.IsActivated = true
-		return dmsetup.ActivateDevice(p.poolName, info.Name, info.DeviceID, info.Size, "")
-	})
+	// Activate snapshot
+	if err := p.transition(ctx, snapInfo.Name, Activating, Activated, func() error {
+		return dmsetup.ActivateDevice(p.poolName, snapInfo.Name, snapInfo.DeviceID, snapInfo.Size, "")
+	}); err != nil {
+		return errors.Wrapf(err, "failed to activate snapshot device %q (dev: %d)", snapInfo.Name, snapInfo.DeviceID)
+	}
 
-	return err
+	return nil
 }
 
-// RemoveDevice removes thin device and metadata from store
-func (p *PoolDevice) RemoveDevice(ctx context.Context, deviceName string, deferred bool) error {
+// 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
+	}
+
 	opts := []dmsetup.RemoveDeviceOpt{dmsetup.RemoveWithForce, dmsetup.RemoveWithRetries}
 	if deferred {
 		opts = append(opts, dmsetup.RemoveDeferred)
 	}
 
-	return p.metadata.UpdateDevice(ctx, deviceName, func(info *DeviceInfo) error {
-		info.IsActivated = false
+	if err := p.transition(ctx, deviceName, Deactivating, Deactivated, func() error {
 		return dmsetup.RemoveDevice(deviceName, opts...)
-	})
+	}); err != nil {
+		return errors.Wrapf(err, "failed to deactivate device %q", deviceName)
+	}
+
+	return nil
+}
+
+// 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 {
+		return err
+	}
+
+	if err := p.transition(ctx, deviceName, Removing, Removed, func() error {
+		// Send 'delete' message to thin-pool
+		return dmsetup.DeleteDevice(p.poolName, info.DeviceID)
+	}); err != nil {
+		return errors.Wrapf(err, "failed to delete device %q (dev id: %d)", info.Name, info.DeviceID)
+	}
+
+	// Remove record from meta store and free device ID
+	if err := p.metadata.RemoveDevice(ctx, deviceName); err != nil {
+		return errors.Wrapf(err, "can't remove device %q metadata from store after removal", deviceName)
+	}
+
+	return nil
 }
 
 // RemovePool deactivates all child thin-devices and removes thin-pool device
@@ -165,17 +268,10 @@ func (p *PoolDevice) RemovePool(ctx context.Context) error {
 
 	var result *multierror.Error
 
+	// Deactivate devices if any
 	for _, name := range deviceNames {
-		info, err := p.metadata.GetDevice(ctx, name)
-		if err != nil {
-			result = multierror.Append(result, errors.Wrapf(err, "failed to get device info %q", name))
-			continue
-		}
-
-		if info.IsActivated {
-			if err := p.RemoveDevice(ctx, name, true); err != nil {
-				result = multierror.Append(result, errors.Wrapf(err, "failed to remove %q", name))
-			}
+		if err := p.DeactivateDevice(ctx, name, true); err != nil && err != ErrNotFound {
+			result = multierror.Append(result, errors.Wrapf(err, "failed to remove %q", name))
 		}
 	}
 
diff --git a/snapshotter/devmapper/snapshotter.go b/snapshotter/devmapper/snapshotter.go
index ebbfd96..d08f474 100644
--- a/snapshotter/devmapper/snapshotter.go
+++ b/snapshotter/devmapper/snapshotter.go
@@ -205,7 +205,7 @@ func (s *Snapshotter) removeDevice(ctx context.Context, key string) error {
 	}
 
 	deviceName := s.getDeviceName(snapID)
-	if err := s.pool.RemoveDevice(ctx, deviceName, true); err != nil {
+	if err := s.pool.RemoveDevice(ctx, deviceName); err != nil {
 		log.G(ctx).WithError(err).Errorf("failed to remove device")
 		return err
 	}
diff --git a/snapshotter/pkg/dmsetup/dmsetup.go b/snapshotter/pkg/dmsetup/dmsetup.go
index 34ecea9..d177a17 100644
--- a/snapshotter/pkg/dmsetup/dmsetup.go
+++ b/snapshotter/pkg/dmsetup/dmsetup.go
@@ -162,7 +162,7 @@ func CreateSnapshot(poolName string, deviceID uint32, baseDeviceID uint32) error
 }
 
 // DeleteDevice sends "delete <deviceID>" message to the given thin-pool
-func DeleteDevice(poolName string, deviceID int) error {
+func DeleteDevice(poolName string, deviceID uint32) error {
 	_, err := dmsetup("message", poolName, "0", fmt.Sprintf("delete %d", deviceID))
 	return err
 }
diff --git a/snapshotter/pkg/losetup/losetup.go b/snapshotter/pkg/losetup/losetup.go
index ef00c39..2e37814 100644
--- a/snapshotter/pkg/losetup/losetup.go
+++ b/snapshotter/pkg/losetup/losetup.go
@@ -22,7 +22,7 @@ import (
 
 // FindAssociatedLoopDevices returns a list of loop devices attached to a given image
 func FindAssociatedLoopDevices(imagePath string) ([]string, error) {
-	output, err := losetup("--list", "--output", "NAME", "--noheadings", "--associated", imagePath)
+	output, err := losetup("--list", "--output", "NAME", "--associated", imagePath)
 	if err != nil {
 		return nil, errors.Wrapf(err, "failed to get loop devices: '%s'", output)
 	}
@@ -31,7 +31,13 @@ func FindAssociatedLoopDevices(imagePath string) ([]string, error) {
 		return []string{}, nil
 	}
 
-	return strings.Split(output, "\n"), nil
+	items := strings.Split(output, "\n")
+	if len(items) <= 1 {
+		return []string{}, nil
+	}
+
+	// Skip header with column names
+	return items[1:], nil
 }
 
 // AttachLoopDevice finds first available loop device and associates it with an image.


Test patch:
diff --git a/snapshotter/devmapper/metadata_test.go b/snapshotter/devmapper/metadata_test.go
index 015f6cd..ce877d4 100644
--- a/snapshotter/devmapper/metadata_test.go
+++ b/snapshotter/devmapper/metadata_test.go
@@ -15,7 +15,6 @@ package devmapper
 
 import (
 	"context"
-	"errors"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -26,9 +25,7 @@ import (
 )
 
 var (
-	testCtx             = context.Background()
-	testDevIDCallback   = func(uint32) error { return nil }
-	testDevInfoCallback = func(*DeviceInfo) error { return nil }
+	testCtx = context.Background()
 )
 
 func TestPoolMetadata_AddDevice(t *testing.T) {
@@ -36,13 +33,13 @@ func TestPoolMetadata_AddDevice(t *testing.T) {
 	defer cleanupStore(t, tempDir, store)
 
 	expected := &DeviceInfo{
-		Name:        "test2",
-		ParentName:  "test1",
-		Size:        1,
-		IsActivated: true,
+		Name:       "test2",
+		ParentName: "test1",
+		Size:       1,
+		State:      Activated,
 	}
 
-	err := store.AddDevice(testCtx, expected, testDevIDCallback)
+	err := store.AddDevice(testCtx, expected)
 	assert.NoError(t, err)
 
 	result, err := store.GetDevice(testCtx, "test2")
@@ -51,7 +48,7 @@ func TestPoolMetadata_AddDevice(t *testing.T) {
 	assert.Equal(t, expected.Name, result.Name)
 	assert.Equal(t, expected.ParentName, result.ParentName)
 	assert.Equal(t, expected.Size, result.Size)
-	assert.Equal(t, expected.IsActivated, result.IsActivated)
+	assert.Equal(t, expected.State, result.State)
 	assert.NotZero(t, result.DeviceID)
 	assert.Equal(t, expected.DeviceID, result.DeviceID)
 }
@@ -60,11 +57,10 @@ func TestPoolMetadata_AddDeviceRollback(t *testing.T) {
 	tempDir, store := createStore(t)
 	defer cleanupStore(t, tempDir, store)
 
-	expectedErr := errors.New("add failed")
-	err := store.AddDevice(testCtx, &DeviceInfo{Name: "test2"}, func(uint32) error { return expectedErr })
-	assert.Equal(t, expectedErr, err)
+	err := store.AddDevice(testCtx, &DeviceInfo{Name: ""})
+	assert.Error(t, err)
 
-	_, err = store.GetDevice(testCtx, "test2")
+	_, err = store.GetDevice(testCtx, "")
 	assert.Equal(t, ErrNotFound, err)
 }
 
@@ -72,10 +68,10 @@ func TestPoolMetadata_AddDeviceDuplicate(t *testing.T) {
 	tempDir, store := createStore(t)
 	defer cleanupStore(t, tempDir, store)
 
-	err := store.AddDevice(testCtx, &DeviceInfo{Name: "test"}, testDevIDCallback)
+	err := store.AddDevice(testCtx, &DeviceInfo{Name: "test"})
 	assert.NoError(t, err)
 
-	err = store.AddDevice(testCtx, &DeviceInfo{Name: "test"}, testDevIDCallback)
+	err = store.AddDevice(testCtx, &DeviceInfo{Name: "test"})
 	assert.Equal(t, ErrAlreadyExists, err)
 }
 
@@ -84,21 +80,21 @@ func TestPoolMetadata_ReuseDeviceID(t *testing.T) {
 	defer cleanupStore(t, tempDir, store)
 
 	info1 := &DeviceInfo{Name: "test1"}
-	err := store.AddDevice(testCtx, info1, testDevIDCallback)
+	err := store.AddDevice(testCtx, info1)
 	assert.NoError(t, err)
 
 	info2 := &DeviceInfo{Name: "test2"}
-	err = store.AddDevice(testCtx, info2, testDevIDCallback)
+	err = store.AddDevice(testCtx, info2)
 	assert.NoError(t, err)
 
 	assert.NotEqual(t, info1.DeviceID, info2.DeviceID)
 	assert.NotZero(t, info1.DeviceID)
 
-	err = store.RemoveDevice(testCtx, info2.Name, testDevInfoCallback)
+	err = store.RemoveDevice(testCtx, info2.Name)
 	assert.NoError(t, err)
 
 	info3 := &DeviceInfo{Name: "test3"}
-	err = store.AddDevice(testCtx, info3, testDevIDCallback)
+	err = store.AddDevice(testCtx, info3)
 	assert.NoError(t, err)
 
 	assert.Equal(t, info2.DeviceID, info3.DeviceID)
@@ -108,10 +104,10 @@ func TestPoolMetadata_RemoveDevice(t *testing.T) {
 	tempDir, store := createStore(t)
 	defer cleanupStore(t, tempDir, store)
 
-	err := store.AddDevice(testCtx, &DeviceInfo{Name: "test"}, testDevIDCallback)
+	err := store.AddDevice(testCtx, &DeviceInfo{Name: "test"})
 	assert.NoError(t, err)
 
-	err = store.RemoveDevice(testCtx, "test", testDevInfoCallback)
+	err = store.RemoveDevice(testCtx, "test")
 	assert.NoError(t, err)
 
 	_, err = store.GetDevice(testCtx, "test")
@@ -123,19 +119,19 @@ func TestPoolMetadata_UpdateDevice(t *testing.T) {
 	defer cleanupStore(t, tempDir, store)
 
 	oldInfo := &DeviceInfo{
-		Name:        "test1",
-		ParentName:  "test2",
-		Size:        3,
-		IsActivated: true,
+		Name:       "test1",
+		ParentName: "test2",
+		Size:       3,
+		State:      Activated,
 	}
 
-	err := store.AddDevice(testCtx, oldInfo, testDevIDCallback)
+	err := store.AddDevice(testCtx, oldInfo)
 	assert.NoError(t, err)
 
 	err = store.UpdateDevice(testCtx, oldInfo.Name, func(info *DeviceInfo) error {
 		info.ParentName = "test5"
 		info.Size = 6
-		info.IsActivated = false
+		info.State = Created
 		return nil
 	})
 
@@ -147,17 +143,17 @@ func TestPoolMetadata_UpdateDevice(t *testing.T) {
 	assert.Equal(t, "test1", newInfo.Name)
 	assert.Equal(t, "test5", newInfo.ParentName)
 	assert.EqualValues(t, 6, newInfo.Size)
-	assert.False(t, newInfo.IsActivated)
+	assert.Equal(t, Created, newInfo.State)
 }
 
 func TestPoolMetadata_GetDeviceNames(t *testing.T) {
 	tempDir, store := createStore(t)
 	defer cleanupStore(t, tempDir, store)
 
-	err := store.AddDevice(testCtx, &DeviceInfo{Name: "test1"}, testDevIDCallback)
+	err := store.AddDevice(testCtx, &DeviceInfo{Name: "test1"})
 	assert.NoError(t, err)
 
-	err = store.AddDevice(testCtx, &DeviceInfo{Name: "test2"}, testDevIDCallback)
+	err = store.AddDevice(testCtx, &DeviceInfo{Name: "test2"})
 	assert.NoError(t, err)
 
 	names, err := store.GetDeviceNames(testCtx)
diff --git a/snapshotter/devmapper/pool_device_test.go b/snapshotter/devmapper/pool_device_test.go
index 3eaefbd..cec373a 100644
--- a/snapshotter/devmapper/pool_device_test.go
+++ b/snapshotter/devmapper/pool_device_test.go
@@ -128,6 +128,10 @@ func TestPoolDevice(t *testing.T) {
 	output, err = exec.Command("umount", thin1MountPath, snap1MountPath).CombinedOutput()
 	assert.NoErrorf(t, err, "failed to unmount devices: %s", string(output))
 
+	t.Run("DeactivateDevice", func(t *testing.T) {
+		testDeactivateThinDevice(t, pool)
+	})
+
 	t.Run("RemoveDevice", func(t *testing.T) {
 		testRemoveThinDevice(t, pool)
 	})
@@ -171,22 +175,26 @@ func testCreateSnapshot(t *testing.T, pool *PoolDevice) {
 	assert.NoErrorf(t, err, "failed to create snapshot from '%s' volume", thinDevice1)
 }
 
-func testRemoveThinDevice(t *testing.T, pool *PoolDevice) {
+func testDeactivateThinDevice(t *testing.T, pool *PoolDevice) {
 	deviceList := []string{
-		thinDevice1,
 		thinDevice2,
 		snapDevice1,
 	}
 
 	for _, deviceName := range deviceList {
-		err := pool.RemoveDevice(context.Background(), deviceName, false)
+		err := pool.DeactivateDevice(context.Background(), deviceName, false)
 		assert.NoErrorf(t, err, "failed to remove '%s'", deviceName)
 	}
 
-	err := pool.RemoveDevice(context.Background(), "not-existing-device", false)
+	err := pool.DeactivateDevice(context.Background(), "not-existing-device", false)
 	assert.Error(t, err, "should return an error if trying to remove not existing device")
 }
 
+func testRemoveThinDevice(t *testing.T, pool *PoolDevice) {
+	err := pool.RemoveDevice(testCtx, thinDevice1)
+	assert.NoErrorf(t, err, "should delete thin device from pool")
+}
+
 func tempMountPath(t *testing.T) string {
 	path, err := ioutil.TempDir("", "devmapper-snapshotter-mount-")
 	require.NoError(t, err, "failed to get temp directory for mount")