All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC][PATCH 0/4][user-cr]: First try at integrating LXC and USER-CR
@ 2010-02-24  8:34 Sukadev Bhattiprolu
       [not found] ` <20100224083452.GB18758-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
  0 siblings, 1 reply; 17+ messages in thread
From: Sukadev Bhattiprolu @ 2010-02-24  8:34 UTC (permalink / raw)
  To: serue-r/Jw6+rmf7HQT0dZR+AlfA, dlezcano-NmTC/0ZBporQT0dZR+AlfA,
	Oren Laadan
  Cc: Containers, sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8


Following two sets of patches is an early attempt to integrate LXC and
USER-CR. 

Overview:

Have USER-CR export the core checkpoint and restart functionality into a
library (/lib/libcheckpoint.a and <usercr.h>) and have LXC link with this
library.

TODO: 

	1. For now, libcheckpoint.a implements only the restart functionality
	   and so only lxc_restart command is implemented. Implementing the
	   checkpoint functionality and lxc_checkpoint command can be done
	   similarly and is hopefully easier than the restart functionality.

	2. The restart() functionality in user-cr makes extensive use of global
	   variables and debug code. The API must be extended to properly
	   include these variables/debug code in the API.

	   Similarly, the 'struct restart_args' may need to be sanitized for
	   use in a formal API.

	3. lxc_restart command  restarts entire containers only (specifically
	   it simulates the --pidns --pids --mount-pty arguments to
	   /bin/restart).

	4. Link lxc_restart and lxc_checkpoint with the shared library
	   liblxc.so (currently links statically)

	5. ...


STATUS:
	I was able to checkpoint/restart a simple '/bin/sleep 1000' LXC
	container, except for a cgroup naming issue after restart (see below).

STEPS:

1. [USER-CR] Build/install /lib/libcheckpoint.a, /usr/include/usercr.h
   
   1.1 Apply the attached [user-cr] patches to the user-cr git tree
       (I tested with following commit as base)

	commit 67cfee9329670ab28eb1a52e94745252b614718f
	Author: Oren Laadan <orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
	Date:   Mon Feb 22 18:00:06 2010 -0500

   1.2 Build/install user-cr binaries/libraries/includes

   	$ make all

	$ make install

	This should install /lib/libcheckpoint.a and /usr/include/usercr.h

2. [LXC] Build lxc_restart using USER-CR API (usercr.h, libcheckpoint.a)

   2.1 Apply attached [lxc] patches to Daniel Lezcano's lxc.git tree (0.6.5)

   2.2 Build lxc_restart (this uses static linking for now)

   	$ make -f Makefile2 lxc_restart

3. Create and checkpoint a simple LXC container

	$ lxc-execute --name foo --rcfile lxc-macvlan.conf -- /bin/sleep 1000

	$ lxc-freeze --name foo

	TODO: 
		lxc_checkpoint --name foo should checkpoint the container,
		For now, use "lxc-ps --name foo" to find pid of lxc-init and
		checkpoint using:

		$ /bin/checkpoint --output=/tmp/sleep.ckpt <pid-of-lxc-init>

	$ lxc-unfreeze --name foo

	$ lxc-stop --name foo

4. Restart a checkpointed LXC container

	$ ./lxc_restart --statefile /tmp/sleep.ckpt --name bar

   	# Test some common lxc commands after restart

	$ lxc-ps --name "bar/1"
	CONTAINER    PID TTY          TIME CMD
	bar/1       8511 ?        00:00:00 lxc-init
	bar/1       8512 ?        00:00:00 sleep

	$ lxc-freeze --name "bar/1"

	$ grep State /proc/8511/status 
	State:	D (disk sleep)

	$ grep State /proc/8512/status 
	State:	D (disk sleep)

	NOTE: 	For some reason, the container name after restart is "bar/1"
		instead of "bar".  Due to this, when the lxc_restart is
		exiting, I get a "-EBUSY - failed to remove "/cgroup/bar"
		error.  I need to fix this still.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/4][user-cr] Move common definitions to restart.h
       [not found] ` <20100224083452.GB18758-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
@ 2010-02-24  8:35   ` Sukadev Bhattiprolu
       [not found]     ` <20100224083534.GC18758-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
  2010-02-24  8:36   ` [PATCH 2/4][user-cr] Rename struct args to struct restart_args Sukadev Bhattiprolu
                     ` (6 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Sukadev Bhattiprolu @ 2010-02-24  8:35 UTC (permalink / raw)
  To: serue-r/Jw6+rmf7HQT0dZR+AlfA, dlezcano-NmTC/0ZBporQT0dZR+AlfA,
	Oren Laadan
  Cc: Containers


From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Date: Tue, 23 Feb 2010 15:25:31 -0800
Subject: [PATCH 1/4][user-cr] Move common definitions to restart.h

To make restart() a library interface, the main() and related code in
restart.c should be moved to a separate file. To prepare for the move,
move some common definitions to a new file, 'restart.h'
---
 restart.c |   44 ++------------------------------------------
 restart.h |   45 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 42 deletions(-)
 create mode 100644 restart.h

diff --git a/restart.c b/restart.c
index f5d23bb..7140786 100644
--- a/restart.c
+++ b/restart.c
@@ -39,6 +39,7 @@
 #include "eclone.h"
 #include "genstack.h"
 #include "compat.h"
+#include "restart.h"
 
 static char usage_str[] =
 "usage: restart [opts]\n"
@@ -128,12 +129,7 @@ static char usage_str[] =
 			printf(__VA_ARGS__);	\
 	} while(0)
 
-#define SIGNAL_ENTRY(signal)  { SIG ## signal, #signal }
-
-struct {
-	int signum;
-	char *sigstr;
-} signal_array[] = {
+struct signal_array signal_array[] = {
 	{ 0, "NONE" },
 	SIGNAL_ENTRY(ALRM),
 	SIGNAL_ENTRY(HUP),
@@ -196,7 +192,6 @@ inline static int restart(pid_t pid, int fd, unsigned long flags, int logfd)
 	return syscall(__NR_restart, pid, fd, flags, logfd);
 }
 
-#define BUFSIZE  (4 * 4096)
 
 struct hashent {
 	long key;
@@ -205,29 +200,6 @@ struct hashent {
 };
 
 struct task;
-struct ckpt_ctx;
-
-struct task {
-	int flags;		/* state and (later) actions */
-
-	struct task *children;	/* pointers to first child, next and prev */
-	struct task *next_sib;	/*   sibling, and the creator of a process */
-	struct task *prev_sib;
-	struct task *creator;
-
-	struct task *phantom;	/* pointer to place-holdler task (if any) */
-
-	pid_t pid;		/* process IDs, our bread-&-butter */
-	pid_t ppid;
-	pid_t tgid;
-	pid_t sid;
-	
-	pid_t rpid;		/* [restart without vpids] actual (real) pid */
-
-	struct ckpt_ctx *ctx;	/* points back to the c/r context */
-
-	pid_t real_parent;	/* pid of task's real parent */
-};
 
 /* zero_task represents creator of root_task (all pids 0) */
 struct task zero_task;
@@ -360,18 +332,6 @@ struct args {
 	int keep_lsm;
 };
 
-#define CKPT_COND_PIDZERO  0x1
-#define CKPT_COND_MNTPROC  0x2
-#define CKPT_COND_MNTPTY   0x4
-
-#define CKPT_COND_NONE     0
-#define CKPT_COND_ANY      ULONG_MAX
-
-/* default for skip/warn/fail */
-#define CKPT_COND_WARN     (CKPT_COND_MNTPROC | \
-			    CKPT_COND_MNTPTY)
-#define CKPT_COND_FAIL     (CKPT_COND_NONE)
-
 static void usage(char *str)
 {
 	fprintf(stderr, "%s", str);
diff --git a/restart.h b/restart.h
new file mode 100644
index 0000000..6782399
--- /dev/null
+++ b/restart.h
@@ -0,0 +1,45 @@
+struct ckpt_ctx;
+
+struct task {
+	int flags;		/* state and (later) actions */
+
+	struct task *children;	/* pointers to first child, next and prev */
+	struct task *next_sib;	/*   sibling, and the creator of a process */
+	struct task *prev_sib;
+	struct task *creator;
+
+	struct task *phantom;	/* pointer to place-holdler task (if any) */
+
+	pid_t pid;		/* process IDs, our bread-&-butter */
+	pid_t ppid;
+	pid_t tgid;
+	pid_t sid;
+	
+	pid_t rpid;		/* [restart without vpids] actual (real) pid */
+
+	struct ckpt_ctx *ctx;	/* points back to the c/r context */
+
+	pid_t real_parent;	/* pid of task's real parent */
+};
+
+#define BUFSIZE  (4 * 4096)
+
+#define CKPT_COND_PIDZERO  0x1
+#define CKPT_COND_MNTPROC  0x2
+#define CKPT_COND_MNTPTY   0x4
+
+#define CKPT_COND_NONE     0
+#define CKPT_COND_ANY      ULONG_MAX
+
+/* default for skip/warn/fail */
+#define CKPT_COND_WARN     (CKPT_COND_MNTPROC | \
+			    CKPT_COND_MNTPTY)
+#define CKPT_COND_FAIL     (CKPT_COND_NONE)
+
+#define SIGNAL_ENTRY(signal)  { SIG ## signal, #signal }
+
+struct signal_array {
+	int signum;
+	char *sigstr;
+};
+
-- 
1.6.6.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/4][user-cr] Rename struct args to struct restart_args
       [not found] ` <20100224083452.GB18758-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
  2010-02-24  8:35   ` [PATCH 1/4][user-cr] Move common definitions to restart.h Sukadev Bhattiprolu
