[RFC][PATCH 0/5][lxc]: Link with USERCR

[RFC][PATCH 0/5][lxc]: Link with USERCR

[Sukadev Bhattiprolu]

Link lxc_checkpoint, lxc_restart with USERCR[2]

Following set of patches, when applied to LXC 0.6.5 and built with
USERCR, enable lxc_checkpoint and lxc_restart of some simple containers.

TODO:
	- For simplicity, this patchset currently links *statically* with
	  USERCR. We should eventually build checkpoint.o and restart.o in
	  USERCR as shared objects and link lxc_checkpoint/lxc_restart with
	  these shared objects and liblxc.so. (This also required rebuilding
	  lxc-execute statically, so lxc_checkpoint can communicate with
	  lxc-execute to get the root-pid of container).

	- Determine if lxc_checkpoint needs a --container option (see
	  TODOs in src/lxc/checkpoint.c)

	- This patchset was tested using lxc-macvlan.conf. But for the VNC
	  server testing I ran into a problem with lxc-macvlan.conf. I
	  switched to 'lxc-no-netns.conf' and was able to C/R the VNC server.
	  I have not debugged the problem with VNC and lxc-macvlan.conf.

	- 'global_send_sigint' is still a global variable in USERCR. We need
	  to define a better interface to expose its functionality to callers
	  of app_restart().

	- Choose better names and API for USERCR :-)

Changelog[v2]: 
	(Based on feedback from Oren Laadan, Serge Hallyn, Daniel Lezcano
	and Cedric Le Goater)

	- Rather than drop --directory option to lxc_checkpoint/lxc_restart
	  add a new option (--image).

	- Integrate lxc_checkpoint to work with USERCR

	- USERCR renamed usercr.h to "app-checkpoint.h"

	- USERCR does not create/install libcheckpoint.a and usercr.h for now.
	  So link directly with app-checkpoint.h, restart.o and checkpoint.o

	- USERCR renames the interfaces to app_checkpoint() and app_restart()
	  'struct app_checkpoint_args' 'struct app_restart_args'.
	  
USAGE:

1. Build USERCR

	$ cd /root

	$ git-clone git://git.ncl.cs.columbia.edu/pub/git/user-cr.git user-cr

	$ git-checkout ckpt-v20 
	
	# Apply patches in the following set:

	# https://lists.linux-foundation.org/pipermail/containers/2010-March/023793.html

	$ cd /root/user-cr

	$ make 

		This should create restart.o and checkpoint.o

2. Build/install lxc_checkpoint and lxc_restart

	# Do normal LXC build/install of 0.6.5.

	# Then replace 'lxc-execute', 'lxc-checkpoint', 'lxc-restart' with
	# new version. i.e Apply attached patches and:

	$ cd src/lxc

	$ USERCR=/root/user-cr \
		make -f Makefile2 lxc_checkpoint lxc_restart lxc_execute

	$ cp lxc_checkpoint lxc_restart lxc-execute  /usr/local/bin

3. Checkpoint/restart a simple LXC container

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

	$ lxc-freeze --name foo

	$ lxc-checkpoint --name foo --image /root/lxc-foo.ckpt

	$ lxc-unfreeze --name foo

	$ lxc-stop --name foo

	$ lxc-restart --name foo --image /root/lxc-foo.ckpt

4. Checkpoint/restart other LXC containers such as:

	- a file-io session (see run-fileio1 in cr-tests[1])

	- process-tree (see run-ptree1 in cr-tests[1])

	- A vi session inside a VNC Server using "twm". i.e

		$ cat /root/.vnc/xstartup
		#!/bin/sh

		xsetroot -solid grey
		vncconfig -iconic &
		xterm -geometry 80x24+10+10 -ls -title "$VNCDESKTOP Desktop" &
		twm &

		$ lxc-execute --name foo --rcfile lxc-no-netns.conf -- \
			/usr/bin/vncserver :1

		$ vncviewer :1

			# Open a vi session

		$ lxc-freeze --name foo 

			etc

[1]: cr-tests:	git://git.sr71.net/~hallyn/cr_tests.git
[2]: USERCR: git://git.ncl.cs.columbia.edu/pub/git/user-cr.git

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

[PATCH 1/5][lxc] Enable static linking of some lxc binaries

[Sukadev Bhattiprolu]

From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Date: Wed, 10 Mar 2010 22:12:34 -0800
Subject: [PATCH 1/5][lxc] Enable static linking of some lxc binaries

This is a temporary commit to statically link lxc_checkpoint, lxc_execute
and lxc_restart.

Signed-off-by: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
 src/lxc/Makefile2 |   35 +++++++++++++++++++++++++++++++++++
 1 files changed, 35 insertions(+), 0 deletions(-)
 create mode 100644 src/lxc/Makefile2

diff --git a/src/lxc/Makefile2 b/src/lxc/Makefile2
new file mode 100644
index 0000000..26b9cd3
--- /dev/null
+++ b/src/lxc/Makefile2
@@ -0,0 +1,35 @@
+BASEDIR = ../..
+
+USERCR ?= 	$(BASEDIR)/../user-cr-lxc/
+
+CFLAGS = 	-static -I . -I .. -I $(USERCR) -I $(USERCR)/include
+LDFLAGS = 	$(USERCR)/libeclone.a -lutil
+
+LXC_OBJS = 	start.o stop.o conf.o confile.o arguments.o monitor.o	\
+		log.o mainloop.o utils.o commands.o state.o cgroup.o	\
+		error.o namespace.o parse.o network.o af_unix.o 	\
+		console.o nl.o
+
+USERCR_CHECKPOINT_OBJ =	$(USERCR)/checkpoint.o
+
+USERCR_RESTART_OBJ = 	$(USERCR)/restart.o
+
+LXC_CHECKPOINT_OBJS = 	$(LXC_OBJS) $(USERCR_CHECKPOINT_OBJ) \
+			lxc_checkpoint.o checkpoint.o
+
+LXC_RESTART_OBJS = 	$(LXC_OBJS) $(USERCR_RESTART_OBJ) \
+			lxc_restart.o restart.o 
+
+LXC_EXECUTE_OBJS =	$(LXC_OBJS) lxc_execute.o
+
+lxc_checkpoint: $(LXC_CHECKPOINT_OBJS)
+	$(CC) -o lxc_checkpoint $(LXC_CHECKPOINT_OBJS) $(LDFLAGS)
+
+lxc_restart: $(LXC_RESTART_OBJS)
+	$(CC) -o lxc_restart $(LXC_RESTART_OBJS) $(LDFLAGS)
+
+lxc_execute: $(LXC_EXECUTE_OBJS)
+	$(CC) -o lxc_execute $(LXC_EXECUTE_OBJS) $(LDFLAGS)
+
+clean:
+	rm -f $(RESTART_OBJS)
-- 
1.6.6.1

[PATCH 2/5][lxc] lxc_restart: Add --image option

[Sukadev Bhattiprolu]

From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Date: Wed, 10 Mar 2010 21:59:39 -0800
Subject: [PATCH 2/5][lxc] lxc_restart: Add --image option

The existing --directory option to lxc_restart expects the checkpoint state
to be a directory. USERCR however uses a single regular file to store the
checkpoint image. So add a --image option to enable checkpointing and
restarting applications using USERCR.

Depending on how the application was checkpointed, users should specify
either --image=CHECKPOINT-IMAGE or the --directory=STATEFILE option (but not
both).

