Skip to content

Commit

Permalink
Fixes Issue # 23418: Race condition between device deferred removal a…
Browse files Browse the repository at this point in the history 
…nd resume device.

Problem Description:

An example scenario that involves deferred removal
1. A new base image gets created (e.g. 'docker load -i'). The base device is activated and
mounted at some point in time during image creation.
2. While image creation is in progress, a privileged container is started
from another image and the host's mount name space is shared with this
container ('docker run --privileged -v /:/host').
3. Image creation completes and the base device gets unmounted. However,
as the privileged container still holds a reference on the base image
mount point, the base device cannot be removed right away. So it gets
flagged for deferred removal.
4. Next, the privileged container terminates and thus its reference to the
base image mount point gets released. The base device (which is flagged
for deferred removal) may now be cleaned up by the device-mapper. This
opens up an opportunity for a race between a 'kworker' thread (executing
the do_deferred_remove() function) and the Docker daemon (executing the
CreateSnapDevice() function).

This PR cancel the deferred removal, if the device is marked for it. And reschedule the
deferred removal later after the device is resumed successfully.

cherry-picked from docker upstream:
	moby#23497

Signed-off-by: Shishir Mahajan <shishir.mahajan@redhat.com>
Signed-off-by: Deng Guangxing <dengguangxing@huawei.com>
  • Loading branch information