@ 2010-02-24  8:36   ` Sukadev Bhattiprolu
  2010-02-24  8:36   ` [PATCH 3/4][user-cr] Move main() in restart.c to restart-main.c Sukadev Bhattiprolu
                     ` (5 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Sukadev Bhattiprolu @ 2010-02-24  8:36 UTC (permalink / raw)
  To: serue-r/Jw6+rmf7HQT0dZR+AlfA, dlezcano-NmTC/0ZBporQT0dZR+AlfA,
	Oren Laadan
  Cc: Containers


From 8a6fad2170fbbeb7a5e44c32239f9703c0b7d2f9 Mon Sep 17 00:00:00 2001
From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Date: Tue, 23 Feb 2010 15:33:04 -0800
Subject: [PATCH 2/4][user-cr] Rename struct args to struct restart_args

Rename struct args to struct restart_args and move to a new header file,
usercr.h. usercr.h will become part of the external API for application
checkpoint restart code.

Also move the bulk of the restart code into a separate function,
app_restart() and have main() call the function.

TODO:
	Cleanup 'struct restart_args' for a more consistent external API.
---
 restart.c |   77 ++++++++++++++++++++++++------------------------------------
 usercr.h  |   24 +++++++++++++++++++
 2 files changed, 55 insertions(+), 46 deletions(-)
 create mode 100644 usercr.h

diff --git a/restart.c b/restart.c
index 7140786..b2281fb 100644
--- a/restart.c
+++ b/restart.c
@@ -40,6 +40,7 @@
 #include "genstack.h"
 #include "compat.h"
 #include "restart.h"
+#include "usercr.h"
 
 static char usage_str[] =
 "usage: restart [opts]\n"
@@ -237,7 +238,7 @@ struct ckpt_ctx {
 	char container[BUFSIZE];
 	char tree[BUFSIZE];
 	char buf[BUFSIZE];
-	struct args *args;
+	struct restart_args *args;
 
 	char *freezer;
 };
@@ -310,28 +311,6 @@ struct pid_swap {
 	pid_t new;
 };
 
-struct args {
-	int self;
-	int pids;
-	int pidns;
-	int no_pidns;
-	int inspect;
-	char *root;
-	int wait;
-	int mntns;
-	int mnt_pty;
-	int show_status;
-	int copy_status;
-	char *freezer;
-	char *input;
-	int infd;
-	char *logfile;
-	int logfd;
-	long warn;
-	long fail;
-	int keep_lsm;
-};
-
 static void usage(char *str)
 {
 	fprintf(stderr, "%s", str);
@@ -382,7 +361,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 restart_args *args, int argc, char *argv[])
 {
 	static struct option opts[] = {
 		{ "help",	no_argument,		NULL, 'h' },
@@ -712,71 +691,77 @@ static int freezer_register(struct ckpt_ctx *ctx, pid_t pid)
 
 int main(int argc, char *argv[])
 {
+	struct restart_args args;
+
+	parse_args(&args, argc, argv);
+
+	return app_restart(&args);
+}
+
+int app_restart(struct restart_args *args)
+{
 	struct ckpt_ctx ctx;
-	struct args args;
 	int ret;
 
 	memset(&ctx, 0, sizeof(ctx));
-
-	parse_args(&args, argc, argv);
-	ctx.args = &args;
+	ctx.args = args;
 
 	/* input file ? */
-	if (args.input) {
-		args.infd = open(args.input, O_RDONLY, 0);
-		if (args.infd < 0) {
+	if (args->input) {
+		args->infd = open(args->input, O_RDONLY, 0);
+		if (args->infd < 0) {
 			perror("open input file");
 			exit(1);
 		}
 	}
 
 	/* 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) {
 			perror("dup2 input file");
 			exit(1);
 		}
-		if (args.infd != STDIN_FILENO)
-			close(args.infd);
+		if (args->infd != STDIN_FILENO)
+			close(args->infd);
 	}
 
 	/* (optional) log file */
-	if (args.logfile) {
-		args.logfd = open(args.logfile,
+	if (args->logfile) {
+		args->logfd = open(args->logfile,
 				  O_RDWR | O_CREAT | O_EXCL, 0644);
-		if (args.logfd < 0) {
+		if (args->logfd < 0) {
 			perror("open log file");
 			exit(1);
 		}
 	}
 
 	/* output file descriptor (default: none) */
-	if (args.logfd < 0)
-		args.logfd = CHECKPOINT_FD_NONE;
+	if (args->logfd < 0)
+		args->logfd = 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) {
 		perror("unshare");
 		exit(1);
 	}
 
 	/* chroot ? */
-	if (args.root && chroot(args.root) < 0) {
+	if (args->root && chroot(args->root) < 0) {
 		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.logfd);
+	if (args->self) {
+		restart(getpid(), STDIN_FILENO, RESTART_TASKSELF, args->logfd);
 		/* reach here if restart(2) failed ! */
 		perror("restart");
 		exit(1);
diff --git a/usercr.h b/usercr.h
new file mode 100644
index 0000000..71a36e6
--- /dev/null
+++ b/usercr.h
@@ -0,0 +1,24 @@
+
+struct restart_args {
+	int self;
+	int pids;
+	int pidns;
+	int no_pidns;
+	int inspect;
+	char *root;
+	int wait;
+	int mntns;
+	int mnt_pty;
+	int show_status;
+	int copy_status;
+	char *freezer;
+	char *input;
+	int infd;
+	char *logfile;
+	int logfd;
+	long warn;
+	long fail;
+	int keep_lsm;
+};
+
+extern int app_restart(struct restart_args *args);
-- 
1.6.6.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 3/4][user-cr] Move main() in restart.c to restart-main.c
       [not found] ` <20100224083452.GB18758-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
  2010-02-24  8:35   ` [PATCH 1/4][user-cr] Move common definitions to restart.h Sukadev Bhattiprolu
  2010-02-24  8:36   ` [PATCH 2/4][user-cr] Rename struct args to struct restart_args Sukadev Bhattiprolu
