From a760e325f3180638f9fedd0ee79d4c6695d8ba64 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Tue, 3 Sep 2019 15:03:44 -0400 Subject: Add ability for volumes with options to mount/umount When volume options and the local volume driver are specified, the volume is intended to be mounted using the 'mount' command. Supported options will be used to volume the volume before the first container using it starts, and unmount the volume after the last container using it dies. This should work for any local filesystem, though at present I've only tested with tmpfs and btrfs. Signed-off-by: Matthew Heon --- libpod/volume_internal.go | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) (limited to 'libpod/volume_internal.go') diff --git a/libpod/volume_internal.go b/libpod/volume_internal.go index 35f0ca19d..8c11b907f 100644 --- a/libpod/volume_internal.go +++ b/libpod/volume_internal.go @@ -3,6 +3,8 @@ package libpod import ( "os" "path/filepath" + + "github.com/containers/libpod/libpod/define" ) // Creates a new volume @@ -20,3 +22,25 @@ func newVolume(runtime *Runtime) (*Volume, error) { func (v *Volume) teardownStorage() error { return os.RemoveAll(filepath.Join(v.runtime.config.VolumePath, v.Name())) } + +// Volumes with options set, or a filesystem type, or a device to mount need to +// be mounted and unmounted. +func (v *Volume) needsMount() bool { + return len(v.config.Options) > 0 && v.config.Driver == "local" +} + +// update() updates the volume state from the DB. +func (v *Volume) update() error { + if err := v.runtime.state.UpdateVolume(v); err != nil { + return err + } + if !v.valid { + return define.ErrVolumeRemoved + } + return nil +} + +// save() saves the volume state to the DB +func (v *Volume) save() error { + return v.runtime.state.SaveVolume(v) +} -- cgit v1.2.3-54-g00ecf From de9a394fcff19ae4422a3f65502c8790787351fd Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Thu, 5 Sep 2019 10:00:50 -0400 Subject: Correctly report errors on unmounting SHM When we fail to remove a container's SHM, that's an error, and we need to report it as such. This may be part of our lingering storage woes. Also, remove MNT_DETACH. It may be another cause of the storage removal failures. Signed-off-by: Matthew Heon --- libpod/boltdb_state.go | 3 +-- libpod/container_internal.go | 21 +++++++++++++-------- libpod/container_internal_linux.go | 8 ++++---- libpod/define/config.go | 4 ++++ libpod/runtime_volume_linux.go | 4 ++-- libpod/volume_internal.go | 2 +- libpod/volume_internal_linux.go | 2 +- pkg/varlinkapi/volumes.go | 3 +-- 8 files changed, 27 insertions(+), 20 deletions(-) (limited to 'libpod/volume_internal.go') diff --git a/libpod/boltdb_state.go b/libpod/boltdb_state.go index acba61f2a..31b551b36 100644 --- a/libpod/boltdb_state.go +++ b/libpod/boltdb_state.go @@ -1403,11 +1403,10 @@ func (s *BoltState) AddVolume(volume *Volume) error { // Volume state is allowed to not exist var volStateJSON []byte if volume.state != nil { - stateJSON, err := json.Marshal(volume.state) + volStateJSON, err = json.Marshal(volume.state) if err != nil { return errors.Wrapf(err, "error marshalling volume %s state to JSON", volume.Name()) } - volStateJSON = stateJSON } db, err := s.getDBCon() diff --git a/libpod/container_internal.go b/libpod/container_internal.go index 67503f148..ffc6c11ee 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -1253,10 +1253,10 @@ func (c *Container) mountStorage() (_ string, Err error) { return } vol.lock.Lock() - defer vol.lock.Unlock() if err := vol.unmount(false); err != nil { logrus.Errorf("Error unmounting volume %s after error mounting container %s: %v", vol.Name(), c.ID(), err) } + vol.lock.Unlock() }() } } @@ -1281,14 +1281,19 @@ func (c *Container) cleanupStorage() error { return nil } + var cleanupErr error + for _, containerMount := range c.config.Mounts { if err := c.unmountSHM(containerMount); err != nil { - return err + if cleanupErr != nil { + logrus.Errorf("Error unmounting container %s: %v", c.ID(), cleanupErr) + } + cleanupErr = err } } if c.config.Rootfs != "" { - return nil + return cleanupErr } if err := c.unmount(false); err != nil { @@ -1298,14 +1303,14 @@ func (c *Container) cleanupStorage() error { // state if errors.Cause(err) == storage.ErrNotAContainer || errors.Cause(err) == storage.ErrContainerUnknown { logrus.Errorf("Storage for container %s has been removed", c.ID()) - return nil + } else { + if cleanupErr != nil { + logrus.Errorf("Error cleaning up container %s storage: %v", c.ID(), cleanupErr) + } + cleanupErr = err } - - return err } - var cleanupErr error - // Request an unmount of all named volumes for _, v := range c.config.NamedVolumes { vol, err := c.runtime.state.Volume(v.Name) diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index 33c57729b..a84db9aa5 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -49,12 +49,12 @@ func (c *Container) mountSHM(shmOptions string) error { } func (c *Container) unmountSHM(mount string) error { - if err := unix.Unmount(mount, unix.MNT_DETACH); err != nil { + if err := unix.Unmount(mount, 0); err != nil { if err != syscall.EINVAL { - logrus.Warnf("container %s failed to unmount %s : %v", c.ID(), mount, err) - } else { - logrus.Debugf("container %s failed to unmount %s : %v", c.ID(), mount, err) + return errors.Wrapf(err, "error unmounting container %s SHM mount %s", c.ID(), mount) } + // If it's just an EINVAL, debug logs only + logrus.Debugf("container %s failed to unmount %s : %v", c.ID(), mount, err) } return nil } diff --git a/libpod/define/config.go b/libpod/define/config.go index d8d6ccf55..c66f67feb 100644 --- a/libpod/define/config.go +++ b/libpod/define/config.go @@ -18,3 +18,7 @@ type InfoData struct { Type string Data map[string]interface{} } + +// VolumeDriverLocal is the "local" volume driver. It is managed by libpod +// itself. +const VolumeDriverLocal = "local" diff --git a/libpod/runtime_volume_linux.go b/libpod/runtime_volume_linux.go index cf8055c7b..9df93faf3 100644 --- a/libpod/runtime_volume_linux.go +++ b/libpod/runtime_volume_linux.go @@ -44,11 +44,11 @@ func (r *Runtime) newVolume(ctx context.Context, options ...VolumeCreateOption) volume.config.Name = stringid.GenerateNonCryptoID() } if volume.config.Driver == "" { - volume.config.Driver = "local" + volume.config.Driver = define.VolumeDriverLocal } volume.config.CreatedTime = time.Now() - if volume.config.Driver == "local" { + if volume.config.Driver == define.VolumeDriverLocal { logrus.Debugf("Validating options for local driver") // Validate options for key := range volume.config.Options { diff --git a/libpod/volume_internal.go b/libpod/volume_internal.go index 8c11b907f..2e886e1b0 100644 --- a/libpod/volume_internal.go +++ b/libpod/volume_internal.go @@ -26,7 +26,7 @@ func (v *Volume) teardownStorage() error { // Volumes with options set, or a filesystem type, or a device to mount need to // be mounted and unmounted. func (v *Volume) needsMount() bool { - return len(v.config.Options) > 0 && v.config.Driver == "local" + return len(v.config.Options) > 0 && v.config.Driver == define.VolumeDriverLocal } // update() updates the volume state from the DB. diff --git a/libpod/volume_internal_linux.go b/libpod/volume_internal_linux.go index 68ddb3c0d..9ae4dcf69 100644 --- a/libpod/volume_internal_linux.go +++ b/libpod/volume_internal_linux.go @@ -67,7 +67,7 @@ func (v *Volume) mount() error { errPipe, err := mountCmd.StderrPipe() if err != nil { - return errors.Wrapf(err, "getting stderr pipe") + return errors.Wrapf(err, "error getting stderr pipe for mount") } if err := mountCmd.Start(); err != nil { out, err2 := ioutil.ReadAll(errPipe) diff --git a/pkg/varlinkapi/volumes.go b/pkg/varlinkapi/volumes.go index a27950b21..6dd86d831 100644 --- a/pkg/varlinkapi/volumes.go +++ b/pkg/varlinkapi/volumes.go @@ -67,8 +67,7 @@ func (i *LibpodAPI) GetVolumes(call iopodman.VarlinkCall, args []string, all boo Labels: v.Labels(), MountPoint: v.MountPoint(), Name: v.Name(), - // TODO change types here to be correct - //Options: v.Options(), + Options: v.Options(), } volumes = append(volumes, newVol) } -- cgit v1.2.3-54-g00ecf