diff --git a/graphdriver/devmapper/deviceset.go b/graphdriver/devmapper/deviceset.go index 8432d92a4..637192dbc 100644 --- a/graphdriver/devmapper/deviceset.go +++ b/graphdriver/devmapper/deviceset.go @@ -12,6 +12,7 @@ import ( "path" "path/filepath" "strconv" + "strings" "sync" "time" ) @@ -29,6 +30,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 +53,7 @@ type DeviceSet struct { TransactionId uint64 NewTransactionId uint64 nextFreeDevice int - activeMounts map[string]int + sawBusy bool } type DiskUsage struct { @@ -69,6 +79,14 @@ type DevStatus struct { HighestMappedSector uint64 } +type UnmountMode int + +const ( + UnmountRegular UnmountMode = iota + UnmountFloat + UnmountSink +) + func getDevName(name string) string { return "/dev/mapper/" + name } @@ -290,7 +308,7 @@ func (devices *DeviceSet) setupBaseImage() error { if oldInfo != nil && !oldInfo.Initialized { utils.Debugf("Removing uninitialized base image") - if err := devices.removeDevice(""); err != nil { + if err := devices.deleteDevice(""); err != nil { utils.Debugf("\n--->Err: %s\n", err) return err } @@ -355,6 +373,10 @@ func (devices *DeviceSet) log(level int, file string, line int, dmError int, mes return // Ignore _LOG_DEBUG } + if strings.Contains(message, "busy") { + devices.sawBusy = true + } + utils.Debugf("libdevmapper(%d): %s:%d (%d) %s", level, file, line, dmError, message) } @@ -562,7 +584,7 @@ func (devices *DeviceSet) AddDevice(hash, baseHash string) error { return nil } -func (devices *DeviceSet) removeDevice(hash string) error { +func (devices *DeviceSet) deleteDevice(hash string) error { info := devices.Devices[hash] if info == nil { return fmt.Errorf("hash %s doesn't exists", hash) @@ -579,7 +601,7 @@ func (devices *DeviceSet) removeDevice(hash string) error { devinfo, _ := getInfo(info.Name()) if devinfo != nil && devinfo.Exists != 0 { - if err := removeDevice(info.Name()); err != nil { + if err := devices.removeDeviceAndWait(info.Name()); err != nil { utils.Debugf("Error removing device: %s\n", err) return err } @@ -610,11 +632,11 @@ func (devices *DeviceSet) removeDevice(hash string) error { return nil } -func (devices *DeviceSet) RemoveDevice(hash string) error { +func (devices *DeviceSet) DeleteDevice(hash string) error { devices.Lock() defer devices.Unlock() - return devices.removeDevice(hash) + return devices.deleteDevice(hash) } func (devices *DeviceSet) deactivateDevice(hash string) error { @@ -632,28 +654,50 @@ func (devices *DeviceSet) deactivateDevice(hash string) error { return err } if devinfo.Exists != 0 { - if err := removeDevice(devname); err != nil { + if err := devices.removeDeviceAndWait(devname); err != nil { utils.Debugf("\n--->Err: %s\n", err) return err } - if err := devices.waitRemove(hash); err != nil { - return err - } } return nil } -// waitRemove blocks until either: -// a) the device registered at - is removed, -// or b) the 1 second timeout expires. -func (devices *DeviceSet) waitRemove(hash string) error { - utils.Debugf("[deviceset %s] waitRemove(%s)", devices.devicePrefix, hash) - defer utils.Debugf("[deviceset %s] waitRemove(%) END", devices.devicePrefix, hash) - devname, err := devices.byHash(hash) +// Issues the underlying dm remove operation and then waits +// for it to finish. +func (devices *DeviceSet) removeDeviceAndWait(devname string) error { + var err error + + for i := 0; i < 10; i++ { + devices.sawBusy = false + err = removeDevice(devname) + if err == nil { + break + } + if !devices.sawBusy { + return err + } + + // If we see EBUSY it may be a transient error, + // sleep a bit a retry a few times. + time.Sleep(5 * time.Millisecond) + } if err != nil { return err } + + if err := devices.waitRemove(devname); err != nil { + return err + } + return nil +} + +// waitRemove blocks until either: +// a) the device registered at - is removed, +// or b) the 1 second timeout expires. +func (devices *DeviceSet) waitRemove(devname string) error { + utils.Debugf("[deviceset %s] waitRemove(%s)", devices.devicePrefix, devname) + defer utils.Debugf("[deviceset %s] waitRemove(%s) END", devices.devicePrefix, devname) i := 0 for ; i < 1000; i += 1 { devinfo, err := getInfo(devname) @@ -728,13 +772,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 { @@ -756,22 +799,35 @@ 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 == nil { + return fmt.Errorf("Unknown device %s", 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, "") @@ -780,20 +836,53 @@ 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 info == nil { + return fmt.Errorf("UnmountDevice: no such device %s\n", 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 } @@ -804,15 +893,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 } @@ -955,9 +1038,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 664899cfb..25b998e59 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 } - return d.DeviceSet.RemoveDevice(id) + // 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 785845ce6..ff4ec45e5 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", ) }()