@ 2010-02-24  8:36   ` Sukadev Bhattiprolu
  2010-02-24  8:37   ` [PATCH 4/4][user-cr] Rename libeclone.a to libcheckpoint.a Sukadev Bhattiprolu
                     ` (4 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Sukadev Bhattiprolu @ 2010-02-24  8:36 UTC (permalink / raw)
  To: serue-r/Jw6+rmf7HQT0dZR+AlfA, dlezcano-NmTC/0ZBporQT0dZR+AlfA,
	Oren Laadan
  Cc: Containers


From 6553b7da33f56e9cb6927e7469af22df56b7f55f Mon Sep 17 00:00:00 2001
From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Date: Tue, 23 Feb 2010 15:55:34 -0800
Subject: [PATCH 3/4][user-cr] Move main() in restart.c to restart-main.c

This would enable us to put restart.o into a library.

TODO:
	restart.c still has a whole bunch of external variables that
	would need to be addressed (maybe encapsulate in structure)

Signed-off-by: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
 Makefile       |    2 +
 restart-main.c |  312 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 restart.c      |  293 ----------------------------------------------------
 3 files changed, 314 insertions(+), 293 deletions(-)
 create mode 100644 restart-main.c

diff --git a/Makefile b/Makefile
index b312358..4449081 100644
--- a/Makefile
+++ b/Makefile
@@ -21,6 +21,8 @@ CFLAGS += -g $(WARNS) $(CKPT_INCLUDE) $(DEBUG)
 BIN_INSTALL_DIR = /bin
 LIB_INSTALL_DIR = /lib
 
+restart: restart.o restart-main.o
+
 ECLONE_PROGS = restart nsexec
 PROGS =	checkpoint ckptinfo $(ECLONE_PROGS)
 LIB_ECLONE = libeclone.a
diff --git a/restart-main.c b/restart-main.c
new file mode 100644
index 0000000..805920f
--- /dev/null
+++ b/restart-main.c
@@ -0,0 +1,312 @@
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+#include <signal.h>
+#include <getopt.h>
+#include <limits.h>
+
+#include "usercr.h"
+#include "restart.h"
+#include "clone.h"
+
+extern int global_verbose;
+extern int global_send_sigint;
+
+static char usage_str[] =
+"usage: restart [opts]\n"
+"  restart restores from a checkpoint image by first creating in userspace\n"
+"  the original tasks tree, and then calling sys_restart by each task.\n"
+"Options:\n"
+"  -h,--help             print this help message\n"
+"  -p,--pidns            create a new pid namspace (default with --pids)\n"
+"  -P,--no-pidns         do not create a new pid namespace (default)\n"
+"     --pids             restore original pids (default with --pidns)\n"
+"     --self             restart a single task, usually from self-checkpoint\n"
+"  -r,--root=ROOT        restart under the directory ROOT instead of current\n"
+"     --signal=SIG       send SIG to root task on SIGINT (default: SIGKILL\n"
+"                        to container root, SIGINT otherwise)\n"
+"     --mntns            restart under a private mounts namespace\n"
+"     --mount-pty        start in a new devpts namespace to supprt ptys\n"
+"  -w,--wait             wait for root task to termiate (default)\n"
+"     --show-status      show exit status of root task (implies -w)\n"
+"     --copy-status      imitate exit status of root task (implies -w)\n"
+"  -W,--no-wait          do not wait for root task to terminate\n"
+"  -k,--keeplsm          try to recreate original LSM labels on all objects\n"
+"  -F,--freezer=CGROUP   freeze tasks in freezer group CGROUP on success\n"
+"  -i,--input=FILE       read data from FILE instead of standard input\n"
+"     --input-fd=FD      read data from file descriptor FD (instead of stdin)\n"
+"  -l,--logfile=FILE     write error and debug data to FILE (default=none)\n"
+"     --logfile-fd=FD    write error and debug data to file desctiptor FD\n"
+"     --inspect          inspect image on-the-fly for error records\n"
+"  -v,--verbose          verbose output\n"
+"  -d,--debug            debugging output\n"
+"     --skip-COND        skip condition COND, and proceed anyway\n"
+"     --warn-COND        warn on condition COND, but proceed anyway\n"
+"     --fail-COND        warn on condition COND, and abort operation\n"
+"  	  COND=any:        any condition\n"
+"  	  COND=pidzero:    task with sid/pgid zero in a --no-pidns restart\n"
+"  	  COND=mntproc:    /proc isn't already mounted at restart (def: warn)\n"
+"";
+
+static void usage(char *str)
+{
+	fprintf(stderr, "%s", str);
+	exit(1);
+}
+
+/* negative retval means error */
+static int str2num(char *str)
+{
+	char *nptr;
+	int num;
+
+	num = strtol(str, &nptr, 10);
+	if (nptr - str != strlen(str))
+		num = -1;
+	return num;
+}
+
+static long cond_to_mask(const char *cond)
+{
+	static struct {
+		char *cond;
+		long mask;
+	} conditions[] = {
+		{"pidzero", CKPT_COND_PIDZERO},
+		{"mntproc", CKPT_COND_MNTPROC},
+		{"any", CKPT_COND_ANY},
+		{NULL, 0}
+	};
+
+	int i;
+
+	for (i = 0; conditions[i].cond; i++)
+		if (!strcmp(cond, conditions[i].cond))
+			return conditions[i].mask;
+
+	printf("restart: invalid warn/fail condition '%s'\n", cond);
+	exit(1);
+}
+
+extern struct signal_array signal_array[];
+extern int global_debug;
+
+static int str2sig(char *str)
+{
+	int sig = 0;
+
+	do {
+		if (!strcmp(signal_array[sig].sigstr, str))
+			return signal_array[sig].signum;
+	} while (signal_array[++sig].signum >= 0);
+
+	return -1;
+}
+
+static void parse_args(struct restart_args *args, int argc, char *argv[])
+{
+	static struct option opts[] = {
+		{ "help",	no_argument,		NULL, 'h' },
+		{ "pidns",	no_argument,		NULL, 'p' },
+		{ "no-pidns",	no_argument,		NULL, 'P' },
+		{ "pids",	no_argument,		NULL, 3 },
+		{ "self",	no_argument,		NULL, 6},
+		{ "signal",	required_argument,	NULL, 4 },
+		{ "inspect",	no_argument,		NULL, 5 },
+		{ "keeplsm",	no_argument,		NULL, 'k' },
+		{ "input",	required_argument,	NULL, 'i' },
+		{ "input-fd",	required_argument,	NULL, 7 },
+		{ "logfile",	required_argument,	NULL, 'l' },
+		{ "logfile-fd",	required_argument,	NULL, 8 },
+		{ "root",	required_argument,	NULL, 'r' },
+		{ "mntns",	no_argument,		NULL, 11 },
+		{ "wait",	no_argument,		NULL, 'w' },
+		{ "show-status",	no_argument,	NULL, 1 },
+		{ "copy-status",	no_argument,	NULL, 2 },
+		{ "no-wait",	no_argument,		NULL, 'W' },
+		{ "freezer",	required_argument,	NULL, 'F' },
+		{ "verbose",	no_argument,		NULL, 'v' },
+		{ "debug",	no_argument,		NULL, 'd' },
+		{ "warn-pidzero",	no_argument,	NULL, 9 },
+		{ "fail-pidzero",	no_argument,	NULL, 10 },
+		{ "mount-pty",	no_argument,		NULL, 12 },
+		{ NULL,		0,			NULL, 0 }
+	};
+	static char optc[] = "hdvkpPwWF:r:i:l:";
+
+	int optind;
+	int sig;
+
+	/* defaults */
+	memset(args, 0, sizeof(*args));
+	args->wait = 1;
+	args->infd = -1;
+	args->logfd = -1;
+	args->warn = CKPT_COND_WARN;
+	args->fail = CKPT_COND_FAIL;
+
+	while (1) {
+		int c = getopt_long(argc, argv, optc, opts, &optind);
+		if (c == -1)
+			break;
+		switch (c) {
+		case '?':
+			exit(1);
+		case 'h':
+			usage(usage_str);
+		case 'v':
+			global_verbose = 1;
+			break;
+		case 5:  /* --inspect */
+			args->inspect = 1;
+			break;
+		case 'i':
+			args->input = optarg;
+			break;
+		case 7:
+			args->infd = str2num(optarg);
+			if (args->infd < 0) {
+				printf("restart: invalid file descriptor\n");
+				exit(1);
+			}
+			break;
+		case 'l':
+			args->logfile = optarg;
+			break;
+		case 8:
+			args->logfd = str2num(optarg);
+			if (args->logfd < 0) {
+				printf("restart: invalid file descriptor\n");
+				exit(1);
+			}
+			break;
+		case 'p':
+			args->pidns = 1;
+			break;
+		case 'P':
+			args->no_pidns = 1;
+			break;
+		case 6:  /* --self */
+			args->self = 1;
+			break;
+		case 4:  /* --signal */
+			sig = str2sig(optarg);
+			if (sig < 0)
+				sig = str2num(optarg);
+			if (sig < 0 || sig >= NSIG) {
+				printf("restart: invalid signal\n");
+				exit(1);
+			}
+			global_send_sigint = sig;
+			break;
+		case 3:  /* --pids */
+			args->pids = 1;
+			args->pidns = 1;  /* implied */
+			break;
+		case 'r':
+			args->root = optarg;
+			break;
+		case 'w':
+			args->wait = 1;
+			break;
+		case 'W':
+			args->wait = 0;
+			break;
+		case 'k':
+			args->keep_lsm = 1;
+			break;
+		case 1:  /* --show-status */
+			args->wait = 1;
+			args->show_status = 1;
+			break;
+		case 2: /* --copy-status */
+			args->wait = 1;
+			args->copy_status = 1;
+			break;
+		case 'd':
+			global_debug = 1;
+			break;
+		case 'F':
+			args->freezer = optarg;
+			break;
+		case 9:
+			args->warn |= cond_to_mask(&opts[optind].name[5]);
+			break;
+		case 10:
+			args->fail |= cond_to_mask(&opts[optind].name[5]);
+			break;
+		case 11:
+			args->mntns = 1;
+			break;
+		case 12:
+			args->mnt_pty = 1;
+			break;
+		default:
+			usage(usage_str);
+		}
+	}
+
+	if (args->no_pidns)
+		args->pidns = 0;
+
+#ifndef CLONE_NEWPID
+	if (args->pidns) {
+		printf("This version of restart was compiled without "
+		       "support for --pidns.\n");
+		exit(1);
+	}
+#endif
+
+#ifndef CHECKPOINT_DEBUG
+	if (global_debug) {
+		printf("This version of restart was compiled without "
+		       "support for --debug.\n");
+		exit(1);
+	}
+#endif
+
+	if (args->pidns)
+		args->pids = 1;
+
+#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 "
+		       "support for --pids.\n");
+		exit(1);
+	}
+#endif
+#endif
+
+	if (args->self &&
+	    (args->pids || args->pidns || args->no_pidns ||
+	     args->show_status || args->copy_status || args->freezer)) {
+		printf("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");
+		exit(1);
+	}
+
+	if (args->logfile && args->logfd >= 0) {
+		printf("Invalid used of both -l/--logfile and --logfile-fd\n");
+		exit(1);
+	}
+
+	if (args->mnt_pty)
+		args->mntns = 1;
+}
+
+int main(int argc, char *argv[])
+{
+	struct restart_args args;
+
+	parse_args(&args, argc, argv);
+
+	return app_restart(&args);
+}
+
diff --git a/restart.c b/restart.c
index b2281fb..b1518f2 100644
--- a/restart.c
+++ b/restart.c
@@ -42,42 +42,6 @@
 #include "restart.h"
 #include "usercr.h"
 
-static char usage_str[] =
-"usage: restart [opts]\n"
-"  restart restores from a checkpoint image by first creating in userspace\n"
-"  the original tasks tree, and then calling sys_restart by each task.\n"
-"Options:\n"
-"  -h,--help             print this help message\n"
-"  -p,--pidns            create a new pid namspace (default with --pids)\n"
-"  -P,--no-pidns         do not create a new pid namespace (default)\n"
-"     --pids             restore original pids (default with --pidns)\n"
-"     --self             restart a single task, usually from self-checkpoint\n"
-"  -r,--root=ROOT        restart under the directory ROOT instead of current\n"
-"     --signal=SIG       send SIG to root task on SIGINT (default: SIGKILL\n"
-"                        to container root, SIGINT otherwise)\n"
-"     --mntns            restart under a private mounts namespace\n"
-"     --mount-pty        start in a new devpts namespace to supprt ptys\n"
-"  -w,--wait             wait for root task to termiate (default)\n"
-"     --show-status      show exit status of root task (implies -w)\n"
-"     --copy-status      imitate exit status of root task (implies -w)\n"
-"  -W,--no-wait          do not wait for root task to terminate\n"
-"  -k,--keeplsm          try to recreate original LSM labels on all objects\n"
-"  -F,--freezer=CGROUP   freeze tasks in freezer group CGROUP on success\n"
-"  -i,--input=FILE       read data from FILE instead of standard input\n"
-"     --input-fd=FD      read data from file descriptor FD (instead of stdin)\n"
-"  -l,--logfile=FILE     write error and debug data to FILE (default=none)\n"
-"     --logfile-fd=FD    write error and debug data to file desctiptor FD\n"
-"     --inspect          inspect image on-the-fly for error records\n"
-"  -v,--verbose          verbose output\n"
-"  -d,--debug            debugging output\n"
-"     --skip-COND        skip condition COND, and proceed anyway\n"
-"     --warn-COND        warn on condition COND, but proceed anyway\n"
-"     --fail-COND        warn on condition COND, and abort operation\n"
-"  	  COND=any:        any condition\n"
-"  	  COND=pidzero:    task with sid/pgid zero in a --no-pidns restart\n"
-"  	  COND=mntproc:    /proc isn't already mounted at restart (def: warn)\n"
-"";
-
 /*
  * By default, 'restart' creates a new pid namespace in which the
  * restart takes place, using the original pids from the time of the
@@ -176,18 +140,6 @@ static char *sig2str(int sig)
 	return "UNKNOWN SIGNAL";
 }
 
-static int str2sig(char *str)
-{
-	int sig = 0;
-
-	do {
-		if (!strcmp(signal_array[sig].sigstr, str))
-			return signal_array[sig].signum;
-	} while (signal_array[++sig].signum >= 0);
-
-	return -1;
-}
-
 inline static int restart(pid_t pid, int fd, unsigned long flags, int logfd)
 {
 	return syscall(__NR_restart, pid, fd, flags, logfd);
@@ -311,46 +263,6 @@ struct pid_swap {
 	pid_t new;
 };
 
-static void usage(char *str)
-{
-	fprintf(stderr, "%s", str);
-	exit(1);
-}
-
-/* negative retval means error */
-static int str2num(char *str)
-{
-	char *nptr;
-	int num;
-
-	num = strtol(str, &nptr, 10);
-	if (nptr - str != strlen(str))
-		num = -1;
-	return num;
-}
-
-static long cond_to_mask(const char *cond)
-{
-	static struct {
-		char *cond;
-		long mask;
-	} conditions[] = {
-		{"pidzero", CKPT_COND_PIDZERO},
-		{"mntproc", CKPT_COND_MNTPROC},
-		{"any", CKPT_COND_ANY},
-		{NULL, 0}
-	};
-
-	int i;
-
-	for (i = 0; conditions[i].cond; i++)
-		if (!strcmp(cond, conditions[i].cond))
-			return conditions[i].mask;
-
-	printf("restart: invalid warn/fail condition '%s'\n", cond);
-	exit(1);
-}
-
 static inline int ckpt_cond_warn(struct ckpt_ctx *ctx, long mask)
 {
 	return (ctx->args->warn & mask);
@@ -361,202 +273,6 @@ static inline int ckpt_cond_fail(struct ckpt_ctx *ctx, long mask)
 	return (ctx->args->fail & mask);
 }
 
-static void parse_args(struct restart_args *args, int argc, char *argv[])
-{
-	static struct option opts[] = {
-		{ "help",	no_argument,		NULL, 'h' },
-		{ "pidns",	no_argument,		NULL, 'p' },
-		{ "no-pidns",	no_argument,		NULL, 'P' },
-		{ "pids",	no_argument,		NULL, 3 },
-		{ "self",	no_argument,		NULL, 6},
-		{ "signal",	required_argument,	NULL, 4 },
-		{ "inspect",	no_argument,		NULL, 5 },
-		{ "keeplsm",	no_argument,		NULL, 'k' },
-		{ "input",	required_argument,	NULL, 'i' },
-		{ "input-fd",	required_argument,	NULL, 7 },
-		{ "logfile",	required_argument,	NULL, 'l' },
-		{ "logfile-fd",	required_argument,	NULL, 8 },
-		{ "root",	required_argument,	NULL, 'r' },
-		{ "mntns",	no_argument,		NULL, 11 },
-		{ "wait",	no_argument,		NULL, 'w' },
-		{ "show-status",	no_argument,	NULL, 1 },
-		{ "copy-status",	no_argument,	NULL, 2 },
-		{ "no-wait",	no_argument,		NULL, 'W' },
-		{ "freezer",	required_argument,	NULL, 'F' },
-		{ "verbose",	no_argument,		NULL, 'v' },
-		{ "debug",	no_argument,		NULL, 'd' },
-		{ "warn-pidzero",	no_argument,	NULL, 9 },
-		{ "fail-pidzero",	no_argument,	NULL, 10 },
-		{ "mount-pty",	no_argument,		NULL, 12 },
-		{ NULL,		0,			NULL, 0 }
-	};
-	static char optc[] = "hdvkpPwWF:r:i:l:";
-
-	int optind;
-	int sig;
-
-	/* defaults */
-	memset(args, 0, sizeof(*args));
-	args->wait = 1;
-	args->infd = -1;
-	args->logfd = -1;
-	args->warn = CKPT_COND_WARN;
-	args->fail = CKPT_COND_FAIL;
-
-	while (1) {
-		int c = getopt_long(argc, argv, optc, opts, &optind);
-		if (c == -1)
-			break;
-		switch (c) {
-		case '?':
-			exit(1);
-		case 'h':
-			usage(usage_str);
-		case 'v':
-			global_verbose = 1;
-			break;
-		case 5:  /* --inspect */
-			args->inspect = 1;
-			break;
-		case 'i':
-			args->input = optarg;
-			break;
-		case 7:
-			args->infd = str2num(optarg);
-			if (args->infd < 0) {
-				printf("restart: invalid file descriptor\n");
-				exit(1);
-			}
-			break;
-		case 'l':
-			args->logfile = optarg;
-			break;
-		case 8:
-			args->logfd = str2num(optarg);
-			if (args->logfd < 0) {
-				printf("restart: invalid file descriptor\n");
-				exit(1);
-			}
-			break;
-		case 'p':
-			args->pidns = 1;
-			break;
-		case 'P':
-			args->no_pidns = 1;
-			break;
-		case 6:  /* --self */
-			args->self = 1;
-			break;
-		case 4:  /* --signal */
-			sig = str2sig(optarg);
-			if (sig < 0)
-				sig = str2num(optarg);
-			if (sig < 0 || sig >= NSIG) {
-				printf("restart: invalid signal\n");
-				exit(1);
-			}
-			global_send_sigint = sig;
-			break;
-		case 3:  /* --pids */
-			args->pids = 1;
-			args->pidns = 1;  /* implied */
-			break;
-		case 'r':
-			args->root = optarg;
-			break;
-		case 'w':
-			args->wait = 1;
-			break;
-		case 'W':
-			args->wait = 0;
-			break;
-		case 'k':
-			args->keep_lsm = 1;
-			break;
-		case 1:  /* --show-status */
-			args->wait = 1;
-			args->show_status = 1;
-			break;
-		case 2: /* --copy-status */
-			args->wait = 1;
-			args->copy_status = 1;
-			break;
-		case 'd':
-			global_debug = 1;
-			break;
-		case 'F':
-			args->freezer = optarg;
-			break;
-		case 9:
-			args->warn |= cond_to_mask(&opts[optind].name[5]);
-			break;
-		case 10:
-			args->fail |= cond_to_mask(&opts[optind].name[5]);
-			break;
-		case 11:
-			args->mntns = 1;
-			break;
-		case 12:
-			args->mnt_pty = 1;
-			break;
-		default:
-			usage(usage_str);
-		}
-	}
-
-	if (args->no_pidns)
-		args->pidns = 0;
-
-#ifndef CLONE_NEWPID
-	if (args->pidns) {
-		printf("This version of restart was compiled without "
-		       "support for --pidns.\n");
-		exit(1);
-	}
-#endif
-
-#ifndef CHECKPOINT_DEBUG
-	if (global_debug) {
-		printf("This version of restart was compiled without "
-		       "support for --debug.\n");
-		exit(1);
-	}
-#endif
-
-	if (args->pidns)
-		args->pids = 1;
-
-#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 "
-		       "support for --pids.\n");
-		exit(1);
-	}
-#endif
-#endif
-
-	if (args->self &&
-	    (args->pids || args->pidns || args->no_pidns ||
-	     args->show_status || args->copy_status || args->freezer)) {
-		printf("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");
-		exit(1);
-	}
-
-	if (args->logfile && args->logfd >= 0) {
-		printf("Invalid used of both -l/--logfile and --logfile-fd\n");
-		exit(1);
-	}
-
-	if (args->mnt_pty)
-		args->mntns = 1;
-}
-
 static void report_exit_status(int status, char *str, int debug)
 {
 	char msg[64];
@@ -689,15 +405,6 @@ static int freezer_register(struct ckpt_ctx *ctx, pid_t pid)
 	return ret;
 }
 
-int main(int argc, char *argv[])
-{
-	struct restart_args args;
-
-	parse_args(&args, argc, argv);
-
-	return app_restart(&args);
-}
-
 int app_restart(struct restart_args *args)
 {
 	struct ckpt_ctx ctx;
-- 
1.6.6.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 4/4][user-cr] Rename libeclone.a to libcheckpoint.a
       [not found] ` <20100224083452.GB18758-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
                     ` (2 preceding siblings ...)
  2010-02-24  8:36   ` [PATCH 3/4][user-cr] Move main() in restart.c to restart-main.c Sukadev Bhattiprolu