Shishir Mahajan authored and Deng Guangxing committed Feb 23, 2017
1 parent fba80bc commit a7f01b4
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 48 deletions.
105 changes: 81 additions & 24 deletions daemon/graphdriver/devmapper/deviceset.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,7 @@ func (devices *DeviceSet) activateDeviceIfNeeded(info *devInfo, ignoreDeleted bo

// Make sure deferred removal on device is canceled, if one was
// scheduled.
if err := devices.cancelDeferredRemoval(info); err != nil {
if err := devices.cancelDeferredRemovalIfNeeded(info); err != nil {
return fmt.Errorf("devmapper: Device Deferred Removal Cancellation Failed: %s", err)
}

Expand Down Expand Up @@ -843,11 +843,56 @@ func (devices *DeviceSet) createRegisterDevice(hash string) (*devInfo, error) {
return info, nil
}

func (devices *DeviceSet) createRegisterSnapDevice(hash string, baseInfo *devInfo, size uint64) error {
if err := devices.poolHasFreeSpace(); err != nil {
func (devices *DeviceSet) takeSnapshot(hash string, baseInfo *devInfo, size uint64) error {
var (
devinfo *devicemapper.Info
err error
)

if err = devices.poolHasFreeSpace(); err != nil {
return err
}

if devices.deferredRemove {
devinfo, err = devicemapper.GetInfoWithDeferred(baseInfo.Name())
if err != nil {
return err
}
if devinfo != nil && devinfo.DeferredRemove != 0 {
err = devices.cancelDeferredRemoval(baseInfo)
if err != nil {
// If Error is ErrEnxio. Device is probably already gone. Continue.
if err != devicemapper.ErrEnxio {
return err
}
} else {
defer devices.deactivateDevice(baseInfo)
}
}
} else {
devinfo, err = devicemapper.GetInfo(baseInfo.Name())
if err != nil {
return err
}
}

doSuspend := devinfo != nil && devinfo.Exists != 0

if doSuspend {
if err = devicemapper.SuspendDevice(baseInfo.Name()); err != nil {
return err
}
defer devicemapper.ResumeDevice(baseInfo.Name())
}

if err = devices.createRegisterSnapDevice(hash, baseInfo, size); err != nil {
return err
}

return nil
}

func (devices *DeviceSet) createRegisterSnapDevice(hash string, baseInfo *devInfo, size uint64) error {
deviceID, err := devices.getNextFreeDeviceID()
if err != nil {
return err
Expand All @@ -860,7 +905,7 @@ func (devices *DeviceSet) createRegisterSnapDevice(hash string, baseInfo *devInf
}

for {
if err := devicemapper.CreateSnapDevice(devices.getPoolDevName(), deviceID, baseInfo.Name(), baseInfo.DeviceID); err != nil {
if err := devicemapper.CreateSnapDeviceRaw(devices.getPoolDevName(), deviceID, baseInfo.DeviceID); err != nil {
if devicemapper.DeviceIDExists(err) {
// Device ID already exists. This should not
// happen. Now we have a mechanism to find
Expand Down Expand Up @@ -1879,7 +1924,7 @@ func (devices *DeviceSet) AddDevice(hash, baseHash string, storageOpt map[string
return fmt.Errorf("devmapper: Container size cannot be smaller than %s", units.HumanSize(float64(baseInfo.Size)))
}

if err := devices.createRegisterSnapDevice(hash, baseInfo, devinfo.Size); err != nil {
if err := devices.takeSnapshot(hash, baseInfo, devinfo.Size); err != nil {
return err
}

Expand Down Expand Up @@ -2123,41 +2168,53 @@ func (devices *DeviceSet) removeDevice(devname string) error {
return err
}

func (devices *DeviceSet) cancelDeferredRemoval(info *devInfo) error {
func (devices *DeviceSet) cancelDeferredRemovalIfNeeded(info *devInfo) error {
if !devices.deferredRemove {
return nil
}

logrus.Debugf("devmapper: cancelDeferredRemoval START(%s)", info.Name())
defer logrus.Debugf("devmapper: cancelDeferredRemoval END(%s)", info.Name())
logrus.Debugf("devmapper: cancelDeferredRemovalIfNeeded START(%s)", info.Name())
defer logrus.Debugf("devmapper: cancelDeferredRemovalIfNeeded END(%s)", info.Name())

devinfo, err := devicemapper.GetInfoWithDeferred(info.Name())
if err != nil {
return err
}

if devinfo != nil && devinfo.DeferredRemove == 0 {
return nil
}

// Cancel deferred remove
for i := 0; i < 100; i++ {
err = devicemapper.CancelDeferredRemove(info.Name())
if err == nil {
break
if err := devices.cancelDeferredRemoval(info); err != nil {
// If Error is ErrEnxio. Device is probably already gone. Continue.
if err != devicemapper.ErrEnxio {
return err
}
}
return nil
}

if err == devicemapper.ErrEnxio {
// Device is probably already gone. Return success.
return nil
}
func (devices *DeviceSet) cancelDeferredRemoval(info *devInfo) error {
logrus.Debugf("devmapper: cancelDeferredRemoval START(%s)", info.Name())
defer logrus.Debugf("devmapper: cancelDeferredRemoval END(%s)", info.Name())

if err != devicemapper.ErrBusy {
return err
}
var err error

// If we see EBUSY it may be a transient error,
// sleep a bit a retry a few times.
devices.Unlock()
time.Sleep(100 * time.Millisecond)
devices.Lock()
// Cancel deferred remove
for i := 0; i < 100; i++ {
err = devicemapper.CancelDeferredRemove(info.Name())
if err != nil {
if err == devicemapper.ErrBusy {
// If we see EBUSY it may be a transient error,
// sleep a bit a retry a few times.
devices.Unlock()
time.Sleep(100 * time.Millisecond)
devices.Lock()
continue
}
}
break
}
return err
}
Expand Down
48 changes: 24 additions & 24 deletions pkg/devicemapper/devmapper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -779,51 +779,51 @@ func activateDevice(poolName string, name string, deviceID int, size uint64, ext
return UdevWait(&cookie)
}

// CreateSnapDevice creates a snapshot based on the device identified by the baseName and baseDeviceId,
func CreateSnapDevice(poolName string, deviceID int, baseName string, baseDeviceID int) error {
devinfo, _ := GetInfo(baseName)
doSuspend := devinfo != nil && devinfo.Exists != 0

if doSuspend {
if err := SuspendDevice(baseName); err != nil {
return err
}
}

// CreateSnapDeviceRaw creates a snapshot device. Caller needs to suspend and resume the origin device if it is active.
func CreateSnapDeviceRaw(poolName string, deviceID int, baseDeviceID int) error {
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("devicemapper: 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("devicemapper: Can't set message %s", err)
}

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
}

return fmt.Errorf("devicemapper: Error running deviceCreate (createSnapDevice) %s", err)
}

return nil
}

// CreateSnapDevice creates a snapshot based on the device identified by the baseName and baseDeviceId,
func CreateSnapDevice(poolName string, deviceID int, baseName string, baseDeviceID int) error {
devinfo, _ := GetInfo(baseName)
doSuspend := devinfo != nil && devinfo.Exists != 0

if doSuspend {
if err := SuspendDevice(baseName); err != nil {
return err
}
}

if err := CreateSnapDeviceRaw(poolName, deviceID, baseDeviceID); err != nil {
if doSuspend {
if err2 := ResumeDevice(baseName); err2 != nil {
return fmt.Errorf("CreateSnapDeviceRaw Error: (%v): ResumeDevice Error: (%v)", err, err2)
}
}
return err
}

if doSuspend {
Expand Down

0 comments on commit a7f01b4

Please sign in to comment.