: Link with USERCR

From: Sukadev Bhattiprolu <[email protected]>
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 <[email protected]>

---
 src/lxc/Makefile2 |   35 +++++++++++++++++++++++++++++++++++
 1 files changed, 35 insertions(+), 0 deletions(-)
 create mode 100644 src/lxc/Makefile2

src/lxc/Makefile2 new file +25 -0

new file mode 100644
index 0000000..26b9cd3
@@ -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)

From: Sukadev Bhattiprolu <[email protected]>
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).

Quoted 9 lines · Sukadev Bhattiprolu

From: Sukadev Bhattiprolu <[email protected]>
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.

Quoted 3 lines

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.

Quoted 10 lines · Daniel Lezcano [dlezcano-NmTC/[email protected]] wrote:

Quoted 9 lines · Sukadev Bhattiprolu

From: Sukadev Bhattiprolu <[email protected]>
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.

Quoted 6 lines

Quoted 3 lines

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.

From: Sukadev Bhattiprolu <[email protected]>
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(-)

src/lxc/restart.c +26 -0

index 467489e..945435c 100644
@@ -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;

src/lxc/restart.c +38 -0

+		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;

src/lxc/restart.c +19 -0

+	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;
 }

From: Sukadev Bhattiprolu <[email protected]>
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 <[email protected]>

---
 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(-)

src/lxc/checkpoint.c +15 -1

index 7e8a93e..0fbabc8 100644
@@ -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;
+

src/lxc/checkpoint.c +13 -0

+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;

src/lxc/checkpoint.c +4 -0

+
+	pid = find_cinit_pid(name);
+	if (pid < 0) {
+		ERROR("Unable to find cinit pid");

src/lxc/checkpoint.c +26 -0

+	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;
 }

Quoted 13 lines

src/lxc/checkpoint.c +12 -0

+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;
+	}

src/lxc/checkpoint.c line 53

Quoted 5 lines

src/lxc/checkpoint.c +5 -0

+
+	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.

Quoted 19 lines

src/lxc/checkpoint.c +18 -0

+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;

src/lxc/checkpoint.c line 77

Quoted 5 lines

src/lxc/checkpoint.c +5 -0

+		} 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.

Quoted 10 lines

src/lxc/checkpoint.c +9 -0

+
+	pid = find_cinit_pid(name);
+	if (pid < 0) {
+		ERROR("Unable to find cinit pid");
+		return -1;
+	}
+
+	memset(&crargs, 0, sizeof(crargs));
+

src/lxc/checkpoint.c line 91

Quoted 5 lines

src/lxc/checkpoint.c +5 -0

+	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 ?

Quoted 12 lines · Daniel Lezcano [dlezcano-NmTC/[email protected]] wrote:

Quoted 12 lines

src/lxc/checkpoint.c +11 -0

+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;

Quoted 8 lines

src/lxc/checkpoint.c line 53

Quoted 6 lines

src/lxc/checkpoint.c +6 -0

+	}
+
+	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.

Quoted 17 lines

Quoted 17 lines

src/lxc/checkpoint.c +17 -0

+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");

Quoted 8 lines

src/lxc/checkpoint.c line 77

Quoted 6 lines

src/lxc/checkpoint.c +6 -0

+			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 ?

Quoted 2 lines

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 ?

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 <[email protected]>

_______________________________________________
Containers mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/containers

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

	$ lxc-stop --name foo

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

Quoted 5 lines · Sukadev Bhattiprolu

Link lxc_checkpoint, lxc_restart with USERCR[2]

3. Checkpoint/restart a simple LXC container

Quoted 8 lines

    $ 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.

Quoted 3 lines

    $ lxc-stop --name foo

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

Is the container restarted in a frozen state ?

Quoted 7 lines · Sukadev Bhattiprolu

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

Quoted 5 lines

	$ 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.

Quoted 4 lines

	$ lxc-stop --name foo

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

Is the container restarted in a frozen state ?

Quoted 5 lines · Daniel Lezcano [dlezcano-NmTC/[email protected]] wrote:

Quoted 4 lines · Sukadev Bhattiprolu

Link lxc_checkpoint, lxc_restart with USERCR[2]

3. Checkpoint/restart a simple LXC container

Quoted 8 lines

Quoted 7 lines

	$ 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 ?

Quoted 4 lines

Quoted 3 lines

	$ 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