@ 2010-02-24  8:37   ` Sukadev Bhattiprolu
       [not found]     ` <20100224083726.GF18758-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
  2010-02-24 15:15   ` [RFC][PATCH 0/4][user-cr]: First try at integrating LXC and USER-CR Serge E. Hallyn
                     ` (3 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Sukadev Bhattiprolu @ 2010-02-24  8:37 UTC (permalink / raw)
  To: serue-r/Jw6+rmf7HQT0dZR+AlfA, dlezcano-NmTC/0ZBporQT0dZR+AlfA,
	Oren Laadan
  Cc: Containers


From c4063a8976fd8eca9b8d62a12b95c3125c8471c7 Mon Sep 17 00:00:00 2001
From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Date: Tue, 23 Feb 2010 16:25:36 -0800
Subject: [PATCH 4/4][user-cr] Rename libeclone.a to libcheckpoint.a

Export app_restart() and usercr.h to user in a new library libcheckpoint.a.
Until eclone() makes it into glibc, include eclone() also in this new library
and remove the libeclone.a.

TODO:
	- cr-tests should be modified to look for libcheckpoint.a instead
	  of libeclone.a

	- libcheckpoint.a should eventually include the app_checkpoint()
	  interface.  I am guessing extracting the app_checkpoint() from
	  checkpoint.c would be similar to (or somewhat easier than) the
	  way we extracted app_restart() from restart.c

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

diff --git a/Makefile b/Makefile
index 4449081..090914a 100644
--- a/Makefile
+++ b/Makefile
@@ -18,14 +18,14 @@ WARNS := -Wall -Wstrict-prototypes -Wno-trigraphs
 CFLAGS += -g $(WARNS) $(CKPT_INCLUDE) $(DEBUG)
 
 # install dir
+INC_INSTALL_DIR = /usr/include
 BIN_INSTALL_DIR = /bin
 LIB_INSTALL_DIR = /lib
 
-restart: restart.o restart-main.o
-
 ECLONE_PROGS = restart nsexec
 PROGS =	checkpoint ckptinfo $(ECLONE_PROGS)
-LIB_ECLONE = libeclone.a
+LIB_CHECKPOINT = libcheckpoint.a
+INC_CHECKPOINT = usercr.h
 
 # other cleanup
 OTHER = ckptinfo_types.c
@@ -34,32 +34,37 @@ LDLIBS = -lm
 
 .PHONY: all distclean clean headers install
 
-all: $(PROGS)
+all: $(PROGS) $(LIB_CHECKPOINT)
 	@make -C test
 
-$(LIB_ECLONE):
-	ar ruv $(LIB_ECLONE) $^
+restart: restart-main.o
+
+$(PROGS): $(LIB_CHECKPOINT) 
+
+$(LIB_CHECKPOINT):
+	ar ruv $(LIB_CHECKPOINT) $^
 
 # restart needs to be thread-safe
 restart: CFLAGS += -D__REENTRANT -pthread
 
+$(LIB_CHECKPOINT): restart.o
+
 # eclone() is architecture specific
 ifneq ($(SUBARCH),)
-$(ECLONE_PROGS): $(LIB_ECLONE) 
 $(ECLONE_PROGS): CFLAGS += -DARCH_HAS_ECLONE
-$(LIB_ECLONE): clone_$(SUBARCH).o genstack.o
+$(LIB_CHECKPOINT): clone_$(SUBARCH).o genstack.o
 endif
 
 # on powerpc, need also assembly file
 ifeq ($(SUBARCH),ppc)
 CFLAGS += -m32
 ASFLAGS += -m32
-$(LIB_ECLONE): clone_$(SUBARCH)_.o
+$(LIB_CHECKPOINT): clone_$(SUBARCH)_.o
 endif
 ifeq ($(SUBARCH),ppc64)
 CFLAGS += -m64
 ASFLAGS += -m64
-$(LIB_ECLONE): clone_$(SUBARCH)_.o
+$(LIB_CHECKPOINT): clone_$(SUBARCH)_.o
 endif
 
 # ckptinfo dependencies
@@ -71,8 +76,10 @@ ckptinfo_types.c: $(CKPT_HEADERS) ckptinfo.py
 install:
 	@echo /usr/bin/install -m 755 checkpoint restart nsexec ckptinfo $(BIN_INSTALL_DIR)
 	@/usr/bin/install -m 755 checkpoint restart ckptinfo nsexec $(BIN_INSTALL_DIR)
-	@echo /usr/bin/install -m 755 $(LIB_ECLONE) $(LIB_INSTALL_DIR)
-	@/usr/bin/install -m 755 $(LIB_ECLONE) $(LIB_INSTALL_DIR)
+	@echo /usr/bin/install -m 755 $(INC_CHECKPOINT) $(INC_INSTALL_DIR)
+	@/usr/bin/install -m 755 $(INC_CHECKPOINT) $(INC_INSTALL_DIR)
+	@echo /usr/bin/install -m 755 $(LIB_CHECKPOINT) $(LIB_INSTALL_DIR)
+	@/usr/bin/install -m 755 $(LIB_CHECKPOINT) $(LIB_INSTALL_DIR)
 
 $(CKPT_HEADERS): %:
 	./scripts/extract-headers.sh -s $(KERNELSRC) -o ./include
@@ -83,5 +90,5 @@ distclean: clean
 	@rm -f $(CKPT_HEADERS)
 
 clean:
-	@rm -f $(PROGS) $(LIB_ECLONE) $(OTHER) *~ *.o headers.h
+	@rm -f $(PROGS) $(LIB_CHECKPOINT) $(OTHER) *~ *.o headers.h
 	@make -C test clean
diff --git a/restart.c b/restart.c
index b1518f2..a163862 100644
--- a/restart.c
+++ b/restart.c
@@ -530,6 +530,12 @@ int app_restart(struct restart_args *args)
 		ret = ckpt_coordinator(&ctx);
 	}
 
