devmapper: Simplify thin pool device id allocation

Instead of globally keeping track of the free device ids we just
start from 0 each run and handle EEXIST error and try the next one.

This way we don't need any global state for the device ids, which
means we can read device metadata lazily. This is important for
multi-process use of the backend.

Docker-DCO-1.1-Signed-off-by: Alexander Larsson <alexl@redhat.com> (github: alexlarsson)

daemon/graphdriver/devmapper/deviceset.go

@@ -60,7 +60,7 @@ type DeviceSet struct {
 	devicePrefix     string
 	TransactionId    uint64
 	NewTransactionId uint64
-	nextFreeDevice   int
+	nextDeviceId     int
 }
 
 type DiskUsage struct {
@@ -156,13 +156,6 @@ func (devices *DeviceSet) ensureImage(name string, size int64) (string, error) {
 	return filename, nil
 }
 
-func (devices *DeviceSet) allocateDeviceId() int {
-	// TODO: Add smarter reuse of deleted devices
-	id := devices.nextFreeDevice
-	devices.nextFreeDevice = devices.nextFreeDevice + 1
-	return id
-}
-
 func (devices *DeviceSet) allocateTransactionId() uint64 {
 	devices.NewTransactionId = devices.NewTransactionId + 1
 	return devices.NewTransactionId
@@ -299,10 +292,6 @@ func (devices *DeviceSet) loadMetaData() error {
 		d.Hash = hash
 		d.devices = devices
 
-		if d.DeviceId >= devices.nextFreeDevice {
-			devices.nextFreeDevice = d.DeviceId + 1
-		}
-
 		// If the transaction id is larger than the actual one we lost the device due to some crash
 		if d.TransactionId > devices.TransactionId {
 			utils.Debugf("Removing lost device %s with id %d", hash, d.TransactionId)
@@ -328,14 +317,17 @@ func (devices *DeviceSet) setupBaseImage() error {
 
 	utils.Debugf("Initializing base device-manager snapshot")
 
-	id := devices.allocateDeviceId()
+	id := devices.nextDeviceId
 
 	// Create initial device
-	if err := createDevice(devices.getPoolDevName(), id); err != nil {
+	if err := createDevice(devices.getPoolDevName(), &id); err != nil {
 		utils.Debugf("\n--->Err: %s\n", err)
 		return err
 	}
 
+	// Ids are 24bit, so wrap around
+	devices.nextDeviceId = (id + 1) & 0xffffff
+
 	utils.Debugf("Registering base device (id %v) with FS size %v", id, DefaultBaseFsSize)
 	info, err := devices.registerDevice(id, "", DefaultBaseFsSize)
 	if err != nil {
@@ -580,13 +572,16 @@ func (devices *DeviceSet) AddDevice(hash, baseHash string) error {
 		return fmt.Errorf("device %s already exists", hash)
 	}
 
-	deviceId := devices.allocateDeviceId()
+	deviceId := devices.nextDeviceId
 
-	if err := createSnapDevice(devices.getPoolDevName(), deviceId, baseInfo.Name(), baseInfo.DeviceId); err != nil {
+	if err := createSnapDevice(devices.getPoolDevName(), &deviceId, baseInfo.Name(), baseInfo.DeviceId); err != nil {
 		utils.Debugf("Error creating snap device: %s\n", err)
 		return err
 	}
 
+	// Ids are 24bit, so wrap around
+	devices.nextDeviceId = (deviceId + 1) & 0xffffff
+
 	if _, err := devices.registerDevice(deviceId, hash, baseInfo.Size); err != nil {
 		deleteDevice(devices.getPoolDevName(), deviceId)
 		utils.Debugf("Error registering device: %s\n", err)

daemon/graphdriver/devmapper/devmapper.go

@@ -64,7 +64,8 @@ var (
 	ErrLoopbackSetCapacity    = errors.New("Unable set loopback capacity")
 	ErrBusy                   = errors.New("Device is Busy")
 
-	dmSawBusy bool
+	dmSawBusy  bool
+	dmSawExist bool
 )
 
 type (
@@ -467,23 +468,33 @@ func resumeDevice(name string) error {
 	return nil
 }
 
-func createDevice(poolName string, deviceId int) error {
+func createDevice(poolName string, deviceId *int) error {
-	utils.Debugf("[devmapper] createDevice(poolName=%v, deviceId=%v)", poolName, deviceId)
+	utils.Debugf("[devmapper] createDevice(poolName=%v, deviceId=%v)", poolName, *deviceId)
-	task, err := createTask(DeviceTargetMsg, poolName)
-	if task == nil {
-		return err
-	}
 
-	if err := task.SetSector(0); err != nil {
+	for {
-		return fmt.Errorf("Can't set sector")
+		task, err := createTask(DeviceTargetMsg, poolName)
-	}
+		if task == nil {
+			return err
+		}
 
-	if err := task.SetMessage(fmt.Sprintf("create_thin %d", deviceId)); err != nil {
+		if err := task.SetSector(0); err != nil {
-		return fmt.Errorf("Can't set message")
+			return fmt.Errorf("Can't set sector")
-	}
+		}
 
-	if err := task.Run(); err != nil {
+		if err := task.SetMessage(fmt.Sprintf("create_thin %d", *deviceId)); err != nil {
-		return fmt.Errorf("Error running createDevice")
+			return fmt.Errorf("Can't set message")
+		}
+
+		dmSawExist = false
+		if err := task.Run(); err != nil {
+			if dmSawExist {
+				// Already exists, try next id
+				*deviceId++
+				continue
+			}
+			return fmt.Errorf("Error running createDevice")
+		}
+		break
 	}
 	return nil
 }
@@ -553,7 +564,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
 
@@ -563,33 +574,44 @@ func createSnapDevice(poolName string, deviceId int, baseName string, baseDevice
 		}
 	}
 
-	task, err := createTask(DeviceTargetMsg, poolName)
+	for {
-	if task == nil {
+		task, err := createTask(DeviceTargetMsg, poolName)
-		if doSuspend {
+		if task == nil {
-			resumeDevice(baseName)
+			if doSuspend {
+				resumeDevice(baseName)
+			}
+			return err
 		}
-		return err
-	}
 
-	if err := task.SetSector(0); err != nil {
+		if err := task.SetSector(0); err != nil {
-		if doSuspend {
+			if doSuspend {
-			resumeDevice(baseName)
+				resumeDevice(baseName)
+			}
+			return fmt.Errorf("Can't set sector")
 		}
-		return fmt.Errorf("Can't set sector")
-	}
 
-	if err := task.SetMessage(fmt.Sprintf("create_snap %d %d", deviceId, baseDeviceId)); err != nil {
+		if err := task.SetMessage(fmt.Sprintf("create_snap %d %d", *deviceId, baseDeviceId)); err != nil {
-		if doSuspend {
+			if doSuspend {
-			resumeDevice(baseName)
+				resumeDevice(baseName)
+			}
+			return fmt.Errorf("Can't set message")
 		}
-		return fmt.Errorf("Can't set message")
-	}
 
-	if err := task.Run(); err != nil {
+		dmSawExist = false
-		if doSuspend {
+		if err := task.Run(); err != nil {
-			resumeDevice(baseName)
+			if dmSawExist {
+				// Already exists, try next id
+				*deviceId++
+				continue
+			}
+
+			if doSuspend {
+				resumeDevice(baseName)
+			}
+			return fmt.Errorf("Error running DeviceCreate (createSnapDevice)")
 		}
-		return fmt.Errorf("Error running DeviceCreate (createSnapDevice)")
+
+		break
 	}
 
 	if doSuspend {

daemon/graphdriver/devmapper/devmapper_log.go

@@ -18,6 +18,10 @@ func DevmapperLogCallback(level C.int, file *C.char, line C.int, dm_errno_or_cla
 		if strings.Contains(msg, "busy") {
 			dmSawBusy = true
 		}
+
+		if strings.Contains(msg, "File exists") {
+			dmSawExist = true
+		}
 	}
 
 	if dmLogger != nil {