[PATCH 00/13][user-cr]: Cleanups checkpoint.c, restart.c

[PATCH 00/13][user-cr]: Cleanups checkpoint.c, restart.c

[Sukadev Bhattiprolu]

[PATCH 00/13][user-cr]: Cleanups checkpoint.c, restart.c

These patches perform some cleanups/reorg of the code in checkpoint.c and
restart.c files.  These changes remove redundant fields in data structures,
rename some fields/functions and avoid directly using stdout/stderr.

The changes will help us later export app_checkpoint() and app_restart()
to larger container implementations such as LXC.

	[PATCH 01/13][user-cr]: restart: Remove args->no_pidns.
	[PATCH 02/13][user-cr]: restart: Rename args->logfd
	[PATCH 03/13][user-cr]: restart: Remove args->logfile 
	[PATCH 04/13][user-cr]: restart: Use ckpt_msg() to log
	[PATCH 05/13][user-cr]: restart: Use ckpt_perror()
	[PATCH 06/13][user-cr]: restart: Remove args->input
	[PATCH 07/13][user-cr]: Define app_restart()
	[PATCH 08/13][user-cr]: restart: Rename struct args.
	[PATCH 09/13][user-cr]: checkpoint: Remove args->output
	[PATCH 10/13][user-cr]: checkpoint: Remove ->logfile
	[PATCH 11/13][user-cr]: checkpoint: Rename struct args
	[PATCH 12/13][user-cr]: Define app_checkpoint()
	[PATCH 13/13] checkpoint: Use ckpt_perror()

Signed-off-by: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>

Re: [PATCH 00/13][user-cr]: Cleanups checkpoint.c, restart.c

[Sukadev Bhattiprolu]

From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Date: Mon, 1 Mar 2010 18:19:44 -0800
Subject: [PATCH 01/13][user-cr]: restart: Remove args->no_pidns.

args->no_pidns does not really need to be a field in the 'args' structure.
It is used to validate command line arguments and can just be a local
variable in parse_args().

Signed-off-by: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
 restart.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/restart.c b/restart.c
index f5d23bb..61abfaa 100644
--- a/restart.c
+++ b/restart.c
@@ -342,7 +342,6 @@ struct args {
 	int self;
 	int pids;
 	int pidns;
-	int no_pidns;
 	int inspect;
 	char *root;
 	int wait;
@@ -455,6 +454,7 @@ static void parse_args(struct args *args, int argc, char *argv[])
 
 	int optind;
 	int sig;
+	int no_pidns;
 
 	/* defaults */
 	memset(args, 0, sizeof(*args));
@@ -463,6 +463,7 @@ static void parse_args(struct args *args, int argc, char *argv[])
 	args->logfd = -1;
 	args->warn = CKPT_COND_WARN;
 	args->fail = CKPT_COND_FAIL;
+	no_pidns = 0;
 
 	while (1) {
 		int c = getopt_long(argc, argv, optc, opts, &optind);
@@ -503,7 +504,7 @@ static void parse_args(struct args *args, int argc, char *argv[])
 			args->pidns = 1;
 			break;
 		case 'P':
-			args->no_pidns = 1;
+			no_pidns = 1;
 			break;
 		case 6:  /* --self */
 			args->self = 1;
@@ -565,7 +566,7 @@ static void parse_args(struct args *args, int argc, char *argv[])
 		}
 	}
 
-	if (args->no_pidns)
+	if (no_pidns)
 		args->pidns = 0;
 
 #ifndef CLONE_NEWPID
