Merge pull request moby#5356 from alexlarsson/devmapper-no-mount-in-c…

…reate devicemapper: Don't mount in Create()
imdaisyadams · Apr 24, 2014 · 56b779c · 56b779c
2 parents df20a0e + 73d9ede
commit 56b779c
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 89 deletions.
diff --git a/daemon/graphdriver/devmapper/deviceset.go b/daemon/graphdriver/devmapper/deviceset.go
@@ -35,12 +35,6 @@ type DevInfo struct {
 
 	mountCount int    `json:"-"`
 	mountPath  string `json:"-"`
-	// A floating mount means one reference is not owned and
-	// will be stolen by the next mount. This allows us to
-	// avoid unmounting directly after creation before the
-	// 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
@@ -94,14 +88,6 @@ type DevStatus struct {
 	HighestMappedSector uint64
 }
 
-type UnmountMode int
-
-const (
-	UnmountRegular UnmountMode = iota
-	UnmountFloat
-	UnmountSink
-)
-
 func getDevName(name string) string {
 	return "/dev/mapper/" + name
 }
@@ -876,12 +862,7 @@ func (devices *DeviceSet) MountDevice(hash, path string, mountLabel string) erro
 			return fmt.Errorf("Trying to mount devmapper device in multple places (%s, %s)", info.mountPath, path)
 		}
 
-		if info.floating {
-			// Steal floating ref
-			info.floating = false
-		} else {
-			info.mountCount++
-		}
+		info.mountCount++
 		return nil
 	}
 
@@ -903,13 +884,12 @@ func (devices *DeviceSet) MountDevice(hash, path string, mountLabel string) erro
 
 	info.mountCount = 1
 	info.mountPath = path
-	info.floating = false
 
 	return devices.setInitialized(info)
 }
 
