devmapper: rollback thin devices on error

Signed-off-by: Maksym Pavlenko <makpav@amazon.com>

snapshots/devmapper/pool_device.go

@@ -58,6 +58,7 @@ func NewPoolDevice(ctx context.Context, config *Config) (*PoolDevice, error) {
 	if _, err := dmsetup.Info(poolPath); err != nil {
 		return nil, errors.Wrapf(err, "failed to query pool %q", poolPath)
 	}
+
 	return &PoolDevice{
 		poolName: config.PoolName,
 		metadata: poolMetaStore,
@@ -108,7 +109,7 @@ func (p *PoolDevice) transition(ctx context.Context, deviceName string, tryingSt
 // 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 {
+func (p *PoolDevice) CreateThinDevice(ctx context.Context, deviceName string, virtualSizeBytes uint64) (retErr error) {
 	info := &DeviceInfo{
 		Name:  deviceName,
 		Size:  virtualSizeBytes,
@@ -120,15 +121,46 @@ func (p *PoolDevice) CreateThinDevice(ctx context.Context, deviceName string, vi
 		return errors.Wrapf(err, "failed to save initial metadata for new thin device %q", deviceName)
 	}
 
+	defer func() {
+		if retErr == nil {
+			return
+		}
+
+		// Rollback metadata
+		retErr = multierror.Append(retErr, p.metadata.RemoveDevice(ctx, info.Name))
+	}()
+
 	// Create thin device
-	if err := p.transition(ctx, deviceName, Creating, Created, func() error {
+	if err := p.createDevice(ctx, info); err != nil {
+		return err
+	}
+
+	defer func() {
+		if retErr == nil {
+			return
+		}
+
+		// Rollback creation
+		retErr = multierror.Append(retErr, p.deleteDevice(ctx, info))
+	}()
+
+	return p.activateDevice(ctx, info)
+}
+
+// createDevice creates thin device
+func (p *PoolDevice) createDevice(ctx context.Context, info *DeviceInfo) error {
+	if err := p.transition(ctx, info.Name, 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
+	return nil
-	if err := p.transition(ctx, deviceName, Activating, Activated, func() error {
+}
+
+// activateDevice activates thin device
+func (p *PoolDevice) activateDevice(ctx context.Context, info *DeviceInfo) error {
+	if err := p.transition(ctx, info.Name, 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)
@@ -138,20 +170,23 @@ func (p *PoolDevice) CreateThinDevice(ctx context.Context, deviceName string, vi
 }
 
 // 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 {
+func (p *PoolDevice) CreateSnapshotDevice(ctx context.Context, deviceName string, snapshotName string, virtualSizeBytes uint64) (retErr error) {
 	baseInfo, err := p.metadata.GetDevice(ctx, deviceName)
 	if err != nil {
 		return errors.Wrapf(err, "failed to query device metadata for %q", deviceName)
 	}
 
-	// Suspend thin device if it was activated previously
+	// Suspend thin device if it was activated previously to avoid corruptions
 	isActivated := p.IsActivated(baseInfo.Name)
 	if isActivated {
-		if err := p.transition(ctx, baseInfo.Name, Suspending, Suspended, func() error {
+		if err := p.suspendDevice(ctx, baseInfo); err != nil {
-			return dmsetup.SuspendDevice(baseInfo.Name)
+			return err
-		}); err != nil {
-			return errors.Wrapf(err, "failed to suspend device %q", baseInfo.Name)
 		}
+
+		// Resume back base thin device on exit
+		defer func() {
+			retErr = multierror.Append(retErr, p.resumeDevice(ctx, baseInfo)).ErrorOrNil()
+		}()
 	}
 
 	snapInfo := &DeviceInfo{
@@ -166,33 +201,63 @@ func (p *PoolDevice) CreateSnapshotDevice(ctx context.Context, deviceName string
 		return errors.Wrapf(err, "failed to save initial metadata for snapshot %q", snapshotName)
 	}
 
+	defer func() {
+		if retErr == nil {
+			return
+		}
+
+		// Rollback metadata
+		retErr = multierror.Append(retErr, p.metadata.RemoveDevice(ctx, snapInfo.Name))
+	}()
+
 	// Create thin device snapshot
+	if err := p.createSnapshot(ctx, baseInfo, snapInfo); err != nil {
+		return err
+	}
+
+	defer func() {
+		if retErr == nil {
+			return
+		}
+
+		// Rollback snapshot creation
+		retErr = multierror.Append(retErr, p.deleteDevice(ctx, snapInfo))
+	}()
+
+	// Activate snapshot device
+	return p.activateDevice(ctx, snapInfo)
+}
+
+func (p *PoolDevice) suspendDevice(ctx context.Context, info *DeviceInfo) error {
+	if err := p.transition(ctx, info.Name, Suspending, Suspended, func() error {
+		return dmsetup.SuspendDevice(info.Name)
+	}); err != nil {
+		return errors.Wrapf(err, "failed to suspend device %q", info.Name)
+	}
+
+	return nil
+}
+
+func (p *PoolDevice) resumeDevice(ctx context.Context, info *DeviceInfo) error {
+	if err := p.transition(ctx, info.Name, Resuming, Resumed, func() error {
+		return dmsetup.ResumeDevice(info.Name)
+	}); err != nil {
+		return errors.Wrapf(err, "failed to resume device %q", info.Name)
+	}
+
+	return nil
+}
+
+func (p *PoolDevice) createSnapshot(ctx context.Context, baseInfo, snapInfo *DeviceInfo) error {
 	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)",
+			"failed to create snapshot %q (dev: %d) from %q (dev: %d)",
 			snapInfo.Name,
 			snapInfo.DeviceID,
 			baseInfo.Name,
-			baseInfo.DeviceID,
+			baseInfo.DeviceID)
-			isActivated)
-	}
-
-	if isActivated {
-		// 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)
-		}
-	}
-
-	// 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 nil
@@ -221,7 +286,7 @@ func (p *PoolDevice) DeactivateDevice(ctx context.Context, deviceName string, de
 // 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 {
+	if err != nil || len(infos) != 1 {
 		// Couldn't query device info, device not active
 		return false
 	}
@@ -244,11 +309,8 @@ func (p *PoolDevice) RemoveDevice(ctx context.Context, deviceName string) error
 		return err
 	}
 
-	if err := p.transition(ctx, deviceName, Removing, Removed, func() error {
+	if err := p.deleteDevice(ctx, info); err != nil {
-		// Send 'delete' message to thin-pool
+		return err
-		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
@@ -259,6 +321,17 @@ func (p *PoolDevice) RemoveDevice(ctx context.Context, deviceName string) error
 	return nil
 }
 
+func (p *PoolDevice) deleteDevice(ctx context.Context, info *DeviceInfo) error {
+	if err := p.transition(ctx, info.Name, 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)
+	}
+
+	return nil
+}
+
 // RemovePool deactivates all child thin-devices and removes thin-pool device
 func (p *PoolDevice) RemovePool(ctx context.Context) error {
 	deviceNames, err := p.metadata.GetDeviceNames(ctx)

snapshots/devmapper/snapshotter.go

@@ -310,7 +310,9 @@ func (s *Snapshotter) createSnapshot(ctx context.Context, kind snapshots.Kind, k
 		}
 
 		if err := s.mkfs(ctx, deviceName); err != nil {
-			return nil, err
+			// Rollback thin device creation if mkfs failed
+			return nil, multierror.Append(err,
+				s.pool.RemoveDevice(ctx, deviceName))
 		}
 	} else {
 		parentDeviceName := s.getDeviceName(snap.ParentIDs[0])