+	/*
+	 * Hack for LXC - which currently only uses pidns == 1, pid0 == 1;
+	 */
+	if (ret >= 0)
+		ret = global_child_pid;
+
 	return ret;
 }
 
-- 
1.6.6.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [RFC][PATCH 0/4][user-cr]: First try at integrating LXC and USER-CR
       [not found] ` <20100224083452.GB18758-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
                     ` (3 preceding siblings ...)
  2010-02-24  8:37   ` [PATCH 4/4][user-cr] Rename libeclone.a to libcheckpoint.a Sukadev Bhattiprolu
@ 2010-02-24 15:15   ` Serge E. Hallyn
  2010-02-24 18:25   ` Cedric Le Goater
                     ` (2 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Serge E. Hallyn @ 2010-02-24 15:15 UTC (permalink / raw)
  To: Sukadev Bhattiprolu; +Cc: dlezcano-NmTC/0ZBporQT0dZR+AlfA, Containers

Quoting Sukadev Bhattiprolu (sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org):
> 
> Following two sets of patches is an early attempt to integrate LXC and
> USER-CR. 
> 
> Overview:
> 
> Have USER-CR export the core checkpoint and restart functionality into a
> library (/lib/libcheckpoint.a and <usercr.h>) and have LXC link with this
> library.
> 
> TODO: 
> 
> 	1. For now, libcheckpoint.a implements only the restart functionality
> 	   and so only lxc_restart command is implemented. Implementing the
> 	   checkpoint functionality and lxc_checkpoint command can be done
> 	   similarly and is hopefully easier than the restart functionality.
> 
> 	2. The restart() functionality in user-cr makes extensive use of global
> 	   variables and debug code. The API must be extended to properly
> 	   include these variables/debug code in the API.
> 
> 	   Similarly, the 'struct restart_args' may need to be sanitized for
> 	   use in a formal API.
> 
> 	3. lxc_restart command  restarts entire containers only (specifically
> 	   it simulates the --pidns --pids --mount-pty arguments to
> 	   /bin/restart).
> 
> 	4. Link lxc_restart and lxc_checkpoint with the shared library
> 	   liblxc.so (currently links statically)
> 
> 	5. ...
> 
> 
> STATUS:
> 	I was able to checkpoint/restart a simple '/bin/sleep 1000' LXC
> 	container, except for a cgroup naming issue after restart (see below).

Very nice, Suka.

-serge

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/4][user-cr] Rename libeclone.a to libcheckpoint.a
       [not found]     ` <20100224083726.GF18758-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
@ 2010-02-24 15:19       ` Serge E. Hallyn
       [not found]         ` <20100224151919.GB6425-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
  2010-02-26 21:53       ` Oren Laadan
  1 sibling, 1 reply; 17+ messages in thread
From: Serge E. Hallyn @ 2010-02-24 15:19 UTC (permalink / raw)
  To: Sukadev Bhattiprolu; +Cc: dlezcano-NmTC/0ZBporQT0dZR+AlfA, Containers

Quoting Sukadev Bhattiprolu (sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org):
> 
> >From c4063a8976fd8eca9b8d62a12b95c3125c8471c7 Mon Sep 17 00:00:00 2001
> From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> Date: Tue, 23 Feb 2010 16:25:36 -0800
> Subject: [PATCH 4/4][user-cr] Rename libeclone.a to libcheckpoint.a
> 
> Export app_restart() and usercr.h to user in a new library libcheckpoint.a.
> Until eclone() makes it into glibc, include eclone() also in this new library
> and remove the libeclone.a.
> 
> TODO:
> 	- cr-tests should be modified to look for libcheckpoint.a instead
> 	  of libeclone.a
> 
> 	- libcheckpoint.a should eventually include the app_checkpoint()
> 	  interface.  I am guessing extracting the app_checkpoint() from
> 	  checkpoint.c would be similar to (or somewhat easier than) the
> 	  way we extracted app_restart() from restart.c
> 
> Signed-off-by: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> ---
>  Makefile  |   33 ++++++++++++++++++++-------------
>  restart.c |    6 ++++++
>  2 files changed, 26 insertions(+), 13 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 4449081..090914a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -18,14 +18,14 @@ WARNS := -Wall -Wstrict-prototypes -Wno-trigraphs
>  CFLAGS += -g $(WARNS) $(CKPT_INCLUDE) $(DEBUG)
> 
>  # install dir
> +INC_INSTALL_DIR = /usr/include
>  BIN_INSTALL_DIR = /bin
>  LIB_INSTALL_DIR = /lib
> 
> -restart: restart.o restart-main.o
> -
>  ECLONE_PROGS = restart nsexec
>  PROGS =	checkpoint ckptinfo $(ECLONE_PROGS)
> -LIB_ECLONE = libeclone.a
> +LIB_CHECKPOINT = libcheckpoint.a
> +INC_CHECKPOINT = usercr.h
> 
>  # other cleanup
>  OTHER = ckptinfo_types.c
> @@ -34,32 +34,37 @@ LDLIBS = -lm
> 
>  .PHONY: all distclean clean headers install
> 
> -all: $(PROGS)
> +all: $(PROGS) $(LIB_CHECKPOINT)
>  	@make -C test
> 
> -$(LIB_ECLONE):
> -	ar ruv $(LIB_ECLONE) $^
> +restart: restart-main.o
> +
> +$(PROGS): $(LIB_CHECKPOINT) 
> +
> +$(LIB_CHECKPOINT):
> +	ar ruv $(LIB_CHECKPOINT) $^
> 
>  # restart needs to be thread-safe
>  restart: CFLAGS += -D__REENTRANT -pthread
> 
> +$(LIB_CHECKPOINT): restart.o
> +
>  # eclone() is architecture specific
>  ifneq ($(SUBARCH),)
> -$(ECLONE_PROGS): $(LIB_ECLONE) 
>  $(ECLONE_PROGS): CFLAGS += -DARCH_HAS_ECLONE
> -$(LIB_ECLONE): clone_$(SUBARCH).o genstack.o
> +$(LIB_CHECKPOINT): clone_$(SUBARCH).o genstack.o
>  endif
> 
>  # on powerpc, need also assembly file
>  ifeq ($(SUBARCH),ppc)
>  CFLAGS += -m32
>  ASFLAGS += -m32
> -$(LIB_ECLONE): clone_$(SUBARCH)_.o
> +$(LIB_CHECKPOINT): clone_$(SUBARCH)_.o
>  endif
>  ifeq ($(SUBARCH),ppc64)
>  CFLAGS += -m64
>  ASFLAGS += -m64
> -$(LIB_ECLONE): clone_$(SUBARCH)_.o
> +$(LIB_CHECKPOINT): clone_$(SUBARCH)_.o
>  endif
> 
>  # ckptinfo dependencies
> @@ -71,8 +76,10 @@ ckptinfo_types.c: $(CKPT_HEADERS) ckptinfo.py
>  install:
>  	@echo /usr/bin/install -m 755 checkpoint restart nsexec ckptinfo $(BIN_INSTALL_DIR)
>  	@/usr/bin/install -m 755 checkpoint restart ckptinfo nsexec $(BIN_INSTALL_DIR)
> -	@echo /usr/bin/install -m 755 $(LIB_ECLONE) $(LIB_INSTALL_DIR)
> -	@/usr/bin/install -m 755 $(LIB_ECLONE) $(LIB_INSTALL_DIR)
> +	@echo /usr/bin/install -m 755 $(INC_CHECKPOINT) $(INC_INSTALL_DIR)
> +	@/usr/bin/install -m 755 $(INC_CHECKPOINT) $(INC_INSTALL_DIR)
> +	@echo /usr/bin/install -m 755 $(LIB_CHECKPOINT) $(LIB_INSTALL_DIR)
> +	@/usr/bin/install -m 755 $(LIB_CHECKPOINT) $(LIB_INSTALL_DIR)
> 
>  $(CKPT_HEADERS): %:
>  	./scripts/extract-headers.sh -s $(KERNELSRC) -o ./include
> @@ -83,5 +90,5 @@ distclean: clean
>  	@rm -f $(CKPT_HEADERS)
> 
>  clean:
> -	@rm -f $(PROGS) $(LIB_ECLONE) $(OTHER) *~ *.o headers.h
> +	@rm -f $(PROGS) $(LIB_CHECKPOINT) $(OTHER) *~ *.o headers.h
>  	@make -C test clean
> diff --git a/restart.c b/restart.c
> index b1518f2..a163862 100644
> --- a/restart.c
> +++ b/restart.c
> @@ -530,6 +530,12 @@ int app_restart(struct restart_args *args)
>  		ret = ckpt_coordinator(&ctx);
>  	}
> 
> +	/*
> +	 * Hack for LXC - which currently only uses pidns == 1, pid0 == 1;
> +	 */
> +	if (ret >= 0)
> +		ret = global_child_pid;
> +

Hmm?  Maybe I'm looking at the wrong context, but isn't this specifically
the existing pidns case?  Should the second block ("new pidns with init")
be the one to execute if your comment is correct?

>  	return ret;
>  }
> 
> -- 
> 1.6.6.1

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC][PATCH 0/4][user-cr]: First try at integrating LXC and USER-CR
       [not found] ` <20100224083452.GB18758-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
                     ` (4 preceding siblings ...)
  2010-02-24 15:15   ` [RFC][PATCH 0/4][user-cr]: First try at integrating LXC and USER-CR Serge E. Hallyn
@ 2010-02-24 18:25   ` Cedric Le Goater
  2010-02-26 21:52   ` Oren Laadan
  2010-03-01 21:22   ` Oren Laadan
  7 siblings, 0 replies; 17+ messages in thread
