[PATCH 1/2][LXC] Rename --directory option to --statefile

[PATCH 1/2][LXC] Rename --directory option to --statefile

[Sukadev Bhattiprolu]

----
From 94fe79c36ac3aed69c8d97f8e4d17c9b02961b97 Mon Sep 17 00:00:00 2001
From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Date: Tue, 23 Feb 2010 17:47:51 -0800
Subject: [PATCH 1/2] Rename --directory option to --statefile

[See following mail for an intro to this patch set

https://lists.linux-foundation.org/pipermail/containers/2010-February/023048.html]

The --directory is misleading since the option seems to expect
an checkpoint image file.

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

diff --git a/src/lxc/lxc_restart.c b/src/lxc/lxc_restart.c
index 7db1d85..83d3d7b 100644
--- a/src/lxc/lxc_restart.c
+++ b/src/lxc/lxc_restart.c
@@ -52,7 +52,7 @@ static int my_checker(const struct lxc_arguments* args)
 static int my_parser(struct lxc_arguments* args, int c, char* arg)
 {
 	switch (c) {
-	case 'd': args->statefile = arg; break;
+	case 'i': args->statefile = arg; break;
 	case 'f': args->rcfile = arg; break;
 	case 'p': args->flags = LXC_FLAG_PAUSE; break;
 	case 's': return lxc_config_define_add(&defines, arg);
@@ -62,7 +62,7 @@ static int my_parser(struct lxc_arguments* args, int c, char* arg)
 }
 
 static const struct option my_longopts[] = {
-	{"directory", required_argument, 0, 'd'},
+	{"statefile", required_argument, 0, 'i'},
 	{"rcfile", required_argument, 0, 'f'},
 	{"pause", no_argument, 0, 'p'},
 	{"define", required_argument, 0, 's'},
@@ -72,14 +72,14 @@ static const struct option my_longopts[] = {
 static struct lxc_arguments my_args = {
 	.progname = "lxc-restart",
 	.help     = "\
---name=NAME --directory STATEFILE\n\
+--name=NAME --statefile STATEFILE\n\
 \n\
-lxc-restart restarts from STATEFILE the NAME container\n\
+lxc-restart restarts from checkpoint image 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\
+  -i, --statefile=STATEFILE for name of statefile\n\
   -f, --rcfile=FILE Load configuration file FILE\n\
   -s, --define KEY=VAL Assign VAL to configuration variable KEY\n",
 	.options  = my_longopts,
-- 
1.6.6.1

[PATCH 2/2][LXC] Have lxc_restart call app_restart()

[Sukadev Bhattiprolu]

From 86023fbab91bc5c7a727e43e853615f27b70fbcf Mon Sep 17 00:00:00 2001
From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Date: Tue, 23 Feb 2010 22:38:14 -0800
Subject: [PATCH 2/2][LXC] Have lxc_restart call app_restart()

Have lxc_restart() call app_restart() exported by libcheckpoint.a from
the USER-CR git tree.

TODO: 	- Similarly implement lxc_checkpoint
	- Use dynamic linking with liblxc.so

Signed-off-by: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
 src/lxc/Makefile2 |   16 +++++++++
 src/lxc/restart.c |   91 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 107 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..9aba841
--- /dev/null
+++ b/src/lxc/Makefile2
@@ -0,0 +1,16 @@
+
+CFLAGS = -static -I . -I ..
+
+LDFLAGS = /lib/libcheckpoint.a -lutil
+
+LXC_OBJS = start.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 stop.o
+
+RESTART_OBJS = $(LXC_OBJS) lxc_restart.o restart.o
+
+lxc_restart: $(RESTART_OBJS)
+	$(CC) -o lxc_restart $(RESTART_OBJS) $(LDFLAGS)
+
+clean:
+	rm -f $(RESTART_OBJS)
diff --git a/src/lxc/restart.c b/src/lxc/restart.c
index 467489e..b49939a 100644
--- a/src/lxc/restart.c
+++ b/src/lxc/restart.c
@@ -22,11 +22,102 @@
  */
 #include <lxc/lxc.h>
 #include <lxc/log.h>
+#include <lxc/start.h>
+#include <lxc/namespace.h>
+#include <errno.h>
+#include <signal.h>
+#include <sys/prctl.h>
+#include <usercr.h>
 
 lxc_log_define(lxc_restart, lxc);
 
+struct lxc_restart_arg {
+	const char *name;
+	const char *statefile;
+	char *const argv;
+	struct lxc_handler *handler;
+};
+
+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 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));
+	restart_args.pids = 1;
+	restart_args.pidns = 1;
+	restart_args.no_pidns = 0;
+	restart_args.mnt_pty = 1;
+	restart_args.input = statefile;
+	restart_args.infd = -1;
+	restart_args.logfd = lxc_log_fd;
+	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);
+
+	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

Re: [PATCH 2/2][LXC] Have lxc_restart call app_restart()

[Cedric Le Goater]

On 02/24/2010 09:41 AM, Sukadev Bhattiprolu wrote:
> diff --git a/src/lxc/Makefile2 b/src/lxc/Makefile2
> new file mode 100644
> index 0000000..9aba841
> --- /dev/null
> +++ b/src/lxc/Makefile2
> @@ -0,0 +1,16 @@
> +
> +CFLAGS = -static -I . -I ..
> +
> +LDFLAGS = /lib/libcheckpoint.a -lutil
> +
> +LXC_OBJS = start.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 stop.o
> +
> +RESTART_OBJS = $(LXC_OBJS) lxc_restart.o restart.o
> +
> +lxc_restart: $(RESTART_OBJS)
> +	$(CC) -o lxc_restart $(RESTART_OBJS) $(LDFLAGS)
> +
> +clean:
> +	rm -f $(RESTART_OBJS)

you should patch Makefile.am.

C.

Re: [PATCH 1/2][LXC] Rename --directory option to --statefile

[Cedric Le Goater]

On 02/24/2010 09:41 AM, Sukadev Bhattiprolu wrote:
> [See following mail for an intro to this patch set
>
> https://lists.linux-foundation.org/pipermail/containers/2010-February/023048.html]
>
> The --directory is misleading since the option seems to expect
> an checkpoint image file.
  
sigh.

This has some major impact on our side as it is a 9 year old API ...
We'll look into it to see how painful it is to change.

C.

Re: [PATCH 1/2][LXC] Rename --directory option to --statefile

[Daniel Lezcano]

Cedric Le Goater wrote:
> On 02/24/2010 09:41 AM, Sukadev Bhattiprolu wrote:
>> [See following mail for an intro to this patch set
>>
>> https://lists.linux-foundation.org/pipermail/containers/2010-February/023048.html] 
>>
>>
>> The --directory is misleading since the option seems to expect
>> an checkpoint image file.
>  
> sigh.
> 
> This has some major impact on our side as it is a 9 year old API ...
> We'll look into it to see how painful it is to change.

We can keep --directory and add --statefile.
For --statefile option we can use a switch for IS_DIR, IS_REG.

--directory becomes deprecated and we switch to the statefile option 
step by step.

Does it make sense ?

   -- Daniel

Re: [PATCH 1/2][LXC] Rename --directory option to --statefile

[Sukadev Bhattiprolu]

Daniel Lezcano [dlezcano-NmTC/0ZBporQT0dZR+AlfA@public.gmane.org] wrote:
> Cedric Le Goater wrote:
>> On 02/24/2010 09:41 AM, Sukadev Bhattiprolu wrote:
>>> [See following mail for an intro to this patch set
>>>
 
>>> https://lists.linux-foundation.org/pipermail/containers/2010-February/023048.html] 
>>> 
>>>
>>>
>>> The --directory is misleading since the option seems to expect
>>> an checkpoint image file.
>>  sigh.
>>
>> This has some major impact on our side as it is a 9 year old API ...
>> We'll look into it to see how painful it is to change.

Ah, did not realize it caused that much pain.

>
> We can keep --directory and add --statefile.
> For --statefile option we can use a switch for IS_DIR, IS_REG.
>
> --directory becomes deprecated and we switch to the statefile option  
> step by step.
>
> Does it make sense ?

Yes, I think it will work.

BTW, what was the rationale for a directory ?  Is it meant to be used
with predefined file names in the specified directory ?

>
>   -- Daniel

Re: [PATCH 1/2][LXC] Rename --directory option to --statefile

[Cedric Le Goater]

>>> This has some major impact on our side as it is a 9 year old API ...
>>> We'll look into it to see how painful it is to change.
>
> Ah, did not realize it caused that much pain.

bah. we should change anyway.

>> We can keep --directory and add --statefile.
>> For --statefile option we can use a switch for IS_DIR, IS_REG.
>>
>> --directory becomes deprecated and we switch to the statefile option
>> step by step.
>>
>> Does it make sense ?
>
> Yes, I think it will work.
>
> BTW, what was the rationale for a directory ?  Is it meant to be used
> with predefined file names in the specified directory ?

the mcr statefile use to be a directory, organized in such a way it was
easy to analyze its contents and easy to restart concurrently tasks
and shared states. it's still a directory but it's shrinking to be a
file and even a stream.

your request makes sense for the long term so we should take it.

c.

Re: [PATCH 1/2][LXC] Rename --directory option to --statefile

[Serge E. Hallyn]

Quoting Cedric Le Goater (clg-NmTC/0ZBporQT0dZR+AlfA@public.gmane.org):
> >>>This has some major impact on our side as it is a 9 year old API ...
> >>>We'll look into it to see how painful it is to change.
> >
> >Ah, did not realize it caused that much pain.
> 
> bah. we should change anyway.
> 
> >>We can keep --directory and add --statefile.
> >>For --statefile option we can use a switch for IS_DIR, IS_REG.
> >>
> >>--directory becomes deprecated and we switch to the statefile option
> >>step by step.
> >>
> >>Does it make sense ?
> >
> >Yes, I think it will work.
> >
> >BTW, what was the rationale for a directory ?  Is it meant to be used
> >with predefined file names in the specified directory ?
> 
> the mcr statefile use to be a directory, organized in such a way it was
> easy to analyze its contents and easy to restart concurrently tasks
> and shared states. it's still a directory but it's shrinking to be a
> file and even a stream.
> 
> your request makes sense for the long term so we should take it.
> 
> c.

Are we sure that we won't find other information which lxc_checkpoint
will want to store which sys_checkpoint() ignores?  A link to a btrfs
snapshot, for instance.

-serge

Re: [PATCH 1/2][LXC] Rename --directory option to --statefile

[Cedric Le Goater]

On 02/24/2010 07:30 PM, Serge E. Hallyn wrote:
> Are we sure that we won't find other information which lxc_checkpoint
> will want to store which sys_checkpoint() ignores?  A link to a btrfs
> snapshot, for instance.

after having froze the container and before running the lxc-checkpoint
command, one should be free to prepend information to the statefile. no ?

C.

Re: [PATCH 1/2][LXC] Rename --directory option to --statefile

[Serge E. Hallyn]

Quoting Cedric Le Goater (clg-NmTC/0ZBporQT0dZR+AlfA@public.gmane.org):
> On 02/24/2010 07:30 PM, Serge E. Hallyn wrote:
> >Are we sure that we won't find other information which lxc_checkpoint
> >will want to store which sys_checkpoint() ignores?  A link to a btrfs
> >snapshot, for instance.
> 
> after having froze the container and before running the lxc-checkpoint
> command, one should be free to prepend information to the statefile. no ?

Hmm, that might work.  I"ll leave it for Oren to answer that :)

-serge