From 7b2b15d3e9f9b7ad898a36bbe5ceb42c9ca58d47 Mon Sep 17 00:00:00 2001 From: Vivek Goyal Date: Wed, 3 Dec 2014 13:06:43 -0500 Subject: [PATCH] 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 --- 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 b4ce1b3b58..db9d2528f7 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 a7306ba55d..c23a3624db 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 {