Ensure volumes can be removed when they fail to unmount

Also, ensure that we don't try to mount them without root - it
appears that it can somehow not error and report that mount was
successful when it clearly did not succeed, which can induce this
case.

We reuse the `--force` flag to indicate that a volume should be
removed even after unmount errors. It seems fairly natural to
expect that --force will remove a volume that is otherwise
presenting problems.

Finally, ignore EINVAL on unmount - if the mount point no longer
exists our job is done.

Fixes: #4247
Fixes: #4248

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
This commit is contained in:
Matthew Heon 2019-10-14 10:29:54 -04:00
parent a8993bab78
commit 0f6b0e8c9c
3 changed files with 37 additions and 1 deletions

View File

@ -65,6 +65,10 @@ var (
// CGroup.
ErrNoCgroups = errors.New("this container does not have a cgroup")
// ErrRootless indicates that the given command cannot but run without
// root.
ErrRootless = errors.New("operation requires root privileges")
// ErrRuntimeStopped indicates that the runtime has already been shut
// down and no further operations can be performed on it
ErrRuntimeStopped = errors.New("runtime has already been stopped")

View File

@ -157,7 +157,14 @@ func (r *Runtime) removeVolume(ctx context.Context, v *Volume, force bool) error
// If the volume is still mounted - force unmount it
if err := v.unmount(true); err != nil {
return errors.Wrapf(err, "error unmounting volume %s", v.Name())
if force {
// If force is set, evict the volume, even if errors
// occur. Otherwise we'll never be able to get rid of
// them.
logrus.Errorf("Error unmounting volume %s: %v", v.Name(), err)
} else {
return errors.Wrapf(err, "error unmounting volume %s", v.Name())
}
}
// Set volume as invalid so it can no longer be used

View File

@ -6,6 +6,8 @@ import (
"io/ioutil"
"os/exec"
"github.com/containers/libpod/libpod/define"
"github.com/containers/libpod/pkg/rootless"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"golang.org/x/sys/unix"
@ -24,6 +26,11 @@ func (v *Volume) mount() error {
return nil
}
// We cannot mount volumes as rootless.
if rootless.IsRootless() {
return errors.Wrapf(define.ErrRootless, "cannot mount volumes without root privileges")
}
// Update the volume from the DB to get an accurate mount counter.
if err := v.update(); err != nil {
return err
@ -108,6 +115,20 @@ func (v *Volume) unmount(force bool) error {
return nil
}
// We cannot unmount volumes as rootless.
if rootless.IsRootless() {
// If force is set, just clear the counter and bail without
// error, so we can remove volumes from the state if they are in
// an awkward configuration.
if force {
logrus.Errorf("Volume %s is mounted despite being rootless - state is not sane", v.Name())
v.state.MountCount = 0
return v.save()
}
return errors.Wrapf(define.ErrRootless, "cannot mount or unmount volumes without root privileges")
}
if !force {
v.state.MountCount = v.state.MountCount - 1
} else {
@ -119,6 +140,10 @@ func (v *Volume) unmount(force bool) error {
if v.state.MountCount == 0 {
// Unmount the volume
if err := unix.Unmount(v.config.MountPoint, unix.MNT_DETACH); err != nil {
if err == unix.EINVAL {
// Ignore EINVAL - the mount no longer exists.
return nil
}
return errors.Wrapf(err, "error unmounting volume %s", v.Name())
}
logrus.Debugf("Unmounted volume %s", v.Name())