Signed-off-by: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
 src/lxc/arguments.h   |    1 +
 src/lxc/lxc_restart.c |   29 ++++++++++++++++++++++-------
 2 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/src/lxc/arguments.h b/src/lxc/arguments.h
index 3bc027d..30953ee 100644
--- a/src/lxc/arguments.h
+++ b/src/lxc/arguments.h
@@ -45,6 +45,7 @@ struct lxc_arguments {
 	int daemonize;
 	const char *rcfile;
 	const char *statefile;
+	const char *imagefile;
 
 	/* for lxc-checkpoint */
 	int flags;
diff --git a/src/lxc/lxc_restart.c b/src/lxc/lxc_restart.c
index 7db1d85..39f7ea8 100644
--- a/src/lxc/lxc_restart.c
+++ b/src/lxc/lxc_restart.c
@@ -41,10 +41,17 @@ static struct lxc_list defines;
 
 static int my_checker(const struct lxc_arguments* args)
 {
-	if (!args->statefile) {
-		lxc_error(args, "no statefile specified");
-		return -1;
-	}
+	int i, s;
+	
+	/* make them boolean */
+	i = !!(args->imagefile);
+	s = !!(args->statefile);
+
+        if (!(i ^ s)) {
+                lxc_error(args, "Must specify exactly one of --directory "
+                                "and --image options");
+                return -1;
+        }
 
 	return 0;
 }
@@ -54,6 +61,7 @@ static int my_parser(struct lxc_arguments* args, int c, char* arg)
 	switch (c) {
 	case 'd': args->statefile = arg; break;
 	case 'f': args->rcfile = arg; break;
+	case 'i': args->imagefile = arg; break;
 	case 'p': args->flags = LXC_FLAG_PAUSE; break;
 	case 's': return lxc_config_define_add(&defines, arg);
 	}
@@ -66,6 +74,7 @@ static const struct option my_longopts[] = {
 	{"rcfile", required_argument, 0, 'f'},
 	{"pause", no_argument, 0, 'p'},
 	{"define", required_argument, 0, 's'},
+	{"image", required_argument, 0, 'i'},
 	LXC_COMMON_OPTIONS
 };
 
@@ -73,14 +82,16 @@ static struct lxc_arguments my_args = {
 	.progname = "lxc-restart",
 	.help     = "\
 --name=NAME --directory STATEFILE\n\
+\tlxc_restart --name=NAME --image CHECKPOINT-IMAGE\n\
 \n\
 lxc-restart restarts from STATEFILE the NAME container\n\
 \n\
 Options :\n\
   -n, --name=NAME      NAME for name of the container\n\
   -p, --pause          do not release the container after the restart\n\
-  -d, --directory=STATEFILE for name of statefile\n\
+  -d, --directory=STATEFILE for name of statefile(legacy mode)\n\
   -f, --rcfile=FILE Load configuration file FILE\n\
+  -i, --image=FILE Load the application state from FILE(user-cr mode)\n\
   -s, --define KEY=VAL Assign VAL to configuration variable KEY\n",
 	.options  = my_longopts,
 	.parser   = my_parser,
@@ -90,6 +101,7 @@ Options :\n\
 int main(int argc, char *argv[])
 {
 	char *rcfile = NULL;
+	const char *image = NULL;
 	struct lxc_conf *conf;
 
 	lxc_list_init(&defines);
@@ -131,6 +143,9 @@ int main(int argc, char *argv[])
 	if (lxc_config_define_load(&defines, conf))
 		return -1;
 
-	return lxc_restart(my_args.name, my_args.statefile, conf,
-			   my_args.flags);
+	image = my_args.imagefile;
+	if (my_args.statefile)
+		image = my_args.statefile;
+
+	return lxc_restart(my_args.name, image, conf, my_args.flags);
 }
-- 
1.6.6.1

[PATCH 3/5][lxc] lxc_checkpoint: Add --image option

[Sukadev Bhattiprolu]

From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Date: Wed, 10 Mar 2010 22:24:17 -0800
Subject: [PATCH 3/5][lxc] lxc_checkpoint: Add --image option

The existing --directory option to lxc_checkpoint expects to save the
checkpoint state in a directory. USERCR however uses a single regular
file to store the checkpoint image. So add a --image option to enable
checkpointing and restarting applications using USERCR.

Users should specify either --image or --directory option (but not both)
to select the file/directory where the application state will be stored.

Signed-off-by: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
 src/lxc/lxc_checkpoint.c |   34 +++++++++++++++++++++++++---------
 1 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/src/lxc/lxc_checkpoint.c b/src/lxc/lxc_checkpoint.c
index a8c74a9..3554518 100644
--- a/src/lxc/lxc_checkpoint.c
+++ b/src/lxc/lxc_checkpoint.c
@@ -39,10 +39,17 @@ lxc_log_define(lxc_checkpoint_ui, lxc_checkpoint);
 
 static int my_checker(const struct lxc_arguments* args)
 {
-	if (!args->statefile) {
-		lxc_error(args, "no statefile specified");
-		return -1;
-	}
+	int i, s;
+	
+	/* make them boolean */
+	i = !!(args->imagefile);
+	s = !!(args->statefile);
+
+        if (!(i ^ s)) {
+                lxc_error(args, "Must specify exactly one of --directory "
+                                "and --image options");
+                return -1;
+        }
 
 	return 0;
 }
@@ -53,6 +60,7 @@ static int my_parser(struct lxc_arguments* args, int c, char* arg)
 	case 'k': args->flags = LXC_FLAG_HALT; break;
 	case 'p': args->flags = LXC_FLAG_PAUSE; break;
 	case 'd': args->statefile = arg; break;
+	case 'i': args->imagefile = arg; break;
 	}
 	return 0;
 }
@@ -61,6 +69,7 @@ static const struct option my_longopts[] = {
 	{"kill", no_argument, 0, 'k'},
 	{"pause", no_argument, 0, 'p'},
 	{"directory", required_argument, 0, 'd'},
+	{"image", required_argument, 0, 'i'},
 	LXC_COMMON_OPTIONS
 };
 
