From eb2f08dc4e68d794ec80c8d867d953b3f7660b67 Mon Sep 17 00:00:00 2001
From: Kir Kolyshkin <kolyshkin@gmail.com>
Date: Thu, 10 Feb 2022 18:52:08 -0800
Subject: [PATCH] checkpoint,restore,list: don't call fatal

There is a mix of styles when handling CLI commands. In most cases we
return an error, which is handled from app.Run in main.go (it calls
fatal if there is an error).

In a few cases, though, we call fatal(err) from random places.

Let's be consistent and always return an error. The only exception is
runc exec, which needs to exit with a particular exit code.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
---
 checkpoint.go | 28 +++++++++++++++++++---------
 list.go       |  4 ++--
 restore.go    | 11 +++++++----
 3 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/checkpoint.go b/checkpoint.go
index 32a62a8bc..7597e552d 100644
--- a/checkpoint.go
+++ b/checkpoint.go
@@ -60,16 +60,24 @@ checkpointed.`,
 			return err
 		}
 		if status == libcontainer.Created || status == libcontainer.Stopped {
-			fatal(fmt.Errorf("Container cannot be checkpointed in %s state", status.String()))
+			return fmt.Errorf("Container cannot be checkpointed in %s state", status.String())
 		}
-		options := criuOptions(context)
+		options, err := criuOptions(context)
+		if err != nil {
+			return err
+		}
+
 		if !(options.LeaveRunning || options.PreDump) {
 			// destroy container unless we tell CRIU to keep it
 			defer destroy(container)
 		}
 		// these are the mandatory criu options for a container
-		setPageServer(context, options)
-		setManageCgroupsMode(context, options)
+		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
 		}
@@ -109,27 +117,28 @@ func prepareImagePaths(context *cli.Context) (string, string, error) {
 	return imagePath, parentPath, nil
 }
 
-func setPageServer(context *cli.Context, options *libcontainer.CriuOpts) {
+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
 	if psOpt := context.String("page-server"); psOpt != "" {
 		address, port, err := net.SplitHostPort(psOpt)
 
 		if err != nil || address == "" || port == "" {
-			fatal(errors.New("Use --page-server ADDRESS:PORT to specify page server"))
+			return errors.New("Use --page-server ADDRESS:PORT to specify page server")
 		}
 		portInt, err := strconv.Atoi(port)
 		if err != nil {
-			fatal(errors.New("Invalid port number"))
+			return errors.New("Invalid port number")
 		}
 		options.PageServer = libcontainer.CriuPageServerInfo{
 			Address: address,
 			Port:    int32(portInt),
 		}
 	}
+	return nil
 }
 
-func setManageCgroupsMode(context *cli.Context, options *libcontainer.CriuOpts) {
+func setManageCgroupsMode(context *cli.Context, options *libcontainer.CriuOpts) error {
 	if cgOpt := context.String("manage-cgroups-mode"); cgOpt != "" {
 		switch cgOpt {
 		case "soft":
@@ -139,9 +148,10 @@ func setManageCgroupsMode(context *cli.Context, options *libcontainer.CriuOpts)
 		case "strict":
 			options.ManageCgroupsMode = criu.CriuCgMode_STRICT
 		default:
-			fatal(errors.New("Invalid manage cgroups mode"))
+			return errors.New("Invalid manage cgroups mode")
 		}
 	}
+	return nil
 }
 
 var namespaceMapping = map[specs.LinuxNamespaceType]int{
diff --git a/list.go b/list.go
index 5aa9bebdd..2a1b0adc7 100644
--- a/list.go
+++ b/list.go
@@ -117,7 +117,7 @@ func getContainers(context *cli.Context) ([]containerState, error) {
 	root := context.GlobalString("root")
 	list, err := os.ReadDir(root)
 	if err != nil {
-		fatal(err)
+		return nil, err
 	}
 
 	var s []containerState
@@ -131,7 +131,7 @@ func getContainers(context *cli.Context) ([]containerState, error) {
 				// Possible race with runc delete.
 				continue
 			}
-			fatal(err)
+			return nil, err
 		}
 		// This cast is safe on Linux.
 		uid := st.Sys().(*syscall.Stat_t).Uid
diff --git a/restore.go b/restore.go
index 59d2904ec..ccd1b232b 100644
--- a/restore.go
+++ b/restore.go
@@ -109,7 +109,10 @@ using the runc checkpoint command.`,
 			logrus.Warn("runc checkpoint is untested with rootless containers")
 		}
 
-		options := criuOptions(context)
+		options, err := criuOptions(context)
+		if err != nil {
+			return err
+		}
 		if err := setEmptyNsMask(context, options); err != nil {
 			return err
 		}
@@ -124,10 +127,10 @@ using the runc checkpoint command.`,
 	},
 }
 
-func criuOptions(context *cli.Context) *libcontainer.CriuOpts {
+func criuOptions(context *cli.Context) (*libcontainer.CriuOpts, error) {
 	imagePath, parentPath, err := prepareImagePaths(context)
 	if err != nil {
-		fatal(err)
+		return nil, err
 	}
 
 	return &libcontainer.CriuOpts{
@@ -145,5 +148,5 @@ func criuOptions(context *cli.Context) *libcontainer.CriuOpts {
 		StatusFd:                context.Int("status-fd"),
 		LsmProfile:              context.String("lsm-profile"),
 		LsmMountContext:         context.String("lsm-mount-context"),
-	}
+	}, nil
 }