From d929589c1fc4538dcd1b2a7a3dc7d4afbdfa72fd Mon Sep 17 00:00:00 2001 From: Vivek Goyal Date: Tue, 6 Oct 2015 17:37:21 -0400 Subject: [PATCH] devmapper: Implement deferred deletion functionality Finally here is the patch to implement deferred deletion functionality. Deferred deleted devices are marked as "Deleted" in device meta file. First we try to delete the device and only if deletion fails and user has enabled deferred deletion, device is marked for deferred deletion. When docker starts up again, we go through list of deleted devices and try to delete these again. Signed-off-by: Vivek Goyal --- daemon/graphdriver/devmapper/deviceset.go | 135 ++++++++++++++++++---- daemon/graphdriver/devmapper/driver.go | 2 +- pkg/devicemapper/devmapper.go | 4 + 3 files changed, 118 insertions(+), 23 deletions(-) diff --git a/daemon/graphdriver/devmapper/deviceset.go b/daemon/graphdriver/devmapper/deviceset.go index 8279d8200e..7b96dd5f57 100644 --- a/daemon/graphdriver/devmapper/deviceset.go +++ b/daemon/graphdriver/devmapper/deviceset.go @@ -58,6 +58,7 @@ type devInfo struct { Size uint64 `json:"size"` TransactionID uint64 `json:"transaction_id"` Initialized bool `json:"initialized"` + Deleted bool `json:"deleted"` devices *DeviceSet mountCount int @@ -425,6 +426,8 @@ func (devices *DeviceSet) deviceFileWalkFunction(path string, finfo os.FileInfo) hash = "" } + // Include deleted devices also as cleanup delete device logic + // will go through it and see if there are any deleted devices. if _, err := devices.lookupDevice(hash); err != nil { return fmt.Errorf("Error looking up device %s:%v", hash, err) } @@ -494,9 +497,13 @@ func (devices *DeviceSet) registerDevice(id int, hash string, size uint64, trans return info, nil } -func (devices *DeviceSet) activateDeviceIfNeeded(info *devInfo) error { +func (devices *DeviceSet) activateDeviceIfNeeded(info *devInfo, ignoreDeleted bool) error { logrus.Debugf("activateDeviceIfNeeded(%v)", info.Hash) + if info.Deleted && !ignoreDeleted { + return fmt.Errorf("devmapper: Can't activate device %v as it is marked for deletion", info.Hash) + } + // Make sure deferred removal on device is canceled, if one was // scheduled. if err := devices.cancelDeferredRemoval(info); err != nil { @@ -570,6 +577,35 @@ func (devices *DeviceSet) migrateOldMetaData() error { return nil } +// Cleanup deleted devices. It assumes that all the devices have been +// loaded in the hash table. Should be called with devices.Lock() held. +// Will drop the lock for device deletion and return with lock acquired. +func (devices *DeviceSet) cleanupDeletedDevices() error { + var deletedDevices []*devInfo + + for _, info := range devices.Devices { + if !info.Deleted { + continue + } + logrus.Debugf("devmapper: Found deleted device %s.", info.Hash) + deletedDevices = append(deletedDevices, info) + } + + // Delete the deleted devices. DeleteDevice() first takes the info lock + // and then devices.Lock(). So drop it to avoid deadlock. + devices.Unlock() + defer devices.Lock() + + for _, info := range deletedDevices { + // This will again try deferred deletion. + if err := devices.DeleteDevice(info.Hash, false); err != nil { + logrus.Warnf("devmapper: Deletion of device %s, device_id=%v failed:%v", info.Hash, info.DeviceID, err) + } + } + + return nil +} + func (devices *DeviceSet) initMetaData() error { devices.Lock() defer devices.Unlock() @@ -594,6 +630,11 @@ func (devices *DeviceSet) initMetaData() error { if err := devices.processPendingTransaction(); err != nil { return err } + + if err := devices.cleanupDeletedDevices(); err != nil { + return err + } + return nil } @@ -758,7 +799,7 @@ func (devices *DeviceSet) verifyBaseDeviceUUID(baseInfo *devInfo) error { devices.Lock() defer devices.Unlock() - if err := devices.activateDeviceIfNeeded(baseInfo); err != nil { + if err := devices.activateDeviceIfNeeded(baseInfo, false); err != nil { return err } @@ -780,7 +821,7 @@ func (devices *DeviceSet) saveBaseDeviceUUID(baseInfo *devInfo) error { devices.Lock() defer devices.Unlock() - if err := devices.activateDeviceIfNeeded(baseInfo); err != nil { + if err := devices.activateDeviceIfNeeded(baseInfo, false); err != nil { return err } @@ -807,7 +848,7 @@ func (devices *DeviceSet) createBaseImage() error { logrus.Debugf("Creating filesystem on base device-mapper thin volume") - if err := devices.activateDeviceIfNeeded(info); err != nil { + if err := devices.activateDeviceIfNeeded(info, false); err != nil { return err } @@ -870,7 +911,7 @@ func (devices *DeviceSet) setupBaseImage() error { // fresh. if oldInfo != nil { - if oldInfo.Initialized { + if oldInfo.Initialized && !oldInfo.Deleted { if err := devices.setupVerifyBaseImageUUID(oldInfo); err != nil { return err } @@ -879,7 +920,10 @@ func (devices *DeviceSet) setupBaseImage() error { } logrus.Debugf("Removing uninitialized base image") - if err := devices.DeleteDevice(""); err != nil { + // If previous base device is in deferred delete state, + // that needs to be cleaned up first. So don't try + // deferred deletion. + if err := devices.DeleteDevice("", true); err != nil { return err } } @@ -1513,19 +1557,26 @@ func (devices *DeviceSet) AddDevice(hash, baseHash string) error { logrus.Debugf("[deviceset] AddDevice(hash=%s basehash=%s)", hash, baseHash) defer logrus.Debugf("[deviceset] AddDevice(hash=%s basehash=%s) END", hash, baseHash) + // If a deleted device exists, return error. baseInfo, err := devices.lookupDeviceWithLock(baseHash) if err != nil { return err } + if baseInfo.Deleted { + return fmt.Errorf("devmapper: Base device %v has been marked for deferred deletion", baseInfo.Hash) + } + baseInfo.lock.Lock() defer baseInfo.lock.Unlock() devices.Lock() defer devices.Unlock() + // Also include deleted devices in case hash of new device is + // same as one of the deleted devices. if info, _ := devices.lookupDevice(hash); info != nil { - return fmt.Errorf("device %s already exists", hash) + return fmt.Errorf("device %s already exists. Deleted=%v", hash, info.Deleted) } if err := devices.createRegisterSnapDevice(hash, baseInfo); err != nil { @@ -1535,8 +1586,26 @@ func (devices *DeviceSet) AddDevice(hash, baseHash string) error { return nil } +func (devices *DeviceSet) markForDeferredDeletion(info *devInfo) error { + // If device is already in deleted state, there is nothing to be done. + if info.Deleted { + return nil + } + + logrus.Debugf("devmapper: Marking device %s for deferred deletion.", info.Hash) + + info.Deleted = true + + // save device metadata to refelect deleted state. + if err := devices.saveMetadata(info); err != nil { + info.Deleted = false + return err + } + return nil +} + // Should be caled with devices.Lock() held. -func (devices *DeviceSet) deleteTransaction(info *devInfo) error { +func (devices *DeviceSet) deleteTransaction(info *devInfo, syncDelete bool) error { if err := devices.openTransaction(info.Hash, info.DeviceID); err != nil { logrus.Debugf("Error opening transaction hash = %s deviceId = %d", "", info.DeviceID) return err @@ -1544,13 +1613,25 @@ func (devices *DeviceSet) deleteTransaction(info *devInfo) error { defer devices.closeTransaction() - if err := devicemapper.DeleteDevice(devices.getPoolDevName(), info.DeviceID); err != nil { - logrus.Debugf("Error deleting device: %s", err) - return err + err := devicemapper.DeleteDevice(devices.getPoolDevName(), info.DeviceID) + if err != nil { + // If syncDelete is true, we want to return error. If deferred + // deletion is not enabled, we return an error. If error is + // something other then EBUSY, return an error. + if syncDelete || !devices.deferredDelete || err != devicemapper.ErrBusy { + logrus.Debugf("Error deleting device: %s", err) + return err + } } - if err := devices.unregisterDevice(info.DeviceID, info.Hash); err != nil { - return err + if err == nil { + if err := devices.unregisterDevice(info.DeviceID, info.Hash); err != nil { + return err + } + } else { + if err := devices.markForDeferredDeletion(info); err != nil { + return err + } } return nil @@ -1562,8 +1643,10 @@ func (devices *DeviceSet) issueDiscard(info *devInfo) error { defer logrus.Debugf("devmapper: issueDiscard(device: %s). END", info.Hash) // This is a workaround for the kernel not discarding block so // on the thin pool when we remove a thinp device, so we do it - // manually - if err := devices.activateDeviceIfNeeded(info); err != nil { + // manually. + // Even if device is deferred deleted, activate it and isue + // discards. + if err := devices.activateDeviceIfNeeded(info, true); err != nil { return err } @@ -1584,7 +1667,7 @@ func (devices *DeviceSet) issueDiscard(info *devInfo) error { } // Should be called with devices.Lock() held. -func (devices *DeviceSet) deleteDevice(info *devInfo) error { +func (devices *DeviceSet) deleteDevice(info *devInfo, syncDelete bool) error { if devices.doBlkDiscard { devices.issueDiscard(info) } @@ -1595,7 +1678,7 @@ func (devices *DeviceSet) deleteDevice(info *devInfo) error { return err } - if err := devices.deleteTransaction(info); err != nil { + if err := devices.deleteTransaction(info, syncDelete); err != nil { return err } @@ -1604,8 +1687,12 @@ func (devices *DeviceSet) deleteDevice(info *devInfo) error { return nil } -// DeleteDevice deletes a device from the hash. -func (devices *DeviceSet) DeleteDevice(hash string) error { +// DeleteDevice will return success if device has been marked for deferred +// removal. If one wants to override that and want DeleteDevice() to fail if +// device was busy and could not be deleted, set syncDelete=true. +func (devices *DeviceSet) DeleteDevice(hash string, syncDelete bool) error { + logrus.Debugf("devmapper: DeleteDevice(hash=%v syncDelete=%v) START", hash, syncDelete) + defer logrus.Debugf("devmapper: DeleteDevice(hash=%v syncDelete=%v) END", hash, syncDelete) info, err := devices.lookupDeviceWithLock(hash) if err != nil { return err @@ -1624,7 +1711,7 @@ func (devices *DeviceSet) DeleteDevice(hash string) error { return fmt.Errorf("devmapper: Can't delete device %v as it is still mounted. mntCount=%v", info.Hash, info.mountCount) } - return devices.deleteDevice(info) + return devices.deleteDevice(info, syncDelete) } func (devices *DeviceSet) deactivatePool() error { @@ -1811,6 +1898,10 @@ func (devices *DeviceSet) MountDevice(hash, path, mountLabel string) error { return err } + if info.Deleted { + return fmt.Errorf("devmapper: Can't mount device %v as it has been marked for deferred deletion", info.Hash) + } + info.lock.Lock() defer info.lock.Unlock() @@ -1826,7 +1917,7 @@ func (devices *DeviceSet) MountDevice(hash, path, mountLabel string) error { return nil } - if err := devices.activateDeviceIfNeeded(info); err != nil { + if err := devices.activateDeviceIfNeeded(info, false); err != nil { return fmt.Errorf("Error activating devmapper device for '%s': %s", hash, err) } @@ -1946,7 +2037,7 @@ func (devices *DeviceSet) GetDeviceStatus(hash string) (*DevStatus, error) { TransactionID: info.TransactionID, } - if err := devices.activateDeviceIfNeeded(info); err != nil { + if err := devices.activateDeviceIfNeeded(info, false); err != nil { return nil, fmt.Errorf("Error activating devmapper device for '%s': %s", hash, err) } diff --git a/daemon/graphdriver/devmapper/driver.go b/daemon/graphdriver/devmapper/driver.go index f117cfb7c5..d59219f1c8 100644 --- a/daemon/graphdriver/devmapper/driver.go +++ b/daemon/graphdriver/devmapper/driver.go @@ -143,7 +143,7 @@ func (d *Driver) Remove(id string) error { } // This assumes the device has been properly Get/Put:ed and thus is unmounted - if err := d.DeviceSet.DeleteDevice(id); err != nil { + if err := d.DeviceSet.DeleteDevice(id, false); err != nil { return err } diff --git a/pkg/devicemapper/devmapper.go b/pkg/devicemapper/devmapper.go index abbd28b5ff..136dd6bbd0 100644 --- a/pkg/devicemapper/devmapper.go +++ b/pkg/devicemapper/devmapper.go @@ -750,7 +750,11 @@ func DeleteDevice(poolName string, deviceID int) error { return fmt.Errorf("Can't set message %s", err) } + dmSawBusy = false if err := task.run(); err != nil { + if dmSawBusy { + return ErrBusy + } return fmt.Errorf("Error running DeleteDevice %s", err) } return nil