From 3c327a4ced7838e1f47e0d3c9e5e6213cf6d2b07 Mon Sep 17 00:00:00 2001 From: Vivek Goyal Date: Tue, 6 Oct 2015 17:37:21 -0400 Subject: [PATCH 1/5] devmapper: construct used device ID map from device Hash map Currently during startup we walk through all the device files and read their device ID and mark in a bitmap that device id is used. We are anyway going through all device files. So we can as well load all that data into device hash map. This will save us little time when container is actually launched later. Also this will help with later patches where cleanup deferred device wants to go through all the devices and see which have been marked for deletion and delete these. So re-organize the code a bit. Signed-off-by: Vivek Goyal Upstream-commit: 6b8b4feaa165af630dd33423f0538a0b987703ae Component: engine --- .../daemon/graphdriver/devmapper/deviceset.go | 31 +++++++++++++------ 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/components/engine/daemon/graphdriver/devmapper/deviceset.go b/components/engine/daemon/graphdriver/devmapper/deviceset.go index 1484648b95..0a9aadbec6 100644 --- a/components/engine/daemon/graphdriver/devmapper/deviceset.go +++ b/components/engine/daemon/graphdriver/devmapper/deviceset.go @@ -375,6 +375,18 @@ func (devices *DeviceSet) lookupDeviceWithLock(hash string) (*devInfo, error) { return info, err } +// This function relies on that device hash map has been loaded in advance. +// Should be called with devices.Lock() held. +func (devices *DeviceSet) constructDeviceIDMap() { + logrus.Debugf("[deviceset] constructDeviceIDMap()") + defer logrus.Debugf("[deviceset] constructDeviceIDMap() END") + + for _, info := range devices.Devices { + devices.markDeviceIDUsed(info.DeviceID) + logrus.Debugf("Added deviceId=%d to DeviceIdMap", info.DeviceID) + } +} + func (devices *DeviceSet) deviceFileWalkFunction(path string, finfo os.FileInfo) error { // Skip some of the meta files which are not device files. @@ -405,19 +417,16 @@ func (devices *DeviceSet) deviceFileWalkFunction(path string, finfo os.FileInfo) hash = "" } - dinfo := devices.loadMetadata(hash) - if dinfo == nil { - return fmt.Errorf("Error loading device metadata file %s", hash) + if _, err := devices.lookupDevice(hash); err != nil { + return fmt.Errorf("Error looking up device %s:%v", hash, err) } - devices.markDeviceIDUsed(dinfo.DeviceID) - logrus.Debugf("Added deviceID=%d to DeviceIDMap", dinfo.DeviceID) return nil } -func (devices *DeviceSet) constructDeviceIDMap() error { - logrus.Debugf("[deviceset] constructDeviceIDMap()") - defer logrus.Debugf("[deviceset] constructDeviceIDMap() END") +func (devices *DeviceSet) loadDeviceFilesOnStart() error { + logrus.Debugf("[deviceset] loadDeviceFilesOnStart()") + defer logrus.Debugf("[deviceset] loadDeviceFilesOnStart() END") var scan = func(path string, info os.FileInfo, err error) error { if err != nil { @@ -568,10 +577,12 @@ func (devices *DeviceSet) initMetaData() error { devices.TransactionID = transactionID - if err := devices.constructDeviceIDMap(); err != nil { - return err + if err := devices.loadDeviceFilesOnStart(); err != nil { + return fmt.Errorf("devmapper: Failed to load device files:%v", err) } + devices.constructDeviceIDMap() + if err := devices.processPendingTransaction(); err != nil { return err } From cd659bcc9c7c12377c11533f56516c2d1ec6eb94 Mon Sep 17 00:00:00 2001 From: Vivek Goyal Date: Tue, 6 Oct 2015 17:37:21 -0400 Subject: [PATCH 2/5] devmapper: Provide option to enabled deferred device deletion Provide a command line option dm.use_deferred_deletion to enable deferred device deletion feature. By default feature will be turned off. Not sure if there is much value in deferred deletion being turned on without deferred removal being turned on. So for now, this feature can be enabled only if deferred removal is on. Signed-off-by: Vivek Goyal Upstream-commit: 51e059e7e90f37848596a0b6ec83f7835e6e4131 Component: engine --- .../daemon/graphdriver/devmapper/deviceset.go | 35 ++++++++++++++-- .../daemon/graphdriver/devmapper/driver.go | 1 + .../docs/reference/commandline/daemon.md | 40 +++++++++++++++++++ components/engine/man/docker-daemon.8.md | 22 ++++++++++ 4 files changed, 95 insertions(+), 3 deletions(-) diff --git a/components/engine/daemon/graphdriver/devmapper/deviceset.go b/components/engine/daemon/graphdriver/devmapper/deviceset.go index 0a9aadbec6..8279d8200e 100644 --- a/components/engine/daemon/graphdriver/devmapper/deviceset.go +++ b/components/engine/daemon/graphdriver/devmapper/deviceset.go @@ -40,6 +40,7 @@ var ( logLevel = devicemapper.LogLevelFatal driverDeferredRemovalSupport = false enableDeferredRemoval = false + enableDeferredDeletion = false ) const deviceSetMetaFile string = "deviceset-metadata" @@ -106,6 +107,7 @@ type DeviceSet struct { transaction `json:"-"` overrideUdevSyncCheck bool deferredRemove bool // use deferred removal + deferredDelete bool // use deferred deletion BaseDeviceUUID string //save UUID of base device } @@ -141,6 +143,12 @@ type Status struct { UdevSyncSupported bool // DeferredRemoveEnabled is true then the device is not unmounted. DeferredRemoveEnabled bool + // True if deferred deletion is enabled. This is different from + // deferred removal. "removal" means that device mapper device is + // deactivated. Thin device is still in thin pool and can be activated + // again. But "deletion" means that thin device will be deleted from + // thin pool and it can't be activated again. + DeferredDeleteEnabled bool } // Structure used to export image/container metadata in docker inspect. @@ -1314,13 +1322,27 @@ func (devices *DeviceSet) initDevmapper(doInit bool) error { return graphdriver.ErrNotSupported } - // If user asked for deferred removal and both library and driver - // supports deferred removal use it. - if enableDeferredRemoval && driverDeferredRemovalSupport && devicemapper.LibraryDeferredRemovalSupport == true { + // If user asked for deferred removal then check both libdm library + // and kernel driver support deferred removal otherwise error out. + if enableDeferredRemoval { + if !driverDeferredRemovalSupport { + return fmt.Errorf("devmapper: Deferred removal can not be enabled as kernel does not support it") + } + if !devicemapper.LibraryDeferredRemovalSupport { + return fmt.Errorf("devmapper: Deferred removal can not be enabled as libdm does not support it") + } logrus.Debugf("devmapper: Deferred removal support enabled.") devices.deferredRemove = true } + if enableDeferredDeletion { + if !devices.deferredRemove { + return fmt.Errorf("devmapper: Deferred deletion can not be enabled as deferred removal is not enabled. Enable deferred removal using --storage-opt dm.use_deferred_removal=true parameter") + } + logrus.Debugf("devmapper: Deferred deletion support enabled.") + devices.deferredDelete = true + } + // https://github.com/docker/docker/issues/4036 if supported := devicemapper.UdevSetSyncSupport(true); !supported { logrus.Warn("Udev sync is not supported. This will lead to unexpected behavior, data loss and errors. For more information, see https://docs.docker.com/reference/commandline/daemon/#daemon-storage-driver-option") @@ -1996,6 +2018,7 @@ func (devices *DeviceSet) Status() *Status { status.MetadataLoopback = devices.metadataLoopFile status.UdevSyncSupported = devicemapper.UdevSyncSupported() status.DeferredRemoveEnabled = devices.deferredRemove + status.DeferredDeleteEnabled = devices.deferredDelete totalSizeInSectors, _, dataUsed, dataTotal, metadataUsed, metadataTotal, err := devices.poolStatus() if err == nil { @@ -2128,6 +2151,12 @@ func NewDeviceSet(root string, doInit bool, options []string) (*DeviceSet, error return nil, err } + case "dm.use_deferred_deletion": + enableDeferredDeletion, err = strconv.ParseBool(val) + if err != nil { + return nil, err + } + default: return nil, fmt.Errorf("Unknown option %s\n", key) } diff --git a/components/engine/daemon/graphdriver/devmapper/driver.go b/components/engine/daemon/graphdriver/devmapper/driver.go index dc2647d11d..f117cfb7c5 100644 --- a/components/engine/daemon/graphdriver/devmapper/driver.go +++ b/components/engine/daemon/graphdriver/devmapper/driver.go @@ -84,6 +84,7 @@ func (d *Driver) Status() [][2]string { {"Metadata Space Available", fmt.Sprintf("%s", units.HumanSize(float64(s.Metadata.Available)))}, {"Udev Sync Supported", fmt.Sprintf("%v", s.UdevSyncSupported)}, {"Deferred Removal Enabled", fmt.Sprintf("%v", s.DeferredRemoveEnabled)}, + {"Deferred Deletion Enabled", fmt.Sprintf("%v", s.DeferredDeleteEnabled)}, } if len(s.DataLoopback) > 0 { status = append(status, [2]string{"Data loop file", s.DataLoopback}) diff --git a/components/engine/docs/reference/commandline/daemon.md b/components/engine/docs/reference/commandline/daemon.md index e3ed62ef42..cef19ab038 100644 --- a/components/engine/docs/reference/commandline/daemon.md +++ b/components/engine/docs/reference/commandline/daemon.md @@ -368,6 +368,46 @@ options for `zfs` start with `zfs`. > Otherwise, set this flag for migrating existing Docker daemons to > a daemon with a supported environment. + * `dm.use_deferred_removal` + + Enables use of deferred device removal if `libdm` and the kernel driver + support the mechanism. + + Deferred device removal means that if device is busy when devices are + being removed/deactivated, then a deferred removal is scheduled on + device. And devices automatically go away when last user of the device + exits. + + For example, when a container exits, its associated thin device is removed. + If that device has leaked into some other mount namespace and can't be + removed, the container exit still succeeds and this option causes the + system to schedule the device for deferred removal. It does not wait in a + loop trying to remove a busy device. + + Example use: `docker daemon --storage-opt dm.use_deferred_removal=true` + + * `dm.use_deferred_deletion` + + Enables use of deferred device deletion for thin pool devices. By default, + thin pool device deletion is synchronous. Before a container is deleted, + the Docker daemon removes any associated devices. If the storage driver + can not remove a device, the container deletion fails and daemon returns. + + `Error deleting container: Error response from daemon: Cannot destroy container` + + To avoid this failure, enable both deferred device deletion and deferred + device removal on the daemon. + + `docker daemon --storage-opt dm.use_deferred_deletion=true --storage-opt dm.use_deferred_removal=true` + + With these two options enabled, if a device is busy when the driver is + deleting a container, the driver marks the device as deleted. Later, when + the device isn't in use, the driver deletes it. + + In general it should be safe to enable this option by default. It will help + when unintentional leaking of mount point happens across multiple mount + namespaces. + Currently supported options of `zfs`: * `zfs.fsname` diff --git a/components/engine/man/docker-daemon.8.md b/components/engine/man/docker-daemon.8.md index b37bd25e0d..59ba4d54f2 100644 --- a/components/engine/man/docker-daemon.8.md +++ b/components/engine/man/docker-daemon.8.md @@ -302,6 +302,28 @@ device. Example use: `docker daemon --storage-opt dm.use_deferred_removal=true` +#### dm.use_deferred_deletion + +Enables use of deferred device deletion for thin pool devices. By default, +thin pool device deletion is synchronous. Before a container is deleted, the +Docker daemon removes any associated devices. If the storage driver can not +remove a device, the container deletion fails and daemon returns. + +`Error deleting container: Error response from daemon: Cannot destroy container` + +To avoid this failure, enable both deferred device deletion and deferred +device removal on the daemon. + +`docker daemon --storage-opt dm.use_deferred_deletion=true --storage-opt dm.use_deferred_removal=true` + +With these two options enabled, if a device is busy when the driver is +deleting a container, the driver marks the device as deleted. Later, when the +device isn't in use, the driver deletes it. + +In general it should be safe to enable this option by default. It will help +when unintentional leaking of mount point happens across multiple mount +namespaces. + #### dm.loopdatasize **Note**: This option configures devicemapper loopback, which should not be used in production. From 7bb486d26e85dafe2271516f4ca08f2ab04ab2c9 Mon Sep 17 00:00:00 2001 From: Vivek Goyal Date: Tue, 6 Oct 2015 17:37:21 -0400 Subject: [PATCH 3/5] 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 Upstream-commit: d929589c1fc4538dcd1b2a7a3dc7d4afbdfa72fd Component: engine --- .../daemon/graphdriver/devmapper/deviceset.go | 135 +++++++++++++++--- .../daemon/graphdriver/devmapper/driver.go | 2 +- .../engine/pkg/devicemapper/devmapper.go | 4 + 3 files changed, 118 insertions(+), 23 deletions(-) diff --git a/components/engine/daemon/graphdriver/devmapper/deviceset.go b/components/engine/daemon/graphdriver/devmapper/deviceset.go index 8279d8200e..7b96dd5f57 100644 --- a/components/engine/daemon/graphdriver/devmapper/deviceset.go +++ b/components/engine/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/components/engine/daemon/graphdriver/devmapper/driver.go b/components/engine/daemon/graphdriver/devmapper/driver.go index f117cfb7c5..d59219f1c8 100644 --- a/components/engine/daemon/graphdriver/devmapper/driver.go +++ b/components/engine/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/components/engine/pkg/devicemapper/devmapper.go b/components/engine/pkg/devicemapper/devmapper.go index abbd28b5ff..136dd6bbd0 100644 --- a/components/engine/pkg/devicemapper/devmapper.go +++ b/components/engine/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 From fb33696b35f26edfedaf18e35cef89b61da2db1e Mon Sep 17 00:00:00 2001 From: Vivek Goyal Date: Tue, 6 Oct 2015 17:37:21 -0400 Subject: [PATCH 4/5] devmapper: Keep track of number of deleted devices Keep track of number of deleted devices and export this information through "docker info". Signed-off-by: Vivek Goyal Upstream-commit: d295dc66521e2734390473ec1f1da8a73ad3288a Component: engine --- .../daemon/graphdriver/devmapper/deviceset.go | 28 ++++++++++++++++++- .../daemon/graphdriver/devmapper/driver.go | 1 + 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/components/engine/daemon/graphdriver/devmapper/deviceset.go b/components/engine/daemon/graphdriver/devmapper/deviceset.go index 7b96dd5f57..5eda2fc521 100644 --- a/components/engine/daemon/graphdriver/devmapper/deviceset.go +++ b/components/engine/daemon/graphdriver/devmapper/deviceset.go @@ -110,6 +110,7 @@ type DeviceSet struct { deferredRemove bool // use deferred removal deferredDelete bool // use deferred deletion BaseDeviceUUID string //save UUID of base device + nrDeletedDevices uint //number of deleted devices } // DiskUsage contains information about disk usage and is used when reporting Status of a device. @@ -149,7 +150,8 @@ type Status struct { // deactivated. Thin device is still in thin pool and can be activated // again. But "deletion" means that thin device will be deleted from // thin pool and it can't be activated again. - DeferredDeleteEnabled bool + DeferredDeleteEnabled bool + DeferredDeletedDeviceCount uint } // Structure used to export image/container metadata in docker inspect. @@ -581,6 +583,11 @@ func (devices *DeviceSet) migrateOldMetaData() error { // 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 { + // If there are no deleted devices, there is nothing to do. + if devices.nrDeletedDevices == 0 { + return nil + } + var deletedDevices []*devInfo for _, info := range devices.Devices { @@ -606,6 +613,15 @@ func (devices *DeviceSet) cleanupDeletedDevices() error { return nil } +func (devices *DeviceSet) countDeletedDevices() { + for _, info := range devices.Devices { + if !info.Deleted { + continue + } + devices.nrDeletedDevices++ + } +} + func (devices *DeviceSet) initMetaData() error { devices.Lock() defer devices.Unlock() @@ -626,6 +642,7 @@ func (devices *DeviceSet) initMetaData() error { } devices.constructDeviceIDMap() + devices.countDeletedDevices() if err := devices.processPendingTransaction(); err != nil { return err @@ -1601,6 +1618,8 @@ func (devices *DeviceSet) markForDeferredDeletion(info *devInfo) error { info.Deleted = false return err } + + devices.nrDeletedDevices++ return nil } @@ -1628,6 +1647,12 @@ func (devices *DeviceSet) deleteTransaction(info *devInfo, syncDelete bool) erro if err := devices.unregisterDevice(info.DeviceID, info.Hash); err != nil { return err } + // If device was already in deferred delete state that means + // deletion was being tried again later. Reduce the deleted + // device count. + if info.Deleted { + devices.nrDeletedDevices-- + } } else { if err := devices.markForDeferredDeletion(info); err != nil { return err @@ -2110,6 +2135,7 @@ func (devices *DeviceSet) Status() *Status { status.UdevSyncSupported = devicemapper.UdevSyncSupported() status.DeferredRemoveEnabled = devices.deferredRemove status.DeferredDeleteEnabled = devices.deferredDelete + status.DeferredDeletedDeviceCount = devices.nrDeletedDevices totalSizeInSectors, _, dataUsed, dataTotal, metadataUsed, metadataTotal, err := devices.poolStatus() if err == nil { diff --git a/components/engine/daemon/graphdriver/devmapper/driver.go b/components/engine/daemon/graphdriver/devmapper/driver.go index d59219f1c8..44dab1f965 100644 --- a/components/engine/daemon/graphdriver/devmapper/driver.go +++ b/components/engine/daemon/graphdriver/devmapper/driver.go @@ -85,6 +85,7 @@ func (d *Driver) Status() [][2]string { {"Udev Sync Supported", fmt.Sprintf("%v", s.UdevSyncSupported)}, {"Deferred Removal Enabled", fmt.Sprintf("%v", s.DeferredRemoveEnabled)}, {"Deferred Deletion Enabled", fmt.Sprintf("%v", s.DeferredDeleteEnabled)}, + {"Deferred Deleted Device Count", fmt.Sprintf("%v", s.DeferredDeletedDeviceCount)}, } if len(s.DataLoopback) > 0 { status = append(status, [2]string{"Data loop file", s.DataLoopback}) From c46b8dfd3a5e9c41b25281773b2e79b369004fd9 Mon Sep 17 00:00:00 2001 From: Vivek Goyal Date: Tue, 6 Oct 2015 17:37:21 -0400 Subject: [PATCH 5/5] devmapper: Implement a goroutine to cleanup deleted devices Start a goroutine which runs every 30 seconds and if there are deferred deleted devices, it tries to clean those up. Also it moves the call to cleanupDeletedDevices() into goroutine and moves the locking completely inside the function. Now function does not assume that device lock is held at the time of entry. Signed-off-by: Vivek Goyal Upstream-commit: 87de04005d61ab986d70a31e3ac82ec92912df55 Component: engine --- .../daemon/graphdriver/devmapper/deviceset.go | 33 +++++++++++++++---- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/components/engine/daemon/graphdriver/devmapper/deviceset.go b/components/engine/daemon/graphdriver/devmapper/deviceset.go index 5eda2fc521..499221ed80 100644 --- a/components/engine/daemon/graphdriver/devmapper/deviceset.go +++ b/components/engine/daemon/graphdriver/devmapper/deviceset.go @@ -111,6 +111,7 @@ type DeviceSet struct { deferredDelete bool // use deferred deletion BaseDeviceUUID string //save UUID of base device nrDeletedDevices uint //number of deleted devices + deletionWorkerTicker *time.Ticker } // DiskUsage contains information about disk usage and is used when reporting Status of a device. @@ -580,9 +581,10 @@ func (devices *DeviceSet) migrateOldMetaData() error { } // 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. +// loaded in the hash table. func (devices *DeviceSet) cleanupDeletedDevices() error { + devices.Lock() + // If there are no deleted devices, there is nothing to do. if devices.nrDeletedDevices == 0 { return nil @@ -601,7 +603,6 @@ func (devices *DeviceSet) cleanupDeletedDevices() error { // 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. @@ -622,6 +623,18 @@ func (devices *DeviceSet) countDeletedDevices() { } } +func (devices *DeviceSet) startDeviceDeletionWorker() { + // Deferred deletion is not enabled. Don't do anything. + if !devices.deferredDelete { + return + } + + logrus.Debugf("devmapper: Worker to cleanup deleted devices started") + for range devices.deletionWorkerTicker.C { + devices.cleanupDeletedDevices() + } +} + func (devices *DeviceSet) initMetaData() error { devices.Lock() defer devices.Unlock() @@ -648,10 +661,8 @@ func (devices *DeviceSet) initMetaData() error { return err } - if err := devices.cleanupDeletedDevices(); err != nil { - return err - } - + // Start a goroutine to cleanup Deleted Devices + go devices.startDeviceDeletionWorker() return nil } @@ -1861,6 +1872,13 @@ func (devices *DeviceSet) Shutdown() error { var devs []*devInfo + // Stop deletion worker. This should start delivering new events to + // ticker channel. That means no new instance of cleanupDeletedDevice() + // will run after this call. If one instance is already running at + // the time of the call, it must be holding devices.Lock() and + // we will block on this lock till cleanup function exits. + devices.deletionWorkerTicker.Stop() + devices.Lock() // Save DeviceSet Metadata first. Docker kills all threads if they // don't finish in certain time. It is possible that Shutdown() @@ -2200,6 +2218,7 @@ func NewDeviceSet(root string, doInit bool, options []string) (*DeviceSet, error doBlkDiscard: true, thinpBlockSize: defaultThinpBlockSize, deviceIDMap: make([]byte, deviceIDMapSz), + deletionWorkerTicker: time.NewTicker(time.Second * 30), } foundBlkDiscard := false