From: Cedric Le Goater @ 2010-02-24 18:25 UTC (permalink / raw)
  To: Sukadev Bhattiprolu; +Cc: dlezcano-NmTC/0ZBporQT0dZR+AlfA, Containers

On 02/24/2010 09:34 AM, Sukadev Bhattiprolu wrote:
>
> Following two sets of patches is an early attempt to integrate LXC and
> USER-CR.
>
> Overview:
>
> Have USER-CR export the core checkpoint and restart functionality into a
> library (/lib/libcheckpoint.a and<usercr.h>) and have LXC link with this
> library.

suka,

if these patches were accepted in the user-cr git tree, it would be easier
for us to follow with your experiments and move forward on some more tests.

cheers,

C.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/4][user-cr] Rename libeclone.a to libcheckpoint.a
       [not found]         ` <20100224151919.GB6425-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
@ 2010-02-24 22:26           ` Sukadev Bhattiprolu
  0 siblings, 0 replies; 17+ messages in thread
From: Sukadev Bhattiprolu @ 2010-02-24 22:26 UTC (permalink / raw)
  To: Serge E. Hallyn; +Cc: dlezcano-NmTC/0ZBporQT0dZR+AlfA, Containers

Serge E. Hallyn [serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org] wrote:
| > diff --git a/restart.c b/restart.c
| > index b1518f2..a163862 100644
| > --- a/restart.c
| > +++ b/restart.c
| > @@ -530,6 +530,12 @@ int app_restart(struct restart_args *args)
| >  		ret = ckpt_coordinator(&ctx);
| >  	}
| > 
| > +	/*
| > +	 * Hack for LXC - which currently only uses pidns == 1, pid0 == 1;
| > +	 */
| > +	if (ret >= 0)
| > +		ret = global_child_pid;
| > +
| 
| Hmm?  Maybe I'm looking at the wrong context, but isn't this specifically
| the existing pidns case?

Yes it is.

| Should the second block ("new pidns with init")
| be the one to execute if your comment is correct?

Oh, you mean I could just return the global_child_pid from within the
"new pid ns with init" block ?. I could do that for now.

BTW, from a long-term API pov, should app_restart():

	- support the other two types of restart ("new pidns without
	  init" and "subtree in current pidns"). LXC may not use those,
	  but any reason for the API to not support those cases ?

	- If the API supports them, should app_restart() always return the
	  pid of the coordinator (i.e global_child_pid). That way the
	  caller knows the root of the restarted process-tree ?

	- Also, like the kernel's API, how about app_checkpoint() and
	  app_restart() take a logfp/logfd and we send all the debugging
	  output to that file ?

Thanks,

Sukadev


| 
| >  	return ret;
| >  }
| > 
| > -- 
| > 1.6.6.1

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC][PATCH 0/4][user-cr]: First try at integrating LXC and USER-CR
       [not found] ` <20100224083452.GB18758-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
                     ` (5 preceding siblings ...)
  2010-02-24 18:25   ` Cedric Le Goater
@ 2010-02-26 21:52   ` Oren Laadan
       [not found]     ` <4B88429B.4000701-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
  2010-03-01 21:22   ` Oren Laadan
  7 siblings, 1 reply; 17+ messages in thread
From: Oren Laadan @ 2010-02-26 21:52 UTC (permalink / raw)
  To: Sukadev Bhattiprolu; +Cc: dlezcano-NmTC/0ZBporQT0dZR+AlfA, Containers

Suka,

This is good stuff: convert restart to a library and intergate
with LXC is excellent target.

We need to give a thought to the API design and the sort of
functionality that the library will offer. I'd prefer to get
a better view of the whole plan before committing to any
incremental step.

For example:

* what prefix will the library calls have ?  e.g. cr_checkpoint(),
cr_restart() etc.  Could also be checkpoint_... or ckpt_... In
any case, must be consistent and unique, and also agree with the
library name: libcheckpoint.a and usercr.h doesn't make sense to
me.

* what arguments are really necessary in struct restart_args ? for
example, a ->logfd is necessary, but ->logfile (pathname) is not
(the caller should already open the file)

* the library should provide an api to initialize the default args
(e.g. the logfd should be -1 by default), e.g. cr_init_restart_args()

* similarly, only constants and macros relevant to a caller
should be exposed, not internal data structures or macros.

* verbosity, debugging, warn and fail: currently that can be
configurable to some extent; but we should never impose on the
caller a perror() - instead the caller should pass FILE * for
stdout and stderr (if NULL - the library will keep silent).

* probably makes sense to add interface to freeze a process tree
identified by its root pid, and thaw a process tree (or a cgroup).

* does the mnt-ns manipulation belong to cr_restart() ?  perhaps
make it a separate api in the library ?

* we need to change the way restart detects errors and cleans up
from relying on signals to, e.g. on pipes. I've been wanting to
do this for the longest time, and it will eliminate most (if not
all) of the global variables.

Oren.


Sukadev Bhattiprolu wrote:
> Following two sets of patches is an early attempt to integrate LXC and
> USER-CR. 
> 
> Overview:
> 
> Have USER-CR export the core checkpoint and restart functionality into a
> library (/lib/libcheckpoint.a and <usercr.h>) and have LXC link with this
> library.
> 
> TODO: 
> 
> 	1. For now, libcheckpoint.a implements only the restart functionality
> 	   and so only lxc_restart command is implemented. Implementing the
> 	   checkpoint functionality and lxc_checkpoint command can be done
> 	   similarly and is hopefully easier than the restart functionality.
> 
> 	2. The restart() functionality in user-cr makes extensive use of global
> 	   variables and debug code. The API must be extended to properly
> 	   include these variables/debug code in the API.
> 
> 	   Similarly, the 'struct restart_args' may need to be sanitized for
> 	   use in a formal API.
> 
> 	3. lxc_restart command  restarts entire containers only (specifically
> 	   it simulates the --pidns --pids --mount-pty arguments to
> 	   /bin/restart).
> 
> 	4. Link lxc_restart and lxc_checkpoint with the shared library
> 	   liblxc.so (currently links statically)
> 
> 	5. ...
> 
> 
> STATUS:
> 	I was able to checkpoint/restart a simple '/bin/sleep 1000' LXC
> 	container, except for a cgroup naming issue after restart (see below).
> 
> STEPS:
> 
> 1. [USER-CR] Build/install /lib/libcheckpoint.a, /usr/include/usercr.h
>    
>    1.1 Apply the attached [user-cr] patches to the user-cr git tree
>        (I tested with following commit as base)
> 
> 	commit 67cfee9329670ab28eb1a52e94745252b614718f
> 	Author: Oren Laadan <orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
> 	Date:   Mon Feb 22 18:00:06 2010 -0500
> 
>    1.2 Build/install user-cr binaries/libraries/includes
> 
>    	$ make all
> 
> 	$ make install
> 
> 	This should install /lib/libcheckpoint.a and /usr/include/usercr.h
> 
> 2. [LXC] Build lxc_restart using USER-CR API (usercr.h, libcheckpoint.a)
> 
>    2.1 Apply attached [lxc] patches to Daniel Lezcano's lxc.git tree (0.6.5)
> 
>    2.2 Build lxc_restart (this uses static linking for now)
> 
>    	$ make -f Makefile2 lxc_restart
> 
> 3. Create and checkpoint a simple LXC container
> 
> 	$ lxc-execute --name foo --rcfile lxc-macvlan.conf -- /bin/sleep 1000
> 
> 	$ lxc-freeze --name foo
> 
> 	TODO: 
> 		lxc_checkpoint --name foo should checkpoint the container,
> 		For now, use "lxc-ps --name foo" to find pid of lxc-init and
> 		checkpoint using:
> 
> 		$ /bin/checkpoint --output=/tmp/sleep.ckpt <pid-of-lxc-init>
> 
> 	$ lxc-unfreeze --name foo
> 
> 	$ lxc-stop --name foo
> 
> 4. Restart a checkpointed LXC container
> 
> 	$ ./lxc_restart --statefile /tmp/sleep.ckpt --name bar
> 
>    	# Test some common lxc commands after restart
> 
> 	$ lxc-ps --name "bar/1"
> 	CONTAINER    PID TTY          TIME CMD
> 	bar/1       8511 ?        00:00:00 lxc-init
> 	bar/1       8512 ?        00:00:00 sleep
> 
> 	$ lxc-freeze --name "bar/1"
> 
> 	$ grep State /proc/8511/status 
> 	State:	D (disk sleep)
> 
> 	$ grep State /proc/8512/status 
> 	State:	D (disk sleep)
> 
> 	NOTE: 	For some reason, the container name after restart is "bar/1"
> 		instead of "bar".  Due to this, when the lxc_restart is
> 		exiting, I get a "-EBUSY - failed to remove "/cgroup/bar"
> 		error.  I need to fix this still.
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/4][user-cr] Rename libeclone.a to libcheckpoint.a
       [not found]     ` <20100224083726.GF18758-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
  2010-02-24 15:19       ` Serge E. Hallyn
@ 2010-02-26 21:53       ` Oren Laadan
       [not found]         ` <4B8842C8.9080707-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
  1 sibling, 1 reply; 17+ messages in thread
From: Oren Laadan @ 2010-02-26 21:53 UTC (permalink / raw)
  To: Sukadev Bhattiprolu; +Cc: dlezcano-NmTC/0ZBporQT0dZR+AlfA, Containers



Sukadev Bhattiprolu wrote:
> From c4063a8976fd8eca9b8d62a12b95c3125c8471c7 Mon Sep 17 00:00:00 2001
> From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> Date: Tue, 23 Feb 2010 16:25:36 -0800
> Subject: [PATCH 4/4][user-cr] Rename libeclone.a to libcheckpoint.a
> 
> Export app_restart() and usercr.h to user in a new library libcheckpoint.a.
> Until eclone() makes it into glibc, include eclone() also in this new library
> and remove the libeclone.a.

I thought libeclone.a is also used by the test suite ?