@@ -68,6 +77,7 @@ static struct lxc_arguments my_args = {
 	.progname = "lxc-checkpoint",
 	.help     = "\
 --name=NAME --directory STATEFILE\n\
+\tlxc_checkpoint --name=NAME --image CHECKPOINT-IMAGE\n\
 \n\
 lxc-checkpoint checkpoints in STATEFILE the NAME container\n\
 \n\
@@ -75,7 +85,8 @@ Options :\n\
   -n, --name=NAME      NAME for name of the container\n\
   -k, --kill           stop the container after checkpoint\n\
   -p, --pause          don't unfreeze the container after the checkpoint\n\
-  -d, --directory=STATEFILE where to store the statefile\n",
+  -d, --directory=STATEFILE where to store the statefile (legacy mode)\n\
+  -i, --image=FILE where to store the checkpoint-image (USERCR mode)\n",
 
 	.options  = my_longopts,
 	.parser   = my_parser,
@@ -97,6 +108,7 @@ static int create_statefile(const char *dir)
 int main(int argc, char *argv[])
 {
 	int ret;
+	const char *image;
 
 	ret = lxc_arguments_parse(&my_args, argc, argv);
 	if (ret)
@@ -107,11 +119,15 @@ int main(int argc, char *argv[])
 	if (ret)
 		return ret;
 
-	ret = create_statefile(my_args.statefile);
-	if (ret)
-		return ret;
+	image = my_args.imagefile;
+	if (my_args.statefile) {
+		image = my_args.statefile;
+		ret = create_statefile(my_args.statefile);
+		if (ret)
+			return ret;
+	}
 
-	ret = lxc_checkpoint(my_args.name, my_args.statefile, my_args.flags);
+	ret = lxc_checkpoint(my_args.name, image, my_args.flags);
 	if (ret)
 		ERROR("failed to checkpoint '%s'", my_args.name);
 	else
-- 
1.6.6.1

[PATCH 4/5][lxc] Hook up lxc_restart() with app_restart()

[Sukadev Bhattiprolu]

From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Date: Thu, 11 Mar 2010 20:47:56 -0800
Subject: [PATCH 4/5][lxc] Hook up lxc_restart() with app_restart()

Have lxc_restart() call app_restart() implemented in the 'restart.o' from
USER-CR git tree.

Changelog[v2]:
	- Link with restart.o from usercr rather than libcheckpoint.a
	- rename 'struct restart_args' to 'struct app_restart_args'
	- Initialize the new field app_restart_args->uerrfd
	- (Oren Laadan)Remove ->send_sigint field from 'struct app_restart_args'
---
 src/lxc/restart.c |  104 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 104 insertions(+), 0 deletions(-)

diff --git a/src/lxc/restart.c b/src/lxc/restart.c
index 467489e..945435c 100644
--- a/src/lxc/restart.c
+++ b/src/lxc/restart.c
@@ -22,11 +22,115 @@
  */
 #include <lxc/lxc.h>
 #include <lxc/log.h>
+#include <lxc/start.h>
+#include <lxc/namespace.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <signal.h>
+#include <sys/prctl.h>
+#include <app-checkpoint.h>
+
+struct lxc_restart_arg {
+	const char *name;
+	const char *statefile;
+	char *const argv;
+	struct lxc_handler *handler;
+};
 
 lxc_log_define(lxc_restart, lxc);
 
+static int do_restart(struct lxc_restart_arg *lxcarg)
+{
+	int pid;
+	struct lxc_handler *handler = lxcarg->handler;
+	const char *name = lxcarg->name;
+	char *const argv = lxcarg->argv;
+	const char *statefile = lxcarg->statefile;
+	struct app_restart_args restart_args;
+
+        if (sigprocmask(SIG_SETMASK, &handler->oldmask, NULL)) {
+                SYSERROR("failed to set sigprocmask");
+                return -1;
+        }
+
+        if (prctl(PR_SET_PDEATHSIG, SIGKILL, 0, 0, 0)) {
+                SYSERROR("failed to set pdeath signal");
+                return -1;
+        }
+
+	memset(&restart_args, 0, sizeof(restart_args));
+
+	if (statefile) {
+		restart_args.infd = open(statefile, O_RDONLY, 0);
+		if (restart_args.infd < 0) {
+			SYSERROR("Failed to open statefile %s\n", statefile);
+			return -1;
+		}
+	}
+
+	restart_args.pids = 1;
+	restart_args.pidns = 1;
+	restart_args.mnt_pty = 1;
+	restart_args.mntns = 1;
+	restart_args.klogfd = -1;
+	restart_args.ulogfd = lxc_log_fd;
+	restart_args.uerrfd = fileno(stderr);
+	restart_args.debug = 1;
+	restart_args.wait = 0;
+
+        pid = app_restart(&restart_args);
+
+        return pid;
+}
+
 int lxc_restart(const char *name, const char *statefile, struct lxc_conf *conf,
 		int flags)
 {
+	int err;
+	int status;
+	struct lxc_handler *handler;
+	struct lxc_restart_arg lxcarg = {
+		.name = name,
+		.statefile = statefile,
+		.handler = NULL,
+	};
+
+	handler = lxc_init(name, conf);
+	if (!handler) {
+		ERROR("failed to initialize the container");
+		return -1;
+	}
+
+	lxcarg.handler = handler;
+	handler->pid = do_restart(&lxcarg);
+
+	INFO("do_restart(): returns pid %d\n", handler->pid);
+	lxc_rename_nsgroup(name, handler);
+
+	err = lxc_close_all_inherited_fd();
+	if (err) {
+		ERROR("unable to close inherited fds");
+		goto out_abort;
+	}
+
+	err = lxc_poll(name, handler);
+	if (err) {
+		ERROR("mainloop exited with an error");
+		goto out_abort;
+	}
+
+	while (waitpid(handler->pid, &status, 0) < 0 && errno == EINTR)
+		continue;
+
+	err =  lxc_error_set_and_log(handler->pid, status);
+
+out_fini:
+	lxc_fini(name, handler);
+	return err;
+
+out_abort:
+	lxc_abort(name, handler);
+	goto out_fini;
+
 	return 0;
 }
-- 
1.6.6.1

[PATCH 5/5][lxc] Hook up lxc_checkpoint() with app_checkpoint()

[Sukadev Bhattiprolu]

From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Date: Thu, 11 Mar 2010 21:32:38 -0800
Subject: [PATCH 5/5][lxc] Hook up lxc_checkpoint() with app_checkpoint()

Have lxc_checkpoint() call app_checkpoint() implemented in checkpoint.o
in the USER-CR git tree

TODO:
	- Map lxc_flags to flags in sys_checkpoint()
	- Initialize app_checkpoint_args.debug and other fields based on
	  command line options to lxc_checkpoint rather than hard-coding
	  them

Signed-off-by: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
 src/lxc/checkpoint.c |   91 +++++++++++++++++++++++++++++++++++++++++++++++++-
 src/lxc/commands.c   |    3 ++
 src/lxc/commands.h   |    1 +
 src/lxc/state.c      |   26 ++++++++++++++
 4 files changed, 120 insertions(+), 1 deletions(-)

diff --git a/src/lxc/checkpoint.c b/src/lxc/checkpoint.c
index 7e8a93e..0fbabc8 100644
--- a/src/lxc/checkpoint.c
+++ b/src/lxc/checkpoint.c
@@ -22,10 +22,99 @@
  */
 #include <lxc/lxc.h>
 #include <lxc/log.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <linux/checkpoint.h>
+
+#include "commands.h"
+#include "arguments.h"
+#include "app-checkpoint.h"
 
 lxc_log_define(lxc_checkpoint, lxc);
 
-int lxc_checkpoint(const char *name, const char *statefile, int flags)
+find_cinit_pid(const char *name)
 {
+	struct lxc_command command = {
+		.request = { .type = LXC_COMMAND_CINIT_PID },
+	};
+
+	int ret, stopped;
+
+	ret = lxc_command(name, &command, &stopped);
+	if (ret < 0) {
+		ERROR("failed to send command");
+		return -1;
+	}
+
+	ERROR("find_cinit_pid %d\n", command.answer.ret);
+
+	return command.answer.ret;
+}
+
+int lxc_checkpoint(const char *name, const char *statefile, int lxc_flags)
+{
+	int ret;
+	int pid;
+	int flags;
+	struct stat statbuf;
+	struct app_checkpoint_args crargs;
+
+	if (access(statefile, F_OK) == 0) {
+		ret = stat(statefile, &statbuf);
+		if (ret < 0) {
+			ERROR("stat(%s): %s\n", statefile, strerror(errno));
+			return -1;
+		}
+
+		if (S_ISDIR(statbuf.st_mode)) {
+			ERROR("--directory option not implemented");
+			return -1;
+		} else {
+			ERROR("Checkpoint image file %s exists\n", statefile);
+			return -1;
+		}
+	}
+
+	pid = find_cinit_pid(name);
+	if (pid < 0) {
+		ERROR("Unable to find cinit pid");
+		return -1;
+	}
+
+	memset(&crargs, 0, sizeof(crargs));
+
+	ret = open(statefile, O_CREAT|O_RDWR|O_EXCL, 0644);
+	if (ret < 0) {
+		ERROR("open(%s) failed\n", statefile);
+		return -1;
+	}
+
+	crargs.outfd = ret;
+	crargs.logfd = lxc_log_fd;
+	crargs.uerrfd = lxc_log_fd;
+	/*
+	 * TODO: Set this to 0 for now - otherwise we get an objhash leak
+	 * 	 due to mismatched references to current PTY which needs to
+	 * 	 be investigated.
+	 *
+	 * TODO: Map @lxc_flags to user-cr flags ?
+	 *
+	 * TODO: We can probably drop the ->container field since @flags
+	 * 	 can provide the same selection. 
+	 *
+	 * TODO: Do we may need a --container option to lxc_checkpoint or
+	 * 	 assume that we always work with full containers ?
+	 */
+	crargs.container = 0;
+
+	flags = CHECKPOINT_SUBTREE;
+
+	ret = app_checkpoint(pid, flags, &crargs);
+	if (ret < 0) {
+		ERROR("checkpoint of %s (pid %d) failed\n", name);
+		return -1;
+	}
+
 	return 0;
 }
diff --git a/src/lxc/commands.c b/src/lxc/commands.c
index 4c48571..af5c947 100644
--- a/src/lxc/commands.c
+++ b/src/lxc/commands.c
@@ -114,6 +114,8 @@ extern void lxc_console_remove_fd(int, struct lxc_tty_info *);
 extern int  lxc_console_callback(int, struct lxc_request *, struct lxc_handler *);
 extern int  lxc_stop_callback(int, struct lxc_request *, struct lxc_handler *);
 extern int  lxc_state_callback(int, struct lxc_request *, struct lxc_handler *);
+extern int  lxc_cinit_pid_callback(int, struct lxc_request *,
+				struct lxc_handler *);
 
 static int trigger_command(int fd, struct lxc_request *request,
 			   struct lxc_handler *handler)
@@ -124,6 +126,7 @@ static int trigger_command(int fd, struct lxc_request *request,
 		[LXC_COMMAND_TTY]   = lxc_console_callback,
 		[LXC_COMMAND_STOP]  = lxc_stop_callback,
 		[LXC_COMMAND_STATE] = lxc_state_callback,
+		[LXC_COMMAND_CINIT_PID] = lxc_cinit_pid_callback,
 	};
 
 	if (request->type < 0 || request->type >= LXC_COMMAND_MAX)
diff --git a/src/lxc/commands.h b/src/lxc/commands.h
index 3c6c082..22b24a9 100644
--- a/src/lxc/commands.h
+++ b/src/lxc/commands.h
@@ -27,6 +27,7 @@ enum {
 	LXC_COMMAND_TTY,
 	LXC_COMMAND_STOP,
 	LXC_COMMAND_STATE,
+	LXC_COMMAND_CINIT_PID,
 	LXC_COMMAND_MAX,
 };
 
diff --git a/src/lxc/state.c b/src/lxc/state.c
index ba63805..837d2da 100644
--- a/src/lxc/state.c
+++ b/src/lxc/state.c
@@ -168,3 +168,29 @@ out:
 	return ret;
 }
 
+extern int lxc_cinit_pid_callback(int fd, struct lxc_request *request,
+		struct lxc_handler *handler)
+{
+	struct lxc_answer answer;
+	int ret;
+	FILE *fp;
+
+	answer.ret = handler->pid;
+
+	INFO("Returning pid %d\n", handler->pid);
+
+	ret = send(fd, &answer, sizeof(answer), 0);
+	if (ret < 0) {
+		WARN("failed to send answer to the peer");
+		goto out;
+	}
+
+	if (ret != sizeof(answer)) {
+		ERROR("partial answer sent");
+		goto out;
+	}
+
+out:
+	return ret;
+}
+
-- 
1.6.6.1

Re: [RFC][PATCH 0/5][lxc]: Link with USERCR

[Michel Normand]

Le jeudi 18 mars 2010 à 23:39 -0700, Sukadev Bhattiprolu a écrit :
> Link lxc_checkpoint, lxc_restart with USERCR[2]
> 
> Following set of patches, when applied to LXC 0.6.5 and built with
> USERCR, enable lxc_checkpoint and lxc_restart of some simple containers.
> 
> TODO:
> 	- For simplicity, this patchset currently links *statically* with
> 	  USERCR. We should eventually build checkpoint.o and restart.o in
> 	  USERCR as shared objects and link lxc_checkpoint/lxc_restart with
> 	  these shared objects and liblxc.so. (This also required rebuilding
> 	  lxc-execute statically, so lxc_checkpoint can communicate with
> 	  lxc-execute to get the root-pid of container).
> 
> 	- Determine if lxc_checkpoint needs a --container option (see
> 	  TODOs in src/lxc/checkpoint.c)
> 
> 	- This patchset was tested using lxc-macvlan.conf. But for the VNC
> 	  server testing I ran into a problem with lxc-macvlan.conf. I
> 	  switched to 'lxc-no-netns.conf' and was able to C/R the VNC server.
> 	  I have not debugged the problem with VNC and lxc-macvlan.conf.
> 
> 	- 'global_send_sigint' is still a global variable in USERCR. We need
> 	  to define a better interface to expose its functionality to callers
> 	  of app_restart().
> 
> 	- Choose better names and API for USERCR :-)
> 
> Changelog[v2]: 
> 	(Based on feedback from Oren Laadan, Serge Hallyn, Daniel Lezcano
> 	and Cedric Le Goater)
> 
> 	- Rather than drop --directory option to lxc_checkpoint/lxc_restart
> 	  add a new option (--image).
> 
> 	- Integrate lxc_checkpoint to work with USERCR
> 
> 	- USERCR renamed usercr.h to "app-checkpoint.h"
> 
> 	- USERCR does not create/install libcheckpoint.a and usercr.h for now.
> 	  So link directly with app-checkpoint.h, restart.o and checkpoint.o
> 
> 	- USERCR renames the interfaces to app_checkpoint() and app_restart()
> 	  'struct app_checkpoint_args' 'struct app_restart_args'.
> 	  
> USAGE:
> 
> 1. Build USERCR
> 
> 	$ cd /root
> 
> 	$ git-clone git://git.ncl.cs.columbia.edu/pub/git/user-cr.git user-cr
> 
> 	$ git-checkout ckpt-v20 
> 	
> 	# Apply patches in the following set:
> 
> 	# https://lists.linux-foundation.org/pipermail/containers/2010-March/023793.html
> 
> 	$ cd /root/user-cr
> 
> 	$ make 
> 
> 		This should create restart.o and checkpoint.o
> 
> 2. Build/install lxc_checkpoint and lxc_restart
> 
> 	# Do normal LXC build/install of 0.6.5.
> 
> 	# Then replace 'lxc-execute', 'lxc-checkpoint', 'lxc-restart' with
> 	# new version. i.e Apply attached patches and:
> 
> 	$ cd src/lxc

