From ff3b4f3bb4a5d1adf0de646b73eee495e1cf9a69 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 1 Aug 2022 15:44:44 -0700 Subject: [PATCH] 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 --- checkpoint.go | 86 ++++++++++++++++++++++++++++----------------------- restore.go | 28 ----------------- 2 files changed, 47 insertions(+), 67 deletions(-) diff --git a/checkpoint.go b/checkpoint.go index b8bfa045d..ac32bbb50 100644 --- a/checkpoint.go +++ b/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 } diff --git a/restore.go b/restore.go index ccd1b232b..30ce89a2d 100644 --- a/restore.go +++ b/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 -}