Skip to content

Commit

Permalink
Ensure volumes can be removed when they fail to unmount
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
mheon committed Oct 14, 2019
1 parent a8993ba commit 0f6b0e8
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 1 deletion.
4 changes: 4 additions & 0 deletions libpod/define/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
9 changes: 8 additions & 1 deletion libpod/runtime_volume_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
25 changes: 25 additions & 0 deletions libpod/volume_internal_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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())
Expand Down

0 comments on commit 0f6b0e8

Please sign in to comment.