Your patch set is not usable as is, with requirement to apply the patch
between two sequences of build, and lxc unable to be built with it.

Please do a patch of automake files and related configure.ac
rather than to create a Makefile2 file.

> 
> 	$ USERCR=/root/user-cr \
> 		make -f Makefile2 lxc_checkpoint lxc_restart lxc_execute
> 
> 	$ cp lxc_checkpoint lxc_restart lxc-execute  /usr/local/bin
> 
> 3. Checkpoint/restart a simple LXC container
> 
> 	$ lxc-execute --name foo --rcfile lxc-no-netns.conf -- /bin/sleep 1000
> 
> 	$ lxc-freeze --name foo
> 
> 	$ lxc-checkpoint --name foo --image /root/lxc-foo.ckpt
> 
> 	$ lxc-unfreeze --name foo
> 
> 	$ lxc-stop --name foo
> 
> 	$ lxc-restart --name foo --image /root/lxc-foo.ckpt
> 
> 4. Checkpoint/restart other LXC containers such as:
> 
> 	- a file-io session (see run-fileio1 in cr-tests[1])
> 
> 	- process-tree (see run-ptree1 in cr-tests[1])
> 
> 	- A vi session inside a VNC Server using "twm". i.e
> 
> 		$ cat /root/.vnc/xstartup
> 		#!/bin/sh
> 
> 		xsetroot -solid grey
> 		vncconfig -iconic &
> 		xterm -geometry 80x24+10+10 -ls -title "$VNCDESKTOP Desktop" &
> 		twm &
> 
> 		$ lxc-execute --name foo --rcfile lxc-no-netns.conf -- \
> 			/usr/bin/vncserver :1
> 
> 		$ vncviewer :1
> 
> 			# Open a vi session
> 
> 		$ lxc-freeze --name foo 
> 
> 			etc
> 
> [1]: cr-tests:	git://git.sr71.net/~hallyn/cr_tests.git
> [2]: USERCR: git://git.ncl.cs.columbia.edu/pub/git/user-cr.git
> 
> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> _______________________________________________
> Containers mailing list
> Containers@lists.linux-foundation.org
> https://lists.linux-foundation.org/mailman/listinfo/containers


