From bb00453e58a86b9787ac4b3e7df3c48d8ddc3f87 Mon Sep 17 00:00:00 2001
From: Vivek Goyal <vgoyal@redhat.com>
Date: Wed, 3 Dec 2014 13:06:43 -0500
Subject: [PATCH 01/20] devmapper: Do not check for pool transaction id during
 old metadata migration

Current code is associating a transaction id with each device and if pool
transaction id is greater that value, then current code assumes that device
is there in pool.

Transaction id of pool is a mechanism so that during device creation and
removal one can define a transaction and during startup figure out if
transaction was complete or not. I think we are using transaction id
throughout the code little inappropriately.

For example, if a device is being deleted, it is possible that we deleted
the device from pool but before we could delete metafile docker crashed.
When docker comes back it will think that device is in the pool (due to
device transaction id being less than pool transaction id) but device
is not in the pool.

Similary, it could happen that some data in the pool is corrupted and
during pool repair some devices are lost (without docker knowing about
it). In that case tool pool transaction id will be higher than device
transaction id and there are no guaratees that device is actually in
the pool.

So move away from this model where we think that a device is in pool if pool
transaction id is greater than device transaction Id. Per device
transaction Id just says that after device creation this should be pool's
transaction Id and nothing more.

Transaction id is per pool property (as opposed to per device property) and
will be used internally to figure out if last transaction was complete or
not and recover from failure during docker startup.