-func (devices *DeviceSet) UnmountDevice(hash string, mode UnmountMode) error {
-	utils.Debugf("[devmapper] UnmountDevice(hash=%s, mode=%d)", hash, mode)
+func (devices *DeviceSet) UnmountDevice(hash string) error {
+	utils.Debugf("[devmapper] UnmountDevice(hash=%s)", hash)
 	defer utils.Debugf("[devmapper] UnmountDevice END")
 
 	info, err := devices.lookupDevice(hash)
@@ -923,24 +903,6 @@ func (devices *DeviceSet) UnmountDevice(hash string, mode UnmountMode) error {
 	devices.Lock()
 	defer devices.Unlock()
 
-	if mode == UnmountFloat {
-		if info.floating {
-			return fmt.Errorf("UnmountDevice: can't float floating reference %s\n", hash)
-		}
-
-		// Leave this reference floating
-		info.floating = true
-		return nil
-	}
-
-	if mode == UnmountSink {
-		if !info.floating {
-			// Someone already sunk this
-			return nil
-		}
-		// Otherwise, treat this as a regular unmount
-	}
-
 	if info.mountCount == 0 {
 		return fmt.Errorf("UnmountDevice: device not-mounted id %s\n", hash)
 	}

diff --git a/daemon/graphdriver/devmapper/driver.go b/daemon/graphdriver/devmapper/driver.go
@@ -64,26 +64,6 @@ func (d *Driver) Create(id, parent string, mountLabel string) error {
 	if err := d.DeviceSet.AddDevice(id, parent); err != nil {
 		return err
 	}
-	mp := path.Join(d.home, "mnt", id)
-	if err := d.mount(id, mp); err != nil {
-		return err
-	}
-
-	if err := osMkdirAll(path.Join(mp, "rootfs"), 0755); err != nil && !osIsExist(err) {
-		return err
-	}
-
-	// Create an "id" file with the container/image id in it to help reconscruct this in case
-	// of later problems
-	if err := ioutil.WriteFile(path.Join(mp, "id"), []byte(id), 0600); err != nil {
-		return err
-	}
-
-	// We float this reference so that the next Get call can
-	// steal it, so we don't have to unmount
-	if err := d.DeviceSet.UnmountDevice(id, UnmountFloat); err != nil {
-		return err
-	}
 
 	return nil
 }
@@ -96,10 +76,6 @@ func (d *Driver) Remove(id string) error {
 		return nil
 	}
 
-	// Sink the float from create in case no Get() call was made
-	if err := d.DeviceSet.UnmountDevice(id, UnmountSink); err != nil {
-		return err
-	}
 	// This assumes the device has been properly Get/Put:ed and thus is unmounted
 	if err := d.DeviceSet.DeleteDevice(id); err != nil {
 		return err
@@ -115,28 +91,42 @@ func (d *Driver) Remove(id string) error {
 
 func (d *Driver) Get(id string) (string, error) {
 	mp := path.Join(d.home, "mnt", id)
-	if err := d.mount(id, mp); err != nil {
+
+	// Create the target directories if they don't exist
+	if err := osMkdirAll(mp, 0755); err != nil && !osIsExist(err) {
 		return "", err
 	}
 
-	return path.Join(mp, "rootfs"), nil
-}
+	// Mount the device
+	if err := d.DeviceSet.MountDevice(id, mp, ""); err != nil {
+		return "", err
+	}
 
-func (d *Driver) Put(id string) {
-	if err := d.DeviceSet.UnmountDevice(id, UnmountRegular); err != nil {
-		utils.Errorf("Warning: error unmounting device %s: %s\n", id, err)
+	rootFs := path.Join(mp, "rootfs")
+	if err := osMkdirAll(rootFs, 0755); err != nil && !osIsExist(err) {
+		d.DeviceSet.UnmountDevice(id)
+		return "", err
 	}
+
+	idFile := path.Join(mp, "id")
+	if _, err := osStat(idFile); err != nil && osIsNotExist(err) {
+		// Create an "id" file with the container/image id in it to help reconscruct this in case
+		// of later problems
+		if err := ioutil.WriteFile(idFile, []byte(id), 0600); err != nil {
+			d.DeviceSet.UnmountDevice(id)
+			return "", err
+		}
+	}
+
+	return rootFs, nil
 }
 
-func (d *Driver) mount(id, mountPoint string) error {
-	// Create the target directories if they don't exist
-	if err := osMkdirAll(mountPoint, 0755); err != nil && !osIsExist(err) {
-		return err
+func (d *Driver) Put(id string) {
+	if err := d.DeviceSet.UnmountDevice(id); err != nil {
+		utils.Errorf("Warning: error unmounting device %s: %s\n", id, err)
 	}
-	// Mount the device
-	return d.DeviceSet.MountDevice(id, mountPoint, "")
 }
 
 func (d *Driver) Exists(id string) bool {
-	return d.Devices[id] != nil
+	return d.DeviceSet.HasDevice(id)
 }
diff --git a/daemon/graphdriver/devmapper/driver_test.go b/daemon/graphdriver/devmapper/driver_test.go
@@ -500,15 +500,10 @@ func TestDriverCreate(t *testing.T) {
 		calls.Assert(t,
 			"DmTaskCreate",
 			"DmTaskGetInfo",
-			"sysMount",
 			"DmTaskRun",
-			"DmTaskSetTarget",
 			"DmTaskSetSector",
-			"DmTaskSetCookie",
-			"DmUdevWait",
 			"DmTaskSetName",
 			"DmTaskSetMessage",
-			"DmTaskSetAddNode",
 		)
 
 	}()
@@ -619,15 +614,10 @@ func TestDriverRemove(t *testing.T) {
 		calls.Assert(t,
 			"DmTaskCreate",
 			"DmTaskGetInfo",
-			"sysMount",
 			"DmTaskRun",
-			"DmTaskSetTarget",
 			"DmTaskSetSector",
-			"DmTaskSetCookie",
-			"DmUdevWait",
 			"DmTaskSetName",
 			"DmTaskSetMessage",
-			"DmTaskSetAddNode",
 		)
 
 		Mounted = func(mnt string) (bool, error) {
@@ -650,7 +640,6 @@ func TestDriverRemove(t *testing.T) {
 			"DmTaskSetTarget",
 			"DmTaskSetAddNode",
 			"DmUdevWait",
-			"sysUnmount",
 		)
 	}()
 	runtime.GC()