_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers

Re: [PATCH 1/5][lxc] Enable static linking of some lxc binaries

[Daniel Lezcano]

Sukadev Bhattiprolu wrote:
> From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> Date: Wed, 10 Mar 2010 22:12:34 -0800
> Subject: [PATCH 1/5][lxc] Enable static linking of some lxc binaries
> 
> This is a temporary commit to statically link lxc_checkpoint, lxc_execute
> and lxc_restart.
> 
> Signed-off-by: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> ---
>  src/lxc/Makefile2 |   35 +++++++++++++++++++++++++++++++++++
>  1 files changed, 35 insertions(+), 0 deletions(-)
>  create mode 100644 src/lxc/Makefile2
> 
> diff --git a/src/lxc/Makefile2 b/src/lxc/Makefile2
> new file mode 100644
> index 0000000..26b9cd3
> --- /dev/null
> +++ b/src/lxc/Makefile2
> @@ -0,0 +1,35 @@
> +BASEDIR = ../..
> +
> +USERCR ?= 	$(BASEDIR)/../user-cr-lxc/
> +
> +CFLAGS = 	-static -I . -I .. -I $(USERCR) -I $(USERCR)/include
> +LDFLAGS = 	$(USERCR)/libeclone.a -lutil
> +
> +LXC_OBJS = 	start.o stop.o conf.o confile.o arguments.o monitor.o	\
> +		log.o mainloop.o utils.o commands.o state.o cgroup.o	\
> +		error.o namespace.o parse.o network.o af_unix.o 	\
> +		console.o nl.o
> +
> +USERCR_CHECKPOINT_OBJ =	$(USERCR)/checkpoint.o
> +
> +USERCR_RESTART_OBJ = 	$(USERCR)/restart.o
> +
> +LXC_CHECKPOINT_OBJS = 	$(LXC_OBJS) $(USERCR_CHECKPOINT_OBJ) \
> +			lxc_checkpoint.o checkpoint.o
> +
> +LXC_RESTART_OBJS = 	$(LXC_OBJS) $(USERCR_RESTART_OBJ) \
> +			lxc_restart.o restart.o 
> +
> +LXC_EXECUTE_OBJS =	$(LXC_OBJS) lxc_execute.o
> +
> +lxc_checkpoint: $(LXC_CHECKPOINT_OBJS)
> +	$(CC) -o lxc_checkpoint $(LXC_CHECKPOINT_OBJS) $(LDFLAGS)
> +
> +lxc_restart: $(LXC_RESTART_OBJS)
> +	$(CC) -o lxc_restart $(LXC_RESTART_OBJS) $(LDFLAGS)
> +
> +lxc_execute: $(LXC_EXECUTE_OBJS)
> +	$(CC) -o lxc_execute $(LXC_EXECUTE_OBJS) $(LDFLAGS)
> +
> +clean:
> +	rm -f $(RESTART_OBJS)

Is it possible to add a 'configure' option and merge that with the lxc 
Makefile.

Re: [PATCH 5/5][lxc] Hook up lxc_checkpoint() with app_checkpoint()

[Daniel Lezcano]

Sukadev Bhattiprolu wrote:
> From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> Date: Thu, 11 Mar 2010 21:32:38 -0800
> Subject: [PATCH 5/5][lxc] Hook up lxc_checkpoint() with app_checkpoint()
> 
> Have lxc_checkpoint() call app_checkpoint() implemented in checkpoint.o
> in the USER-CR git tree
> 
> TODO:
> 	- Map lxc_flags to flags in sys_checkpoint()
> 	- Initialize app_checkpoint_args.debug and other fields based on
> 	  command line options to lxc_checkpoint rather than hard-coding
> 	  them
> 
> Signed-off-by: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> ---
>  src/lxc/checkpoint.c |   91 +++++++++++++++++++++++++++++++++++++++++++++++++-
>  src/lxc/commands.c   |    3 ++
>  src/lxc/commands.h   |    1 +
>  src/lxc/state.c      |   26 ++++++++++++++
>  4 files changed, 120 insertions(+), 1 deletions(-)
> 
> diff --git a/src/lxc/checkpoint.c b/src/lxc/checkpoint.c
> index 7e8a93e..0fbabc8 100644
> --- a/src/lxc/checkpoint.c
> +++ b/src/lxc/checkpoint.c
> @@ -22,10 +22,99 @@
>   */
>  #include <lxc/lxc.h>
>  #include <lxc/log.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +#include <errno.h>
> +#include <linux/checkpoint.h>
> +
> +#include "commands.h"
> +#include "arguments.h"
> +#include "app-checkpoint.h"
> 
>  lxc_log_define(lxc_checkpoint, lxc);
> 
> -int lxc_checkpoint(const char *name, const char *statefile, int flags)
> +find_cinit_pid(const char *name)
>  {
> +	struct lxc_command command = {
> +		.request = { .type = LXC_COMMAND_CINIT_PID },
> +	};
> +
> +	int ret, stopped;
> +
> +	ret = lxc_command(name, &command, &stopped);
> +	if (ret < 0) {
> +		ERROR("failed to send command");
> +		return -1;
> +	}
> +
> +	ERROR("find_cinit_pid %d\n", command.answer.ret);
> +
> +	return command.answer.ret;
> +}

I just committed the same function for the lxc_attach command. I think 
you can reuse it.

> +int lxc_checkpoint(const char *name, const char *statefile, int lxc_flags)
> +{
> +	int ret;
> +	int pid;
> +	int flags;
> +	struct stat statbuf;
> +	struct app_checkpoint_args crargs;
> +
> +	if (access(statefile, F_OK) == 0) {
> +		ret = stat(statefile, &statbuf);
> +		if (ret < 0) {
> +			ERROR("stat(%s): %s\n", statefile, strerror(errno));
> +			return -1;
> +		}
> +
> +		if (S_ISDIR(statbuf.st_mode)) {
> +			ERROR("--directory option not implemented");
> +			return -1;
> +		} else {
> +			ERROR("Checkpoint image file %s exists\n", statefile);
> +			return -1;
> +		}
> +	}

For the checkpoint, you don't need to check if it's a directory or a 
file (but it should be done at restart time). I am not sure 'access' is 
really necessary because the O_EXCL is set in the open below.

> +
> +	pid = find_cinit_pid(name);
> +	if (pid < 0) {
> +		ERROR("Unable to find cinit pid");
> +		return -1;
> +	}
> +
> +	memset(&crargs, 0, sizeof(crargs));
> +
> +	ret = open(statefile, O_CREAT|O_RDWR|O_EXCL, 0644);
> +	if (ret < 0) {
> +		ERROR("open(%s) failed\n", statefile);
> +		return -1;
> +	}

As the statefile may contain sensible data, it would be preferable to 
set it 0600, no ?

> +	crargs.outfd = ret;
> +	crargs.logfd = lxc_log_fd;
> +	crargs.uerrfd = lxc_log_fd;
> +	/*
> +	 * TODO: Set this to 0 for now - otherwise we get an objhash leak
> +	 * 	 due to mismatched references to current PTY which needs to
> +	 * 	 be investigated.
> +	 *
> +	 * TODO: Map @lxc_flags to user-cr flags ?
> +	 *
> +	 * TODO: We can probably drop the ->container field since @flags
> +	 * 	 can provide the same selection. 
> +	 *
> +	 * TODO: Do we may need a --container option to lxc_checkpoint or
> +	 * 	 assume that we always work with full containers ?
> +	 */
> +	crargs.container = 0;
> +
> +	flags = CHECKPOINT_SUBTREE;
> +
> +	ret = app_checkpoint(pid, flags, &crargs);
> +	if (ret < 0) {
> +		ERROR("checkpoint of %s (pid %d) failed\n", name);
> +		return -1;
> +	}
> +
>  	return 0;
>  }

Re: [RFC][PATCH 0/5][lxc]: Link with USERCR

[Daniel Lezcano]

Sukadev Bhattiprolu wrote:
> Link lxc_checkpoint, lxc_restart with USERCR[2]
> 

> 3. Checkpoint/restart a simple LXC container
> 
> 	$ lxc-execute --name foo --rcfile lxc-no-netns.conf -- /bin/sleep 1000
> 
> 	$ lxc-freeze --name foo
> 
> 	$ lxc-checkpoint --name foo --image /root/lxc-foo.ckpt
> 
> 	$ lxc-unfreeze --name foo

IMO, the freeze / thaw should be done in the lxc_checkpoint function code.

> 	$ lxc-stop --name foo
> 
> 	$ lxc-restart --name foo --image /root/lxc-foo.ckpt

Is the container restarted in a frozen state ?

Re: [PATCH 2/5][lxc] lxc_restart: Add --image option

[Daniel Lezcano]

Sukadev Bhattiprolu wrote:
> From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> Date: Wed, 10 Mar 2010 21:59:39 -0800
> Subject: [PATCH 2/5][lxc] lxc_restart: Add --image option
> 
> The existing --directory option to lxc_restart expects the checkpoint state
> to be a directory. USERCR however uses a single regular file to store the
> checkpoint image. So add a --image option to enable checkpointing and
> restarting applications using USERCR.
> 
> Depending on how the application was checkpointed, users should specify
> either --image=CHECKPOINT-IMAGE or the --directory=STATEFILE option (but not
> both).

It's a detail but I would prefer to replace --image by --statefile. So 
we can put --directory deprecated and use --statefile as a more generic 
name.

Re: [PATCH 4/5][lxc] Hook up lxc_restart() with app_restart()

[Daniel Lezcano]

Sukadev Bhattiprolu wrote:
> From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> Date: Thu, 11 Mar 2010 20:47:56 -0800
> Subject: [PATCH 4/5][lxc] Hook up lxc_restart() with app_restart()
> 
> Have lxc_restart() call app_restart() implemented in the 'restart.o' from
> USER-CR git tree.
> 
> Changelog[v2]:
> 	- Link with restart.o from usercr rather than libcheckpoint.a
> 	- rename 'struct restart_args' to 'struct app_restart_args'
> 	- Initialize the new field app_restart_args->uerrfd
> 	- (Oren Laadan)Remove ->send_sigint field from 'struct app_restart_args'
> ---
>  src/lxc/restart.c |  104 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 104 insertions(+), 0 deletions(-)
> 
> diff --git a/src/lxc/restart.c b/src/lxc/restart.c
> index 467489e..945435c 100644
> --- a/src/lxc/restart.c
> +++ b/src/lxc/restart.c
> @@ -22,11 +22,115 @@
>   */
>  #include <lxc/lxc.h>
>  #include <lxc/log.h>
> +#include <lxc/start.h>
> +#include <lxc/namespace.h>
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <signal.h>
> +#include <sys/prctl.h>
> +#include <app-checkpoint.h>
> +
> +struct lxc_restart_arg {
> +	const char *name;
> +	const char *statefile;
> +	char *const argv;
> +	struct lxc_handler *handler;
> +};
> 
>  lxc_log_define(lxc_restart, lxc);
> 
> +static int do_restart(struct lxc_restart_arg *lxcarg)
> +{
> +	int pid;
> +	struct lxc_handler *handler = lxcarg->handler;
> +	const char *name = lxcarg->name;
> +	char *const argv = lxcarg->argv;
> +	const char *statefile = lxcarg->statefile;
> +	struct app_restart_args restart_args;
> +
> +        if (sigprocmask(SIG_SETMASK, &handler->oldmask, NULL)) {
> +                SYSERROR("failed to set sigprocmask");
> +                return -1;
> +        }
> +
> +        if (prctl(PR_SET_PDEATHSIG, SIGKILL, 0, 0, 0)) {
> +                SYSERROR("failed to set pdeath signal");
> +                return -1;
> +        }
> +
> +	memset(&restart_args, 0, sizeof(restart_args));
> +
> +	if (statefile) {

Why do you need to check if statefile is valid ? It's a mandatory 
option, no ? if it is not specified, we should not reach this point.

> +		restart_args.infd = open(statefile, O_RDONLY, 0);
> +		if (restart_args.infd < 0) {
> +			SYSERROR("Failed to open statefile %s\n", statefile);
> +			return -1;
> +		}
> +	}
> +
> +	restart_args.pids = 1;
> +	restart_args.pidns = 1;
> +	restart_args.mnt_pty = 1;
> +	restart_args.mntns = 1;
> +	restart_args.klogfd = -1;
> +	restart_args.ulogfd = lxc_log_fd;
> +	restart_args.uerrfd = fileno(stderr);
> +	restart_args.debug = 1;
> +	restart_args.wait = 0;
> +
> +        pid = app_restart(&restart_args);
> +
> +        return pid;
> +}
> +
>  int lxc_restart(const char *name, const char *statefile, struct lxc_conf *conf,
>  		int flags)
>  {
> +	int err;
> +	int status;
> +	struct lxc_handler *handler;
> +	struct lxc_restart_arg lxcarg = {
> +		.name = name,
> +		.statefile = statefile,
> +		.handler = NULL,
> +	};
> +
> +	handler = lxc_init(name, conf);
> +	if (!handler) {
> +		ERROR("failed to initialize the container");
> +		return -1;
> +	}
> +
> +	lxcarg.handler = handler;
> +	handler->pid = do_restart(&lxcarg);
> +
> +	INFO("do_restart(): returns pid %d\n", handler->pid);
> +	lxc_rename_nsgroup(name, handler);
> +
> +	err = lxc_close_all_inherited_fd();
> +	if (err) {
> +		ERROR("unable to close inherited fds");
> +		goto out_abort;
> +	}

A small change here, recently committed. Now we check there is *no* 
inherited fd at this point instead of trying to detect them.

> +	err = lxc_poll(name, handler);
> +	if (err) {
> +		ERROR("mainloop exited with an error");
> +		goto out_abort;
> +	}
> +
> +	while (waitpid(handler->pid, &status, 0) < 0 && errno == EINTR)
> +		continue;
> +
> +	err =  lxc_error_set_and_log(handler->pid, status);
> +
> +out_fini:
> +	lxc_fini(name, handler);
> +	return err;
> +
> +out_abort:
> +	lxc_abort(name, handler);
> +	goto out_fini;
> +
>  	return 0;
>  }

