Merge pull request #4504 from alexlarsson/devicemapper-waits

devmapper: Increase sleep times and unlock while sleeping

graphdriver/devmapper/deviceset.go

@@ -39,6 +39,13 @@ type DevInfo struct {
 	// first get (since we need to mount to set up the device
 	// a bit first).
 	floating bool `json:"-"`
+
+	// The global DeviceSet lock guarantees that we serialize all
+	// the calls to libdevmapper (which is not threadsafe), but we
+	// sometimes release that lock while sleeping. In that case
+	// this per-device lock is still held, protecting against
+	// other accesses to the device that we're doing the wait on.
+	lock sync.Mutex `json:"-"`
 }
 
 type MetaData struct {
@@ -47,7 +54,7 @@ type MetaData struct {
 
 type DeviceSet struct {
 	MetaData
-	sync.Mutex
+	sync.Mutex       // Protects Devices map and serializes calls into libdevmapper
 	root             string
 	devicePrefix     string
 	TransactionId    uint64
@@ -569,6 +576,9 @@ func (devices *DeviceSet) AddDevice(hash, baseHash string) error {
 		return fmt.Errorf("Error adding device for '%s': can't find device for parent '%s'", hash, baseHash)
 	}
 
+	baseInfo.lock.Lock()
+	defer baseInfo.lock.Unlock()
+
 	deviceId := devices.allocateDeviceId()
 
 	if err := devices.createSnapDevice(devices.getPoolDevName(), deviceId, baseInfo.Name(), baseInfo.DeviceId); err != nil {
@@ -636,6 +646,14 @@ func (devices *DeviceSet) DeleteDevice(hash string) error {
 	devices.Lock()
 	defer devices.Unlock()
 
+	info := devices.Devices[hash]
+	if info == nil {
+		return fmt.Errorf("Unknown device %s", hash)
+	}
+
+	info.lock.Lock()
+	defer info.lock.Unlock()
+
 	return devices.deleteDevice(hash)
 }
 
@@ -683,7 +701,7 @@ func (devices *DeviceSet) deactivateDevice(hash string) error {
 func (devices *DeviceSet) removeDeviceAndWait(devname string) error {
 	var err error
 
-	for i := 0; i < 10; i++ {
+	for i := 0; i < 1000; i++ {
 		devices.sawBusy = false
 		err = removeDevice(devname)
 		if err == nil {
@@ -695,7 +713,9 @@ func (devices *DeviceSet) removeDeviceAndWait(devname string) error {
 
 		// If we see EBUSY it may be a transient error,
 		// sleep a bit a retry a few times.
-		time.Sleep(5 * time.Millisecond)
+		devices.Unlock()
+		time.Sleep(10 * time.Millisecond)
+		devices.Lock()
 	}
 	if err != nil {
 		return err
@@ -728,7 +748,9 @@ func (devices *DeviceSet) waitRemove(devname string) error {
 			break
 		}
 
-		time.Sleep(1 * time.Millisecond)
+		devices.Unlock()
+		time.Sleep(10 * time.Millisecond)
+		devices.Lock()
 	}
 	if i == 1000 {
 		return fmt.Errorf("Timeout while waiting for device %s to be removed", devname)
@@ -773,20 +795,26 @@ func (devices *DeviceSet) Shutdown() error {
 	defer utils.Debugf("[deviceset %s] shutdown END", devices.devicePrefix)
 
 	for _, info := range devices.Devices {
+		info.lock.Lock()
 		if info.mountCount > 0 {
 			if err := sysUnmount(info.mountPath, 0); err != nil {
 				utils.Debugf("Shutdown unmounting %s, error: %s\n", info.mountPath, err)
 			}
 		}
+		info.lock.Unlock()
 	}
 
 	for _, d := range devices.Devices {
+		d.lock.Lock()
+
 		if err := devices.waitClose(d.Hash); err != nil {
 			utils.Errorf("Warning: error waiting for device %s to unmount: %s\n", d.Hash, err)
 		}
 		if err := devices.deactivateDevice(d.Hash); err != nil {
 			utils.Debugf("Shutdown deactivate %s , error: %s\n", d.Hash, err)
 		}
+
+		d.lock.Unlock()
 	}
 
 	if err := devices.deactivatePool(); err != nil {
@@ -805,6 +833,9 @@ func (devices *DeviceSet) MountDevice(hash, path string) error {
 		return fmt.Errorf("Unknown device %s", hash)
 	}
 
+	info.lock.Lock()
+	defer info.lock.Unlock()
+
 	if info.mountCount > 0 {
 		if path != info.mountPath {
 			return fmt.Errorf("Trying to mount devmapper device in multple places (%s, %s)", info.mountPath, path)
@@ -851,6 +882,9 @@ func (devices *DeviceSet) UnmountDevice(hash string, mode UnmountMode) error {
 		return fmt.Errorf("UnmountDevice: no such device %s\n", hash)
 	}
 
+	info.lock.Lock()
+	defer info.lock.Unlock()
+
 	if mode == UnmountFloat {
 		if info.floating {
 			return fmt.Errorf("UnmountDevice: can't float floating reference %s\n", hash)
@@ -920,6 +954,10 @@ func (devices *DeviceSet) HasActivatedDevice(hash string) bool {
 	if info == nil {
 		return false
 	}
+
+	info.lock.Lock()
+	defer info.lock.Unlock()
+
 	devinfo, _ := getInfo(info.Name())
 	return devinfo != nil && devinfo.Exists != 0
 }
@@ -974,6 +1012,9 @@ func (devices *DeviceSet) GetDeviceStatus(hash string) (*DevStatus, error) {
 		return nil, fmt.Errorf("No device %s", hash)
 	}
 
+	info.lock.Lock()
+	defer info.lock.Unlock()
+
 	status := &DevStatus{
 		DeviceId:      info.DeviceId,
 		Size:          info.Size,