restore: fix ignoring --manage-cgroups-mode

Merge the logic of setPageServer, setManageCgroupsMode, and
setEmptyNsMask into criuOptions. This does three things:

1. Fixes ignoring --manage-cgroups-mode on restore;
2. Simplifies the code in checkpoint.go and restore.go;
3. Ensures issues like 1 won't happen again.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>

checkpoint.go

@@ -67,17 +67,6 @@ checkpointed.`,
 			return err
 		}
 
-		// these are the mandatory criu options for a container
-		if err := setPageServer(context, options); err != nil {
-			return err
-		}
-		if err := setManageCgroupsMode(context, options); err != nil {
-			return err
-		}
-		if err := setEmptyNsMask(context, options); err != nil {
-			return err
-		}
-
 		err = container.Checkpoint(options)
 		if err == nil && !(options.LeaveRunning || options.PreDump) {
 			// Destroy the container unless we tell CRIU to keep it.
@@ -119,59 +108,78 @@ func prepareImagePaths(context *cli.Context) (string, string, error) {
 	return imagePath, parentPath, nil
 }
 
-func setPageServer(context *cli.Context, options *libcontainer.CriuOpts) error {
-	// xxx following criu opts are optional
-	// The dump image can be sent to a criu page server
+func criuOptions(context *cli.Context) (*libcontainer.CriuOpts, error) {
+	imagePath, parentPath, err := prepareImagePaths(context)
+	if err != nil {
+		return nil, err
+	}
+
+	opts := &libcontainer.CriuOpts{
+		ImagesDirectory:         imagePath,
+		WorkDirectory:           context.String("work-path"),
+		ParentImage:             parentPath,
+		LeaveRunning:            context.Bool("leave-running"),
+		TcpEstablished:          context.Bool("tcp-established"),
+		ExternalUnixConnections: context.Bool("ext-unix-sk"),
+		ShellJob:                context.Bool("shell-job"),
+		FileLocks:               context.Bool("file-locks"),
+		PreDump:                 context.Bool("pre-dump"),
+		AutoDedup:               context.Bool("auto-dedup"),
+		LazyPages:               context.Bool("lazy-pages"),
+		StatusFd:                context.Int("status-fd"),
+		LsmProfile:              context.String("lsm-profile"),
+		LsmMountContext:         context.String("lsm-mount-context"),
+	}
+
+	// CRIU options below may or may not be set.
+
 	if psOpt := context.String("page-server"); psOpt != "" {
 		address, port, err := net.SplitHostPort(psOpt)
 
 		if err != nil || address == "" || port == "" {
-			return errors.New("Use --page-server ADDRESS:PORT to specify page server")
+			return nil, errors.New("Use --page-server ADDRESS:PORT to specify page server")
 		}
 		portInt, err := strconv.Atoi(port)
 		if err != nil {
-			return errors.New("Invalid port number")
+			return nil, errors.New("Invalid port number")
 		}
-		options.PageServer = libcontainer.CriuPageServerInfo{
+		opts.PageServer = libcontainer.CriuPageServerInfo{
 			Address: address,
 			Port:    int32(portInt),
 		}
 	}
-	return nil
-}
 
-func setManageCgroupsMode(context *cli.Context, options *libcontainer.CriuOpts) error {
 	if cgOpt := context.String("manage-cgroups-mode"); cgOpt != "" {
 		switch cgOpt {
 		case "soft":
-			options.ManageCgroupsMode = criu.CriuCgMode_SOFT
+			opts.ManageCgroupsMode = criu.CriuCgMode_SOFT
 		case "full":
-			options.ManageCgroupsMode = criu.CriuCgMode_FULL
+			opts.ManageCgroupsMode = criu.CriuCgMode_FULL
 		case "strict":
-			options.ManageCgroupsMode = criu.CriuCgMode_STRICT
+			opts.ManageCgroupsMode = criu.CriuCgMode_STRICT
 		default:
-			return errors.New("Invalid manage cgroups mode")
+			return nil, errors.New("Invalid manage cgroups mode")
 		}
 	}
-	return nil
-}
 
-var namespaceMapping = map[specs.LinuxNamespaceType]int{
-	specs.NetworkNamespace: unix.CLONE_NEWNET,
-}
-
-func setEmptyNsMask(context *cli.Context, options *libcontainer.CriuOpts) error {
-	/* Runc doesn't manage network devices and their configuration */
+	// runc doesn't manage network devices and their configuration.
 	nsmask := unix.CLONE_NEWNET
 
-	for _, ns := range context.StringSlice("empty-ns") {
-		f, exists := namespaceMapping[specs.LinuxNamespaceType(ns)]
-		if !exists {
-			return fmt.Errorf("namespace %q is not supported", ns)
+	if context.IsSet("empty-ns") {
+		namespaceMapping := map[specs.LinuxNamespaceType]int{
+			specs.NetworkNamespace: unix.CLONE_NEWNET,
+		}
+
+		for _, ns := range context.StringSlice("empty-ns") {
+			f, exists := namespaceMapping[specs.LinuxNamespaceType(ns)]
+			if !exists {
+				return nil, fmt.Errorf("namespace %q is not supported", ns)
+			}
+			nsmask |= f
 		}
-		nsmask |= f
 	}
 
-	options.EmptyNs = uint32(nsmask)
-	return nil
+	opts.EmptyNs = uint32(nsmask)
+
+	return opts, nil
 }

restore.go

@@ -3,7 +3,6 @@ package main
 import (
 	"os"
 
-	"github.com/opencontainers/runc/libcontainer"
 	"github.com/opencontainers/runc/libcontainer/userns"
 	"github.com/sirupsen/logrus"
 	"github.com/urfave/cli"
@@ -113,9 +112,6 @@ using the runc checkpoint command.`,
 		if err != nil {
 			return err
 		}
-		if err := setEmptyNsMask(context, options); err != nil {
-			return err
-		}
 		status, err := startContainer(context, CT_ACT_RESTORE, options)
 		if err != nil {
 			return err
@@ -126,27 +122,3 @@ using the runc checkpoint command.`,
 		return nil
 	},
 }
-
-func criuOptions(context *cli.Context) (*libcontainer.CriuOpts, error) {
-	imagePath, parentPath, err := prepareImagePaths(context)
-	if err != nil {
-		return nil, err
-	}
-
-	return &libcontainer.CriuOpts{
-		ImagesDirectory:         imagePath,
-		WorkDirectory:           context.String("work-path"),
-		ParentImage:             parentPath,
-		LeaveRunning:            context.Bool("leave-running"),
-		TcpEstablished:          context.Bool("tcp-established"),
-		ExternalUnixConnections: context.Bool("ext-unix-sk"),
-		ShellJob:                context.Bool("shell-job"),
-		FileLocks:               context.Bool("file-locks"),
-		PreDump:                 context.Bool("pre-dump"),
-		AutoDedup:               context.Bool("auto-dedup"),
-		LazyPages:               context.Bool("lazy-pages"),
-		StatusFd:                context.Int("status-fd"),
-		LsmProfile:              context.String("lsm-profile"),
-		LsmMountContext:         context.String("lsm-mount-context"),
-	}, nil
-}