If for some reason metafile is present but device is not in pool, then
device activation will fail later.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 daemon/graphdriver/devmapper/deviceset.go | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/daemon/graphdriver/devmapper/deviceset.go b/daemon/graphdriver/devmapper/deviceset.go
index b9d6e7616..9731d633c 100644
--- a/daemon/graphdriver/devmapper/deviceset.go
+++ b/daemon/graphdriver/devmapper/deviceset.go
@@ -367,11 +367,7 @@ func (devices *DeviceSet) initMetaData() error {
 
 		for hash, info := range m.Devices {
 			info.Hash = hash
-
-			// If the transaction id is larger than the actual one we lost the device due to some crash
-			if info.TransactionId <= devices.TransactionId {
-				devices.saveMetadata(info)
-			}
+			devices.saveMetadata(info)
 		}
 		if err := os.Rename(devices.oldMetadataFile(), devices.oldMetadataFile()+".migrated"); err != nil {
 			return err

From b721d6d8d0313fbb4b80e12318c96fc4004ee96b Mon Sep 17 00:00:00 2001
From: Vivek Goyal <vgoyal@redhat.com>
Date: Wed, 3 Dec 2014 13:06:43 -0500
Subject: [PATCH 02/20] devmapper: Do not check for transaction id during
 device metadata loading

Again, just because device transaction id is greater than pool transaction
id, it does not guarantee that device is in the pool. So do not check
of this during loading of device metadata.

Docker needs to deal with it. And device activation will fail when we try
to activate a device for whom metafile is present but there is no device
in the pool.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 daemon/graphdriver/devmapper/deviceset.go | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/daemon/graphdriver/devmapper/deviceset.go b/daemon/graphdriver/devmapper/deviceset.go
index 9731d633c..bc83bb7fa 100644
--- a/daemon/graphdriver/devmapper/deviceset.go
+++ b/daemon/graphdriver/devmapper/deviceset.go
@@ -390,11 +390,6 @@ func (devices *DeviceSet) loadMetadata(hash string) *DevInfo {
 		return nil
 	}
 
-	// If the transaction id is larger than the actual one we lost the device due to some crash
-	if info.TransactionId > devices.TransactionId {
-		return nil
-	}
-
 	return info
 }
 

From 004d8b9b337f4a6cf68c124e89e02e673c6320fc Mon Sep 17 00:00:00 2001
From: Vivek Goyal <vgoyal@redhat.com>
Date: Wed, 3 Dec 2014 13:06:43 -0500
Subject: [PATCH 03/20] devmapper: Remove unnecessary call to
 allocateTransactionId() during device removal

Remove call to allocateTransactionId() during device removal. This seems to
be unnecessary and it is not clear what this call is doing.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 daemon/graphdriver/devmapper/deviceset.go | 1 -
 1 file changed, 1 deletion(-)

diff --git a/daemon/graphdriver/devmapper/deviceset.go b/daemon/graphdriver/devmapper/deviceset.go
index bc83bb7fa..7ffabc3e2 100644
--- a/daemon/graphdriver/devmapper/deviceset.go
+++ b/daemon/graphdriver/devmapper/deviceset.go
@@ -793,7 +793,6 @@ func (devices *DeviceSet) deleteDevice(info *DevInfo) error {
 		return err
 	}
 
-	devices.allocateTransactionId()
 	devices.devicesLock.Lock()
 	delete(devices.Devices, info.Hash)
 	devices.devicesLock.Unlock()

From 0db6cc85edfccb16ce5308eea767530e1a3f6906 Mon Sep 17 00:00:00 2001
From: Vivek Goyal <vgoyal@redhat.com>
Date: Wed, 3 Dec 2014 13:06:43 -0500
Subject: [PATCH 04/20] devmapper: Remove transaction Id update from
 saveMetaData()

Right now saveMetaData() is kind of little overloaded function. It is
supposed to save file metadata to disk. But in addition if user has
bumped up NewTransactionId before calling saveMetaData(), then it will
also update the transaction ID in pool.

Keep saveMetaData() simple and let it just save the file. Any update
of pool transaction ID is done inline in the code which needs it.

Also create an helper function updatePoolTransactionId() to update pool
transaction Id.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 daemon/graphdriver/devmapper/deviceset.go | 26 +++++++++++++++++------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/daemon/graphdriver/devmapper/deviceset.go b/daemon/graphdriver/devmapper/deviceset.go
index 7ffabc3e2..d6891f809 100644
--- a/daemon/graphdriver/devmapper/deviceset.go
+++ b/daemon/graphdriver/devmapper/deviceset.go
@@ -204,6 +204,16 @@ func (devices *DeviceSet) allocateTransactionId() uint64 {
 	return devices.NewTransactionId
 }
 
+func (devices *DeviceSet) updatePoolTransactionId() error {
+	if devices.NewTransactionId != devices.TransactionId {
+		if err := devicemapper.SetTransactionId(devices.getPoolDevName(), devices.TransactionId, devices.NewTransactionId); err != nil {
+			return fmt.Errorf("Error setting devmapper transaction ID: %s", err)
+		}
+		devices.TransactionId = devices.NewTransactionId
+	}
+	return nil
+}
+
 func (devices *DeviceSet) removeMetadata(info *DevInfo) error {
 	if err := os.RemoveAll(devices.metadataFile(info)); err != nil {
 		return fmt.Errorf("Error removing metadata file %s: %s", devices.metadataFile(info), err)
@@ -246,13 +256,6 @@ func (devices *DeviceSet) saveMetadata(info *DevInfo) error {
 	if err := devices.writeMetaFile(jsonData, devices.metadataFile(info)); err != nil {
 		return err
 	}
-
-	if devices.NewTransactionId != devices.TransactionId {
-		if err = devicemapper.SetTransactionId(devices.getPoolDevName(), devices.TransactionId, devices.NewTransactionId); err != nil {
-			return fmt.Errorf("Error setting devmapper transition ID: %s", err)
-		}
-		devices.TransactionId = devices.NewTransactionId
-	}
 	return nil
 }
 
@@ -294,6 +297,15 @@ func (devices *DeviceSet) registerDevice(id int, hash string, size uint64) (*Dev
 		return nil, err
 	}
 
+	if err := devices.updatePoolTransactionId(); err != nil {
+		// Remove unused device
+		devices.devicesLock.Lock()
+		delete(devices.Devices, hash)
+		devices.devicesLock.Unlock()
+		devices.removeMetadata(info)
+		return nil, err
+	}
+
 	return info, nil
 }
 

From 824a87f7efb94e4f307d920c3c3689156d6e633f Mon Sep 17 00:00:00 2001
From: Vivek Goyal <vgoyal@redhat.com>
Date: Wed, 3 Dec 2014 13:06:43 -0500
Subject: [PATCH 05/20] devmapper: Move pool id query and migration of old data
 in separate function

Right now initMetaData() first queries the pool for current transaciton Id
and then it migrates the old metafile.

Move pool transaction Id query and file migration in separate functions
for better code reuse and organization.

Given we have removed device transaction Id dependency from saveMetaData(),
we don't have to query pool transaction Id before migrating files.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 daemon/graphdriver/devmapper/deviceset.go | 28 +++++++++++++----------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/daemon/graphdriver/devmapper/deviceset.go b/daemon/graphdriver/devmapper/deviceset.go
index d6891f809..26a3da8cf 100644
--- a/daemon/graphdriver/devmapper/deviceset.go
+++ b/daemon/graphdriver/devmapper/deviceset.go
@@ -352,19 +352,8 @@ func (devices *DeviceSet) createFilesystem(info *DevInfo) error {
 	return nil
 }
 
-func (devices *DeviceSet) initMetaData() error {
-	_, _, _, params, err := devicemapper.GetStatus(devices.getPoolName())
-	if err != nil {
-		return err
-	}
-
-	if _, err := fmt.Sscanf(params, "%d", &devices.TransactionId); err != nil {
-		return err
-	}
-	devices.NewTransactionId = devices.TransactionId
-
+func (devices *DeviceSet) migrateOldMetaData() error {
 	// Migrate old metadatafile
-
 	jsonData, err := ioutil.ReadFile(devices.oldMetadataFile())
 	if err != nil && !os.IsNotExist(err) {
 		return err
@@ -390,6 +379,21 @@ func (devices *DeviceSet) initMetaData() error {
 	return nil
 }
 
+func (devices *DeviceSet) initMetaData() error {
+	if err := devices.migrateOldMetaData(); err != nil {
+		return err
+	}
+
+	_, transactionId, _, _, _, _, err := devices.poolStatus()
+	if err != nil {
+		return err
+	}
+
+	devices.TransactionId = transactionId
+	devices.NewTransactionId = devices.TransactionId
+	return nil
+}
+
 func (devices *DeviceSet) loadMetadata(hash string) *DevInfo {
 	info := &DevInfo{Hash: hash, devices: devices}
 

From 5be77901cd505aad002b912b5febe2ba6baa23fd Mon Sep 17 00:00:00 2001
From: Vivek Goyal <vgoyal@redhat.com>
Date: Wed, 3 Dec 2014 13:06:43 -0500
Subject: [PATCH 06/20] devmapper: Do not add back device into hash map if meta
 file removal failed

When we are deleting a device, we also delete associated metadata file. If
that file removal fails, we are adding back the device in in-memory
table. I really can't see what's the point. When next lookup takes place
it will be automatically loaded if need be. Remove that code.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 daemon/graphdriver/devmapper/deviceset.go | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/daemon/graphdriver/devmapper/deviceset.go b/daemon/graphdriver/devmapper/deviceset.go
index 26a3da8cf..b4ce1b3b5 100644
--- a/daemon/graphdriver/devmapper/deviceset.go
+++ b/daemon/graphdriver/devmapper/deviceset.go
@@ -814,9 +814,6 @@ func (devices *DeviceSet) deleteDevice(info *DevInfo) error {
 	devices.devicesLock.Unlock()
 
 	if err := devices.removeMetadata(info); err != nil {
-		devices.devicesLock.Lock()
-		devices.Devices[info.Hash] = info
-		devices.devicesLock.Unlock()
 		log.Debugf("Error removing meta data: %s", err)
 		return err
 	}

From 7b2b15d3e9f9b7ad898a36bbe5ceb42c9ca58d47 Mon Sep 17 00:00:00 2001
From: Vivek Goyal <vgoyal@redhat.com>
Date: Wed, 3 Dec 2014 13:06:43 -0500
Subject: [PATCH 07/20] devmapper: Use device id as specified by caller

Currently devicemapper CreateDevice and CreateSnapDevice keep on retrying
device creation till a suitable device id is found.

With new transaction mechanism we need to store device id in transaction
before it has been created.

So change the logic in such a way that caller decides the devices Id to
use. If that device Id is not available, caller bumps up the device Id
and retries.

That way caller can update transaciton too when it tries a new Id. Transaction
related patches will come later in the series.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 daemon/graphdriver/devmapper/deviceset.go |  67 +++++++++++---
 pkg/devicemapper/devmapper.go             | 108 +++++++++++-----------
 2 files changed, 108 insertions(+), 67 deletions(-)

diff --git a/daemon/graphdriver/devmapper/deviceset.go b/daemon/graphdriver/devmapper/deviceset.go
index b4ce1b3b5..db9d2528f 100644
--- a/daemon/graphdriver/devmapper/deviceset.go
+++ b/daemon/graphdriver/devmapper/deviceset.go
@@ -394,6 +394,50 @@ func (devices *DeviceSet) initMetaData() error {
 	return nil
 }
 
+func (devices *DeviceSet) incNextDeviceId() {
+	// Ids are 24bit, so wrap around
+	devices.NextDeviceId = (devices.NextDeviceId + 1) & 0xffffff
+}
+
+func (devices *DeviceSet) createDevice(deviceId *int) error {
+	for {
+		if err := devicemapper.CreateDevice(devices.getPoolDevName(), *deviceId); err != nil {
+			if devicemapper.DeviceIdExists(err) {
+				// Device Id already exists. Try a new one.
+				devices.incNextDeviceId()
+				*deviceId = devices.NextDeviceId
+				continue
+			}
+			log.Debugf("Error creating device: %s", err)
+			return err
+		}
+		break
+	}
+	devices.incNextDeviceId()
+	return nil
+}
+
+func (devices *DeviceSet) createSnapDevice(baseInfo *DevInfo, deviceId *int) error {
+	log.Debugf("[deviceset] createSnapDevice() DeviceId=%d", *deviceId)
+	defer log.Debugf("[deviceset] createSnapDevice() END DeviceId=%d", *deviceId)
+
+	for {
+		if err := devicemapper.CreateSnapDevice(devices.getPoolDevName(), *deviceId, baseInfo.Name(), baseInfo.DeviceId); err != nil {
+			if devicemapper.DeviceIdExists(err) {
+				// Device Id already exists. Try a new one.
+				devices.incNextDeviceId()
+				*deviceId = devices.NextDeviceId
+				continue
+			}
+			log.Debugf("Error creating snap device: %s", err)
+			return err
+		}
+		break
+	}
+	devices.incNextDeviceId()
+	return nil
+}
+
 func (devices *DeviceSet) loadMetadata(hash string) *DevInfo {
 	info := &DevInfo{Hash: hash, devices: devices}
 
@@ -439,20 +483,16 @@ func (devices *DeviceSet) setupBaseImage() error {
 
 	log.Debugf("Initializing base device-mapper thin volume")
 
-	id := devices.NextDeviceId
-
 	// Create initial device
-	if err := devicemapper.CreateDevice(devices.getPoolDevName(), &id); err != nil {
+	deviceId := devices.NextDeviceId
+	if err := devices.createDevice(&deviceId); err != nil {
 		return err
 	}
 
-	// Ids are 24bit, so wrap around
-	devices.NextDeviceId = (id + 1) & 0xffffff
-
-	log.Debugf("Registering base device (id %v) with FS size %v", id, devices.baseFsSize)
-	info, err := devices.registerDevice(id, "", devices.baseFsSize)
+	log.Debugf("Registering base device (id %v) with FS size %v", deviceId, devices.baseFsSize)
+	info, err := devices.registerDevice(deviceId, "", devices.baseFsSize)
 	if err != nil {
-		_ = devicemapper.DeleteDevice(devices.getPoolDevName(), id)
+		_ = devicemapper.DeleteDevice(devices.getPoolDevName(), deviceId)
 		return err
 	}
 
@@ -751,6 +791,9 @@ func (devices *DeviceSet) initDevmapper(doInit bool) error {
 }
 
 func (devices *DeviceSet) AddDevice(hash, baseHash string) error {
+	log.Debugf("[deviceset] AddDevice() hash=%s basehash=%s", hash, baseHash)
+	defer log.Debugf("[deviceset] AddDevice END")
+
 	baseInfo, err := devices.lookupDevice(baseHash)
 	if err != nil {
 		return err
@@ -767,15 +810,11 @@ func (devices *DeviceSet) AddDevice(hash, baseHash string) error {
 	}
 
 	deviceId := devices.NextDeviceId
-
-	if err := devicemapper.CreateSnapDevice(devices.getPoolDevName(), &deviceId, baseInfo.Name(), baseInfo.DeviceId); err != nil {
+	if err := devices.createSnapDevice(baseInfo, &deviceId); err != nil {
 		log.Debugf("Error creating snap device: %s", err)
 		return err
 	}
 
-	// Ids are 24bit, so wrap around
-	devices.NextDeviceId = (deviceId + 1) & 0xffffff
-
 	if _, err := devices.registerDevice(deviceId, hash, baseInfo.Size); err != nil {
 		devicemapper.DeleteDevice(devices.getPoolDevName(), deviceId)
 		log.Debugf("Error registering device: %s", err)
diff --git a/pkg/devicemapper/devmapper.go b/pkg/devicemapper/devmapper.go
index a7306ba55..c23a3624d 100644
--- a/pkg/devicemapper/devmapper.go
+++ b/pkg/devicemapper/devmapper.go
@@ -67,6 +67,7 @@ var (
 	ErrGetLoopbackBackingFile = errors.New("Unable to get loopback backing file")
 	ErrLoopbackSetCapacity    = errors.New("Unable set loopback capacity")
 	ErrBusy                   = errors.New("Device is Busy")
+	ErrDeviceIdExists         = errors.New("Device Id Exists")
 
 	dmSawBusy  bool
 	dmSawExist bool
@@ -97,6 +98,16 @@ type (
 	AddNodeType int
 )
 
+// Returns whether error conveys the information about device Id already
+// exist or not. This will be true if device creation or snap creation
+// operation fails if device or snap device already exists in pool.
+// Current implementation is little crude as it scans the error string
+// for exact pattern match. Replacing it with more robust implementation
+// is desirable.
+func DeviceIdExists(err error) bool {
+	return fmt.Sprint(err) == fmt.Sprint(ErrDeviceIdExists)
+}
+
 func (t *Task) destroy() {
 	if t != nil {
 		DmTaskDestroy(t.unmanaged)
@@ -528,33 +539,29 @@ func ResumeDevice(name string) error {
 	return nil
 }
 
-func CreateDevice(poolName string, deviceId *int) error {
-	log.Debugf("[devmapper] CreateDevice(poolName=%v, deviceId=%v)", poolName, *deviceId)
+func CreateDevice(poolName string, deviceId int) error {
+	log.Debugf("[devmapper] CreateDevice(poolName=%v, deviceId=%v)", poolName, deviceId)
+	task, err := TaskCreateNamed(DeviceTargetMsg, poolName)
+	if task == nil {
+		return err
+	}
 
-	for {
-		task, err := TaskCreateNamed(DeviceTargetMsg, poolName)
-		if task == nil {
-			return err
-		}
+	if err := task.SetSector(0); err != nil {
+		return fmt.Errorf("Can't set sector %s", err)
+	}
 
-		if err := task.SetSector(0); err != nil {
-			return fmt.Errorf("Can't set sector %s", err)
-		}
+	if err := task.SetMessage(fmt.Sprintf("create_thin %d", deviceId)); err != nil {
+		return fmt.Errorf("Can't set message %s", err)
+	}
 
-		if err := task.SetMessage(fmt.Sprintf("create_thin %d", *deviceId)); err != nil {
-			return fmt.Errorf("Can't set message %s", err)
-		}
-
-		dmSawExist = false // reset before the task is run
-		if err := task.Run(); err != nil {
-			if dmSawExist {
-				// Already exists, try next id
-				*deviceId++
-				continue
-			}
+	dmSawExist = false // reset before the task is run
+	if err := task.Run(); err != nil {
+		// Caller wants to know about ErrDeviceIdExists so that it can try with a different device id.
+		if dmSawExist {
+			return ErrDeviceIdExists
+		} else {
 			return fmt.Errorf("Error running CreateDevice %s", err)
 		}
-		break
 	}
 	return nil
 }
@@ -607,7 +614,7 @@ func ActivateDevice(poolName string, name string, deviceId int, size uint64) err
 	return nil
 }
 
-func CreateSnapDevice(poolName string, deviceId *int, baseName string, baseDeviceId int) error {
+func CreateSnapDevice(poolName string, deviceId int, baseName string, baseDeviceId int) error {
 	devinfo, _ := GetInfo(baseName)
 	doSuspend := devinfo != nil && devinfo.Exists != 0
 
@@ -617,44 +624,39 @@ func CreateSnapDevice(poolName string, deviceId *int, baseName string, baseDevic
 		}
 	}
 
-	for {
-		task, err := TaskCreateNamed(DeviceTargetMsg, poolName)
-		if task == nil {
-			if doSuspend {
-				ResumeDevice(baseName)
-			}
-			return err
+	task, err := TaskCreateNamed(DeviceTargetMsg, poolName)
+	if task == nil {
+		if doSuspend {
+			ResumeDevice(baseName)
 		}
+		return err
+	}
 
-		if err := task.SetSector(0); err != nil {
-			if doSuspend {
-				ResumeDevice(baseName)
-			}
-			return fmt.Errorf("Can't set sector %s", err)
+	if err := task.SetSector(0); err != nil {
+		if doSuspend {
+			ResumeDevice(baseName)
 		}
+		return fmt.Errorf("Can't set sector %s", err)
+	}
 
-		if err := task.SetMessage(fmt.Sprintf("create_snap %d %d", *deviceId, baseDeviceId)); err != nil {
-			if doSuspend {
-				ResumeDevice(baseName)
-			}
-			return fmt.Errorf("Can't set message %s", err)
+	if err := task.SetMessage(fmt.Sprintf("create_snap %d %d", deviceId, baseDeviceId)); err != nil {
+		if doSuspend {
+			ResumeDevice(baseName)
 		}
+		return fmt.Errorf("Can't set message %s", err)
+	}
 
-		dmSawExist = false // reset before the task is run
-		if err := task.Run(); err != nil {
-			if dmSawExist {
-				// Already exists, try next id
-				*deviceId++
-				continue
-			}
-
-			if doSuspend {
-				ResumeDevice(baseName)
-			}
+	dmSawExist = false // reset before the task is run
+	if err := task.Run(); err != nil {
+		if doSuspend {
+			ResumeDevice(baseName)
+		}
+		// Caller wants to know about ErrDeviceIdExists so that it can try with a different device id.
+		if dmSawExist {
+			return ErrDeviceIdExists
+		} else {
 			return fmt.Errorf("Error running DeviceCreate (createSnapDevice) %s", err)
 		}
-
-		break
 	}
 
 	if doSuspend {

From 442247927b8e6c102ce1f94de58c7f93aab3d271 Mon Sep 17 00:00:00 2001
From: Vivek Goyal <vgoyal@redhat.com>
Date: Wed, 3 Dec 2014 13:06:43 -0500
Subject: [PATCH 08/20] devmapper: Provide a function unregisterDevice()

Currently registerDevice() adds a device to in-memory table, saves metadata
and also updates the pool transaction ID.

Now move transaciton Id update out of registerDevice() and provide a new
function unregisterDevice() which does the reverse of registerDevice().
This will simplify some code down the line and make it more structured.

This is just code reorganization and should not change functionality.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 daemon/graphdriver/devmapper/deviceset.go | 57 +++++++++++++++--------
 1 file changed, 38 insertions(+), 19 deletions(-)

diff --git a/daemon/graphdriver/devmapper/deviceset.go b/daemon/graphdriver/devmapper/deviceset.go
index db9d2528f..796385456 100644
--- a/daemon/graphdriver/devmapper/deviceset.go
+++ b/daemon/graphdriver/devmapper/deviceset.go
@@ -274,13 +274,32 @@ func (devices *DeviceSet) lookupDevice(hash string) (*DevInfo, error) {
 	return info, nil
 }
 
-func (devices *DeviceSet) registerDevice(id int, hash string, size uint64) (*DevInfo, error) {
+func (devices *DeviceSet) unregisterDevice(id int, hash string) error {
+	log.Debugf("unregisterDevice(%v, %v)", id, hash)
+	info := &DevInfo{
+		Hash:     hash,
+		DeviceId: id,
+	}
+
+	devices.devicesLock.Lock()
+	delete(devices.Devices, hash)
+	devices.devicesLock.Unlock()
+
+	if err := devices.removeMetadata(info); err != nil {
+		log.Debugf("Error removing meta data: %s", err)
+		return err
+	}
+
+	return nil
+}
+
+func (devices *DeviceSet) registerDevice(id int, hash string, size uint64, transactionId uint64) (*DevInfo, error) {
 	log.Debugf("registerDevice(%v, %v)", id, hash)
 	info := &DevInfo{
 		Hash:          hash,
 		DeviceId:      id,
 		Size:          size,
-		TransactionId: devices.allocateTransactionId(),
+		TransactionId: transactionId,
 		Initialized:   false,
 		devices:       devices,
 	}
@@ -297,15 +316,6 @@ func (devices *DeviceSet) registerDevice(id int, hash string, size uint64) (*Dev
 		return nil, err
 	}
 
-	if err := devices.updatePoolTransactionId(); err != nil {
-		// Remove unused device
-		devices.devicesLock.Lock()
-		delete(devices.Devices, hash)
-		devices.devicesLock.Unlock()
-		devices.removeMetadata(info)
-		return nil, err
-	}
-
 	return info, nil
 }
 
@@ -489,13 +499,20 @@ func (devices *DeviceSet) setupBaseImage() error {
 		return err
 	}
 
+	transactionId := devices.allocateTransactionId()
 	log.Debugf("Registering base device (id %v) with FS size %v", deviceId, devices.baseFsSize)
-	info, err := devices.registerDevice(deviceId, "", devices.baseFsSize)
+	info, err := devices.registerDevice(deviceId, "", devices.baseFsSize, transactionId)
 	if err != nil {
 		_ = devicemapper.DeleteDevice(devices.getPoolDevName(), deviceId)
 		return err
 	}
 
+	if err := devices.updatePoolTransactionId(); err != nil {
+		devices.unregisterDevice(deviceId, "")
+		devicemapper.DeleteDevice(devices.getPoolDevName(), deviceId)
+		return err
+	}
+
 	log.Debugf("Creating filesystem on base device-mapper thin volume")
 
 	if err = devices.activateDeviceIfNeeded(info); err != nil {
@@ -815,11 +832,18 @@ func (devices *DeviceSet) AddDevice(hash, baseHash string) error {
 		return err
 	}
 
-	if _, err := devices.registerDevice(deviceId, hash, baseInfo.Size); err != nil {
+	transactionId := devices.allocateTransactionId()
+	if _, err := devices.registerDevice(deviceId, hash, baseInfo.Size, transactionId); err != nil {
 		devicemapper.DeleteDevice(devices.getPoolDevName(), deviceId)
 		log.Debugf("Error registering device: %s", err)
 		return err
 	}
+
+	if err := devices.updatePoolTransactionId(); err != nil {
+		devices.unregisterDevice(deviceId, hash)
+		devicemapper.DeleteDevice(devices.getPoolDevName(), deviceId)
+		return err
+	}
 	return nil
 }
 
@@ -848,12 +872,7 @@ func (devices *DeviceSet) deleteDevice(info *DevInfo) error {
 		return err
 	}
 
-	devices.devicesLock.Lock()
-	delete(devices.Devices, info.Hash)
-	devices.devicesLock.Unlock()
-
-	if err := devices.removeMetadata(info); err != nil {
-		log.Debugf("Error removing meta data: %s", err)
+	if err := devices.unregisterDevice(info.DeviceId, info.Hash); err != nil {
 		return err
 	}
 

From ad9118c696c0953ec48eec15ea4b7546296d7c20 Mon Sep 17 00:00:00 2001
From: Vivek Goyal <vgoyal@redhat.com>
Date: Wed, 3 Dec 2014 13:06:43 -0500
Subject: [PATCH 09/20] devmapper: Create new helper function for device and
 snap creation

Create two new helper functions for device and snap device creation. These
functions will not only create the device and also register the device.

Again, makes the code structure better and keeps all transaction logic
contained to functions instead of spilling over into functions like
setupBaseImage or AddDevice().

Just the code reorganization. No functionality change.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 daemon/graphdriver/devmapper/deviceset.go | 76 ++++++++++++++---------
 1 file changed, 46 insertions(+), 30 deletions(-)

diff --git a/daemon/graphdriver/devmapper/deviceset.go b/daemon/graphdriver/devmapper/deviceset.go
index 796385456..713d1d60b 100644
--- a/daemon/graphdriver/devmapper/deviceset.go
+++ b/daemon/graphdriver/devmapper/deviceset.go
@@ -427,6 +427,28 @@ func (devices *DeviceSet) createDevice(deviceId *int) error {
 	return nil
 }
 
+func (devices *DeviceSet) createRegisterDevice(hash string) (*DevInfo, error) {
+	deviceId := devices.NextDeviceId
+	if err := devices.createDevice(&deviceId); err != nil {
+		return nil, err
+	}
+
+	transactionId := devices.allocateTransactionId()
+	log.Debugf("Registering device (id %v) with FS size %v", deviceId, devices.baseFsSize)
+	info, err := devices.registerDevice(deviceId, hash, devices.baseFsSize, transactionId)
+	if err != nil {
+		_ = devicemapper.DeleteDevice(devices.getPoolDevName(), deviceId)
+		return nil, err
+	}
+
+	if err := devices.updatePoolTransactionId(); err != nil {
+		devices.unregisterDevice(deviceId, hash)
+		devicemapper.DeleteDevice(devices.getPoolDevName(), deviceId)
+		return nil, err
+	}
+	return info, nil
+}
+
 func (devices *DeviceSet) createSnapDevice(baseInfo *DevInfo, deviceId *int) error {
 	log.Debugf("[deviceset] createSnapDevice() DeviceId=%d", *deviceId)
 	defer log.Debugf("[deviceset] createSnapDevice() END DeviceId=%d", *deviceId)
@@ -448,6 +470,28 @@ func (devices *DeviceSet) createSnapDevice(baseInfo *DevInfo, deviceId *int) err
 	return nil
 }
 
+func (devices *DeviceSet) createRegisterSnapDevice(hash string, baseInfo *DevInfo) error {
+	deviceId := devices.NextDeviceId
+	if err := devices.createSnapDevice(baseInfo, &deviceId); err != nil {
+		log.Debugf("Error creating snap device: %s", err)
+		return err
+	}
+
+	transactionId := devices.allocateTransactionId()
+	if _, err := devices.registerDevice(deviceId, hash, baseInfo.Size, transactionId); err != nil {
+		devicemapper.DeleteDevice(devices.getPoolDevName(), deviceId)
+		log.Debugf("Error registering device: %s", err)
+		return err
+	}
+
+	if err := devices.updatePoolTransactionId(); err != nil {
+		devices.unregisterDevice(deviceId, hash)
+		devicemapper.DeleteDevice(devices.getPoolDevName(), deviceId)
+		return err
+	}
+	return nil
+}
+
 func (devices *DeviceSet) loadMetadata(hash string) *DevInfo {
 	info := &DevInfo{Hash: hash, devices: devices}
 
@@ -494,22 +538,8 @@ func (devices *DeviceSet) setupBaseImage() error {
 	log.Debugf("Initializing base device-mapper thin volume")
 
 	// Create initial device
-	deviceId := devices.NextDeviceId
-	if err := devices.createDevice(&deviceId); err != nil {
-		return err
-	}
-
-	transactionId := devices.allocateTransactionId()
-	log.Debugf("Registering base device (id %v) with FS size %v", deviceId, devices.baseFsSize)
-	info, err := devices.registerDevice(deviceId, "", devices.baseFsSize, transactionId)
+	info, err := devices.createRegisterDevice("")
 	if err != nil {
-		_ = devicemapper.DeleteDevice(devices.getPoolDevName(), deviceId)
-		return err
-	}
-
-	if err := devices.updatePoolTransactionId(); err != nil {
-		devices.unregisterDevice(deviceId, "")
-		devicemapper.DeleteDevice(devices.getPoolDevName(), deviceId)
 		return err
 	}
 
@@ -826,24 +856,10 @@ func (devices *DeviceSet) AddDevice(hash, baseHash string) error {
 		return fmt.Errorf("device %s already exists", hash)
 	}
 
-	deviceId := devices.NextDeviceId
-	if err := devices.createSnapDevice(baseInfo, &deviceId); err != nil {
-		log.Debugf("Error creating snap device: %s", err)
+	if err := devices.createRegisterSnapDevice(hash, baseInfo); err != nil {
 		return err
 	}
 
-	transactionId := devices.allocateTransactionId()
-	if _, err := devices.registerDevice(deviceId, hash, baseInfo.Size, transactionId); err != nil {
-		devicemapper.DeleteDevice(devices.getPoolDevName(), deviceId)
-		log.Debugf("Error registering device: %s", err)
-		return err
-	}
-
-	if err := devices.updatePoolTransactionId(); err != nil {
-		devices.unregisterDevice(deviceId, hash)
-		devicemapper.DeleteDevice(devices.getPoolDevName(), deviceId)
-		return err
-	}
 	return nil
 }
 

From 6d347aeb6984ebdcb1051212ab3103880ef69ab0 Mon Sep 17 00:00:00 2001
From: Vivek Goyal <vgoyal@redhat.com>
Date: Wed, 3 Dec 2014 13:06:43 -0500
Subject: [PATCH 10/20] devmapper: Remove unnecessary condition check in
 updatePoolTransactionId()

Currently updatePoolTransactionId() checks if NewTransactionId and
TransactionId are not same only then update the transaction Id in pool. This
check is redundant. Currently we call updatePoolTransactionId() only from
two places and both of these first allocate a new transaction Id.

Also updatePoolTransactionId() should only be called after allocating
new transaction Id otherwise it does not make any sense.

Remove the redundant check and reduce confusion.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 daemon/graphdriver/devmapper/deviceset.go | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/daemon/graphdriver/devmapper/deviceset.go b/daemon/graphdriver/devmapper/deviceset.go
index 713d1d60b..40fb4e2d4 100644
--- a/daemon/graphdriver/devmapper/deviceset.go
+++ b/daemon/graphdriver/devmapper/deviceset.go
@@ -205,12 +205,10 @@ func (devices *DeviceSet) allocateTransactionId() uint64 {
 }
 
 func (devices *DeviceSet) updatePoolTransactionId() error {
-	if devices.NewTransactionId != devices.TransactionId {
-		if err := devicemapper.SetTransactionId(devices.getPoolDevName(), devices.TransactionId, devices.NewTransactionId); err != nil {
-			return fmt.Errorf("Error setting devmapper transaction ID: %s", err)
-		}
-		devices.TransactionId = devices.NewTransactionId
+	if err := devicemapper.SetTransactionId(devices.getPoolDevName(), devices.TransactionId, devices.NewTransactionId); err != nil {
+		return fmt.Errorf("Error setting devmapper transaction ID: %s", err)
 	}
+	devices.TransactionId = devices.NewTransactionId
 	return nil
 }
 

From 7b0a1b814b8f13e30df466dd66c3fdc2114eac28 Mon Sep 17 00:00:00 2001
From: Vivek Goyal <vgoyal@redhat.com>
Date: Wed, 3 Dec 2014 13:06:43 -0500
Subject: [PATCH 11/20] devmapper: Allocate new transaction Id using current
 transaction Id

Currently new transaction Id is created using allocateTransactionId()
function. This function takes NewTransactionId and bumps up by one
to create NewTransactionId.

I think ideally we should be bumping up devices.TransactionId by 1
to come up with NewTransactionId. Because idea is that devices.TransactionId
contains the current pool transaction Id and to come up with a new
transaction Id bump it up by one.

Current code is not wrong as we are keeping NewTransactionId and
TransactionId in sync. But it will be more direct if we look at
devices.TransactionId to come up with NewTransactionId. That way
we don't have to even initialize NewTransactionId during startup
as first time somebody wants to do a transaction, it will be
allocated fresh.

So simplify the code a bit. No functionality change.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 daemon/graphdriver/devmapper/deviceset.go | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/daemon/graphdriver/devmapper/deviceset.go b/daemon/graphdriver/devmapper/deviceset.go
index 40fb4e2d4..2660ade29 100644
--- a/daemon/graphdriver/devmapper/deviceset.go
+++ b/daemon/graphdriver/devmapper/deviceset.go
@@ -200,7 +200,7 @@ func (devices *DeviceSet) ensureImage(name string, size int64) (string, error) {
 }
 
 func (devices *DeviceSet) allocateTransactionId() uint64 {
-	devices.NewTransactionId = devices.NewTransactionId + 1
+	devices.NewTransactionId = devices.TransactionId + 1
 	return devices.NewTransactionId
 }
 
@@ -398,7 +398,6 @@ func (devices *DeviceSet) initMetaData() error {
 	}
 
 	devices.TransactionId = transactionId
-	devices.NewTransactionId = devices.TransactionId
 	return nil
 }
 

From f078bcd8e50913fd8b05022ebd047c5a1f2e3d52 Mon Sep 17 00:00:00 2001
From: Vivek Goyal <vgoyal@redhat.com>
Date: Wed, 3 Dec 2014 13:06:43 -0500
Subject: [PATCH 12/20] devmapper: Rename NewTransactionId to OpenTransactionId

Very soon we will have the notion of an open transaction and keep its
details in a metafile.

When a new transaction is opened, we allocate a new transaction Id,
do the device creation/deletion and then we will close the transaction.

I thought that OpenTransactionId better represents the semantics of
transaction Id associated with an open transaction instead of NewtransactionId.

This patch just does the renaming. No functionality change.

I have also introduced a structure "Transaction" which will keep all
the details associated with a transaction. Later patches will add more
fields in this structure.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 daemon/graphdriver/devmapper/deviceset.go | 26 +++++++++++++----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/daemon/graphdriver/devmapper/deviceset.go b/daemon/graphdriver/devmapper/deviceset.go
index 2660ade29..f13257535 100644
--- a/daemon/graphdriver/devmapper/deviceset.go
+++ b/daemon/graphdriver/devmapper/deviceset.go
@@ -35,6 +35,10 @@ var (
 
 const deviceSetMetaFile string = "deviceset-metadata"
 
+type Transaction struct {
+	OpenTransactionId uint64 `json:"-"`
+}
+
 type DevInfo struct {
 	Hash          string     `json:"-"`
 	DeviceId      int        `json:"device_id"`
@@ -65,13 +69,12 @@ type MetaData struct {
 }
 
 type DeviceSet struct {
-	MetaData         `json:"-"`
-	sync.Mutex       `json:"-"` // Protects Devices map and serializes calls into libdevmapper
-	root             string
-	devicePrefix     string
-	TransactionId    uint64 `json:"-"`
-	NewTransactionId uint64 `json:"-"`
-	NextDeviceId     int    `json:"next_device_id"`
+	MetaData      `json:"-"`
+	sync.Mutex    `json:"-"` // Protects Devices map and serializes calls into libdevmapper
+	root          string
+	devicePrefix  string
+	TransactionId uint64 `json:"-"`
+	NextDeviceId  int    `json:"next_device_id"`
 
 	// Options
 	dataLoopbackSize     int64
@@ -85,6 +88,7 @@ type DeviceSet struct {
 	doBlkDiscard         bool
 	thinpBlockSize       uint32
 	thinPoolDevice       string
+	Transaction          `json:"-"`
 }
 
 type DiskUsage struct {
@@ -200,15 +204,15 @@ func (devices *DeviceSet) ensureImage(name string, size int64) (string, error) {
 }
 
 func (devices *DeviceSet) allocateTransactionId() uint64 {
-	devices.NewTransactionId = devices.TransactionId + 1
-	return devices.NewTransactionId
+	devices.OpenTransactionId = devices.TransactionId + 1
+	return devices.OpenTransactionId
 }
 
 func (devices *DeviceSet) updatePoolTransactionId() error {
-	if err := devicemapper.SetTransactionId(devices.getPoolDevName(), devices.TransactionId, devices.NewTransactionId); err != nil {
+	if err := devicemapper.SetTransactionId(devices.getPoolDevName(), devices.TransactionId, devices.OpenTransactionId); err != nil {
 		return fmt.Errorf("Error setting devmapper transaction ID: %s", err)
 	}
-	devices.TransactionId = devices.NewTransactionId
+	devices.TransactionId = devices.OpenTransactionId
 	return nil
 }
 

From 359a38b26a164f430c79fe542babb77c6e48dcc3 Mon Sep 17 00:00:00 2001
From: Vivek Goyal <vgoyal@redhat.com>
Date: Wed, 3 Dec 2014 13:06:43 -0500
Subject: [PATCH 13/20] devmapper: Use a common delete function for all device
 deletion operation

Right now setupBaseImage() uses deleteDevice() to delete uninitialized
base image while rest of the code uses DeleteDevice(). Change it and
use a common function everywhere for the sake of uniformity.

I can't see what harm can be done by doing little extra locking done
by DeleteDevice().

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 daemon/graphdriver/devmapper/deviceset.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/daemon/graphdriver/devmapper/deviceset.go b/daemon/graphdriver/devmapper/deviceset.go
index f13257535..802d2e656 100644
--- a/daemon/graphdriver/devmapper/deviceset.go
+++ b/daemon/graphdriver/devmapper/deviceset.go
@@ -516,7 +516,7 @@ func (devices *DeviceSet) setupBaseImage() error {
 
 	if oldInfo != nil && !oldInfo.Initialized {
 		log.Debugf("Removing uninitialized base image")
-		if err := devices.deleteDevice(oldInfo); err != nil {
+		if err := devices.DeleteDevice(""); err != nil {
 			return err
 		}
 	}

From 4d39e056aac2fadffcb8560101f3c31a2b7db3ae Mon Sep 17 00:00:00 2001
From: Vivek Goyal <vgoyal@redhat.com>
Date: Wed, 3 Dec 2014 13:06:43 -0500
Subject: [PATCH 14/20] devmapper: Keep track of used device Ids in a bitmap

Currently devicemapper backend does not keep track of used device Ids in
the pool. It tries a device Id and if that device Id exists in pool, it
tries with a different Id and keeps on doing this in a loop till it succeeds.

This worked fine so far but now we are moving to transaction based
device creation and deletion. We will keep deviceId information in
transaction which will be rolled back if docker crashed before transaction
was complete.

If we store a deviceId in transaction and later figure out it already
existed in pool and docker crashed, then we will rollback and remove
that existing device Id from pool (which we should not have).

That means, we should know free device Id in pool in advance before
we put that device Id in transaction.

Hence this patch creates a bitmap (one bit each for a deviceId), and
sets the bit if device Id is used otherwise resets it. This patch
is just preparing the ground right now. Actual usage will follow
in later patches.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 daemon/graphdriver/devmapper/deviceset.go | 32 +++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/daemon/graphdriver/devmapper/deviceset.go b/daemon/graphdriver/devmapper/deviceset.go
index 802d2e656..d72186128 100644
--- a/daemon/graphdriver/devmapper/deviceset.go
+++ b/daemon/graphdriver/devmapper/deviceset.go
@@ -30,7 +30,9 @@ var (
 	DefaultDataLoopbackSize     int64  = 100 * 1024 * 1024 * 1024
 	DefaultMetaDataLoopbackSize int64  = 2 * 1024 * 1024 * 1024
 	DefaultBaseFsSize           uint64 = 10 * 1024 * 1024 * 1024
-	DefaultThinpBlockSize       uint32 = 128 // 64K = 128 512b sectors
+	DefaultThinpBlockSize       uint32 = 128      // 64K = 128 512b sectors
+	MaxDeviceId                 int    = 0xffffff // 24 bit, pool limit
+	DeviceIdMapSz               int    = (MaxDeviceId + 1) / 8
 )
 
 const deviceSetMetaFile string = "deviceset-metadata"
@@ -75,6 +77,7 @@ type DeviceSet struct {
 	devicePrefix  string
 	TransactionId uint64 `json:"-"`
 	NextDeviceId  int    `json:"next_device_id"`
+	deviceIdMap   []byte
 
 	// Options
 	dataLoopbackSize     int64
@@ -261,6 +264,30 @@ func (devices *DeviceSet) saveMetadata(info *DevInfo) error {
 	return nil
 }
 
+func (devices *DeviceSet) markDeviceIdUsed(deviceId int) {
+	var mask byte
+	i := deviceId % 8
+	mask = 1 << uint(i)
+	devices.deviceIdMap[deviceId/8] = devices.deviceIdMap[deviceId/8] | mask
+}
+
+func (devices *DeviceSet) markDeviceIdFree(deviceId int) {
+	var mask byte
+	i := deviceId % 8
+	mask = ^(1 << uint(i))
+	devices.deviceIdMap[deviceId/8] = devices.deviceIdMap[deviceId/8] & mask
+}
+
+func (devices *DeviceSet) isDeviceIdFree(deviceId int) bool {
+	var mask byte
+	i := deviceId % 8
+	mask = (1 << uint(i))
+	if (devices.deviceIdMap[deviceId/8] & mask) != 0 {
+		return false
+	}
+	return true
+}
+
 func (devices *DeviceSet) lookupDevice(hash string) (*DevInfo, error) {
 	devices.devicesLock.Lock()
 	defer devices.devicesLock.Unlock()
@@ -407,7 +434,7 @@ func (devices *DeviceSet) initMetaData() error {
 
 func (devices *DeviceSet) incNextDeviceId() {
 	// Ids are 24bit, so wrap around
-	devices.NextDeviceId = (devices.NextDeviceId + 1) & 0xffffff
+	devices.NextDeviceId = (devices.NextDeviceId + 1) & MaxDeviceId
 }
 
 func (devices *DeviceSet) createDevice(deviceId *int) error {
@@ -1333,6 +1360,7 @@ func NewDeviceSet(root string, doInit bool, options []string) (*DeviceSet, error
 		filesystem:           "ext4",
 		doBlkDiscard:         true,
 		thinpBlockSize:       DefaultThinpBlockSize,
+		deviceIdMap:          make([]byte, DeviceIdMapSz),
 	}
 
 	foundBlkDiscard := false

From 39dc7829dea87d4be8e6e9b2a598fb354ebf4ba0 Mon Sep 17 00:00:00 2001
From: Vivek Goyal <vgoyal@redhat.com>
Date: Wed, 3 Dec 2014 13:06:43 -0500
Subject: [PATCH 15/20] devmapper: Construct initial device Id map from device
 meta files

When docker starts, build a used/free Device Id map from the per
device meta files we already have. These meta files have the data
which device Ids are in use. Parse these files and mark device as
used in the map.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 daemon/graphdriver/devmapper/deviceset.go | 63 +++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/daemon/graphdriver/devmapper/deviceset.go b/daemon/graphdriver/devmapper/deviceset.go
index d72186128..fd4a11e5e 100644
--- a/daemon/graphdriver/devmapper/deviceset.go
+++ b/daemon/graphdriver/devmapper/deviceset.go
@@ -303,6 +303,65 @@ func (devices *DeviceSet) lookupDevice(hash string) (*DevInfo, error) {
 	return info, nil
 }
 
+func (devices *DeviceSet) deviceFileWalkFunction(path string, finfo os.FileInfo) error {
+
+	// Skip some of the meta files which are not device files.
+	if strings.HasSuffix(finfo.Name(), ".migrated") {
+		log.Debugf("Skipping file %s", path)
+		return nil
+	}
+
+	if finfo.Name() == deviceSetMetaFile {
+		log.Debugf("Skipping file %s", path)
+		return nil
+	}
+
+	log.Debugf("Loading data for file %s", path)
+
+	hash := finfo.Name()
+	if hash == "base" {
+		hash = ""
+	}
+
+	dinfo := devices.loadMetadata(hash)
+	if dinfo == nil {
+		return fmt.Errorf("Error loading device metadata file %s", hash)
+	}
+
+	if dinfo.DeviceId > MaxDeviceId {
+		log.Errorf("Warning: Ignoring Invalid DeviceId=%d", dinfo.DeviceId)
+		return nil
+	}
+
+	devices.Lock()
+	devices.markDeviceIdUsed(dinfo.DeviceId)
+	devices.Unlock()
+
+	log.Debugf("Added deviceId=%d to DeviceIdMap", dinfo.DeviceId)
+	return nil
+}
+
+func (devices *DeviceSet) constructDeviceIdMap() error {
+	log.Debugf("[deviceset] constructDeviceIdMap()")
+	defer log.Debugf("[deviceset] constructDeviceIdMap() END")
+
+	var scan = func(path string, info os.FileInfo, err error) error {
+		if err != nil {
+			log.Debugf("Can't walk the file %s", path)
+			return nil
+		}
+
+		// Skip any directories
+		if info.IsDir() {
+			return nil
+		}
+
+		return devices.deviceFileWalkFunction(path, info)
+	}
+
+	return filepath.Walk(devices.metadataDir(), scan)
+}
+
 func (devices *DeviceSet) unregisterDevice(id int, hash string) error {
 	log.Debugf("unregisterDevice(%v, %v)", id, hash)
 	info := &DevInfo{
@@ -429,6 +488,10 @@ func (devices *DeviceSet) initMetaData() error {
 	}
 
 	devices.TransactionId = transactionId
+
+	if err := devices.constructDeviceIdMap(); err != nil {
+		return err
+	}
 	return nil
 }
 

From a44c23fe6604d1de59c64bbb9dc234c7c3dbede9 Mon Sep 17 00:00:00 2001
From: Vivek Goyal <vgoyal@redhat.com>
Date: Wed, 3 Dec 2014 13:06:43 -0500
Subject: [PATCH 16/20] devmapper: Provide a helper function getNextDeviceId()

Right now we are accessing devices.NextDeviceId directly and also
incrementing it at various places.

Instead provide a helper function which is responsile for
incrementing NextDeviceId and return next deviceId.

This is just code structuring. This will help later once we
convert this function to find a free device Id and it goes
through a bitmap of used/free device Ids.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 daemon/graphdriver/devmapper/deviceset.go | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/daemon/graphdriver/devmapper/deviceset.go b/daemon/graphdriver/devmapper/deviceset.go
index fd4a11e5e..bf24b5164 100644
--- a/daemon/graphdriver/devmapper/deviceset.go
+++ b/daemon/graphdriver/devmapper/deviceset.go
@@ -500,13 +500,17 @@ func (devices *DeviceSet) incNextDeviceId() {
 	devices.NextDeviceId = (devices.NextDeviceId + 1) & MaxDeviceId
 }
 
+func (devices *DeviceSet) getNextDeviceId() int {
+	devices.incNextDeviceId()
+	return devices.NextDeviceId
+}
+
 func (devices *DeviceSet) createDevice(deviceId *int) error {
 	for {
 		if err := devicemapper.CreateDevice(devices.getPoolDevName(), *deviceId); err != nil {
 			if devicemapper.DeviceIdExists(err) {
 				// Device Id already exists. Try a new one.
-				devices.incNextDeviceId()
-				*deviceId = devices.NextDeviceId
+				*deviceId = devices.getNextDeviceId()
 				continue
 			}
 			log.Debugf("Error creating device: %s", err)
@@ -514,12 +518,11 @@ func (devices *DeviceSet) createDevice(deviceId *int) error {
 		}
 		break
 	}
-	devices.incNextDeviceId()
 	return nil
 }
 
 func (devices *DeviceSet) createRegisterDevice(hash string) (*DevInfo, error) {
-	deviceId := devices.NextDeviceId
+	deviceId := devices.getNextDeviceId()
 	if err := devices.createDevice(&deviceId); err != nil {
 		return nil, err
 	}
@@ -548,8 +551,7 @@ func (devices *DeviceSet) createSnapDevice(baseInfo *DevInfo, deviceId *int) err
 		if err := devicemapper.CreateSnapDevice(devices.getPoolDevName(), *deviceId, baseInfo.Name(), baseInfo.DeviceId); err != nil {
 			if devicemapper.DeviceIdExists(err) {
 				// Device Id already exists. Try a new one.
-				devices.incNextDeviceId()
-				*deviceId = devices.NextDeviceId
+				*deviceId = devices.getNextDeviceId()
 				continue
 			}
 			log.Debugf("Error creating snap device: %s", err)
@@ -557,12 +559,11 @@ func (devices *DeviceSet) createSnapDevice(baseInfo *DevInfo, deviceId *int) err
 		}
 		break
 	}
-	devices.incNextDeviceId()
 	return nil
 }
 
 func (devices *DeviceSet) createRegisterSnapDevice(hash string, baseInfo *DevInfo) error {
-	deviceId := devices.NextDeviceId
+	deviceId := devices.getNextDeviceId()
 	if err := devices.createSnapDevice(baseInfo, &deviceId); err != nil {
 		log.Debugf("Error creating snap device: %s", err)
 		return err

From 14d0dd855ee1e7cd1a3185c3d5a00e7afccb5c43 Mon Sep 17 00:00:00 2001
From: Vivek Goyal <vgoyal@redhat.com>
Date: Wed, 3 Dec 2014 13:06:43 -0500
Subject: [PATCH 17/20] devmapper: Open code createDevice() and
 createSnapDevice()

Open code createDevice() and createSnapDevice() and move all the logic
in the caller.

This is a sheer code reorganization so that all device Id allocation
logic is in one function. That way in case of erros, one can easily
cleanup and mark device Id free again. (Later patches benefit from
it).

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 daemon/graphdriver/devmapper/deviceset.go | 36 ++++++-----------------
 1 file changed, 9 insertions(+), 27 deletions(-)

diff --git a/daemon/graphdriver/devmapper/deviceset.go b/daemon/graphdriver/devmapper/deviceset.go
index bf24b5164..17fbf715a 100644
--- a/daemon/graphdriver/devmapper/deviceset.go
+++ b/daemon/graphdriver/devmapper/deviceset.go
@@ -505,27 +505,20 @@ func (devices *DeviceSet) getNextDeviceId() int {
 	return devices.NextDeviceId
 }
 
-func (devices *DeviceSet) createDevice(deviceId *int) error {
+func (devices *DeviceSet) createRegisterDevice(hash string) (*DevInfo, error) {
+	deviceId := devices.getNextDeviceId()
 	for {
-		if err := devicemapper.CreateDevice(devices.getPoolDevName(), *deviceId); err != nil {
+		if err := devicemapper.CreateDevice(devices.getPoolDevName(), deviceId); err != nil {
 			if devicemapper.DeviceIdExists(err) {
 				// Device Id already exists. Try a new one.
-				*deviceId = devices.getNextDeviceId()
+				deviceId = devices.getNextDeviceId()
 				continue
 			}
 			log.Debugf("Error creating device: %s", err)
-			return err
+			return nil, err
 		}
 		break
 	}
-	return nil
-}
-
-func (devices *DeviceSet) createRegisterDevice(hash string) (*DevInfo, error) {
-	deviceId := devices.getNextDeviceId()
-	if err := devices.createDevice(&deviceId); err != nil {
-		return nil, err
-	}
 
 	transactionId := devices.allocateTransactionId()
 	log.Debugf("Registering device (id %v) with FS size %v", deviceId, devices.baseFsSize)
@@ -543,15 +536,13 @@ func (devices *DeviceSet) createRegisterDevice(hash string) (*DevInfo, error) {
 	return info, nil
 }
 
-func (devices *DeviceSet) createSnapDevice(baseInfo *DevInfo, deviceId *int) error {
-	log.Debugf("[deviceset] createSnapDevice() DeviceId=%d", *deviceId)
-	defer log.Debugf("[deviceset] createSnapDevice() END DeviceId=%d", *deviceId)
-
+func (devices *DeviceSet) createRegisterSnapDevice(hash string, baseInfo *DevInfo) error {
+	deviceId := devices.getNextDeviceId()
 	for {
-		if err := devicemapper.CreateSnapDevice(devices.getPoolDevName(), *deviceId, baseInfo.Name(), baseInfo.DeviceId); err != nil {
+		if err := devicemapper.CreateSnapDevice(devices.getPoolDevName(), deviceId, baseInfo.Name(), baseInfo.DeviceId); err != nil {
 			if devicemapper.DeviceIdExists(err) {
 				// Device Id already exists. Try a new one.
-				*deviceId = devices.getNextDeviceId()
+				deviceId = devices.getNextDeviceId()
 				continue
 			}
 			log.Debugf("Error creating snap device: %s", err)
@@ -559,15 +550,6 @@ func (devices *DeviceSet) createSnapDevice(baseInfo *DevInfo, deviceId *int) err
 		}
 		break
 	}
-	return nil
-}
-
-func (devices *DeviceSet) createRegisterSnapDevice(hash string, baseInfo *DevInfo) error {
-	deviceId := devices.getNextDeviceId()
-	if err := devices.createSnapDevice(baseInfo, &deviceId); err != nil {
-		log.Debugf("Error creating snap device: %s", err)
-		return err
-	}
 
 	transactionId := devices.allocateTransactionId()
 	if _, err := devices.registerDevice(deviceId, hash, baseInfo.Size, transactionId); err != nil {

From e28a419e1197bf50bbb378b02f0226c3115edeaa Mon Sep 17 00:00:00 2001
From: Vivek Goyal <vgoyal@redhat.com>
Date: Wed, 3 Dec 2014 13:06:43 -0500
Subject: [PATCH 18/20] devmapper: Find a free device Id to use for device
 creation

Finally, we seem to have all the bits to keep track of all used device
Ids and find a free device Id to use when creating a  new device. Start
using it.

Ideally we should completely move away from retry logic when pool returns
-EEXISTS. For now I have retained that logic and I simply output a warning.
When things are stable, we should be able to get rid of it.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 daemon/graphdriver/devmapper/deviceset.go | 54 +++++++++++++++++++----
 1 file changed, 46 insertions(+), 8 deletions(-)

diff --git a/daemon/graphdriver/devmapper/deviceset.go b/daemon/graphdriver/devmapper/deviceset.go
index 17fbf715a..04437ce9f 100644
--- a/daemon/graphdriver/devmapper/deviceset.go
+++ b/daemon/graphdriver/devmapper/deviceset.go
@@ -500,21 +500,41 @@ func (devices *DeviceSet) incNextDeviceId() {
 	devices.NextDeviceId = (devices.NextDeviceId + 1) & MaxDeviceId
 }
 
-func (devices *DeviceSet) getNextDeviceId() int {
+func (devices *DeviceSet) getNextFreeDeviceId() (int, error) {
 	devices.incNextDeviceId()
-	return devices.NextDeviceId
+	for i := 0; i <= MaxDeviceId; i++ {
+		if devices.isDeviceIdFree(devices.NextDeviceId) {
+			devices.markDeviceIdUsed(devices.NextDeviceId)
+			return devices.NextDeviceId, nil
+		}
+		devices.incNextDeviceId()
+	}
+
+	return 0, fmt.Errorf("Unable to find a free device Id")
 }
 
 func (devices *DeviceSet) createRegisterDevice(hash string) (*DevInfo, error) {
-	deviceId := devices.getNextDeviceId()
+	deviceId, err := devices.getNextFreeDeviceId()
+	if err != nil {
+		return nil, err
+	}
+
 	for {
 		if err := devicemapper.CreateDevice(devices.getPoolDevName(), deviceId); err != nil {
 			if devicemapper.DeviceIdExists(err) {
-				// Device Id already exists. Try a new one.
-				deviceId = devices.getNextDeviceId()
+				// Device Id already exists. This should not
+				// happen. Now we have a mechianism to find
+				// a free device Id. So something is not right.
+				// Give a warning and continue.
+				log.Errorf("Warning: Device Id %d exists in pool but it is supposed to be unused", deviceId)
+				deviceId, err = devices.getNextFreeDeviceId()
+				if err != nil {
+					return nil, err
+				}
 				continue
 			}
 			log.Debugf("Error creating device: %s", err)
+			devices.markDeviceIdFree(deviceId)
 			return nil, err
 		}
 		break
@@ -525,27 +545,41 @@ func (devices *DeviceSet) createRegisterDevice(hash string) (*DevInfo, error) {
 	info, err := devices.registerDevice(deviceId, hash, devices.baseFsSize, transactionId)
 	if err != nil {
 		_ = devicemapper.DeleteDevice(devices.getPoolDevName(), deviceId)
+		devices.markDeviceIdFree(deviceId)
 		return nil, err
 	}
 
 	if err := devices.updatePoolTransactionId(); err != nil {
 		devices.unregisterDevice(deviceId, hash)
 		devicemapper.DeleteDevice(devices.getPoolDevName(), deviceId)
+		devices.markDeviceIdFree(deviceId)
 		return nil, err
 	}
 	return info, nil
 }
 
 func (devices *DeviceSet) createRegisterSnapDevice(hash string, baseInfo *DevInfo) error {
-	deviceId := devices.getNextDeviceId()
+	deviceId, err := devices.getNextFreeDeviceId()
+	if err != nil {
+		return err
+	}
+
 	for {
 		if err := devicemapper.CreateSnapDevice(devices.getPoolDevName(), deviceId, baseInfo.Name(), baseInfo.DeviceId); err != nil {
 			if devicemapper.DeviceIdExists(err) {
-				// Device Id already exists. Try a new one.
-				deviceId = devices.getNextDeviceId()
+				// Device Id already exists. This should not
+				// happen. Now we have a mechianism to find
+				// a free device Id. So something is not right.
+				// Give a warning and continue.
+				log.Errorf("Warning: Device Id %d exists in pool but it is supposed to be unused", deviceId)
+				deviceId, err = devices.getNextFreeDeviceId()
+				if err != nil {
+					return err
+				}
 				continue
 			}
 			log.Debugf("Error creating snap device: %s", err)
+			devices.markDeviceIdFree(deviceId)
 			return err
 		}
 		break
@@ -554,6 +588,7 @@ func (devices *DeviceSet) createRegisterSnapDevice(hash string, baseInfo *DevInf
 	transactionId := devices.allocateTransactionId()
 	if _, err := devices.registerDevice(deviceId, hash, baseInfo.Size, transactionId); err != nil {
 		devicemapper.DeleteDevice(devices.getPoolDevName(), deviceId)
+		devices.markDeviceIdFree(deviceId)
 		log.Debugf("Error registering device: %s", err)
 		return err
 	}
@@ -561,6 +596,7 @@ func (devices *DeviceSet) createRegisterSnapDevice(hash string, baseInfo *DevInf
 	if err := devices.updatePoolTransactionId(); err != nil {
 		devices.unregisterDevice(deviceId, hash)
 		devicemapper.DeleteDevice(devices.getPoolDevName(), deviceId)
+		devices.markDeviceIdFree(deviceId)
 		return err
 	}
 	return nil
@@ -966,6 +1002,8 @@ func (devices *DeviceSet) deleteDevice(info *DevInfo) error {
 		return err
 	}
 
+	devices.markDeviceIdFree(info.DeviceId)
+
 	return nil
 }
 

From c115c4aa45ba82f27859b0afba5724d437857879 Mon Sep 17 00:00:00 2001
From: Vivek Goyal <vgoyal@redhat.com>
Date: Wed, 3 Dec 2014 13:06:43 -0500
Subject: [PATCH 19/20] devmapper: Use transaction mechanism during device or
 snap device creation

Finally this patch uses the notion of transaction for device or snapshot
device creation.

Following is sequence of event.

- Open a trasaction and save details in a file.
- Create a new device/snapshot device
- If a new device id is used, refresh transaction with new device id details.
- Create device metadata file
- Close transaction.

If docker crashes anywhere in between without closing transaction, then
upon next start, docker will figure out that there was a pending transaction
and it will roll back transaction. That is it will do following.

- Delete Device from pool
- Delete device metadata file
- Remove transaction file to mark no transaction is pending.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 daemon/graphdriver/devmapper/deviceset.go | 149 +++++++++++++++++++++-
 1 file changed, 142 insertions(+), 7 deletions(-)

diff --git a/daemon/graphdriver/devmapper/deviceset.go b/daemon/graphdriver/devmapper/deviceset.go
index 04437ce9f..1e388baa6 100644
--- a/daemon/graphdriver/devmapper/deviceset.go
+++ b/daemon/graphdriver/devmapper/deviceset.go
@@ -36,9 +36,12 @@ var (
 )
 
 const deviceSetMetaFile string = "deviceset-metadata"
+const transactionMetaFile string = "transaction-metadata"
 
 type Transaction struct {
-	OpenTransactionId uint64 `json:"-"`
+	OpenTransactionId uint64 `json:"open_transaction_id"`
+	DeviceIdHash      string `json:"device_hash"`
+	DeviceId          int    `json:"device_id"`
 }
 
 type DevInfo struct {
@@ -149,6 +152,10 @@ func (devices *DeviceSet) metadataFile(info *DevInfo) string {
 	return path.Join(devices.metadataDir(), file)
 }
 
+func (devices *DeviceSet) transactionMetaFile() string {
+	return path.Join(devices.metadataDir(), transactionMetaFile)
+}
+
 func (devices *DeviceSet) deviceSetMetaFile() string {
 	return path.Join(devices.metadataDir(), deviceSetMetaFile)
 }
@@ -492,6 +499,10 @@ func (devices *DeviceSet) initMetaData() error {
 	if err := devices.constructDeviceIdMap(); err != nil {
 		return err
 	}
+
+	if err := devices.processPendingTransaction(); err != nil {
+		return err
+	}
 	return nil
 }
 
@@ -519,6 +530,12 @@ func (devices *DeviceSet) createRegisterDevice(hash string) (*DevInfo, error) {
 		return nil, err
 	}
 
+	if err := devices.openTransaction(hash, deviceId); err != nil {
+		log.Debugf("Error opening transaction hash = %s deviceId = %d", hash, deviceId)
+		devices.markDeviceIdFree(deviceId)
+		return nil, err
+	}
+
 	for {
 		if err := devicemapper.CreateDevice(devices.getPoolDevName(), deviceId); err != nil {
 			if devicemapper.DeviceIdExists(err) {
@@ -531,6 +548,8 @@ func (devices *DeviceSet) createRegisterDevice(hash string) (*DevInfo, error) {
 				if err != nil {
 					return nil, err
 				}
+				// Save new device id into transaction
+				devices.refreshTransaction(deviceId)
 				continue
 			}
 			log.Debugf("Error creating device: %s", err)
@@ -540,16 +559,15 @@ func (devices *DeviceSet) createRegisterDevice(hash string) (*DevInfo, error) {
 		break
 	}
 
-	transactionId := devices.allocateTransactionId()
 	log.Debugf("Registering device (id %v) with FS size %v", deviceId, devices.baseFsSize)
-	info, err := devices.registerDevice(deviceId, hash, devices.baseFsSize, transactionId)
+	info, err := devices.registerDevice(deviceId, hash, devices.baseFsSize, devices.OpenTransactionId)
 	if err != nil {
 		_ = devicemapper.DeleteDevice(devices.getPoolDevName(), deviceId)
 		devices.markDeviceIdFree(deviceId)
 		return nil, err
 	}
 
-	if err := devices.updatePoolTransactionId(); err != nil {
+	if err := devices.closeTransaction(); err != nil {
 		devices.unregisterDevice(deviceId, hash)
 		devicemapper.DeleteDevice(devices.getPoolDevName(), deviceId)
 		devices.markDeviceIdFree(deviceId)
@@ -564,6 +582,12 @@ func (devices *DeviceSet) createRegisterSnapDevice(hash string, baseInfo *DevInf
 		return err
 	}
 
+	if err := devices.openTransaction(hash, deviceId); err != nil {
+		log.Debugf("Error opening transaction hash = %s deviceId = %d", hash, deviceId)
+		devices.markDeviceIdFree(deviceId)
+		return err
+	}
+
 	for {
 		if err := devicemapper.CreateSnapDevice(devices.getPoolDevName(), deviceId, baseInfo.Name(), baseInfo.DeviceId); err != nil {
 			if devicemapper.DeviceIdExists(err) {
@@ -576,6 +600,8 @@ func (devices *DeviceSet) createRegisterSnapDevice(hash string, baseInfo *DevInf
 				if err != nil {
 					return err
 				}
+				// Save new device id into transaction
+				devices.refreshTransaction(deviceId)
 				continue
 			}
 			log.Debugf("Error creating snap device: %s", err)
@@ -585,15 +611,14 @@ func (devices *DeviceSet) createRegisterSnapDevice(hash string, baseInfo *DevInf
 		break
 	}
 
-	transactionId := devices.allocateTransactionId()
-	if _, err := devices.registerDevice(deviceId, hash, baseInfo.Size, transactionId); err != nil {
+	if _, err := devices.registerDevice(deviceId, hash, baseInfo.Size, devices.OpenTransactionId); err != nil {
 		devicemapper.DeleteDevice(devices.getPoolDevName(), deviceId)
 		devices.markDeviceIdFree(deviceId)
 		log.Debugf("Error registering device: %s", err)
 		return err
 	}
 
-	if err := devices.updatePoolTransactionId(); err != nil {
+	if err := devices.closeTransaction(); err != nil {
 		devices.unregisterDevice(deviceId, hash)
 		devicemapper.DeleteDevice(devices.getPoolDevName(), deviceId)
 		devices.markDeviceIdFree(deviceId)
@@ -775,6 +800,90 @@ func (devices *DeviceSet) ResizePool(size int64) error {
 	return nil
 }
 
+func (devices *DeviceSet) loadTransactionMetaData() error {
+	jsonData, err := ioutil.ReadFile(devices.transactionMetaFile())
+	if err != nil {
+		// There is no active transaction. This will be the case
+		// during upgrade.
+		if os.IsNotExist(err) {
+			devices.OpenTransactionId = devices.TransactionId
+			return nil
+		}
+		return err
+	}
+
+	json.Unmarshal(jsonData, &devices.Transaction)
+	return nil
+}
+
+func (devices *DeviceSet) saveTransactionMetaData() error {
+	jsonData, err := json.Marshal(&devices.Transaction)
+	if err != nil {
+		return fmt.Errorf("Error encoding metadata to json: %s", err)
+	}
+
+	return devices.writeMetaFile(jsonData, devices.transactionMetaFile())
+}
+
+func (devices *DeviceSet) removeTransactionMetaData() error {
+	if err := os.RemoveAll(devices.transactionMetaFile()); err != nil {
+		return err
+	}
+	return nil
+}
+
+func (devices *DeviceSet) rollbackTransaction() error {
+	log.Debugf("Rolling back open transaction: TransactionId=%d hash=%s device_id=%d", devices.OpenTransactionId, devices.DeviceIdHash, devices.DeviceId)
+
+	// A device id might have already been deleted before transaction
+	// closed. In that case this call will fail. Just leave a message
+	// in case of failure.
+	if err := devicemapper.DeleteDevice(devices.getPoolDevName(), devices.DeviceId); err != nil {
+		log.Errorf("Warning: Unable to delete device: %s", err)
+	}
+
+	dinfo := &DevInfo{Hash: devices.DeviceIdHash}
+	if err := devices.removeMetadata(dinfo); err != nil {
+		log.Errorf("Warning: Unable to remove meta data: %s", err)
+	} else {
+		devices.markDeviceIdFree(devices.DeviceId)
+	}
+
+	if err := devices.removeTransactionMetaData(); err != nil {
+		log.Errorf("Warning: Unable to remove transaction meta file %s: %s", devices.transactionMetaFile(), err)
+	}
+
+	return nil
+}
+
+func (devices *DeviceSet) processPendingTransaction() error {
+	if err := devices.loadTransactionMetaData(); err != nil {
+		return err
+	}
+
+	// If there was open transaction but pool transaction Id is same
+	// as open transaction Id, nothing to roll back.
+	if devices.TransactionId == devices.OpenTransactionId {
+		return nil
+	}
+
+	// If open transaction Id is less than pool transaction Id, something
+	// is wrong. Bail out.
+	if devices.OpenTransactionId < devices.TransactionId {
+		log.Errorf("Warning: Open Transaction id %d is less than pool transaction id %d", devices.OpenTransactionId, devices.TransactionId)
+		return nil
+	}
+
+	// Pool transaction Id is not same as open transaction. There is
+	// a transaction which was not completed.
+	if err := devices.rollbackTransaction(); err != nil {
+		return fmt.Errorf("Rolling back open transaction failed: %s", err)
+	}
+
+	devices.OpenTransactionId = devices.TransactionId
+	return nil
+}
+
 func (devices *DeviceSet) loadDeviceSetMetaData() error {
 	jsonData, err := ioutil.ReadFile(devices.deviceSetMetaFile())
 	if err != nil {
@@ -798,6 +907,32 @@ func (devices *DeviceSet) saveDeviceSetMetaData() error {
 	return devices.writeMetaFile(jsonData, devices.deviceSetMetaFile())
 }
 
+func (devices *DeviceSet) openTransaction(hash string, DeviceId int) error {
+	devices.allocateTransactionId()
+	devices.DeviceIdHash = hash
+	devices.DeviceId = DeviceId
+	if err := devices.saveTransactionMetaData(); err != nil {
+		return fmt.Errorf("Error saving transaction meta data: %s", err)
+	}
+	return nil
+}
+
+func (devices *DeviceSet) refreshTransaction(DeviceId int) error {
+	devices.DeviceId = DeviceId
+	if err := devices.saveTransactionMetaData(); err != nil {
+		return fmt.Errorf("Error saving transaction meta data: %s", err)
+	}
+	return nil
+}
+
+func (devices *DeviceSet) closeTransaction() error {
+	if err := devices.updatePoolTransactionId(); err != nil {
+		log.Debugf("Failed to close Transaction")
+		return err
+	}
+	return nil
+}
+
 func (devices *DeviceSet) initDevmapper(doInit bool) error {
 	// give ourselves to libdm as a log handler
 	devicemapper.LogInit(devices)

From 17b75a21a667a27a9a27565ab282cd615dbdb66e Mon Sep 17 00:00:00 2001
From: Vivek Goyal <vgoyal@redhat.com>
Date: Wed, 3 Dec 2014 13:06:43 -0500
Subject: [PATCH 20/20] devmapper: Use transactions during device deletion

Use transaction logic during device deletion and do rollback if transaction
is not complete. Following is the sequence of events.

- Open transaction and save to metafile
- Delete device from pool
- Delete device metadata file from disk
- Close Transaction

If docker crashes without closing transaction then rollback will take
place upon next docker start.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 daemon/graphdriver/devmapper/deviceset.go | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/daemon/graphdriver/devmapper/deviceset.go b/daemon/graphdriver/devmapper/deviceset.go
index 1e388baa6..71502a483 100644
--- a/daemon/graphdriver/devmapper/deviceset.go
+++ b/daemon/graphdriver/devmapper/deviceset.go
@@ -1128,6 +1128,11 @@ func (devices *DeviceSet) deleteDevice(info *DevInfo) error {
 		}
 	}
 
+	if err := devices.openTransaction(info.Hash, info.DeviceId); err != nil {
+		log.Debugf("Error opening transaction hash = %s deviceId = %d", "", info.DeviceId)
+		return err
+	}
+
 	if err := devicemapper.DeleteDevice(devices.getPoolDevName(), info.DeviceId); err != nil {
 		log.Debugf("Error deleting device: %s", err)
 		return err
@@ -1137,6 +1142,10 @@ func (devices *DeviceSet) deleteDevice(info *DevInfo) error {
 		return err
 	}
 
+	if err := devices.closeTransaction(); err != nil {
+		return err
+	}
+
 	devices.markDeviceIdFree(info.DeviceId)
 
 	return nil