Oren.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC][PATCH 0/4][user-cr]: First try at integrating LXC and USER-CR
       [not found]     ` <4B88429B.4000701-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
@ 2010-02-27  0:10       ` Sukadev Bhattiprolu
       [not found]         ` <20100227001002.GA22965-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
  0 siblings, 1 reply; 17+ messages in thread
From: Sukadev Bhattiprolu @ 2010-02-27  0:10 UTC (permalink / raw)
  To: Oren Laadan; +Cc: dlezcano-NmTC/0ZBporQT0dZR+AlfA, Containers

Oren Laadan [orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org] wrote:
> Suka,
>
> This is good stuff: convert restart to a library and intergate
> with LXC is excellent target.
>
> We need to give a thought to the API design and the sort of
> functionality that the library will offer. I'd prefer to get
> a better view of the whole plan before committing to any
> incremental step.
>
> For example:
>
> * what prefix will the library calls have ?  e.g. cr_checkpoint(),
> cr_restart() etc.  Could also be checkpoint_... or ckpt_... In
> any case, must be consistent and unique, and also agree with the
> library name: libcheckpoint.a and usercr.h doesn't make sense to
> me.

Agree. I have been worried about publishing an interface and getting
stuck with a bad interface. But I have also been trying to prototype
interface with LIBLXC available so did not yet spend lot of time
on the interface.

I have been meaning to add a disclaimer to usercr.h that the structures
will change without regard to backward compatibility

libcheckpoint.a looked like a reasonable name but for the header,
we already have a checkpoint.h from the kernel :-) Open to new/better
names/prefixes for the library calls.

Anyway, these names are just for the prototype and to start a discussion,
My guess is that we are not ready to publish a formal interface at
this point, but we can start moving towards that.

My current pass has been to eliminate all global references in
app_restart() and app_checkpoint() (like perror(), stderr, stdout etc).
while preserving the current behavior. Once we have an interface
that LIBLXC can use, maybe we could focus more on stabilizing
the interface ? It seemed to hard to do that with all the
globals in the code.

>
> * what arguments are really necessary in struct restart_args ? for
> example, a ->logfd is necessary, but ->logfile (pathname) is not
> (the caller should already open the file)

I have several small patches to clean this up. Was hoping to post
later today. They remove ->logfile, ->input, ->output fromt he
structures and just leave the relevant fds.

>
> * the library should provide an api to initialize the default args
> (e.g. the logfd should be -1 by default), e.g. cr_init_restart_args()
>
> * similarly, only constants and macros relevant to a caller
> should be exposed, not internal data structures or macros.
>
> * verbosity, debugging, warn and fail: currently that can be
> configurable to some extent; but we should never impose on the
> caller a perror() - instead the caller should pass FILE * for
> stdout and stderr (if NULL - the library will keep silent).

Yes, my current patchset uses a restart_args->ulogfd for the
user-space error messages. The restart-main.c sets this field
to fileno(stderr).

>
> * probably makes sense to add interface to freeze a process tree
> identified by its root pid, and thaw a process tree (or a cgroup).

True. For the near-term, LIBLXC has a way to freeze/thaw the containers

>
> * does the mnt-ns manipulation belong to cr_restart() ?  perhaps
> make it a separate api in the library ?

Good point. We need to think about that.

>
> * we need to change the way restart detects errors and cleans up
> from relying on signals to, e.g. on pipes. I've been wanting to
> do this for the longest time, and it will eliminate most (if not
> all) of the global variables.

Ok.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/4][user-cr] Rename libeclone.a to libcheckpoint.a
       [not found]         ` <4B8842C8.9080707-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
@ 2010-02-27  1:44           ` Sukadev Bhattiprolu
  0 siblings, 0 replies; 17+ messages in thread
From: Sukadev Bhattiprolu @ 2010-02-27  1:44 UTC (permalink / raw)
  To: Oren Laadan; +Cc: dlezcano-NmTC/0ZBporQT0dZR+AlfA, Containers

Oren Laadan [orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org] wrote:
>
>
> Sukadev Bhattiprolu wrote:
>> From c4063a8976fd8eca9b8d62a12b95c3125c8471c7 Mon Sep 17 00:00:00 2001
>> From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
>> Date: Tue, 23 Feb 2010 16:25:36 -0800
>> Subject: [PATCH 4/4][user-cr] Rename libeclone.a to libcheckpoint.a
>>
>> Export app_restart() and usercr.h to user in a new library libcheckpoint.a.
>> Until eclone() makes it into glibc, include eclone() also in this new library
>> and remove the libeclone.a.
>
> I thought libeclone.a is also used by the test suite ?

Yes and I have a todo in the patchset to fix cr-tests if/when we do end up
changing the name.

libeclone.a looks like a short-term name (if/when eclone() is merged in libc,
the library is no longer required).

libcheckpoint.a (or equivalent) has more long term need. If we are having a
libcheckpoint, no point keeping libeclone around too ?

Sukadev

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC][PATCH 0/4][user-cr]: First try at integrating LXC and USER-CR
       [not found]         ` <20100227001002.GA22965-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
@ 2010-02-27  2:14           ` Oren Laadan
  0 siblings, 0 replies; 17+ messages in thread
From: Oren Laadan @ 2010-02-27  2:14 UTC (permalink / raw)
  To: Sukadev Bhattiprolu; +Cc: dlezcano-NmTC/0ZBporQT0dZR+AlfA, Containers



Sukadev Bhattiprolu wrote:
> Oren Laadan [orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org] wrote:
>> Suka,
>>
>> This is good stuff: convert restart to a library and intergate
>> with LXC is excellent target.
>>
>> We need to give a thought to the API design and the sort of
>> functionality that the library will offer. I'd prefer to get
>> a better view of the whole plan before committing to any
>> incremental step.
>>
>> For example:
>>
>> * what prefix will the library calls have ?  e.g. cr_checkpoint(),
>> cr_restart() etc.  Could also be checkpoint_... or ckpt_... In
>> any case, must be consistent and unique, and also agree with the
>> library name: libcheckpoint.a and usercr.h doesn't make sense to
>> me.
> 
> Agree. I have been worried about publishing an interface and getting
> stuck with a bad interface. But I have also been trying to prototype
> interface with LIBLXC available so did not yet spend lot of time
> on the interface.
> 
> I have been meaning to add a disclaimer to usercr.h that the structures
> will change without regard to backward compatibility
> 
> libcheckpoint.a looked like a reasonable name but for the header,
> we already have a checkpoint.h from the kernel :-) Open to new/better
> names/prefixes for the library calls.

we have sys/checkpoint.h, so it makes sense to have checkpoint.h :)

> 
> Anyway, these names are just for the prototype and to start a discussion,
> My guess is that we are not ready to publish a formal interface at
> this point, but we can start moving towards that.
> 
> My current pass has been to eliminate all global references in
> app_restart() and app_checkpoint() (like perror(), stderr, stdout etc).
> while preserving the current behavior. Once we have an interface
> that LIBLXC can use, maybe we could focus more on stabilizing
> the interface ? It seemed to hard to do that with all the
> globals in the code.
> 
>> * what arguments are really necessary in struct restart_args ? for
>> example, a ->logfd is necessary, but ->logfile (pathname) is not
>> (the caller should already open the file)
> 
> I have several small patches to clean this up. Was hoping to post
> later today. They remove ->logfile, ->input, ->output fromt he
> structures and just leave the relevant fds.

What I suggest is to avoid unnecessary work and fixes, it's worth
our while to discuss what could be the api and functionality even
before getting into the implementation details.etc.

Also, two important points that I had meant to mention are:

* versioning: we should provision for api extensions, e.g. in the
struct restart_args, or at the very least a way to figure out the
version.

* hooks / plugins: allow the user to request hooks or plugins,
for example after checkpoint completes but before applications
are thawed, or in restart after the tree is created but before
tasks call sys_restart(). another example - handling the mount-
namespace (and remounting the pty) could be a plugin.

Oren.

> 
>> * the library should provide an api to initialize the default args
>> (e.g. the logfd should be -1 by default), e.g. cr_init_restart_args()
>>
>> * similarly, only constants and macros relevant to a caller
>> should be exposed, not internal data structures or macros.
>>
>> * verbosity, debugging, warn and fail: currently that can be
>> configurable to some extent; but we should never impose on the
>> caller a perror() - instead the caller should pass FILE * for
>> stdout and stderr (if NULL - the library will keep silent).
> 
> Yes, my current patchset uses a restart_args->ulogfd for the
> user-space error messages. The restart-main.c sets this field
> to fileno(stderr).
> 
>> * probably makes sense to add interface to freeze a process tree
>> identified by its root pid, and thaw a process tree (or a cgroup).
> 
> True. For the near-term, LIBLXC has a way to freeze/thaw the containers
> 
>> * does the mnt-ns manipulation belong to cr_restart() ?  perhaps
>> make it a separate api in the library ?
> 
> Good point. We need to think about that.
> 
>> * we need to change the way restart detects errors and cleans up
>> from relying on signals to, e.g. on pipes. I've been wanting to
>> do this for the longest time, and it will eliminate most (if not
>> all) of the global variables.
> 
> Ok.
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC][PATCH 0/4][user-cr]: First try at integrating LXC and USER-CR
       [not found] ` <20100224083452.GB18758-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
                     ` (6 preceding siblings ...)
  2010-02-26 21:52   ` Oren Laadan