@@ -598,7 +599,7 @@ static void parse_args(struct args *args, int argc, char *argv[])
 #endif
 
 	if (args->self &&
-	    (args->pids || args->pidns || args->no_pidns ||
+	    (args->pids || args->pidns || no_pidns ||
 	     args->show_status || args->copy_status || args->freezer)) {
 		printf("Invalid mix of --self with multiprocess options\n");
 		exit(1);
-- 
1.6.0.4

[PATCH 02/13][user-cr]: restart: Rename args->logfd

[Sukadev Bhattiprolu]

From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Date: Mon, 1 Mar 2010 18:24:51 -0800
Subject: [PATCH 02/13][user-cr]: restart: Rename args->logfd

User-space restart code needs at least two different log files. The restart()
system call code in the kernel writes useful messages to one (currently,
args->logfd).

The user-space restart code writes messages to the other (currently via stdout/
stderr). Note that the user-level restart code creates several processes and
some processes can be in user-space and others in the kernel (sys_restart()).
If both kernel and user-space portions of the restart code logged messages to
the same file, the output would be garbled and hard to use.

To use different file descriptors for the kernel and user-space, first rename
the current ->logfd field to ->klogfd.

Signed-off-by: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
 restart.c |   26 +++++++++++++-------------
 1 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/restart.c b/restart.c
index 61abfaa..7697189 100644
--- a/restart.c
+++ b/restart.c
@@ -191,9 +191,9 @@ static int str2sig(char *str)
 	return -1;
 }
 
-inline static int restart(pid_t pid, int fd, unsigned long flags, int logfd)
+inline static int restart(pid_t pid, int fd, unsigned long flags, int klogfd)
 {
-	return syscall(__NR_restart, pid, fd, flags, logfd);
+	return syscall(__NR_restart, pid, fd, flags, klogfd);
 }
 
 #define BUFSIZE  (4 * 4096)
@@ -353,7 +353,7 @@ struct args {
 	char *input;
 	int infd;
 	char *logfile;
-	int logfd;
+	int klogfd;
 	long warn;
 	long fail;
 	int keep_lsm;
@@ -460,7 +460,7 @@ static void parse_args(struct args *args, int argc, char *argv[])
 	memset(args, 0, sizeof(*args));
 	args->wait = 1;
 	args->infd = -1;
-	args->logfd = -1;
+	args->klogfd = -1;
 	args->warn = CKPT_COND_WARN;
 	args->fail = CKPT_COND_FAIL;
 	no_pidns = 0;
@@ -494,8 +494,8 @@ static void parse_args(struct args *args, int argc, char *argv[])
 			args->logfile = optarg;
 			break;
 		case 8:
-			args->logfd = str2num(optarg);
-			if (args->logfd < 0) {
+			args->klogfd = str2num(optarg);
+			if (args->klogfd < 0) {
 				printf("restart: invalid file descriptor\n");
 				exit(1);
 			}
@@ -610,7 +610,7 @@ static void parse_args(struct args *args, int argc, char *argv[])
 		exit(1);
 	}
 
-	if (args->logfile && args->logfd >= 0) {
+	if (args->logfile && args->klogfd >= 0) {
 		printf("Invalid used of both -l/--logfile and --logfile-fd\n");
 		exit(1);
 	}
@@ -783,17 +783,17 @@ int main(int argc, char *argv[])
 
 	/* (optional) log file */
 	if (args.logfile) {
-		args.logfd = open(args.logfile,
+		args.klogfd = open(args.logfile,
 				  O_RDWR | O_CREAT | O_EXCL, 0644);
-		if (args.logfd < 0) {
+		if (args.klogfd < 0) {
 			perror("open log file");
 			exit(1);
 		}
 	}
 
 	/* output file descriptor (default: none) */
-	if (args.logfd < 0)
-		args.logfd = CHECKPOINT_FD_NONE;
+	if (args.klogfd < 0)
+		args.klogfd = CHECKPOINT_FD_NONE;
 
 	/* freezer preparation */
 	if (args.freezer && freezer_prepare(&ctx) < 0)
@@ -817,7 +817,7 @@ int main(int argc, char *argv[])
 
 	/* self-restart ends here: */
 	if (args.self) {
-		restart(getpid(), STDIN_FILENO, RESTART_TASKSELF, args.logfd);
+		restart(getpid(), STDIN_FILENO, RESTART_TASKSELF, args.klogfd);
 		/* reach here if restart(2) failed ! */
 		perror("restart");
 		exit(1);
@@ -1202,7 +1202,7 @@ static int ckpt_coordinator(struct ckpt_ctx *ctx)
 	if (ctx->args->keep_lsm)
 		flags |= RESTART_KEEP_LSM;
 
-	ret = restart(root_pid, STDIN_FILENO, flags, ctx->args->logfd);
+	ret = restart(root_pid, STDIN_FILENO, flags, ctx->args->klogfd);
 
 	if (ret < 0) {
 		perror("restart failed");
-- 
1.6.0.4

[PATCH 03/13][user-cr]: restart: Remove args->logfile

[Sukadev Bhattiprolu]

From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Date: Mon, 1 Mar 2010 18:45:59 -0800
Subject: [PATCH 03/13][user-cr]: restart: Remove args->logfile 

The args->logfile field is used to store the the name of the logfile to
which the kernel must write its output. If we move the code that opens this
args->logfile from main() to parse_args() and use the existing args->klogfd
field, we can remove the ->logfile field from struct args.

Signed-off-by: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
 restart.c |   29 ++++++++++++++++-------------
 1 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/restart.c b/restart.c
index 7697189..1839330 100644
--- a/restart.c
+++ b/restart.c
@@ -352,7 +352,6 @@ struct args {
 	char *freezer;
 	char *input;
 	int infd;
-	char *logfile;
 	int klogfd;
 	long warn;
 	long fail;
@@ -456,6 +455,8 @@ static void parse_args(struct args *args, int argc, char *argv[])
 	int sig;
 	int no_pidns;
 
+	char *klogfile;
+
 	/* defaults */
 	memset(args, 0, sizeof(*args));
 	args->wait = 1;
@@ -465,6 +466,8 @@ static void parse_args(struct args *args, int argc, char *argv[])
 	args->fail = CKPT_COND_FAIL;
 	no_pidns = 0;
 
+	klogfile = NULL;
+
 	while (1) {
 		int c = getopt_long(argc, argv, optc, opts, &optind);
 		if (c == -1)
@@ -491,7 +494,7 @@ static void parse_args(struct args *args, int argc, char *argv[])
 			}
 			break;
 		case 'l':
-			args->logfile = optarg;
+			klogfile = optarg;
 			break;
 		case 8:
 			args->klogfd = str2num(optarg);
@@ -610,11 +613,21 @@ static void parse_args(struct args *args, int argc, char *argv[])
 		exit(1);
 	}
 
-	if (args->logfile && args->klogfd >= 0) {
+	if (klogfile && args->klogfd >= 0) {
 		printf("Invalid used of both -l/--logfile and --logfile-fd\n");
 		exit(1);
 	}
 
+	/* (optional) log file */
+	if (klogfile) {
+		args->klogfd = open(klogfile, O_RDWR | O_CREAT | O_EXCL, 0644);
+		if (args->klogfd < 0) {
+			perror("open log file");
+			exit(1);
+		}
+	}
+
+
 	if (args->mnt_pty)
 		args->mntns = 1;
 }
@@ -781,16 +794,6 @@ int main(int argc, char *argv[])
 			close(args.infd);
 	}
 
-	/* (optional) log file */
-	if (args.logfile) {
-		args.klogfd = open(args.logfile,
-				  O_RDWR | O_CREAT | O_EXCL, 0644);
-		if (args.klogfd < 0) {
-			perror("open log file");
-			exit(1);
-		}
-	}
-
 	/* output file descriptor (default: none) */
 	if (args.klogfd < 0)
 		args.klogfd = CHECKPOINT_FD_NONE;
-- 
1.6.0.4

[PATCH 04/13][user-cr]: restart: Use ckpt_msg() to log

[Sukadev Bhattiprolu]

From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Date: Mon, 1 Mar 2010 20:04:12 -0800
Subject: [PATCH 04/13][user-cr]: restart: Use ckpt_msg() to log

Rather than implicitly redirecting messages to 'stdout' or 'stderr'
use global variables, 'global_ulogfd' and 'global_uerrfd'. That would
make it easier to implement additional command line options to redirect
these fds to other files, If we also later implement a library interface
to this restart code, it would enable callers to specify fds other than
'stdout' and 'stderr'.

Signed-off-by: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
 restart.c |   90 ++++++++++++++++++++++++++++++++++++++++++------------------
 1 files changed, 63 insertions(+), 27 deletions(-)

diff --git a/restart.c b/restart.c
index 1839330..c4c2158 100644
--- a/restart.c
+++ b/restart.c
@@ -31,6 +31,7 @@
 #include <sys/syscall.h>
 #include <sys/prctl.h>
 #include <sys/mount.h>
+#include <stdarg.h>
 
 #include <linux/sched.h>
 #include <linux/checkpoint.h>
@@ -101,16 +102,38 @@ static char usage_str[] =
  * of the checkpoint image stream.
  */
 
+#define BUFSIZE  (4 * 4096)
+
+static inline void ckpt_msg(int fd, char *format, ...)
+{
+	va_list ap;
+	char *bufp;
+	if (fd < 0)
+		return;
+
+	va_start(ap, format);
+
+	bufp = malloc(BUFSIZE);
+	if(bufp) {
+		vsnprintf(bufp, BUFSIZE, format, ap);
+		write(fd, bufp, strlen(bufp));
+	}
+	free(bufp);
+
+	va_end(ap);
+}
+
 #ifdef CHECKPOINT_DEBUG
 #define ckpt_dbg(_format, _args...)					\
 	do {								\
 		if (global_debug)					\
-			fprintf(stderr, "<%d>" _format, _gettid(), ##_args); \
+			ckpt_msg(global_uerrfd, "<%d>" _format, 	\
+					_gettid(), ##_args); 		\
 	} while (0)
-#define ckpt_dbg_cont(_format, _args...)			\
-	do {							\
-		if (global_debug)				\
-			fprintf(stderr, _format, ##_args);	\
+#define ckpt_dbg_cont(_format, _args...)				\
+	do {								\
+		if (global_debug)					\
+			ckpt_msg(global_uerrfd, _format, ##_args);	\
 	} while (0)
 #else
 #define ckpt_dbg(_format, _args...)  \
@@ -120,12 +143,12 @@ static char usage_str[] =
 #endif
 
 #define ckpt_err(...)  \
-	fprintf(stderr, __VA_ARGS__)
+	ckpt_msg(global_uerrfd, __VA_ARGS__)
 
-#define ckpt_verbose(...)			\
-	do {					\
-		if (global_verbose)		\
-			printf(__VA_ARGS__);	\
+#define ckpt_verbose(...)					\
+	do {							\
+		if (global_verbose)				\
+			ckpt_msg(global_ulogfd, __VA_ARGS__);	\
 	} while(0)
 
 #define SIGNAL_ENTRY(signal)  { SIG ## signal, #signal }
@@ -196,8 +219,6 @@ inline static int restart(pid_t pid, int fd, unsigned long flags, int klogfd)
 	return syscall(__NR_restart, pid, fd, flags, klogfd);
 }
 
-#define BUFSIZE  (4 * 4096)
-
 struct hashent {
 	long key;
 	void *data;
@@ -270,6 +291,12 @@ struct ckpt_ctx {
 	char *freezer;
 };
 
+/*
+ * TODO: Do we need to direct user-space restart messages to two different
+ * 	 fds (like stdout and stderr) or can we just use one ?
+ */
+int global_ulogfd;
+int global_uerrfd;
 int global_debug;
 int global_verbose;
 pid_t global_child_pid;
@@ -372,7 +399,7 @@ struct args {
 
 static void usage(char *str)
 {
-	fprintf(stderr, "%s", str);
+	ckpt_err("%s", str);
 	exit(1);
 }
 
@@ -406,7 +433,7 @@ static long cond_to_mask(const char *cond)
 		if (!strcmp(cond, conditions[i].cond))
 			return conditions[i].mask;
 
-	printf("restart: invalid warn/fail condition '%s'\n", cond);
+	ckpt_err("restart: invalid warn/fail condition '%s'\n", cond);
 	exit(1);
 }
 
@@ -489,7 +516,7 @@ static void parse_args(struct args *args, int argc, char *argv[])
 		case 7:
 			args->infd = str2num(optarg);
 			if (args->infd < 0) {
-				printf("restart: invalid file descriptor\n");
+				ckpt_err("restart: invalid file descriptor\n");
 				exit(1);
 			}
 			break;
@@ -499,7 +526,7 @@ static void parse_args(struct args *args, int argc, char *argv[])
 		case 8:
 			args->klogfd = str2num(optarg);
 			if (args->klogfd < 0) {
-				printf("restart: invalid file descriptor\n");
+				ckpt_err("restart: invalid file descriptor\n");
 				exit(1);
 			}
 			break;
@@ -517,7 +544,7 @@ static void parse_args(struct args *args, int argc, char *argv[])
 			if (sig < 0)
 				sig = str2num(optarg);
 			if (sig < 0 || sig >= NSIG) {
-				printf("restart: invalid signal\n");
+				ckpt_err("restart: invalid signal\n");
 				exit(1);
 			}
 			global_send_sigint = sig;
@@ -574,7 +601,7 @@ static void parse_args(struct args *args, int argc, char *argv[])
 
 #ifndef CLONE_NEWPID
 	if (args->pidns) {
-		printf("This version of restart was compiled without "
+		ckpt_err("This version of restart was compiled without "
 		       "support for --pidns.\n");
 		exit(1);
 	}
@@ -582,7 +609,7 @@ static void parse_args(struct args *args, int argc, char *argv[])
 
 #ifndef CHECKPOINT_DEBUG
 	if (global_debug) {
-		printf("This version of restart was compiled without "
+		ckpt_err("This version of restart was compiled without "
 		       "support for --debug.\n");
 		exit(1);
 	}
@@ -594,7 +621,7 @@ static void parse_args(struct args *args, int argc, char *argv[])
 #if 0   /* Defered until __NR_eclone makes it to standard headers */
 #ifndef __NR_eclone
 	if (args->pids) {
-		printf("This version of restart was compiled without "
+		ckpt_err("This version of restart was compiled without "
 		       "support for --pids.\n");
 		exit(1);
 	}
@@ -604,17 +631,17 @@ static void parse_args(struct args *args, int argc, char *argv[])
 	if (args->self &&
 	    (args->pids || args->pidns || no_pidns ||
 	     args->show_status || args->copy_status || args->freezer)) {
-		printf("Invalid mix of --self with multiprocess options\n");
+		ckpt_err("Invalid mix of --self with multiprocess options\n");
 		exit(1);
 	}
 
 	if (args->input && args->infd >= 0) {
-		printf("Invalid used of both -i/--input and --input-fd\n");
+		ckpt_err("Invalid use of both -i/--input and --input-fd\n");
 		exit(1);
 	}
 
 	if (klogfile && args->klogfd >= 0) {
-		printf("Invalid used of both -l/--logfile and --logfile-fd\n");
+		ckpt_err("Invalid use of both -l/--logfile and --logfile-fd\n");
 		exit(1);
 	}
 
@@ -770,6 +797,15 @@ int main(int argc, char *argv[])
 	struct args args;
 	int ret;
 
+	/*
+	 * Initialize the log/error fds early so even parse_args() errors
+	 * are redirected here. Even if we later implement command line options
+	 * that override these, any errors/messages that occur before those
+	 * new options are parsed still go to stdout/stderr
+	 */
+	global_ulogfd = fileno(stdout);
+	global_uerrfd = fileno(stderr);
+
 	memset(&ctx, 0, sizeof(ctx));
 
 	parse_args(&args, argc, argv);
@@ -891,16 +927,16 @@ static int ckpt_parse_status(int status, int mimic, int verbose)
 	int ret = 0;
 
 	if (verbose && global_sent_sigint)
-		printf("Terminated\n");
+		ckpt_verbose("Terminated\n");
 	if (WIFSIGNALED(status)) {
 		sig = WTERMSIG(status);
 		if (verbose && !global_sent_sigint)
-			printf("Killed %d\n", sig);
+			ckpt_verbose("Killed %d\n", sig);
 		ckpt_dbg("task terminated with signal %d\n", sig);
 	} else if (WIFEXITED(status)) {
 		ret = WEXITSTATUS(status);
 		if (verbose)
-			printf("Exited %d\n", ret);
+			ckpt_verbose("Exited %d\n", ret);
 		ckpt_dbg("task exited with status %d\n", ret);
 	}
 
@@ -1172,7 +1208,7 @@ static int ckpt_coordinator_pidns(struct ckpt_ctx *ctx)
 #else
 static int ckpt_coordinator_pidns(struct ckpt_ctx *ctx)
 {
-	printf("logical error: ckpt_coordinator_pidns unexpected\n");
+	ckpt_err("logical error: ckpt_coordinator_pidns unexpected\n");
 	exit(1);
 }
 #endif
-- 
1.6.0.4

[PATCH 05/13][user-cr]: restart: Use ckpt_perror()

[Sukadev Bhattiprolu]

From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Date: Mon, 1 Mar 2010 20:18:31 -0800
Subject: [PATCH 05/13][user-cr]: restart: Use ckpt_perror()

Remove the implicit dependency on 'stderr' and redirect messages to
global_uerrfd. This would simplify implementing command line options
for these file descriptors and also enable implementing the restart
functionality as a library interface

Signed-off-by: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
 restart.c |   92 ++++++++++++++++++++++++++++++++----------------------------
 1 files changed, 49 insertions(+), 43 deletions(-)

diff --git a/restart.c b/restart.c
index c4c2158..eda864d 100644
--- a/restart.c
+++ b/restart.c
@@ -123,6 +123,12 @@ static inline void ckpt_msg(int fd, char *format, ...)
 	va_end(ap);
 }
 
+#define ckpt_perror(s) 							\
+	do {								\
+		ckpt_msg(global_uerrfd, s);				\
+		ckpt_msg(global_uerrfd, ": %s\n", strerror(errno));	\
+	} while (0)
+
 #ifdef CHECKPOINT_DEBUG
 #define ckpt_dbg(_format, _args...)					\
 	do {								\
@@ -649,7 +655,7 @@ static void parse_args(struct args *args, int argc, char *argv[])
 	if (klogfile) {
 		args->klogfd = open(klogfile, O_RDWR | O_CREAT | O_EXCL, 0644);
 		if (args->klogfd < 0) {
-			perror("open log file");
+			ckpt_perror("open log file");
 			exit(1);
 		}
 	}
@@ -701,7 +707,7 @@ static void sigchld_handler(int sig)
 		} else if (errno == ECHILD) {
 			break;
 		} else {
-			perror("WEIRD !! child collection failed");
+			ckpt_perror("WEIRD !! child collection failed");
 			exit(1);
 		}
 	}
@@ -741,7 +747,7 @@ static int freezer_prepare(struct ckpt_ctx *ctx)
 
 	freezer = malloc(strlen(ctx->args->freezer) + 32);
 	if (!freezer) {
-		perror("malloc freezer buf");
+		ckpt_perror("malloc freezer buf");
 		return -1;
 	}
 
@@ -749,13 +755,13 @@ static int freezer_prepare(struct ckpt_ctx *ctx)
 
 	fd = open(freezer, O_WRONLY, 0);
 	if (fd < 0) {
-		perror("freezer path");
+		ckpt_perror("freezer path");
 		free(freezer);
 		exit(1);
 	}
 	ret = write(fd, FREEZER_THAWED, sizeof(FREEZER_THAWED)); 
 	if (ret != sizeof(FREEZER_THAWED)) {
-		perror("thawing freezer");
+		ckpt_perror("thawing freezer");
 		free(freezer);
 		exit(1);
 	}
@@ -774,14 +780,14 @@ static int freezer_register(struct ckpt_ctx *ctx, pid_t pid)
 
 	fd = open(ctx->freezer, O_WRONLY, 0);
 	if (fd < 0) {
-		perror("freezer path");
+		ckpt_perror("freezer path");
 		return -1;
 	}
 
 	n = sprintf(pidstr, "%d", pid);
 	ret = write(fd, pidstr, n);
 	if (ret != n) {
-		perror("adding pid %d to freezer");
+		ckpt_perror("adding pid %d to freezer");
 		ret = -1;
 	} else {
 		ret = 0;
@@ -815,7 +821,7 @@ int main(int argc, char *argv[])
 	if (args.input) {
 		args.infd = open(args.input, O_RDONLY, 0);
 		if (args.infd < 0) {
-			perror("open input file");
+			ckpt_perror("open input file");
 			exit(1);
 		}
 	}
@@ -823,7 +829,7 @@ int main(int argc, char *argv[])
 	/* input file descriptor (default: stdin) */
 	if (args.infd >= 0) {
 		if (dup2(args.infd, STDIN_FILENO) < 0) {
-			perror("dup2 input file");
+			ckpt_perror("dup2 input file");
 			exit(1);
 		}
 		if (args.infd != STDIN_FILENO)
@@ -840,13 +846,13 @@ int main(int argc, char *argv[])
 
 	/* private mounts namespace ? */
 	if (args.mntns && unshare(CLONE_NEWNS | CLONE_FS) < 0) {
-		perror("unshare");
+		ckpt_perror("unshare");
 		exit(1);
 	}
 
 	/* chroot ? */
 	if (args.root && chroot(args.root) < 0) {
-		perror("chroot");
+		ckpt_perror("chroot");
 		exit(1);
 	}
 
@@ -858,7 +864,7 @@ int main(int argc, char *argv[])
 	if (args.self) {
 		restart(getpid(), STDIN_FILENO, RESTART_TASKSELF, args.klogfd);
 		/* reach here if restart(2) failed ! */
-		perror("restart");
+		ckpt_perror("restart");
 		exit(1);
 	}
 
@@ -866,25 +872,25 @@ int main(int argc, char *argv[])
 
 	ret = ckpt_read_header(&ctx);
 	if (ret < 0) {
-		perror("read c/r header");
+		ckpt_perror("read c/r header");
 		exit(1);
 	}
 		
 	ret = ckpt_read_header_arch(&ctx);
 	if (ret < 0) {
-		perror("read c/r header arch");
+		ckpt_perror("read c/r header arch");
 		exit(1);
 	}
 
 	ret = ckpt_read_container(&ctx);
 	if (ret < 0) {
-		perror("read c/r container section");
+		ckpt_perror("read c/r container section");
 		exit(1);
 	}
 
 	ret = ckpt_read_tree(&ctx);
 	if (ret < 0) {
-		perror("read c/r tree");
+		ckpt_perror("read c/r tree");
 		exit(1);
 	}
 
@@ -978,7 +984,7 @@ static int ckpt_collect_child(struct ckpt_ctx *ctx)
 	if (global_child_collected) {
 		status = global_child_status;
 	} else if (pid < 0) {
-		perror("WEIRD: collect child task");
+		ckpt_perror("WEIRD: collect child task");
 		exit(1);
 	}
 
@@ -991,7 +997,7 @@ static int ckpt_remount_devpts(struct ckpt_ctx *ctx)
 
 	/* make sure /dev/ptmx is a link else we'll just break */
 	if (lstat("/dev/ptmx", &ptystat) < 0) {
-		perror("stat /dev/ptmx");
+		ckpt_perror("stat /dev/ptmx");
 		return -1;
 	}
 	if ((ptystat.st_mode & S_IFMT) != S_IFLNK) {
@@ -1002,7 +1008,7 @@ static int ckpt_remount_devpts(struct ckpt_ctx *ctx)
 	/* this is unlikely, but maybe we don't want to fail */
 	if (umount2("/dev/pts", MNT_DETACH) < 0) {
 		if (ckpt_cond_fail(ctx, CKPT_COND_MNTPTY)) {
-			perror("umount -l /dev/pts");
+			ckpt_perror("umount -l /dev/pts");
 			return -1;
 		}
 		if (ckpt_cond_warn(ctx, CKPT_COND_MNTPTY))
@@ -1010,7 +1016,7 @@ static int ckpt_remount_devpts(struct ckpt_ctx *ctx)
 	}
 	if (mount("pts", "/dev/pts", "devpts", 0,
 		  "ptmxmode=666,newinstance") < 0) {
-		perror("mount -t devpts -o newinstance");
+		ckpt_perror("mount -t devpts -o newinstance");
 		return -1;
 	}
 
@@ -1092,20 +1098,20 @@ static int ckpt_probe_child(pid_t pid, char *str)
 static int ckpt_remount_proc(struct ckpt_ctx *ctx)
 {
 	if (unshare(CLONE_NEWNS | CLONE_FS) < 0) {
-		perror("unshare");
+		ckpt_perror("unshare");
 		return -1;
 	}
 	/* this is unlikely, but we don't want to fail */
 	if (umount2("/proc", MNT_DETACH) < 0) {
 		if (ckpt_cond_fail(ctx, CKPT_COND_MNTPROC)) {
-			perror("umount -l /proc");
+			ckpt_perror("umount -l /proc");
 			return -1;
 		}
 		if (ckpt_cond_warn(ctx, CKPT_COND_MNTPROC))
 			ckpt_err("[warn] failed to un-mount old /proc\n");
 	}
 	if (mount("proc", "/proc", "proc", 0, NULL) < 0) {
-		perror("mount -t proc");
+		ckpt_perror("mount -t proc");
 		return -1;
 	}
 
@@ -1135,7 +1141,7 @@ static int ckpt_coordinator_status(struct ckpt_ctx *ctx)
 
 	ret = read(ctx->pipe_coord[0], &status, sizeof(status));
 	if (ret < 0)
-		perror("read coordinator status");
+		ckpt_perror("read coordinator status");
 	else if (ret == 0) {
 		/* coordinator failed to report */
 		ckpt_dbg("Coordinator failed to report status.");
@@ -1162,13 +1168,13 @@ static int ckpt_coordinator_pidns(struct ckpt_ctx *ctx)
 	 * root-task's return value).
 	 */
 	if (pipe(ctx->pipe_coord) < 0) {
-		perror("pipe");
+		ckpt_perror("pipe");
 		return -1;
 	}
 
 	stk = genstack_alloc(PTHREAD_STACK_MIN);
 	if (!stk) {
-		perror("coordinator genstack_alloc");
+		ckpt_perror("coordinator genstack_alloc");
 		return -1;
 	}
 	sp = genstack_sp(stk);
@@ -1179,7 +1185,7 @@ static int ckpt_coordinator_pidns(struct ckpt_ctx *ctx)
 	coord_pid = clone(__ckpt_coordinator, sp, CLONE_NEWPID|SIGCHLD, ctx);
 	genstack_release(stk);
 	if (coord_pid < 0) {
-		perror("clone coordinator");
+		ckpt_perror("clone coordinator");
 		return coord_pid;
 	}
 	global_child_pid = coord_pid;
@@ -1244,7 +1250,7 @@ static int ckpt_coordinator(struct ckpt_ctx *ctx)
 	ret = restart(root_pid, STDIN_FILENO, flags, ctx->args->klogfd);
 
 	if (ret < 0) {
-		perror("restart failed");
+		ckpt_perror("restart failed");
 		ckpt_verbose("Failed\n");
 		ckpt_dbg("restart failed ?\n");
 		exit(1);
@@ -1258,7 +1264,7 @@ static int ckpt_coordinator(struct ckpt_ctx *ctx)
 	if (ctx->args->pidns && ctx->tasks_arr[0].pid != 1) {
 		/* Report success/failure to the parent */
 		if (write(ctx->pipe_coord[1], &ret, sizeof(ret)) < 0) {
-			perror("failed to report status");
+			ckpt_perror("failed to report status");
 			exit(1);
 		}
 
@@ -1314,7 +1320,7 @@ static int ckpt_build_tree(struct ckpt_ctx *ctx)
 	ctx->tasks_max = ctx->pids_nr * 4;
 	ctx->tasks_arr = malloc(sizeof(*ctx->tasks_arr) * ctx->tasks_max);
 	if (!ctx->tasks_arr) {
-		perror("malloc tasks array");
+		ckpt_perror("malloc tasks array");
 		return -1;
 	}
 
@@ -1915,7 +1921,7 @@ static int ckpt_make_tree(struct ckpt_ctx *ctx, struct task *task)
 	if (task->pid == task->sid) {
 		ret = setsid();
 		if (ret < 0 && task != ckpt_init_task(ctx)) {
-			perror("setsid");
+			ckpt_perror("setsid");
 			return -1;
 		}
 	}
@@ -1948,7 +1954,7 @@ static int ckpt_make_tree(struct ckpt_ctx *ctx, struct task *task)
 	swap.new = _gettid();
 	ret = write(ctx->pipe_out, &swap, sizeof(swap));
 	if (ret != sizeof(swap)) {
-		perror("write swap");
+		ckpt_perror("write swap");
 		return -1;
 	}
 	close(ctx->pipe_out);
@@ -1975,7 +1981,7 @@ static int ckpt_make_tree(struct ckpt_ctx *ctx, struct task *task)
 	ckpt_dbg("about to call sys_restart(), flags %#lx\n", flags);
 	ret = restart(0, STDIN_FILENO, flags, CHECKPOINT_FD_NONE);
 	if (ret < 0)
-		perror("task restore failed");
+		ckpt_perror("task restore failed");
 	return ret;
 }
 
@@ -2007,7 +2013,7 @@ int ckpt_fork_stub(void *data)
 	 */
 	if (!ctx->args->pidns) {
 		if (prctl(PR_SET_PDEATHSIG, SIGKILL, 0, 0, 0) < 0) {
-			perror("prctl");
+			ckpt_perror("prctl");
 			return -1;
 		}
 		if (getppid() != task->real_parent) {
@@ -2044,7 +2050,7 @@ static pid_t ckpt_fork_child(struct ckpt_ctx *ctx, struct task *child)
 
 	stk = genstack_alloc(PTHREAD_STACK_MIN);
 	if (!stk) {
-		perror("ckpt_fork_child genstack_alloc");
+		ckpt_perror("ckpt_fork_child genstack_alloc");
 		return -1;
 	}
 
@@ -2078,7 +2084,7 @@ static pid_t ckpt_fork_child(struct ckpt_ctx *ctx, struct task *child)
 
 	pid = eclone(ckpt_fork_stub, child, flags, &clone_args, &pid);
 	if (pid < 0) {
-		perror("eclone");
+		ckpt_perror("eclone");
 		genstack_release(stk);
 		return -1;
 	}
@@ -2105,12 +2111,12 @@ static int ckpt_fork_feeder(struct ckpt_ctx *ctx)
 	pid_t pid;
 
 	if (pipe(ctx->pipe_feed)) {
-		perror("pipe");
+		ckpt_perror("pipe");
 		exit(1);
 	}
 
 	if (pipe(ctx->pipe_child) < 0) {
-		perror("pipe");
+		ckpt_perror("pipe");
 		exit(1);
 	}
 
@@ -2122,14 +2128,14 @@ static int ckpt_fork_feeder(struct ckpt_ctx *ctx)
 
 	stk = genstack_alloc(PTHREAD_STACK_MIN);
 	if (!stk) {
-		perror("ckpt_fork_feeder genstack_alloc");
+		ckpt_perror("ckpt_fork_feeder genstack_alloc");
 		return -1;
 	}
 
 	pid = clone(ckpt_do_feeder, genstack_sp(stk),
 		    CLONE_THREAD | CLONE_SIGHAND | CLONE_VM, ctx);
 	if (pid < 0) {
-		perror("feeder thread");
+		ckpt_perror("feeder thread");
 		return -1;
 	}
 
@@ -2148,7 +2154,7 @@ static int ckpt_fork_feeder(struct ckpt_ctx *ctx)
 
 static void ckpt_abort(struct ckpt_ctx *ctx, char *str)
 {
-	perror(str);
+	ckpt_perror(str);
 	kill(-(ctx->root_pid), SIGKILL);
 	exit(1);
 }
@@ -2685,7 +2691,7 @@ static int hash_init(struct ckpt_ctx *ctx)
 
 	ctx->hash_arr = malloc(sizeof(*hash) * HASH_BUCKETS);
 	if (!ctx->hash_arr) {
-		perror("malloc hash table");
+		ckpt_perror("malloc hash table");
 		return -1;
 	}
 	memset(ctx->hash_arr, 0, sizeof(*hash) * HASH_BUCKETS);
@@ -2725,7 +2731,7 @@ static int hash_insert(struct ckpt_ctx *ctx, long key, void *data)
 
 	hash = malloc(sizeof(*hash));
 	if (!hash) {
-		perror("malloc hash");
+		ckpt_perror("malloc hash");
 		return -1;
 	}
 	hash->key = key;
-- 
1.6.0.4

[PATCH 06/13][user-cr]: restart: Remove args->input

[Sukadev Bhattiprolu]

From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Date: Mon, 1 Mar 2010 20:24:15 -0800
Subject: [PATCH 06/13][user-cr]: restart: Remove args->input

Having both ->input and ->infd in 'struct args' is redundant.

The args->input field is used to store the name of the input file
(checkpoint-image).  If we move the code that opens this file from
main() to parse_args() and use the existing args->infd field, we
can remove the ->input field from struct args.

Signed-off-by: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
 restart.c |   25 +++++++++++++------------
 1 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/restart.c b/restart.c
index eda864d..d028890 100644
--- a/restart.c
+++ b/restart.c
@@ -383,7 +383,6 @@ struct args {
 	int show_status;
 	int copy_status;
 	char *freezer;
-	char *input;
 	int infd;
 	int klogfd;
 	long warn;
@@ -489,6 +488,7 @@ static void parse_args(struct args *args, int argc, char *argv[])
 	int no_pidns;
 
 	char *klogfile;
+	char *input;
 
 	/* defaults */
 	memset(args, 0, sizeof(*args));
@@ -500,6 +500,7 @@ static void parse_args(struct args *args, int argc, char *argv[])
 	no_pidns = 0;
 
 	klogfile = NULL;
+	input = NULL;
 
 	while (1) {
 		int c = getopt_long(argc, argv, optc, opts, &optind);
@@ -517,7 +518,7 @@ static void parse_args(struct args *args, int argc, char *argv[])
 			args->inspect = 1;
 			break;
 		case 'i':
-			args->input = optarg;
+			input = optarg;
 			break;
 		case 7:
 			args->infd = str2num(optarg);
@@ -641,11 +642,20 @@ static void parse_args(struct args *args, int argc, char *argv[])
 		exit(1);
 	}
 
-	if (args->input && args->infd >= 0) {
+	if (input && args->infd >= 0) {
 		ckpt_err("Invalid use of both -i/--input and --input-fd\n");
 		exit(1);
 	}
 
+	/* input file ? */
+	if (input) {
+		args->infd = open(input, O_RDONLY, 0);
+		if (args->infd < 0) {
+			ckpt_perror("open input file");
+			exit(1);
+		}
+	}
+
 	if (klogfile && args->klogfd >= 0) {
 		ckpt_err("Invalid use of both -l/--logfile and --logfile-fd\n");
 		exit(1);
@@ -817,15 +827,6 @@ int main(int argc, char *argv[])
 	parse_args(&args, argc, argv);
 	ctx.args = &args;
 
-	/* input file ? */
-	if (args.input) {
-		args.infd = open(args.input, O_RDONLY, 0);
-		if (args.infd < 0) {
-			ckpt_perror("open input file");
-			exit(1);
-		}
-	}
-
 	/* input file descriptor (default: stdin) */
 	if (args.infd >= 0) {
 		if (dup2(args.infd, STDIN_FILENO) < 0) {
-- 
1.6.0.4

[PATCH 07/13][user-cr]: Define app_restart()

[Sukadev Bhattiprolu]

From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Date: Tue, 2 Mar 2010 09:29:55 -0800
Subject: [PATCH 07/13][user-cr]: Define app_restart()

Move the bulk of code that implements application restart into a
separate function, and make main() a wrapper to that function.
This would help export the core restart functionality as a library
interface.

Signed-off-by: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
 restart.c |   58 ++++++++++++++++++++++++++++++++--------------------------
 1 files changed, 32 insertions(+), 26 deletions(-)

diff --git a/restart.c b/restart.c
index d028890..369e9e6 100644
--- a/restart.c
+++ b/restart.c
@@ -807,63 +807,51 @@ static int freezer_register(struct ckpt_ctx *ctx, pid_t pid)
 	return ret;
 }
 
-int main(int argc, char *argv[])
+int app_restart(struct args *args)
 {
 	struct ckpt_ctx ctx;
-	struct args args;
 	int ret;
 
-	/*
-	 * Initialize the log/error fds early so even parse_args() errors
-	 * are redirected here. Even if we later implement command line options
-	 * that override these, any errors/messages that occur before those
-	 * new options are parsed still go to stdout/stderr
-	 */
-	global_ulogfd = fileno(stdout);
-	global_uerrfd = fileno(stderr);
-
 	memset(&ctx, 0, sizeof(ctx));
-
-	parse_args(&args, argc, argv);
-	ctx.args = &args;
+	ctx.args = args;
 
 	/* input file descriptor (default: stdin) */
-	if (args.infd >= 0) {
-		if (dup2(args.infd, STDIN_FILENO) < 0) {
+	if (args->infd >= 0) {
+		if (dup2(args->infd, STDIN_FILENO) < 0) {
 			ckpt_perror("dup2 input file");
 			exit(1);
 		}
-		if (args.infd != STDIN_FILENO)
-			close(args.infd);
+		if (args->infd != STDIN_FILENO)
+			close(args->infd);
 	}
 
 	/* output file descriptor (default: none) */
-	if (args.klogfd < 0)
-		args.klogfd = CHECKPOINT_FD_NONE;
+	if (args->klogfd < 0)
+		args->klogfd = CHECKPOINT_FD_NONE;
 
 	/* freezer preparation */
-	if (args.freezer && freezer_prepare(&ctx) < 0)
+	if (args->freezer && freezer_prepare(&ctx) < 0)
 		exit(1);
 
 	/* private mounts namespace ? */
-	if (args.mntns && unshare(CLONE_NEWNS | CLONE_FS) < 0) {
+	if (args->mntns && unshare(CLONE_NEWNS | CLONE_FS) < 0) {
 		ckpt_perror("unshare");
 		exit(1);
 	}
 
 	/* chroot ? */
-	if (args.root && chroot(args.root) < 0) {
+	if (args->root && chroot(args->root) < 0) {
 		ckpt_perror("chroot");
 		exit(1);
 	}
 
 	/* remount /dev/pts ? */
-	if (args.mnt_pty && ckpt_remount_devpts(&ctx) < 0)
+	if (args->mnt_pty && ckpt_remount_devpts(&ctx) < 0)
 		exit(1);
 
 	/* self-restart ends here: */
-	if (args.self) {
-		restart(getpid(), STDIN_FILENO, RESTART_TASKSELF, args.klogfd);
+	if (args->self) {
+		restart(getpid(), STDIN_FILENO, RESTART_TASKSELF, args->klogfd);
 		/* reach here if restart(2) failed ! */
 		ckpt_perror("restart");
 		exit(1);
@@ -928,6 +916,24 @@ int main(int argc, char *argv[])
 	return ret;
 }
 
+int main(int argc, char *argv[])
+{
+	struct args args;
+
+	/*
+	 * Initialize the log/error fds early so even parse_args() errors
+	 * are redirected here. Even if we later implement command line options
+	 * that override these, any errors/messages that occur before those
+	 * new options are parsed still go to stdout/stderr
+	 */
+	global_ulogfd = fileno(stdout);
+	global_uerrfd = fileno(stderr);
+
+	parse_args(&args, argc, argv);
+
+	return app_restart(&args);
+}
+
 static int ckpt_parse_status(int status, int mimic, int verbose)
 {
 	int sig = 0;
-- 
1.6.0.4

[PATCH 08/13][user-cr]: restart: Rename struct args.

[Sukadev Bhattiprolu]

From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Date: Mon, 1 Mar 2010 20:33:36 -0800
Subject: [PATCH 08/13][user-cr]: restart: Rename struct args.

Rename 'struct args' in restart.c to 'struct app_restart_args'. This
would enable us to distinguish this struct from the 'struct args' in
checkpoint.c when checkpoint/restart code is implemented as a library
interface

Signed-off-by: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
 restart.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/restart.c b/restart.c
index 369e9e6..6994298 100644
--- a/restart.c
+++ b/restart.c
@@ -292,7 +292,7 @@ struct ckpt_ctx {
 	char container[BUFSIZE];
 	char tree[BUFSIZE];
 	char buf[BUFSIZE];
-	struct args *args;
+	struct app_restart_args *args;
 
 	char *freezer;
 };
@@ -371,7 +371,7 @@ struct pid_swap {
 	pid_t new;
 };
 
-struct args {
+struct app_restart_args {
 	int self;
 	int pids;
 	int pidns;
@@ -452,7 +452,7 @@ static inline int ckpt_cond_fail(struct ckpt_ctx *ctx, long mask)
 	return (ctx->args->fail & mask);
 }
 
-static void parse_args(struct args *args, int argc, char *argv[])
+static void parse_args(struct app_restart_args *args, int argc, char *argv[])
 {
 	static struct option opts[] = {
 		{ "help",	no_argument,		NULL, 'h' },
@@ -807,7 +807,7 @@ static int freezer_register(struct ckpt_ctx *ctx, pid_t pid)
 	return ret;
 }
 
-int app_restart(struct args *args)
+int app_restart(struct app_restart_args *args)
 {
 	struct ckpt_ctx ctx;
 	int ret;
@@ -918,7 +918,7 @@ int app_restart(struct args *args)
 
 int main(int argc, char *argv[])
 {
-	struct args args;
+	struct app_restart_args args;
 
 	/*
 	 * Initialize the log/error fds early so even parse_args() errors
-- 
1.6.0.4

[PATCH 09/13][user-cr]: checkpoint: Remove args->output

[Sukadev Bhattiprolu]

From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Date: Wed, 3 Mar 2010 10:49:19 -0800
Subject: [PATCH 09/13][user-cr]: checkpoint: Remove args->output

'struct args' has both an 'outfd' and an 'output' field. The ->output
field is only needed to parse arguments and by moving the open of
the output file into parse_args(), we can drop the ->output field
and just use ->outfd.

Signed-off-by: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
 checkpoint.c |   29 +++++++++++++++--------------
 1 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/checkpoint.c b/checkpoint.c
index 464e359..df405cb 100644
--- a/checkpoint.c
+++ b/checkpoint.c
@@ -40,7 +40,6 @@ static char usage_str[] =
 "";
 
 struct args {
-	char *output;
 	int outfd;
 	char *logfile;
 	int logfd;
@@ -84,10 +83,12 @@ static void parse_args(struct args *args, int argc, char *argv[])
 		{ NULL,		0,			NULL, 0 }
 	};
 	static char optc[] = "hvco:l:";
+	char *output;
 
 	/* defaults */
 	args->outfd = -1;
 	args->logfd = -1;
+	output = NULL;
 
 	while (1) {
 		int c = getopt_long(argc, argv, optc, opts, NULL);
@@ -99,7 +100,7 @@ static void parse_args(struct args *args, int argc, char *argv[])
 		case 'h':
 			usage(usage_str);
 		case 'o':
-			args->output = optarg;
+			output = optarg;
 			break;
 		case 1:
 			args->outfd = str2num(optarg);
@@ -129,10 +130,20 @@ static void parse_args(struct args *args, int argc, char *argv[])
 		}
 	}
 
-	if (args->output && args->outfd >= 0) {
-		printf("Invalid used of both -o/--output and --output-fd\n");
+	if (output && args->outfd >= 0) {
+		printf("Invalid use of both -o/--output and --output-fd\n");
 		exit(1);
 	}
+
+	/* output file */
+	if (output) {
+		args->outfd = open(output, O_RDWR | O_CREAT | O_EXCL, 0644);
+		if (args->outfd < 0) {
+			perror("open output file");
+			exit(1);
+		}
+	}
+
 	if (args->logfile && args->logfd >= 0) {
 		printf("Invalid used of both -l/--logfile and --logfile-fd\n");
 		exit(1);
@@ -162,16 +173,6 @@ int main(int argc, char *argv[])
 		exit(1);
 	}
 
-	/* output file */
-	if (args.output) {
-		args.outfd = open(args.output,
-				     O_RDWR | O_CREAT | O_EXCL, 0644);
-		if (args.outfd < 0) {
-			perror("open output file");
-			exit(1);
-		}
-	}
-
 	/* output file descriptor (default: stdout) */
 	if (args.outfd < 0)
 		args.outfd = STDOUT_FILENO;
-- 
1.6.0.4

[PATCH 10/13][user-cr]: checkpoint: Remove ->logfile

[Sukadev Bhattiprolu]

From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Date: Wed, 3 Mar 2010 11:03:52 -0800
Subject: [PATCH 10/13][user-cr]: checkpoint: Remove ->logfile

'struct args' has both an 'logfd' and a 'logfile' field. The ->logfile
field is only needed to parse arguments and by moving the open of
the logfile into parse_args(), we can drop the ->logfile field and just
use ->logfd.

Signed-off-by: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
 checkpoint.c |   28 ++++++++++++++--------------
 1 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/checkpoint.c b/checkpoint.c
index df405cb..ca77d8b 100644
--- a/checkpoint.c
+++ b/checkpoint.c
@@ -41,7 +41,6 @@ static char usage_str[] =
 
 struct args {
 	int outfd;
-	char *logfile;
 	int logfd;
 	int container;
 	int verbose;
@@ -84,11 +83,13 @@ static void parse_args(struct args *args, int argc, char *argv[])
 	};
 	static char optc[] = "hvco:l:";
 	char *output;
+	char *logfile;
 
 	/* defaults */
 	args->outfd = -1;
 	args->logfd = -1;
 	output = NULL;
+	logfile = NULL;
 
 	while (1) {
 		int c = getopt_long(argc, argv, optc, opts, NULL);
@@ -110,7 +111,7 @@ static void parse_args(struct args *args, int argc, char *argv[])
 			}
 			break;
 		case 'l':
-			args->logfile = optarg;
+			logfile = optarg;
 			break;
 		case 2:
 			args->logfd = str2num(optarg);
@@ -144,10 +145,19 @@ static void parse_args(struct args *args, int argc, char *argv[])
 		}
 	}
 
-	if (args->logfile && args->logfd >= 0) {
-		printf("Invalid used of both -l/--logfile and --logfile-fd\n");
+	if (logfile && args->logfd >= 0) {
+		printf("Invalid use of both -l/--logfile and --logfile-fd\n");
 		exit(1);
 	}
+
+	/* (optional) log file */
+	if (logfile) {
+		args->logfd = open(logfile, O_RDWR | O_CREAT | O_EXCL, 0644);
+		if (args->logfd < 0) {
+			perror("open log file");
+			exit(1);
+		}
+	}
 }
 
 int main(int argc, char *argv[])
@@ -177,16 +187,6 @@ int main(int argc, char *argv[])
 	if (args.outfd < 0)
 		args.outfd = STDOUT_FILENO;
 
-	/* (optional) log file */
-	if (args.logfile) {
-		args.logfd = open(args.logfile,
-				  O_RDWR | O_CREAT | O_EXCL, 0644);
-		if (args.logfd < 0) {
-			perror("open log file");
-			exit(1);
-		}
-	}
-
 	/* output file descriptor (default: none) */
 	if (args.logfd < 0)
 		args.logfd = CHECKPOINT_FD_NONE;
-- 
1.6.0.4

[PATCH 11/13][user-cr]: checkpoint: Rename struct args

[Sukadev Bhattiprolu]

From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Date: Wed, 3 Mar 2010 11:24:54 -0800
Subject: [PATCH 11/13][user-cr]: checkpoint: Rename struct args

Rename 'struct args' in checkpoint.c to 'struct app_checkpoint_args'.
This would give the struct a more specific name when checkpoint/restart
code is implemented as a library interface.

Signed-off-by: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
 checkpoint.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/checkpoint.c b/checkpoint.c
index ca77d8b..d5fcfee 100644
--- a/checkpoint.c
+++ b/checkpoint.c
@@ -39,7 +39,7 @@ static char usage_str[] =
 "  -v,--verbose          verbose output\n"
 "";
 
-struct args {
+struct app_checkpoint_args {
 	int outfd;
 	int logfd;
 	int container;
@@ -69,7 +69,7 @@ static int str2num(char *str)
 	return num;
 }
 
-static void parse_args(struct args *args, int argc, char *argv[])
+static void parse_args(struct app_checkpoint_args *args, int argc, char *argv[])
 {
 	static struct option opts[] = {
 		{ "help",	no_argument,		NULL, 'h' },
@@ -162,7 +162,7 @@ static void parse_args(struct args *args, int argc, char *argv[])
 
 int main(int argc, char *argv[])
 {
-	struct args args;
+	struct app_checkpoint_args args;
 	unsigned long flags = 0;
 	pid_t pid;
 	int ret;
-- 
1.6.0.4

[PATCH 12/13][user-cr]: Define app_checkpoint()

[Sukadev Bhattiprolu]

From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Date: Wed, 3 Mar 2010 11:35:47 -0800
Subject: [PATCH 12/13][user-cr]: Define app_checkpoint()

Move the bulk of the code that implements application checkpoint into
a separate function, app_checkpoint() and make a main() a wrapper to
that function. This would help export the core checkpoint functionality
as a library interface in a future patch.

Signed-off-by: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
 checkpoint.c |   50 +++++++++++++++++++++++++++++---------------------
 1 files changed, 29 insertions(+), 21 deletions(-)

diff --git a/checkpoint.c b/checkpoint.c
index d5fcfee..4044da8 100644
--- a/checkpoint.c
+++ b/checkpoint.c
@@ -160,12 +160,36 @@ static void parse_args(struct app_checkpoint_args *args, int argc, char *argv[])
 	}
 }
 
+int app_checkpoint(int pid, unsigned long flags,
+				struct app_checkpoint_args *args)
+{
+	int ret;
+
+	/* output file descriptor (default: stdout) */
+	if (args->outfd < 0)
+		args->outfd = STDOUT_FILENO;
+
+	/* output file descriptor (default: none) */
+	if (args->logfd < 0)
+		args->logfd = CHECKPOINT_FD_NONE;
+
+	ret = checkpoint(pid, args->outfd, flags, args->logfd);
+
+	if (ret < 0) {
+		perror("checkpoint");
+		fprintf(stderr, "(you may use 'ckptinfo -e' for more info)\n"); 
+	} else if (args->verbose) {
+		fprintf(stderr, "checkpoint id %d\n", ret);
+	}
+
+	return (ret > 0 ? 0 : 1);
+}
+
 int main(int argc, char *argv[])
 {
 	struct app_checkpoint_args args;
 	unsigned long flags = 0;
 	pid_t pid;
-	int ret;
 
 	memset(&args, 0, sizeof(args));
 	parse_args(&args, argc, argv);
@@ -174,31 +198,15 @@ int main(int argc, char *argv[])
 	if (argc != 1)
 		usage(usage_str);
 
-	if (!args.container)
-		flags |= CHECKPOINT_SUBTREE;
-
 	pid = atoi(argv[optind]);
 	if (pid <= 0) {
 		printf("invalid pid\n");
 		exit(1);
 	}
 
-	/* output file descriptor (default: stdout) */
-	if (args.outfd < 0)
-		args.outfd = STDOUT_FILENO;
-
-	/* output file descriptor (default: none) */
-	if (args.logfd < 0)
-		args.logfd = CHECKPOINT_FD_NONE;
-
-	ret = checkpoint(pid, args.outfd, flags, args.logfd);
-
-	if (ret < 0) {
-		perror("checkpoint");
-		fprintf(stderr, "(you may use 'ckptinfo -e' for more info)\n"); 
-	} else if (args.verbose) {
-		fprintf(stderr, "checkpoint id %d\n", ret);
-	}
+	if (!args.container)
+		flags |= CHECKPOINT_SUBTREE;
 
-	return (ret > 0 ? 0 : 1);
+	return app_checkpoint(pid, flags, &args);
 }
+
-- 
1.6.0.4

[PATCH 13/13][user-cr]: checkpoint: Use ckpt_perror()

[Sukadev Bhattiprolu]

From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Date: Wed, 3 Mar 2010 15:22:47 -0800
Subject: [PATCH 13/13] checkpoint: Use ckpt_perror()

Remove the implicit dependency on 'stderr' and redirect messages to
global_uerrfd. This would simplify implementing command line options
for these file descriptors and also enable implementing the checkpoint
functionality as a library interface

Signed-off-by: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
 checkpoint.c |   60 +++++++++++++++++++++++++++++++++++++++++++++++----------
 1 files changed, 49 insertions(+), 11 deletions(-)

diff --git a/checkpoint.c b/checkpoint.c
index 4044da8..ace17e2 100644
--- a/checkpoint.c
+++ b/checkpoint.c
@@ -14,6 +14,7 @@
 #include <errno.h>
 #include <getopt.h>
 #include <unistd.h>
+#include <stdarg.h>
 #include <fcntl.h>
 #include <sys/types.h>
 #include <sys/syscall.h>
@@ -39,9 +40,12 @@ static char usage_str[] =
 "  -v,--verbose          verbose output\n"
 "";
 
+static int global_uerrfd = -1;
+
 struct app_checkpoint_args {
 	int outfd;
 	int logfd;
+	int uerrfd;
 	int container;
 	int verbose;
 };
@@ -51,9 +55,38 @@ inline static int checkpoint(pid_t pid, int fd, unsigned long flags, int logfd)
 	return syscall(__NR_checkpoint, pid, fd, flags, logfd);
 }
 
+#define BUFSIZE  (4 * 4096)
+static inline void ckpt_msg(int fd, char *format, ...)
+{
+        va_list ap;
+        char *bufp;
+        if (fd < 0)
+                return;
+
+        va_start(ap, format);
+
+        bufp = malloc(BUFSIZE);
+        if(bufp) {
+                vsnprintf(bufp, BUFSIZE, format, ap);
+                write(fd, bufp, strlen(bufp));
+        }
+        free(bufp);
+
+        va_end(ap);
+}
+
+#define ckpt_err(...)  \
+	ckpt_msg(global_uerrfd, __VA_ARGS__)
+
+#define ckpt_perror(s)                                                  \
+        do {                                                            \
+                ckpt_msg(global_uerrfd, s);                             \
+                ckpt_msg(global_uerrfd, ": %s\n", strerror(errno));     \
+        } while (0)
+
 static void usage(char *str)
 {
-	fprintf(stderr, "%s", str);
+	ckpt_err("%s", str);
 	exit(1);
 }
 
@@ -88,6 +121,7 @@ static void parse_args(struct app_checkpoint_args *args, int argc, char *argv[])
 	/* defaults */
 	args->outfd = -1;
 	args->logfd = -1;
+	args->uerrfd = fileno(stderr);
 	output = NULL;
 	logfile = NULL;
 
@@ -106,7 +140,7 @@ static void parse_args(struct app_checkpoint_args *args, int argc, char *argv[])
 		case 1:
 			args->outfd = str2num(optarg);
 			if (args->outfd < 0) {
-				printf("checkpoint: invalid file descriptor\n");
+				ckpt_err("checkpoint: invalid file descriptor\n");
 				exit(1);
 			}
 			break;
@@ -116,7 +150,7 @@ static void parse_args(struct app_checkpoint_args *args, int argc, char *argv[])
 		case 2:
 			args->logfd = str2num(optarg);
 			if (args->logfd < 0) {
-				printf("checkpoint: invalid file descriptor\n");
+				ckpt_err("checkpoint: invalid file descriptor\n");
 				exit(1);
 			}
 			break;
@@ -132,7 +166,7 @@ static void parse_args(struct app_checkpoint_args *args, int argc, char *argv[])
 	}
 
 	if (output && args->outfd >= 0) {
-		printf("Invalid use of both -o/--output and --output-fd\n");
+		ckpt_err("Invalid use of both -o/--output and --output-fd\n");
 		exit(1);
 	}
 
@@ -140,13 +174,13 @@ static void parse_args(struct app_checkpoint_args *args, int argc, char *argv[])
 	if (output) {
 		args->outfd = open(output, O_RDWR | O_CREAT | O_EXCL, 0644);
 		if (args->outfd < 0) {
-			perror("open output file");
+			ckpt_perror("open output file");
 			exit(1);
 		}
 	}
 
 	if (logfile && args->logfd >= 0) {
-		printf("Invalid use of both -l/--logfile and --logfile-fd\n");
+		ckpt_err("Invalid use of both -l/--logfile and --logfile-fd\n");
 		exit(1);
 	}
 
@@ -154,7 +188,7 @@ static void parse_args(struct app_checkpoint_args *args, int argc, char *argv[])
 	if (logfile) {
 		args->logfd = open(logfile, O_RDWR | O_CREAT | O_EXCL, 0644);
 		if (args->logfd < 0) {
-			perror("open log file");
+			ckpt_perror("open log file");
 			exit(1);
 		}
 	}
@@ -165,6 +199,8 @@ int app_checkpoint(int pid, unsigned long flags,
 {
 	int ret;
 
+	global_uerrfd = args->uerrfd;
+
 	/* output file descriptor (default: stdout) */
 	if (args->outfd < 0)
 		args->outfd = STDOUT_FILENO;
@@ -176,10 +212,10 @@ int app_checkpoint(int pid, unsigned long flags,
 	ret = checkpoint(pid, args->outfd, flags, args->logfd);
 
 	if (ret < 0) {
-		perror("checkpoint");
-		fprintf(stderr, "(you may use 'ckptinfo -e' for more info)\n"); 
+		ckpt_perror("checkpoint");
+		ckpt_err("(you may use 'ckptinfo -e' for more info)\n"); 
 	} else if (args->verbose) {
-		fprintf(stderr, "checkpoint id %d\n", ret);
+		ckpt_err("checkpoint id %d\n", ret);
 	}
 
 	return (ret > 0 ? 0 : 1);
@@ -191,6 +227,8 @@ int main(int argc, char *argv[])
 	unsigned long flags = 0;
 	pid_t pid;
 
+	global_uerrfd = fileno(stderr);
+
 	memset(&args, 0, sizeof(args));
 	parse_args(&args, argc, argv);
 
@@ -200,7 +238,7 @@ int main(int argc, char *argv[])
 
 	pid = atoi(argv[optind]);
 	if (pid <= 0) {
-		printf("invalid pid\n");
+		ckpt_err("invalid pid\n");
 		exit(1);
 	}
 
-- 
1.6.0.4

Re: [PATCH 00/13][user-cr]: Cleanups checkpoint.c, restart.c

[Serge E. Hallyn]

Quoting Sukadev Bhattiprolu (sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org):
> 
> [PATCH 00/13][user-cr]: Cleanups checkpoint.c, restart.c
> 
> These patches perform some cleanups/reorg of the code in checkpoint.c and
> restart.c files.  These changes remove redundant fields in data structures,
> rename some fields/functions and avoid directly using stdout/stderr.
> 
> The changes will help us later export app_checkpoint() and app_restart()
> to larger container implementations such as LXC.
> 
> 	[PATCH 01/13][user-cr]: restart: Remove args->no_pidns.
> 	[PATCH 02/13][user-cr]: restart: Rename args->logfd
> 	[PATCH 03/13][user-cr]: restart: Remove args->logfile 
> 	[PATCH 04/13][user-cr]: restart: Use ckpt_msg() to log
> 	[PATCH 05/13][user-cr]: restart: Use ckpt_perror()
> 	[PATCH 06/13][user-cr]: restart: Remove args->input
> 	[PATCH 07/13][user-cr]: Define app_restart()
> 	[PATCH 08/13][user-cr]: restart: Rename struct args.
> 	[PATCH 09/13][user-cr]: checkpoint: Remove args->output
> 	[PATCH 10/13][user-cr]: checkpoint: Remove ->logfile
> 	[PATCH 11/13][user-cr]: checkpoint: Rename struct args
> 	[PATCH 12/13][user-cr]: Define app_checkpoint()
> 	[PATCH 13/13] checkpoint: Use ckpt_perror()
> 
> Signed-off-by: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>

Thanks, Suka, very nice.

Acked-by: Serge Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

-serge

Re: [PATCH 00/13][user-cr]: Cleanups checkpoint.c, restart.c

[Oren Laadan]

Series applied (will push shortly).

Oren.

Sukadev Bhattiprolu wrote:
> [PATCH 00/13][user-cr]: Cleanups checkpoint.c, restart.c
> 
> These patches perform some cleanups/reorg of the code in checkpoint.c and
> restart.c files.  These changes remove redundant fields in data structures,
> rename some fields/functions and avoid directly using stdout/stderr.
> 
> The changes will help us later export app_checkpoint() and app_restart()
> to larger container implementations such as LXC.
> 
> 	[PATCH 01/13][user-cr]: restart: Remove args->no_pidns.
> 	[PATCH 02/13][user-cr]: restart: Rename args->logfd
> 	[PATCH 03/13][user-cr]: restart: Remove args->logfile 
> 	[PATCH 04/13][user-cr]: restart: Use ckpt_msg() to log
> 	[PATCH 05/13][user-cr]: restart: Use ckpt_perror()
> 	[PATCH 06/13][user-cr]: restart: Remove args->input
> 	[PATCH 07/13][user-cr]: Define app_restart()
> 	[PATCH 08/13][user-cr]: restart: Rename struct args.
> 	[PATCH 09/13][user-cr]: checkpoint: Remove args->output
> 	[PATCH 10/13][user-cr]: checkpoint: Remove ->logfile
> 	[PATCH 11/13][user-cr]: checkpoint: Rename struct args
> 	[PATCH 12/13][user-cr]: Define app_checkpoint()
> 	[PATCH 13/13] checkpoint: Use ckpt_perror()
> 
> Signed-off-by: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
>