diff --git a/graphdriver/devmapper/deviceset.go b/graphdriver/devmapper/deviceset.go index e4c835a3de..4d708c840a 100644 --- a/graphdriver/devmapper/deviceset.go +++ b/graphdriver/devmapper/deviceset.go @@ -29,6 +29,15 @@ type DevInfo struct { TransactionId uint64 `json:"transaction_id"` Initialized bool `json:"initialized"` devices *DeviceSet `json:"-"` + + 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:"-"` } type MetaData struct { @@ -43,7 +52,6 @@ type DeviceSet struct { TransactionId uint64 NewTransactionId uint64 nextFreeDevice int - activeMounts map[string]int } type DiskUsage struct { @@ -69,6 +77,14 @@ type DevStatus struct { HighestMappedSector uint64 } +type UnmountMode int + +const ( + UnmountRegular UnmountMode = iota + UnmountFloat + UnmountSink +) + func getDevName(name string) string { return "/dev/mapper/" + name } @@ -733,13 +749,12 @@ func (devices *DeviceSet) Shutdown() error { utils.Debugf("[devmapper] Shutting down DeviceSet: %s", devices.root) defer utils.Debugf("[deviceset %s] shutdown END", devices.devicePrefix) - for path, count := range devices.activeMounts { - for i := count; i > 0; i-- { - if err := sysUnmount(path, 0); err != nil { - utils.Debugf("Shutdown unmounting %s, error: %s\n", path, err) + for _, info := range devices.Devices { + if info.mountCount > 0 { + if err := sysUnmount(info.mountPath, 0); err != nil { + utils.Debugf("Shutdown unmounting %s, error: %s\n", info.mountPath, err) } } - delete(devices.activeMounts, path) } for _, d := range devices.Devices { @@ -761,22 +776,32 @@ func (devices *DeviceSet) Shutdown() error { return nil } -func (devices *DeviceSet) MountDevice(hash, path string, readOnly bool) error { +func (devices *DeviceSet) MountDevice(hash, path string) error { devices.Lock() defer devices.Unlock() + info := devices.Devices[hash] + + if info.mountCount > 0 { + if path != info.mountPath { + 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++ + } + return nil + } + if err := devices.activateDeviceIfNeeded(hash); err != nil { return fmt.Errorf("Error activating devmapper device for '%s': %s", hash, err) } - info := devices.Devices[hash] - var flags uintptr = sysMsMgcVal - if readOnly { - flags = flags | sysMsRdOnly - } - err := sysMount(info.DevName(), path, "ext4", flags, "discard") if err != nil && err == sysEInval { err = sysMount(info.DevName(), path, "ext4", flags, "") @@ -785,20 +810,50 @@ func (devices *DeviceSet) MountDevice(hash, path string, readOnly bool) error { return fmt.Errorf("Error mounting '%s' on '%s': %s", info.DevName(), path, err) } - count := devices.activeMounts[path] - devices.activeMounts[path] = count + 1 + info.mountCount = 1 + info.mountPath = path + info.floating = false return devices.setInitialized(hash) } -func (devices *DeviceSet) UnmountDevice(hash, path string, deactivate bool) error { - utils.Debugf("[devmapper] UnmountDevice(hash=%s path=%s)", hash, path) +func (devices *DeviceSet) UnmountDevice(hash string, mode UnmountMode) error { + utils.Debugf("[devmapper] UnmountDevice(hash=%s, mode=%d)", hash, mode) defer utils.Debugf("[devmapper] UnmountDevice END") devices.Lock() defer devices.Unlock() - utils.Debugf("[devmapper] Unmount(%s)", path) - if err := sysUnmount(path, 0); err != nil { + info := devices.Devices[hash] + + 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) + } + + info.mountCount-- + if info.mountCount > 0 { + return nil + } + + utils.Debugf("[devmapper] Unmount(%s)", info.mountPath) + if err := sysUnmount(info.mountPath, 0); err != nil { utils.Debugf("\n--->Err: %s\n", err) return err } @@ -809,15 +864,9 @@ func (devices *DeviceSet) UnmountDevice(hash, path string, deactivate bool) erro return err } - if count := devices.activeMounts[path]; count > 1 { - devices.activeMounts[path] = count - 1 - } else { - delete(devices.activeMounts, path) - } + devices.deactivateDevice(hash) - if deactivate { - devices.deactivateDevice(hash) - } + info.mountPath = "" return nil } @@ -960,9 +1009,8 @@ func NewDeviceSet(root string, doInit bool) (*DeviceSet, error) { SetDevDir("/dev") devices := &DeviceSet{ - root: root, - MetaData: MetaData{Devices: make(map[string]*DevInfo)}, - activeMounts: make(map[string]int), + root: root, + MetaData: MetaData{Devices: make(map[string]*DevInfo)}, } if err := devices.initDevmapper(doInit); err != nil { diff --git a/graphdriver/devmapper/driver.go b/graphdriver/devmapper/driver.go index 9b6cb109d2..25b998e59f 100644 --- a/graphdriver/devmapper/driver.go +++ b/graphdriver/devmapper/driver.go @@ -8,7 +8,6 @@ import ( "github.com/dotcloud/docker/utils" "io/ioutil" "path" - "sync" ) func init() { @@ -22,9 +21,7 @@ func init() { type Driver struct { *DeviceSet - home string - sync.Mutex // Protects concurrent modification to active - active map[string]int + home string } var Init = func(home string) (graphdriver.Driver, error) { @@ -35,7 +32,6 @@ var Init = func(home string) (graphdriver.Driver, error) { d := &Driver{ DeviceSet: deviceSet, home: home, - active: make(map[string]int), } return d, nil } @@ -83,55 +79,36 @@ func (d *Driver) Create(id, parent string) error { 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 } func (d *Driver) Remove(id string) error { - // Protect the d.active from concurrent access - d.Lock() - defer d.Unlock() - - if d.active[id] != 0 { - utils.Errorf("Warning: removing active id %s\n", id) - } - - mp := path.Join(d.home, "mnt", id) - if err := d.unmount(id, mp); err != 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 return d.DeviceSet.DeleteDevice(id) } func (d *Driver) Get(id string) (string, error) { - // Protect the d.active from concurrent access - d.Lock() - defer d.Unlock() - - count := d.active[id] - mp := path.Join(d.home, "mnt", id) - if count == 0 { - if err := d.mount(id, mp); err != nil { - return "", err - } + if err := d.mount(id, mp); err != nil { + return "", err } - d.active[id] = count + 1 - return path.Join(mp, "rootfs"), nil } func (d *Driver) Put(id string) { - // Protect the d.active from concurrent access - d.Lock() - defer d.Unlock() - - if count := d.active[id]; count > 1 { - d.active[id] = count - 1 - } else { - mp := path.Join(d.home, "mnt", id) - d.unmount(id, mp) - delete(d.active, id) + if err := d.DeviceSet.UnmountDevice(id, UnmountRegular); err != nil { + utils.Errorf("Warning: error unmounting device %s: %s\n", id, err) } } @@ -140,25 +117,8 @@ func (d *Driver) mount(id, mountPoint string) error { if err := osMkdirAll(mountPoint, 0755); err != nil && !osIsExist(err) { return err } - // If mountpoint is already mounted, do nothing - if mounted, err := Mounted(mountPoint); err != nil { - return fmt.Errorf("Error checking mountpoint: %s", err) - } else if mounted { - return nil - } // Mount the device - return d.DeviceSet.MountDevice(id, mountPoint, false) -} - -func (d *Driver) unmount(id, mountPoint string) error { - // If mountpoint is not mounted, do nothing - if mounted, err := Mounted(mountPoint); err != nil { - return fmt.Errorf("Error checking mountpoint: %s", err) - } else if !mounted { - return nil - } - // Unmount the device - return d.DeviceSet.UnmountDevice(id, mountPoint, true) + return d.DeviceSet.MountDevice(id, mountPoint) } func (d *Driver) Exists(id string) bool { diff --git a/graphdriver/devmapper/driver_test.go b/graphdriver/devmapper/driver_test.go index 785845ce6e..ff4ec45e5d 100644 --- a/graphdriver/devmapper/driver_test.go +++ b/graphdriver/devmapper/driver_test.go @@ -495,7 +495,6 @@ func TestDriverCreate(t *testing.T) { "DmTaskCreate", "DmTaskGetInfo", "sysMount", - "Mounted", "DmTaskRun", "DmTaskSetTarget", "DmTaskSetSector", @@ -614,7 +613,6 @@ func TestDriverRemove(t *testing.T) { "DmTaskCreate", "DmTaskGetInfo", "sysMount", - "Mounted", "DmTaskRun", "DmTaskSetTarget", "DmTaskSetSector", @@ -645,7 +643,6 @@ func TestDriverRemove(t *testing.T) { "DmTaskSetTarget", "DmTaskSetAddNode", "DmUdevWait", - "Mounted", "sysUnmount", ) }()