Ensure unmount before removing local volume.
When there is an error unmounting a local volume, it is still possible
to call `Remove()` on the volume causing removal of the mounted
resources which is generally not desirable.
This ensures that resources are unmounted before attempting removal.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
(cherry picked from commit db3576f8a08ca70287bd3fdf9b21e162537f9d3a)
Signed-off-by: Victor Vieux <victorvieux@gmail.com>
diff --git a/integration-cli/docker_cli_daemon_test.go b/integration-cli/docker_cli_daemon_test.go
index bab94d8..0e7cb27 100644
--- a/integration-cli/docker_cli_daemon_test.go
+++ b/integration-cli/docker_cli_daemon_test.go
@@ -88,8 +88,8 @@
s.d.Restart(c)
- if _, err := s.d.Cmd("run", "-d", "--volumes-from", "volrestarttest1", "--name", "volrestarttest2", "busybox", "top"); err != nil {
- c.Fatal(err)
+ if out, err := s.d.Cmd("run", "-d", "--volumes-from", "volrestarttest1", "--name", "volrestarttest2", "busybox", "top"); err != nil {
+ c.Fatal(err, out)
}
if out, err := s.d.Cmd("rm", "-fv", "volrestarttest2"); err != nil {
diff --git a/volume/local/local.go b/volume/local/local.go
index d29559d..6631423 100644
--- a/volume/local/local.go
+++ b/volume/local/local.go
@@ -218,6 +218,14 @@
return fmt.Errorf("unknown volume type %T", v)
}
+ if lv.active.count > 0 {
+ return fmt.Errorf("volume has active mounts")
+ }
+
+ if err := lv.unmount(); err != nil {
+ return err
+ }
+
realPath, err := filepath.EvalSymlinks(lv.path)
if err != nil {
if !os.IsNotExist(err) {
@@ -306,6 +314,7 @@
}
// Mount implements the localVolume interface, returning the data location.
+// If there are any provided mount options, the resources will be mounted at this point
func (v *localVolume) Mount(id string) (string, error) {
v.m.Lock()
defer v.m.Unlock()
@@ -321,19 +330,35 @@
return v.path, nil
}
-// Umount is for satisfying the localVolume interface and does not do anything in this driver.
+// Unmount dereferences the id, and if it is the last reference will unmount any resources
+// that were previously mounted.
func (v *localVolume) Unmount(id string) error {
v.m.Lock()
defer v.m.Unlock()
+
+ // Always decrement the count, even if the unmount fails
+ // Essentially docker doesn't care if this fails, it will send an error, but
+ // ultimately there's nothing that can be done. If we don't decrement the count
+ // this volume can never be removed until a daemon restart occurs.
if v.opts != nil {
v.active.count--
- if v.active.count == 0 {
- if err := mount.Unmount(v.path); err != nil {
- v.active.count++
+ }
+
+ if v.active.count > 0 {
+ return nil
+ }
+
+ return v.unmount()
+}
+
+func (v *localVolume) unmount() error {
+ if v.opts != nil {
+ if err := mount.Unmount(v.path); err != nil {
+ if mounted, mErr := mount.Mounted(v.path); mounted || mErr != nil {
return errors.Wrapf(err, "error while unmounting volume path '%s'", v.path)
}
- v.active.mounted = false
}
+ v.active.mounted = false
}
return nil
}
