Browse code

Fixes Issue # 23418: Race condition between device deferred removal and 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.

Signed-off-by: Shishir Mahajan <shishir.mahajan@redhat.com>

Shishir Mahajan authored on 2016/06/10 02:52:25
Showing 2 changed files
... ...
@@ -528,7 +528,7 @@ func (devices *DeviceSet) activateDeviceIfNeeded(info *devInfo, ignoreDeleted bo
528 528
 
529 529
 	// Make sure deferred removal on device is canceled, if one was
530 530
 	// scheduled.
531
-	if err := devices.cancelDeferredRemoval(info); err != nil {
531
+	if err := devices.cancelDeferredRemovalIfNeeded(info); err != nil {
532 532
 		return fmt.Errorf("devmapper: Device Deferred Removal Cancellation Failed: %s", err)
533 533
 	}
534 534
 
... ...
@@ -841,11 +841,56 @@ func (devices *DeviceSet) createRegisterDevice(hash string) (*devInfo, error) {
841 841
 	return info, nil
842 842
 }
843 843
 
844
-func (devices *DeviceSet) createRegisterSnapDevice(hash string, baseInfo *devInfo, size uint64) error {
845
-	if err := devices.poolHasFreeSpace(); err != nil {
844
+func (devices *DeviceSet) takeSnapshot(hash string, baseInfo *devInfo, size uint64) error {
845
+	var (
846
+		devinfo *devicemapper.Info
847
+		err     error
848
+	)
849
+
850
+	if err = devices.poolHasFreeSpace(); err != nil {
851
+		return err
852
+	}
853
+
854
+	if devices.deferredRemove {
855
+		devinfo, err = devicemapper.GetInfoWithDeferred(baseInfo.Name())
856
+		if err != nil {
857
+			return err
858
+		}
859
+		if devinfo != nil && devinfo.DeferredRemove != 0 {
860
+			err = devices.cancelDeferredRemoval(baseInfo)
861
+			if err != nil {
862
+				// If Error is ErrEnxio. Device is probably already gone. Continue.
863
+				if err != devicemapper.ErrEnxio {
864
+					return err
865
+				}
866
+			} else {
867
+				defer devices.deactivateDevice(baseInfo)
868
+			}
869
+		}
870
+	} else {
871
+		devinfo, err = devicemapper.GetInfo(baseInfo.Name())
872
+		if err != nil {
873
+			return err
874
+		}
875
+	}
876
+
877
+	doSuspend := devinfo != nil && devinfo.Exists != 0
878
+
879
+	if doSuspend {
880
+		if err = devicemapper.SuspendDevice(baseInfo.Name()); err != nil {
881
+			return err
882
+		}
883
+		defer devicemapper.ResumeDevice(baseInfo.Name())
884
+	}
885
+
886
+	if err = devices.createRegisterSnapDevice(hash, baseInfo, size); err != nil {
846 887
 		return err
847 888
 	}
848 889
 
890
+	return nil
891
+}
892
+
893
+func (devices *DeviceSet) createRegisterSnapDevice(hash string, baseInfo *devInfo, size uint64) error {
849 894
 	deviceID, err := devices.getNextFreeDeviceID()
850 895
 	if err != nil {
851 896
 		return err
... ...
@@ -858,7 +903,7 @@ func (devices *DeviceSet) createRegisterSnapDevice(hash string, baseInfo *devInf
858 858
 	}
859 859
 
860 860
 	for {
861
-		if err := devicemapper.CreateSnapDevice(devices.getPoolDevName(), deviceID, baseInfo.Name(), baseInfo.DeviceID); err != nil {
861
+		if err := devicemapper.CreateSnapDeviceRaw(devices.getPoolDevName(), deviceID, baseInfo.DeviceID); err != nil {
862 862
 			if devicemapper.DeviceIDExists(err) {
863 863
 				// Device ID already exists. This should not
864 864
 				// happen. Now we have a mechanism to find
... ...
@@ -1886,7 +1931,7 @@ func (devices *DeviceSet) AddDevice(hash, baseHash string, storageOpt map[string
1886 1886
 		return fmt.Errorf("devmapper: Container size cannot be smaller than %s", units.HumanSize(float64(baseInfo.Size)))
1887 1887
 	}
1888 1888
 
1889
-	if err := devices.createRegisterSnapDevice(hash, baseInfo, size); err != nil {
1889
+	if err := devices.takeSnapshot(hash, baseInfo, size); err != nil {
1890 1890
 		return err
1891 1891
 	}
1892 1892
 
... ...
@@ -2128,41 +2173,53 @@ func (devices *DeviceSet) removeDevice(devname string) error {
2128 2128
 	return err
2129 2129
 }
2130 2130
 
2131
-func (devices *DeviceSet) cancelDeferredRemoval(info *devInfo) error {
2131
+func (devices *DeviceSet) cancelDeferredRemovalIfNeeded(info *devInfo) error {
2132 2132
 	if !devices.deferredRemove {
2133 2133
 		return nil
2134 2134
 	}
2135 2135
 
2136
-	logrus.Debugf("devmapper: cancelDeferredRemoval START(%s)", info.Name())
2137
-	defer logrus.Debugf("devmapper: cancelDeferredRemoval END(%s)", info.Name())
2136
+	logrus.Debugf("devmapper: cancelDeferredRemovalIfNeeded START(%s)", info.Name())
2137
+	defer logrus.Debugf("devmapper: cancelDeferredRemovalIfNeeded END(%s)", info.Name())
2138 2138
 
2139 2139
 	devinfo, err := devicemapper.GetInfoWithDeferred(info.Name())
2140
+	if err != nil {
2141
+		return err
2142
+	}
2140 2143
 
2141 2144
 	if devinfo != nil && devinfo.DeferredRemove == 0 {
2142 2145
 		return nil
2143 2146
 	}
2144 2147
 
2145 2148
 	// Cancel deferred remove
2146
-	for i := 0; i < 100; i++ {
2147
-		err = devicemapper.CancelDeferredRemove(info.Name())
2148
-		if err == nil {
2149
-			break
2149
+	if err := devices.cancelDeferredRemoval(info); err != nil {
2150
+		// If Error is ErrEnxio. Device is probably already gone. Continue.
2151
+		if err != devicemapper.ErrEnxio {
2152
+			return err
2150 2153
 		}
2154
+	}
2155
+	return nil
2156
+}
2151 2157
 
2152
-		if err == devicemapper.ErrEnxio {
2153
-			// Device is probably already gone. Return success.
2154
-			return nil
2155
-		}
2158
+func (devices *DeviceSet) cancelDeferredRemoval(info *devInfo) error {
2159
+	logrus.Debugf("devmapper: cancelDeferredRemoval START(%s)", info.Name())
2160
+	defer logrus.Debugf("devmapper: cancelDeferredRemoval END(%s)", info.Name())
2156 2161
 
2157
-		if err != devicemapper.ErrBusy {
2158
-			return err
2159
-		}
2162
+	var err error
2160 2163
 
2161
-		// If we see EBUSY it may be a transient error,
2162
-		// sleep a bit a retry a few times.
2163
-		devices.Unlock()
2164
-		time.Sleep(100 * time.Millisecond)
2165
-		devices.Lock()
2164
+	// Cancel deferred remove
2165
+	for i := 0; i < 100; i++ {
2166
+		err = devicemapper.CancelDeferredRemove(info.Name())
2167
+		if err != nil {
2168
+			if err == devicemapper.ErrBusy {
2169
+				// If we see EBUSY it may be a transient error,
2170
+				// sleep a bit a retry a few times.
2171
+				devices.Unlock()
2172
+				time.Sleep(100 * time.Millisecond)
2173
+				devices.Lock()
2174
+				continue
2175
+			}
2176
+		}
2177
+		break
2166 2178
 	}
2167 2179
 	return err
2168 2180
 }
... ...
@@ -750,51 +750,51 @@ func activateDevice(poolName string, name string, deviceID int, size uint64, ext
750 750
 	return nil
751 751
 }
752 752
 
753
-// CreateSnapDevice creates a snapshot based on the device identified by the baseName and baseDeviceId,
754
-func CreateSnapDevice(poolName string, deviceID int, baseName string, baseDeviceID int) error {
755
-	devinfo, _ := GetInfo(baseName)
756
-	doSuspend := devinfo != nil && devinfo.Exists != 0
757
-
758
-	if doSuspend {
759
-		if err := SuspendDevice(baseName); err != nil {
760
-			return err
761
-		}
762
-	}
763
-
753
+// CreateSnapDeviceRaw creates a snapshot device. Caller needs to suspend and resume the origin device if it is active.
754
+func CreateSnapDeviceRaw(poolName string, deviceID int, baseDeviceID int) error {
764 755
 	task, err := TaskCreateNamed(deviceTargetMsg, poolName)
765 756
 	if task == nil {
766
-		if doSuspend {
767
-			ResumeDevice(baseName)
768
-		}
769 757
 		return err
770 758
 	}
771 759
 
772 760
 	if err := task.setSector(0); err != nil {
773
-		if doSuspend {
774
-			ResumeDevice(baseName)
775
-		}
776 761
 		return fmt.Errorf("devicemapper: Can't set sector %s", err)
777 762
 	}
778 763
 
779 764
 	if err := task.setMessage(fmt.Sprintf("create_snap %d %d", deviceID, baseDeviceID)); err != nil {
780
-		if doSuspend {
781
-			ResumeDevice(baseName)
782
-		}
783 765
 		return fmt.Errorf("devicemapper: Can't set message %s", err)
784 766
 	}
785 767
 
786 768
 	dmSawExist = false // reset before the task is run
787 769
 	if err := task.run(); err != nil {
788
-		if doSuspend {
789
-			ResumeDevice(baseName)
790
-		}
791 770
 		// Caller wants to know about ErrDeviceIDExists so that it can try with a different device id.
792 771
 		if dmSawExist {
793 772
 			return ErrDeviceIDExists
794 773
 		}
795
-
796 774
 		return fmt.Errorf("devicemapper: Error running deviceCreate (createSnapDevice) %s", err)
775
+	}
776
+
777
+	return nil
778
+}
779
+
780
+// CreateSnapDevice creates a snapshot based on the device identified by the baseName and baseDeviceId,
781
+func CreateSnapDevice(poolName string, deviceID int, baseName string, baseDeviceID int) error {
782
+	devinfo, _ := GetInfo(baseName)
783
+	doSuspend := devinfo != nil && devinfo.Exists != 0
797 784
 
785
+	if doSuspend {
786
+		if err := SuspendDevice(baseName); err != nil {
787
+			return err
788
+		}
789
+	}
790
+
791
+	if err := CreateSnapDeviceRaw(poolName, deviceID, baseDeviceID); err != nil {
792
+		if doSuspend {
793
+			if err2 := ResumeDevice(baseName); err2 != nil {
794
+				return fmt.Errorf("CreateSnapDeviceRaw Error: (%v): ResumeDevice Error: (%v)", err, err2)
795
+			}
796
+		}
797
+		return err
798 798
 	}
799 799
 
800 800
 	if doSuspend {