@ 2010-03-01 21:22   ` Oren Laadan
  7 siblings, 0 replies; 17+ messages in thread
From: Oren Laadan @ 2010-03-01 21:22 UTC (permalink / raw)
  To: Sukadev Bhattiprolu; +Cc: dlezcano-NmTC/0ZBporQT0dZR+AlfA, Containers

Suka,

To make these patches available for those who want to try out
lxc + c/r, I pulled them to a separate branch: ckpt-v19-suka.

(The last two did not apply cleanly because of recent changes
to the Makefile so I merged them manually - let me know if it
breaks).

Oren


Sukadev Bhattiprolu wrote:
> Following two sets of patches is an early attempt to integrate LXC and
> USER-CR. 
> 
> Overview:
> 
> Have USER-CR export the core checkpoint and restart functionality into a
> library (/lib/libcheckpoint.a and <usercr.h>) and have LXC link with this
> library.
> 
> TODO: 
> 
> 	1. For now, libcheckpoint.a implements only the restart functionality
> 	   and so only lxc_restart command is implemented. Implementing the
> 	   checkpoint functionality and lxc_checkpoint command can be done
> 	   similarly and is hopefully easier than the restart functionality.
> 
> 	2. The restart() functionality in user-cr makes extensive use of global
> 	   variables and debug code. The API must be extended to properly
> 	   include these variables/debug code in the API.
> 
> 	   Similarly, the 'struct restart_args' may need to be sanitized for
> 	   use in a formal API.
> 
> 	3. lxc_restart command  restarts entire containers only (specifically
> 	   it simulates the --pidns --pids --mount-pty arguments to
> 	   /bin/restart).
> 
> 	4. Link lxc_restart and lxc_checkpoint with the shared library
> 	   liblxc.so (currently links statically)
> 
> 	5. ...
> 
> 
> STATUS:
> 	I was able to checkpoint/restart a simple '/bin/sleep 1000' LXC
> 	container, except for a cgroup naming issue after restart (see below).
> 
> STEPS:
> 
> 1. [USER-CR] Build/install /lib/libcheckpoint.a, /usr/include/usercr.h
>    
>    1.1 Apply the attached [user-cr] patches to the user-cr git tree
>        (I tested with following commit as base)
> 
> 	commit 67cfee9329670ab28eb1a52e94745252b614718f
> 	Author: Oren Laadan <orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
> 	Date:   Mon Feb 22 18:00:06 2010 -0500
> 
>    1.2 Build/install user-cr binaries/libraries/includes
> 
>    	$ make all
> 
> 	$ make install
> 
> 	This should install /lib/libcheckpoint.a and /usr/include/usercr.h
> 
> 2. [LXC] Build lxc_restart using USER-CR API (usercr.h, libcheckpoint.a)
> 
>    2.1 Apply attached [lxc] patches to Daniel Lezcano's lxc.git tree (0.6.5)
> 
>    2.2 Build lxc_restart (this uses static linking for now)
> 
>    	$ make -f Makefile2 lxc_restart
> 
> 3. Create and checkpoint a simple LXC container
> 
> 	$ lxc-execute --name foo --rcfile lxc-macvlan.conf -- /bin/sleep 1000
> 
> 	$ lxc-freeze --name foo
> 
> 	TODO: 
> 		lxc_checkpoint --name foo should checkpoint the container,
> 		For now, use "lxc-ps --name foo" to find pid of lxc-init and
> 		checkpoint using:
> 
> 		$ /bin/checkpoint --output=/tmp/sleep.ckpt <pid-of-lxc-init>
> 
> 	$ lxc-unfreeze --name foo
> 
> 	$ lxc-stop --name foo
> 
> 4. Restart a checkpointed LXC container
> 
> 	$ ./lxc_restart --statefile /tmp/sleep.ckpt --name bar
> 
>    	# Test some common lxc commands after restart
> 
> 	$ lxc-ps --name "bar/1"
> 	CONTAINER    PID TTY          TIME CMD
> 	bar/1       8511 ?        00:00:00 lxc-init
> 	bar/1       8512 ?        00:00:00 sleep
> 
> 	$ lxc-freeze --name "bar/1"
> 
> 	$ grep State /proc/8511/status 
> 	State:	D (disk sleep)
> 
> 	$ grep State /proc/8512/status 
> 	State:	D (disk sleep)
> 
> 	NOTE: 	For some reason, the container name after restart is "bar/1"
> 		instead of "bar".  Due to this, when the lxc_restart is
> 		exiting, I get a "-EBUSY - failed to remove "/cgroup/bar"
> 		error.  I need to fix this still.
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH] c/r: fix regression (in "fix scheduling in atomic while restoring ipc shm")
       [not found]     ` <20100224083534.GC18758-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
@ 2010-03-03  0:04       ` Oren Laadan
       [not found]         ` <1267574649-14269-1-git-send-email-orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
  0 siblings, 1 reply; 17+ messages in thread
From: Oren Laadan @ 2010-03-03  0:04 UTC (permalink / raw)
  To: Nikita V. Youshchenko
  Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA

So... here's a simple fix to a silly regression - this should bring
an end to my little fiasco. 

Signed-off-by: Oren Laadan <orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
---
 ipc/checkpoint_msg.c |    2 +-
 ipc/checkpoint_sem.c |    2 +-
 ipc/checkpoint_shm.c |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/ipc/checkpoint_msg.c b/ipc/checkpoint_msg.c
index 073a893..0155c20 100644
--- a/ipc/checkpoint_msg.c
+++ b/ipc/checkpoint_msg.c
@@ -359,7 +359,7 @@ int restore_ipc_msg(struct ckpt_ctx *ctx, struct ipc_namespace *ns)
 	 * ipc-ns, we will need to re-examine this.
 	 */
 
-	ipc = idr_find(&msg_ids->ipcs_idr, h->perms.id);
+	ipc = idr_find(&msg_ids->ipcs_idr, ipcid_to_idx(h->perms.id));
 	BUG_ON(!ipc);
 
 	msq = container_of(ipc, struct msg_queue, q_perm);
diff --git a/ipc/checkpoint_sem.c b/ipc/checkpoint_sem.c
index 78c1932..2abfb5a 100644
--- a/ipc/checkpoint_sem.c
+++ b/ipc/checkpoint_sem.c
@@ -216,7 +216,7 @@ int restore_ipc_sem(struct ckpt_ctx *ctx, struct ipc_namespace *ns)
 	 * ipc-ns, we will need to re-examine this.
 	 */
 
-	ipc = idr_find(&sem_ids->ipcs_idr, h->perms.id);
+	ipc = idr_find(&sem_ids->ipcs_idr, ipcid_to_idx(h->perms.id));
 	BUG_ON(!ipc);
 
 	sem = container_of(ipc, struct sem_array, sem_perm);
diff --git a/ipc/checkpoint_shm.c b/ipc/checkpoint_shm.c
index 6329245..a07c051 100644
--- a/ipc/checkpoint_shm.c
+++ b/ipc/checkpoint_shm.c
@@ -270,7 +270,7 @@ int restore_ipc_shm(struct ckpt_ctx *ctx, struct ipc_namespace *ns)
 	 * ipc-ns, we will need to re-examine this.
 	 */
 
-	ipc = idr_find(&shm_ids->ipcs_idr, h->perms.id);
+	ipc = idr_find(&shm_ids->ipcs_idr, ipcid_to_idx(h->perms.id));
 	BUG_ON(!ipc);
 
 	shp = container_of(ipc, struct shmid_kernel, shm_perm);
-- 
1.6.3.3

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH] c/r: fix regression (in "fix scheduling in atomic while restoring ipc shm")
       [not found]         ` <1267574649-14269-1-git-send-email-orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
@ 2010-03-03 15:38           ` Serge E. Hallyn
  0 siblings, 0 replies; 17+ messages in thread
From: Serge E. Hallyn @ 2010-03-03 15:38 UTC (permalink / raw)
  To: Oren Laadan
  Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	Nikita V. Youshchenko

Quoting Oren Laadan (orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org):
> So... here's a simple fix to a silly regression - this should bring
> an end to my little fiasco. 
> 
> Signed-off-by: Oren Laadan <orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>

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

> ---
>  ipc/checkpoint_msg.c |    2 +-
>  ipc/checkpoint_sem.c |    2 +-
>  ipc/checkpoint_shm.c |    2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/ipc/checkpoint_msg.c b/ipc/checkpoint_msg.c
> index 073a893..0155c20 100644
> --- a/ipc/checkpoint_msg.c
> +++ b/ipc/checkpoint_msg.c
> @@ -359,7 +359,7 @@ int restore_ipc_msg(struct ckpt_ctx *ctx, struct ipc_namespace *ns)
>  	 * ipc-ns, we will need to re-examine this.
>  	 */
> 
> -	ipc = idr_find(&msg_ids->ipcs_idr, h->perms.id);
> +	ipc = idr_find(&msg_ids->ipcs_idr, ipcid_to_idx(h->perms.id));
>  	BUG_ON(!ipc);
> 
>  	msq = container_of(ipc, struct msg_queue, q_perm);
> diff --git a/ipc/checkpoint_sem.c b/ipc/checkpoint_sem.c
> index 78c1932..2abfb5a 100644
> --- a/ipc/checkpoint_sem.c
> +++ b/ipc/checkpoint_sem.c
> @@ -216,7 +216,7 @@ int restore_ipc_sem(struct ckpt_ctx *ctx, struct ipc_namespace *ns)
>  	 * ipc-ns, we will need to re-examine this.
>  	 */
> 
> -	ipc = idr_find(&sem_ids->ipcs_idr, h->perms.id);
> +	ipc = idr_find(&sem_ids->ipcs_idr, ipcid_to_idx(h->perms.id));
>  	BUG_ON(!ipc);
> 
>  	sem = container_of(ipc, struct sem_array, sem_perm);
> diff --git a/ipc/checkpoint_shm.c b/ipc/checkpoint_shm.c
> index 6329245..a07c051 100644
> --- a/ipc/checkpoint_shm.c
> +++ b/ipc/checkpoint_shm.c
> @@ -270,7 +270,7 @@ int restore_ipc_shm(struct ckpt_ctx *ctx, struct ipc_namespace *ns)
>  	 * ipc-ns, we will need to re-examine this.
>  	 */
> 
> -	ipc = idr_find(&shm_ids->ipcs_idr, h->perms.id);
> +	ipc = idr_find(&shm_ids->ipcs_idr, ipcid_to_idx(h->perms.id));
>  	BUG_ON(!ipc);
> 
>  	shp = container_of(ipc, struct shmid_kernel, shm_perm);
> -- 
> 1.6.3.3

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2010-03-03 15:38 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-02-24  8:34 [RFC][PATCH 0/4][user-cr]: First try at integrating LXC and USER-CR Sukadev Bhattiprolu
     [not found] ` <20100224083452.GB18758-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2010-02-24  8:35   ` [PATCH 1/4][user-cr] Move common definitions to restart.h Sukadev Bhattiprolu
     [not found]     ` <20100224083534.GC18758-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2010-03-03  0:04       ` [PATCH] c/r: fix regression (in "fix scheduling in atomic while restoring ipc shm") Oren Laadan
     [not found]         ` <1267574649-14269-1-git-send-email-orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
2010-03-03 15:38           ` Serge E. Hallyn
2010-02-24  8:36   ` [PATCH 2/4][user-cr] Rename struct args to struct restart_args Sukadev Bhattiprolu
2010-02-24  8:36   ` [PATCH 3/4][user-cr] Move main() in restart.c to restart-main.c Sukadev Bhattiprolu
2010-02-24  8:37   ` [PATCH 4/4][user-cr] Rename libeclone.a to libcheckpoint.a Sukadev Bhattiprolu
     [not found]     ` <20100224083726.GF18758-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2010-02-24 15:19       ` Serge E. Hallyn
     [not found]         ` <20100224151919.GB6425-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2010-02-24 22:26           ` Sukadev Bhattiprolu
2010-02-26 21:53       ` Oren Laadan
     [not found]         ` <4B8842C8.9080707-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
2010-02-27  1:44           ` Sukadev Bhattiprolu
2010-02-24 15:15   ` [RFC][PATCH 0/4][user-cr]: First try at integrating LXC and USER-CR Serge E. Hallyn
2010-02-24 18:25   ` Cedric Le Goater
2010-02-26 21:52   ` Oren Laadan
     [not found]     ` <4B88429B.4000701-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
2010-02-27  0:10       ` Sukadev Bhattiprolu
     [not found]         ` <20100227001002.GA22965-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2010-02-27  2:14           ` Oren Laadan
2010-03-01 21:22   ` Oren Laadan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.