Merge pull request #2798 from adrianreber/2021-02-08-nested-bind.mounts

Correctly restore containers with nested bind mounts

libcontainer/container_linux.go

@@ -1236,11 +1236,38 @@ func (c *linuxContainer) prepareCriuRestoreMounts(mounts []*configs.Mount) error
 	// Now go through all mounts and create the mountpoints
 	// if the mountpoints are not on a tmpfs, as CRIU will
 	// restore the complete tmpfs content from its checkpoint.
+	umounts := []string{}
+	defer func() {
+		for _, u := range umounts {
+			if e := unix.Unmount(u, unix.MNT_DETACH); e != nil {
+				if e != unix.EINVAL {
+					// Ignore EINVAL as it means 'target is not a mount point.'
+					// It probably has already been unmounted.
+					logrus.Warnf("Error during cleanup unmounting of %q (%v)", u, e)
+				}
+			}
+		}
+	}()
 	for _, m := range mounts {
 		if !isPathInPrefixList(m.Destination, tmpfs) {
 			if err := c.makeCriuRestoreMountpoints(m); err != nil {
 				return err
 			}
+			// If the mount point is a bind mount, we need to mount
+			// it now so that runc can create the necessary mount
+			// points for mounts in bind mounts.
+			// This also happens during initial container creation.
+			// Without this CRIU restore will fail
+			// See: https://github.com/opencontainers/runc/issues/2748
+			// It is also not necessary to order the mount points
+			// because during initial container creation mounts are
+			// set up in the order they are configured.
+			if m.Device == "bind" {
+				if err := unix.Mount(m.Source, m.Destination, "", unix.MS_BIND|unix.MS_REC, ""); err != nil {
+					return errorsf.Wrapf(err, "unable to bind mount %q to %q", m.Source, m.Destination)
+				}
+				umounts = append(umounts, m.Destination)
+			}
 		}
 	}
 	return nil

tests/integration/checkpoint.bats

@@ -313,3 +313,45 @@ function simple_cr() {
 	unlink "$tmp"
 	test -f ./work-dir/"$tmplog2" && unlink ./work-dir/"$tmplog2"
 }
+
+@test "checkpoint and restore with nested bind mounts" {
+	bind1=$(mktemp -d -p .)
+	bind2=$(mktemp -d -p .)
+	update_config '	  .mounts += [{
+					type: "bind",
+					source: "'"$bind1"'",
+					destination: "/test",
+					options: ["rw", "bind"]
+				},
+	                        {
+					type: "bind",
+					source: "'"$bind2"'",
+					destination: "/test/for/nested",
+					options: ["rw", "bind"]
+				}]'
+
+	runc run -d --console-socket "$CONSOLE_SOCKET" test_busybox
+	[ "$status" -eq 0 ]
+
+	testcontainer test_busybox running
+
+	# checkpoint the running container
+	runc --criu "$CRIU" checkpoint --work-path ./work-dir test_busybox
+	grep -B 5 Error ./work-dir/dump.log || true
+	[ "$status" -eq 0 ]
+
+	# after checkpoint busybox is no longer running
+	testcontainer test_busybox checkpointed
+
+	# cleanup mountpoints created by runc during creation
+	# the mountpoints should be recreated during restore - that is the actual thing tested here
+	rm -rf "${bind1:?}"/*
+
+	# restore from checkpoint
+	runc --criu "$CRIU" restore -d --work-path ./work-dir --console-socket "$CONSOLE_SOCKET" test_busybox
+	grep -B 5 Error ./work-dir/restore.log || true
+	[ "$status" -eq 0 ]
+
+	# busybox should be back up and running
+	testcontainer test_busybox running
+}