mirror of
https://github.com/moby/moby.git
synced 2022-11-09 12:21:53 -05:00
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 db3576f8a0
)
Signed-off-by: Victor Vieux <victorvieux@gmail.com>
This commit is contained in:
parent
c4bd13b650
commit
d1c9e9cfe9
2 changed files with 32 additions and 7 deletions
|
@ -88,8 +88,8 @@ func (s *DockerDaemonSuite) TestDaemonRestartWithVolumesRefs(c *check.C) {
|
||||||
|
|
||||||
s.d.Restart(c)
|
s.d.Restart(c)
|
||||||
|
|
||||||
if _, err := s.d.Cmd("run", "-d", "--volumes-from", "volrestarttest1", "--name", "volrestarttest2", "busybox", "top"); err != nil {
|
if out, err := s.d.Cmd("run", "-d", "--volumes-from", "volrestarttest1", "--name", "volrestarttest2", "busybox", "top"); err != nil {
|
||||||
c.Fatal(err)
|
c.Fatal(err, out)
|
||||||
}
|
}
|
||||||
|
|
||||||
if out, err := s.d.Cmd("rm", "-fv", "volrestarttest2"); err != nil {
|
if out, err := s.d.Cmd("rm", "-fv", "volrestarttest2"); err != nil {
|
||||||
|
|
|
@ -218,6 +218,14 @@ func (r *Root) Remove(v volume.Volume) error {
|
||||||
return fmt.Errorf("unknown volume type %T", v)
|
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)
|
realPath, err := filepath.EvalSymlinks(lv.path)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
if !os.IsNotExist(err) {
|
if !os.IsNotExist(err) {
|
||||||
|
@ -306,6 +314,7 @@ func (v *localVolume) Path() string {
|
||||||
}
|
}
|
||||||
|
|
||||||
// Mount implements the localVolume interface, returning the data location.
|
// 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) {
|
func (v *localVolume) Mount(id string) (string, error) {
|
||||||
v.m.Lock()
|
v.m.Lock()
|
||||||
defer v.m.Unlock()
|
defer v.m.Unlock()
|
||||||
|
@ -321,19 +330,35 @@ func (v *localVolume) Mount(id string) (string, error) {
|
||||||
return v.path, nil
|
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 {
|
func (v *localVolume) Unmount(id string) error {
|
||||||
v.m.Lock()
|
v.m.Lock()
|
||||||
defer v.m.Unlock()
|
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 {
|
if v.opts != nil {
|
||||||
v.active.count--
|
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)
|
return errors.Wrapf(err, "error while unmounting volume path '%s'", v.path)
|
||||||
}
|
}
|
||||||
v.active.mounted = false
|
|
||||||
}
|
}
|
||||||
|
v.active.mounted = false
|
||||||
}
|
}
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue