From 4d5a0e19ebe1fd57a4c2fc1f6b36249060bd8b4a Mon Sep 17 00:00:00 2001
From: Maksym Pavlenko <makpav@amazon.com>
Date: Tue, 30 Jul 2019 16:03:09 -0700
Subject: [PATCH] Mark faulty device in one transaction

Signed-off-by: Maksym Pavlenko <makpav@amazon.com>
---
 snapshots/devmapper/metadata.go      | 31 ++++++++++++++--------------
 snapshots/devmapper/metadata_test.go |  2 +-
 snapshots/devmapper/pool_device.go   |  8 +++----
 3 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/snapshots/devmapper/metadata.go b/snapshots/devmapper/metadata.go
index 4a861c5d3..9b0f069b7 100644
--- a/snapshots/devmapper/metadata.go
+++ b/snapshots/devmapper/metadata.go
@@ -24,7 +24,6 @@ import (
 	"fmt"
 	"strconv"
 
-	"github.com/hashicorp/go-multierror"
 	"github.com/pkg/errors"
 	bolt "go.etcd.io/bbolt"
 )
@@ -127,23 +126,25 @@ func (m *PoolMetadata) AddDevice(ctx context.Context, info *DeviceInfo) error {
 // The snapshotter might attempt to recreate a device in 'Faulty' state with another devmapper ID in
 // subsequent calls, and in case of success it's status will be changed to 'Created' or 'Activated'.
 // The devmapper dev ID will remain in 'deviceFaulty' state until manually handled by a user.
-func (m *PoolMetadata) MarkFaulty(ctx context.Context, info *DeviceInfo) error {
-	var result error
+func (m *PoolMetadata) MarkFaulty(ctx context.Context, name string) error {
+	return m.db.Update(func(tx *bolt.Tx) error {
+		var (
+			device    = DeviceInfo{}
+			devBucket = tx.Bucket(devicesBucketName)
+		)
 
-	if err := m.UpdateDevice(ctx, info.Name, func(deviceInfo *DeviceInfo) error {
-		deviceInfo.State = Faulty
-		return nil
-	}); err != nil {
-		result = multierror.Append(result, err)
-	}
+		if err := getObject(devBucket, name, &device); err != nil {
+			return err
+		}
 
-	if err := m.db.Update(func(tx *bolt.Tx) error {
-		return markDeviceID(tx, info.DeviceID, deviceFaulty)
-	}); err != nil {
-		result = multierror.Append(result, err)
-	}
+		device.State = Faulty
 
-	return result
+		if err := putObject(devBucket, name, &device, true); err != nil {
+			return err
+		}
+
+		return markDeviceID(tx, device.DeviceID, deviceFaulty)
+	})
 }
 
 // getNextDeviceID finds the next free device ID by taking a cursor
diff --git a/snapshots/devmapper/metadata_test.go b/snapshots/devmapper/metadata_test.go
index c4309a598..750802733 100644
--- a/snapshots/devmapper/metadata_test.go
+++ b/snapshots/devmapper/metadata_test.go
@@ -162,7 +162,7 @@ func TestPoolMetadata_MarkFaulty(t *testing.T) {
 	err := store.AddDevice(testCtx, info)
 	assert.NilError(t, err)
 
-	err = store.MarkFaulty(testCtx, info)
+	err = store.MarkFaulty(testCtx, "test")
 	assert.NilError(t, err)
 
 	saved, err := store.GetDevice(testCtx, info.Name)
diff --git a/snapshots/devmapper/pool_device.go b/snapshots/devmapper/pool_device.go
index e720f33e8..68e07af87 100644
--- a/snapshots/devmapper/pool_device.go
+++ b/snapshots/devmapper/pool_device.go
@@ -132,7 +132,7 @@ func (p *PoolDevice) CreateThinDevice(ctx context.Context, deviceName string, vi
 			if delErr != nil {
 				// Failed to rollback, mark the device as faulty and keep metadata in order to
 				// preserve the faulty device ID
-				retErr = multierror.Append(retErr, delErr, p.metadata.MarkFaulty(ctx, info))
+				retErr = multierror.Append(retErr, delErr, p.metadata.MarkFaulty(ctx, info.Name))
 				return
 			}
 
@@ -147,7 +147,7 @@ func (p *PoolDevice) CreateThinDevice(ctx context.Context, deviceName string, vi
 
 		// We're unable to create the devmapper device, most likely something wrong with the deviceID
 		if devErr != nil {
-			retErr = multierror.Append(retErr, p.metadata.MarkFaulty(ctx, info))
+			retErr = multierror.Append(retErr, p.metadata.MarkFaulty(ctx, info.Name))
 			return
 		}
 	}()
@@ -223,7 +223,7 @@ func (p *PoolDevice) CreateSnapshotDevice(ctx context.Context, deviceName string
 			if delErr != nil {
 				// Failed to rollback, mark the device as faulty and keep metadata in order to
 				// preserve the faulty device ID
-				retErr = multierror.Append(retErr, delErr, p.metadata.MarkFaulty(ctx, snapInfo))
+				retErr = multierror.Append(retErr, delErr, p.metadata.MarkFaulty(ctx, snapInfo.Name))
 				return
 			}
 
@@ -238,7 +238,7 @@ func (p *PoolDevice) CreateSnapshotDevice(ctx context.Context, deviceName string
 
 		// We're unable to create the devmapper device, most likely something wrong with the deviceID
 		if devErr != nil {
-			retErr = multierror.Append(retErr, p.metadata.MarkFaulty(ctx, snapInfo))
+			retErr = multierror.Append(retErr, p.metadata.MarkFaulty(ctx, snapInfo.Name))
 			return
 		}
 	}()