From f26203cf544db07ae64dffaa495e4217976c0786 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Thu, 24 Apr 2014 22:36:45 +0200 Subject: [PATCH] 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 (github: alexlarsson) --- daemon/graphdriver/devmapper/deviceset.go | 27 +++--- daemon/graphdriver/devmapper/devmapper.go | 94 ++++++++++++------- daemon/graphdriver/devmapper/devmapper_log.go | 4 + 3 files changed, 73 insertions(+), 52 deletions(-) diff --git a/daemon/graphdriver/devmapper/deviceset.go b/daemon/graphdriver/devmapper/deviceset.go index 01d1a5d312..d3a097961c 100644 --- a/daemon/graphdriver/devmapper/deviceset.go +++ b/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) diff --git a/daemon/graphdriver/devmapper/devmapper.go b/daemon/graphdriver/devmapper/devmapper.go index f064f9d615..86af653b4d 100644 --- a/daemon/graphdriver/devmapper/devmapper.go +++ b/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 { - utils.Debugf("[devmapper] createDevice(poolName=%v, deviceId=%v)", poolName, deviceId) - task, err := createTask(DeviceTargetMsg, poolName) - if task == nil { - return err - } +func createDevice(poolName string, deviceId *int) error { + utils.Debugf("[devmapper] createDevice(poolName=%v, deviceId=%v)", poolName, *deviceId) - if err := task.SetSector(0); err != nil { - return fmt.Errorf("Can't set sector") - } + for { + task, err := createTask(DeviceTargetMsg, poolName) + if task == nil { + return err + } - if err := task.SetMessage(fmt.Sprintf("create_thin %d", deviceId)); err != nil { - return fmt.Errorf("Can't set message") - } + if err := task.SetSector(0); err != nil { + return fmt.Errorf("Can't set sector") + } - if err := task.Run(); err != nil { - return fmt.Errorf("Error running createDevice") + if err := task.SetMessage(fmt.Sprintf("create_thin %d", *deviceId)); err != nil { + 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) - if task == nil { - if doSuspend { - resumeDevice(baseName) + for { + task, err := createTask(DeviceTargetMsg, poolName) + if task == nil { + if doSuspend { + resumeDevice(baseName) + } + return err } - return err - } - if err := task.SetSector(0); err != nil { - if doSuspend { - resumeDevice(baseName) + if err := task.SetSector(0); err != nil { + if doSuspend { + 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 doSuspend { - resumeDevice(baseName) + 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") } - return fmt.Errorf("Can't set message") - } - if err := task.Run(); err != nil { - if doSuspend { - resumeDevice(baseName) + dmSawExist = false + if err := task.Run(); err != nil { + 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 { diff --git a/daemon/graphdriver/devmapper/devmapper_log.go b/daemon/graphdriver/devmapper/devmapper_log.go index 7f78bbf0d4..cdeaed2525 100644 --- a/daemon/graphdriver/devmapper/devmapper_log.go +++ b/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 {