Re: [RFC][PATCH 0/5][lxc]: Link with USERCR

[Sukadev Bhattiprolu]

Michel Normand [normand-NmTC/0ZBporQT0dZR+AlfA@public.gmane.org] wrote:
| Le jeudi 18 mars 2010 à 23:39 -0700, Sukadev Bhattiprolu a écrit :
| > 2. Build/install lxc_checkpoint and lxc_restart
| > 
| > 	# Do normal LXC build/install of 0.6.5.
| > 
| > 	# Then replace 'lxc-execute', 'lxc-checkpoint', 'lxc-restart' with
| > 	# new version. i.e Apply attached patches and:
| > 
| > 	$ cd src/lxc
| 
| Your patch set is not usable as is, with requirement to apply the patch
| between two sequences of build, and lxc unable to be built with it.
| 
| Please do a patch of automake files and related configure.ac
| rather than to create a Makefile2 file.

Sure, I am working on it and will resend in a couple of days after
addressing Daniel's comments also. If you have other comments on the
code, please let me know.

Sukadev.

Re: [RFC][PATCH 0/5][lxc]: Link with USERCR

[Oren Laadan]

Daniel Lezcano wrote:
> Sukadev Bhattiprolu wrote:
>> Link lxc_checkpoint, lxc_restart with USERCR[2]
>>
> 
>> 3. Checkpoint/restart a simple LXC container
>>
>>     $ lxc-execute --name foo --rcfile lxc-no-netns.conf -- /bin/sleep 
>> 1000
>>
>>     $ lxc-freeze --name foo
>>
>>     $ lxc-checkpoint --name foo --image /root/lxc-foo.ckpt
>>
>>     $ lxc-unfreeze --name foo
> 
> IMO, the freeze / thaw should be done in the lxc_checkpoint function code.
> 
>>     $ lxc-stop --name foo
>>
>>     $ lxc-restart --name foo --image /root/lxc-foo.ckpt
> 
> Is the container restarted in a frozen state ?
> 
> 

It could if given the proper flag to sys_restart().

Oren.

Re: [PATCH 5/5][lxc] Hook up lxc_checkpoint() with app_checkpoint()

[Sukadev Bhattiprolu]

Daniel Lezcano [dlezcano-NmTC/0ZBporQT0dZR+AlfA@public.gmane.org] wrote:
>> +find_cinit_pid(const char *name)
>>  {
>> +	struct lxc_command command = {
>> +		.request = { .type = LXC_COMMAND_CINIT_PID },
>> +	};
>> +
>> +	int ret, stopped;
>> +
>> +	ret = lxc_command(name, &command, &stopped);
>> +	if (ret < 0) {
>> +		ERROR("failed to send command");
>> +		return -1;
>> +	}
>> +
>> +	ERROR("find_cinit_pid %d\n", command.answer.ret);
>> +
>> +	return command.answer.ret;
>> +}
>
> I just committed the same function for the lxc_attach command. I think  
> you can reuse it.

Ok, will try to reuse it.

