Let containers/storage keep track of mounts

Currently we unmount storage that is still in use.
We should not be unmounting storeage that we mounted
via a different command or by podman mount. This
change relies on containers/storage to umount keep track of
how many times the storage was mounted before really unmounting
it from the system.

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>

cmd/podman/umount.go

@@ -58,7 +58,7 @@ func umountCmd(c *cli.Context) error {
 				continue
 			}
 
-			if err = unmountContainer(ctr); err != nil {
+			if err = ctr.Unmount(); err != nil {
 				if lastError != nil {
 					logrus.Error(lastError)
 				}
@@ -78,7 +78,7 @@ func umountCmd(c *cli.Context) error {
 				continue
 			}
 
-			if err = unmountContainer(ctr); err != nil {
+			if err = ctr.Unmount(); err != nil {
 				if lastError != nil {
 					logrus.Error(lastError)
 				}
@@ -90,12 +90,3 @@ func umountCmd(c *cli.Context) error {
 	}
 	return lastError
 }
-
-func unmountContainer(ctr *libpod.Container) error {
-	if mounted, err := ctr.Mounted(); mounted {
-		return ctr.Unmount()
-	} else {
-		return err
-	}
-	return errors.Errorf("container is not mounted")
-}

libpod/container_api.go

@@ -437,11 +437,7 @@ func (c *Container) Mount() (string, error) {
 		}
 	}
 
-	if err := c.mountStorage(); err != nil {
+	return c.mount()
-		return "", err
-	}
-
-	return c.state.Mountpoint, nil
 }
 
 // Unmount unmounts a container's filesystem on the host
@@ -456,15 +452,24 @@ func (c *Container) Unmount() error {
 	}
 
 	if c.state.State == ContainerStateRunning || c.state.State == ContainerStatePaused {
-		return errors.Wrapf(ErrCtrStateInvalid, "cannot remove storage for container %s as it is running or paused", c.ID())
+		return errors.Wrapf(ErrCtrStateInvalid, "cannot unmount storage for container %s as it is running or paused", c.ID())
 	}
 
 	// Check if we have active exec sessions
 	if len(c.state.ExecSessions) != 0 {
-		return errors.Wrapf(ErrCtrStateInvalid, "container %s has active exec sessions, refusing to clean up", c.ID())
+		return errors.Wrapf(ErrCtrStateInvalid, "container %s has active exec sessions, refusing to unmount", c.ID())
 	}
 
-	return c.cleanupStorage()
+	if c.state.Mounted {
+		mounted, err := c.runtime.storageService.MountedContainerImage(c.ID())
+		if err != nil {
+			return errors.Wrapf(err, "can't determine how many times %s is mounted, refusing to unmount", c.ID())
+		}
+		if mounted == 1 {
+			return errors.Wrapf(err, "can't unmount %s last mount, it is still in use", c.ID())
+		}
+	}
+	return c.unmount()
 }
 
 // Pause pauses a container

libpod/container_internal.go

@@ -753,9 +753,9 @@ func (c *Container) mountStorage() (err error) {
 
 	mountPoint := c.config.Rootfs
 	if mountPoint == "" {
-		mountPoint, err = c.runtime.storageService.MountContainerImage(c.ID())
+		mountPoint, err = c.mount()
 		if err != nil {
-			return errors.Wrapf(err, "error mounting storage for container %s", c.ID())
+			return err
 		}
 	}
 	c.state.Mounted = true
@@ -796,8 +796,7 @@ func (c *Container) cleanupStorage() error {
 		return nil
 	}
 
-	// Also unmount storage
+	if err := c.unmount(); err != nil {
-	if _, err := c.runtime.storageService.UnmountContainerImage(c.ID()); err != nil {
 		// If the container has already been removed, warn but don't
 		// error
 		// We still want to be able to kick the container out of the
@@ -807,7 +806,7 @@ func (c *Container) cleanupStorage() error {
 			return nil
 		}
 
-		return errors.Wrapf(err, "error unmounting container %s root filesystem", c.ID())
+		return err
 	}
 
 	c.state.Mountpoint = ""
@@ -1285,3 +1284,22 @@ func (c *Container) setupOCIHooks(ctx context.Context, config *spec.Spec) (exten
 
 	return manager.Hooks(config, c.Spec().Annotations, len(c.config.UserVolumes) > 0)
 }
+
+// mount mounts the container's root filesystem
+func (c *Container) mount() (string, error) {
+	mountPoint, err := c.runtime.storageService.MountContainerImage(c.ID())
+	if err != nil {
+		return "", errors.Wrapf(err, "error mounting storage for container %s", c.ID())
+	}
+	return mountPoint, nil
+}
+
+// unmount unmounts the container's root filesystem
+func (c *Container) unmount() error {
+	// Also unmount storage
+	if _, err := c.runtime.storageService.UnmountContainerImage(c.ID()); err != nil {
+		return errors.Wrapf(err, "error unmounting container %s root filesystem", c.ID())
+	}
+
+	return nil
+}

libpod/storage.go

@@ -248,6 +248,21 @@ func (r *storageService) UnmountContainerImage(idOrName string) (bool, error) {
 	return mounted, nil
 }
 
+func (r *storageService) MountedContainerImage(idOrName string) (int, error) {
+	if idOrName == "" {
+		return 0, ErrEmptyID
+	}
+	container, err := r.store.Container(idOrName)
+	if err != nil {
+		return 0, err
+	}
+	mounted, err := r.store.Mounted(container.ID)
+	if err != nil {
+		return 0, err
+	}
+	return mounted, nil
+}
+
 func (r *storageService) GetWorkDir(id string) (string, error) {
 	container, err := r.store.Container(id)
 	if err != nil {