>
>> +int lxc_checkpoint(const char *name, const char *statefile, int lxc_flags)
>> +{
>> +	int ret;
>> +	int pid;
>> +	int flags;
>> +	struct stat statbuf;
>> +	struct app_checkpoint_args crargs;
>> +
>> +	if (access(statefile, F_OK) == 0) {
>> +		ret = stat(statefile, &statbuf);
>> +		if (ret < 0) {
>> +			ERROR("stat(%s): %s\n", statefile, strerror(errno));
>> +			return -1;
>> +		}
>> +
>> +		if (S_ISDIR(statbuf.st_mode)) {
>> +			ERROR("--directory option not implemented");
>> +			return -1;
>> +		} else {
>> +			ERROR("Checkpoint image file %s exists\n", statefile);
>> +			return -1;
>> +		}
>> +	}
>
> For the checkpoint, you don't need to check if it's a directory or a  
> file (but it should be done at restart time).

I need to open() the statefile and pass its fd to app_checkpoint().
If I don't check for directory, the open() below would fail, only
because of the O_RDWR flag. By checking, we could give a more useful
error message ?

> I am not sure 'access' is  
> really necessary because the O_EXCL is set in the open below.

Agree. How about just the stat() and the error message ?

Maybe later we could bury this code under say #ifdef USERCR so
if LXC is configured without USERCR, the current behavior is preserved ?

>
>> +
>> +	pid = find_cinit_pid(name);
>> +	if (pid < 0) {
>> +		ERROR("Unable to find cinit pid");
>> +		return -1;
>> +	}
>> +
>> +	memset(&crargs, 0, sizeof(crargs));
>> +
>> +	ret = open(statefile, O_CREAT|O_RDWR|O_EXCL, 0644);
>> +	if (ret < 0) {
>> +		ERROR("open(%s) failed\n", statefile);
>> +		return -1;
>> +	}
>
> As the statefile may contain sensible data, it would be preferable to  
> set it 0600, no ?

Agree.

Re: [RFC][PATCH 0/5][lxc]: Link with USERCR

[Sukadev Bhattiprolu]

Daniel Lezcano [dlezcano-NmTC/0ZBporQT0dZR+AlfA@public.gmane.org] wrote:
> Sukadev Bhattiprolu wrote:
>> Link lxc_checkpoint, lxc_restart with USERCR[2]
>>
>
>> 3. Checkpoint/restart a simple LXC container
>>
>> 	$ lxc-execute --name foo --rcfile lxc-no-netns.conf -- /bin/sleep 1000
>>
>> 	$ lxc-freeze --name foo
>>
>> 	$ lxc-checkpoint --name foo --image /root/lxc-foo.ckpt
>>
>> 	$ lxc-unfreeze --name foo
>
> IMO, the freeze / thaw should be done in the lxc_checkpoint function code.

Well, by separating the freeze and checkpoint, we could do things like
snapshot the filesystem or would lxc_checkpoint make an external call
to do that ?

I will leave it as is for my next version and I am guessing it can be done
easily later ?
>
>> 	$ lxc-stop --name foo
>>
>> 	$ lxc-restart --name foo --image /root/lxc-foo.ckpt
>
> Is the container restarted in a frozen state ?

It can be (by setting the restart_args->freezer). I will leave it in
frozen for now. Maybe later we can add a command line option to let
user choose whether or not to leave it frozen

Re: [PATCH 2/5][lxc] lxc_restart: Add --image option

[Sukadev Bhattiprolu]

Daniel Lezcano [dlezcano-NmTC/0ZBporQT0dZR+AlfA@public.gmane.org] wrote:
> Sukadev Bhattiprolu wrote:
>> From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
>> Date: Wed, 10 Mar 2010 21:59:39 -0800
>> Subject: [PATCH 2/5][lxc] lxc_restart: Add --image option
>>
>> The existing --directory option to lxc_restart expects the checkpoint state
>> to be a directory. USERCR however uses a single regular file to store the
>> checkpoint image. So add a --image option to enable checkpointing and
>> restarting applications using USERCR.
>>
>> Depending on how the application was checkpointed, users should specify
>> either --image=CHECKPOINT-IMAGE or the --directory=STATEFILE option (but not
>> both).
>
> It's a detail but I would prefer to replace --image by --statefile. So  
> we can put --directory deprecated and use --statefile as a more generic  
> name.

Ok will fix.

Re: [PATCH 5/5][lxc] Hook up lxc_checkpoint() with app_checkpoint()

[Daniel Lezcano]

Sukadev Bhattiprolu wrote:
> Daniel Lezcano [dlezcano-NmTC/0ZBporQT0dZR+AlfA@public.gmane.org] wrote:
>>> +find_cinit_pid(const char *name)
>>>  {
>>> +	struct lxc_command command = {
>>> +		.request = { .type = LXC_COMMAND_CINIT_PID },
>>> +	};
>>> +
>>> +	int ret, stopped;
>>> +
>>> +	ret = lxc_command(name, &command, &stopped);
>>> +	if (ret < 0) {
>>> +		ERROR("failed to send command");
>>> +		return -1;
>>> +	}
>>> +
>>> +	ERROR("find_cinit_pid %d\n", command.answer.ret);
>>> +
>>> +	return command.answer.ret;
>>> +}
>> I just committed the same function for the lxc_attach command. I think  
>> you can reuse it.
> 
> Ok, will try to reuse it.
> 
>>> +int lxc_checkpoint(const char *name, const char *statefile, int lxc_flags)
>>> +{
>>> +	int ret;
>>> +	int pid;
>>> +	int flags;
>>> +	struct stat statbuf;
>>> +	struct app_checkpoint_args crargs;
>>> +
>>> +	if (access(statefile, F_OK) == 0) {
>>> +		ret = stat(statefile, &statbuf);
>>> +		if (ret < 0) {
>>> +			ERROR("stat(%s): %s\n", statefile, strerror(errno));
>>> +			return -1;
>>> +		}
>>> +
>>> +		if (S_ISDIR(statbuf.st_mode)) {
>>> +			ERROR("--directory option not implemented");
>>> +			return -1;
>>> +		} else {
>>> +			ERROR("Checkpoint image file %s exists\n", statefile);
>>> +			return -1;
>>> +		}
>>> +	}
>> For the checkpoint, you don't need to check if it's a directory or a  
>> file (but it should be done at restart time).
> 
> I need to open() the statefile and pass its fd to app_checkpoint().
> If I don't check for directory, the open() below would fail, only
> because of the O_RDWR flag. By checking, we could give a more useful
> error message ?

What I meant is the directory shall not exists when checkpointing, if 
the directory or the file exists, the checkpoint should fail.

>> I am not sure 'access' is  
>> really necessary because the O_EXCL is set in the open below.
> 
> Agree. How about just the stat() and the error message ?

IMO, just fail when trying to open the file, the O_EXCL flag will do the 
job for us.

> Maybe later we could bury this code under say #ifdef USERCR so
> if LXC is configured without USERCR, the current behavior is preserved ?

Right, it's an aspect we have to address. But we can do that later, IMHO 
the main objective would be to correctly integrate lxc and the CR with 
the 'configure' script. eg configure --with-linux=<path> .... and the 
makefiles.

Re: [RFC][PATCH 0/5][lxc]: Link with USERCR

[Daniel Lezcano]

Sukadev Bhattiprolu wrote:
> Daniel Lezcano [dlezcano-NmTC/0ZBporQT0dZR+AlfA@public.gmane.org] wrote:
>> Sukadev Bhattiprolu wrote:
>>> Link lxc_checkpoint, lxc_restart with USERCR[2]
>>>
>>> 3. Checkpoint/restart a simple LXC container
>>>
>>> 	$ lxc-execute --name foo --rcfile lxc-no-netns.conf -- /bin/sleep 1000
>>>
>>> 	$ lxc-freeze --name foo
>>>
>>> 	$ lxc-checkpoint --name foo --image /root/lxc-foo.ckpt
>>>
>>> 	$ lxc-unfreeze --name foo
>> IMO, the freeze / thaw should be done in the lxc_checkpoint function code.
> 
> Well, by separating the freeze and checkpoint, we could do things like
> snapshot the filesystem or would lxc_checkpoint make an external call
> to do that ?
> 
> I will leave it as is for my next version and I am guessing it can be done
> easily later ?
>>> 	$ lxc-stop --name foo
>>>
>>> 	$ lxc-restart --name foo --image /root/lxc-foo.ckpt
>> Is the container restarted in a frozen state ?
> 
> It can be (by setting the restart_args->freezer). I will leave it in
> frozen for now. Maybe later we can add a command line option to let
> user choose whether or not to leave it frozen

The lxc_checkpoint / lxc_restart API have already the --pause option 
defined.

If you don't specify the --pause option, the checkpoint and the restart 
release the application right after finishing their job.

There is also the --kill option for the checkpoint, to kill the 
container after the checkpoint.

The lxc_checkpoint function should do:

    freeze();

    checkpoint();

    if (kill) {
        stop();
        unfreeze();
        return;
    }

    if (!pause)
        unfreeze();

    return;

The same for the restart except for the 'kill'.

The functions are already defined in lxc, it takes 5 mins to write this 
code.

Defined in lxc.h :
extern int lxc_stop(const char *name);
extern int lxc_freeze(const char *name);
extern int lxc_unfreeze(const char *name);

The lxc_checkpoint / lxc_restart functions have a 'flags' parameter with 
LXC_FLAG_PAUSE or LXC_FLAG_HALT (if set in the command line).

Re: [PATCH 2/5][lxc] lxc_restart: Add --image option

[Daniel Lezcano]

Sukadev Bhattiprolu wrote:
> Daniel Lezcano [dlezcano-NmTC/0ZBporQT0dZR+AlfA@public.gmane.org] wrote:
>> Sukadev Bhattiprolu wrote:
>>> From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
>>> Date: Wed, 10 Mar 2010 21:59:39 -0800
>>> Subject: [PATCH 2/5][lxc] lxc_restart: Add --image option
>>>
>>> The existing --directory option to lxc_restart expects the checkpoint state
>>> to be a directory. USERCR however uses a single regular file to store the
>>> checkpoint image. So add a --image option to enable checkpointing and
>>> restarting applications using USERCR.
>>>
>>> Depending on how the application was checkpointed, users should specify
>>> either --image=CHECKPOINT-IMAGE or the --directory=STATEFILE option (but not
>>> both).
>> It's a detail but I would prefer to replace --image by --statefile. So  
>> we can put --directory deprecated and use --statefile as a more generic  
>> name.
> 
> Ok will fix.

Thanks.