: Add --with-usercr configure option

Pls let me know if this will work. Will address your other comments and repost entire patchset.

---
From: Sukadev Bhattiprolu <[email protected]>
Date: Wed, 24 Mar 2010 17:26:44 -0700
Subject: [PATCH 1/1] Add --with-usercr configure option

Add a configure option, --with-usercr=dir which would allow linking with external (i.e USERCR) implementation of checkpoint/restart.

For now, USERCR "publishes" a app-checkpoint.h, checkpoint.o and restart.o files which implement the functions app_checkpoint() and app_restart().

Usage:

	$ ./autogen.sh

	$ ./configure --help |grep usercr
	--with-usercr=dir     use the Checkpoint/Restart implementation in 'dir'

	$ ls /home/guest/user-cr/
	app-checkpoint.h    checkpoint.o    restart.o

	$ ./configure --with-usercr=/home/guest/user-cr

TODO:
If names of interfaces in USERCR change, we may want to rename the config option too ?

Can we remove the src/lxc/{checkpoint.o,restart.o} files from liblxc.so and lxc-checkpoint/lxc-restart directly with them ? That way, the USERCR interfaces/object files don't end up in liblxc.so.

USERCR_CFLAGS are only needed for src/lxc/{checkpoint.c,restart.c} but not sure if there is an easy way to define autoconf CFLAGS just for those two files.

---
 configure.ac        |   19 +++++++++++++++++++
 src/lxc/Makefile.am |   10 +++++++++-
 2 files changed, 28 insertions(+), 1 deletions(-)

index f82e7df..dd46e4d 100644
@@ -12,6 +12,25 @@ AM_PROG_CC_C_O
 AC_GNU_SOURCE
 AC_CHECK_PROG(SETCAP, setcap, yes, no, $PATH$PATH_SEPARATOR/sbin)
 
+AC_ARG_WITH(usercr, [AS_HELP_STRING([--with-usercr=dir], \
+           [use the Checkpoint/Restart implementation in 'dir'])], [], \
+	   [with_usercr=no])
+
+if test "x$with_usercr" != "xno"; then
+       AS_AC_EXPAND(USERCR_OBJS, "${with_usercr}/checkpoint.o ${with_usercr}/restart.o")
+       AS_AC_EXPAND(USERCR_CFLAGS, "-DUSERCR -I${with_usercr}")
+
+       AC_CHECK_FILE([$with_usercr/app-checkpoint.h], [], \
+               AC_MSG_ERROR([--with-usercr specified directory $with_usercr but $with_usercr/app-checkpoint.h was not found]))
+
+       AC_CHECK_FILE([${with_usercr}/checkpoint.o], [], \
+               AC_MSG_ERROR([--with-usercr specified directory $with_usercr but ${with_usercr}/checkpoint.o was not found]))
+
+       AC_CHECK_FILE([${with_usercr}/restart.o], [], \
+               AC_MSG_ERROR([--with-usercr specified directory $with_usercr but ${with_usercr}/restart.o was not found]))
+fi
+
+
 AC_ARG_ENABLE([doc],
 	[AC_HELP_STRING([--enable-doc], [make mans (require docbook2man installed) [default=auto]])],
 	[], [enable_doc=auto])

index be64b2a..e8748b5 100644
@@ -45,12 +45,20 @@ liblxc_so_SOURCES = \
 	mainloop.c mainloop.h \
 	af_unix.c af_unix.h
 
-AM_CFLAGS=-I$(top_srcdir)/src
+# We only need $(USERCR_CFLAGS) for lxc_checkpoint and lxc_restart files
+# but for now, just set it for all.
+AM_CFLAGS=-I$(top_srcdir)/src $(USERCR_CFLAGS)
 
 liblxc_so_CFLAGS = -fPIC -DPIC $(AM_CFLAGS)
 
+# TODO: Adding $(USERCR_OBJS) here ensures we don't have undefined references
+# 	when building liblxc.so, but this has the side-effect of putting the
+# 	app_checkpoint/restart functions in liblxc.so. Or alternatively,
+# 	we could remove src/lxc/{checkpoint.o,restart.o} from liblxc.so
+# 	and link lxc-checkpoint/lxc-restart with them directly.
 liblxc_so_LDFLAGS = \
 	-shared \
+	$(USERCR_OBJS) \
 	-Wl,-soname,liblxc.so.$(firstword $(subst ., ,$(VERSION)))
 
 liblxc_so_LDADD = -lutil

1.6.6.1

Suka,
I am adding my own comments, even if mail addressed to Daniel.

not sure to understand, but the --with-usercr to stay lowercase.

exemple should work (I did not tried) lxc_checkpoint_SOURCES = lxc_checkpoint.c lxc_checkpoint_CFLAGS = -I$(top_srcdir)/src $(USERCR_CFLAGS)

Michel Normand [normand-NmTC/[email protected]] wrote:
| Suka,
| I am adding my own comments, even if mail addressed to Daniel.

I did mean to CC you since you had a comment on this earlier. But even otherwise, comments always welcome :-)

| 
| Le mercredi 24 mars 2010 à 18:05 -0700, Sukadev Bhattiprolu a écrit :
| > Pls let me know if this will work. Will address your other comments
| > and repost entire patchset.
| > 
| > ---
| > From: Sukadev Bhattiprolu <[email protected]>
| > Date: Wed, 24 Mar 2010 17:26:44 -0700
| > Subject: [PATCH 1/1] Add --with-usercr configure option
| > 
| > Add a configure option, --with-usercr=dir which would allow linking
| > with external (i.e USERCR) implementation  of checkpoint/restart.
| > 
| > For now, USERCR "publishes" a app-checkpoint.h, checkpoint.o and
| > restart.o files which implement the functions app_checkpoint() and
| > app_restart().
| > 
| > Usage:
| > 	$ ./autogen.sh
| > 
| > 	$ ./configure --help |grep usercr
| > 	--with-usercr=dir     use the Checkpoint/Restart implementation in 'dir'
| > 
| > 	$ ls /home/guest/user-cr/
| > 	app-checkpoint.h    checkpoint.o    restart.o
| > 
| > 	$ ./configure --with-usercr=/home/guest/user-cr
| > 
| > TODO:
| > 	If names of interfaces in USERCR change, we may want to rename
| > 	the config option too ?
| 
| not sure to understand, but the --with-usercr to stay lowercase.

I was just not sure if the name 'with-usercr' made sense for long term :-) Like Daniel suggested, I will go with --with-cr for now. Maybe --with-libcr would make more sense in the long run.

| 
| > 
| > 	Can we remove the src/lxc/{checkpoint.o,restart.o} files from
| > 	liblxc.so and lxc-checkpoint/lxc-restart directly with them ?
| > 	That way, the USERCR interfaces/object files don't end up in
| > 	liblxc.so.
| > 
| > 	USERCR_CFLAGS are only needed for src/lxc/{checkpoint.c,restart.c}
| > 	but not sure if there is an easy way to define autoconf CFLAGS
| > 	just for those two files.
| 
| exemple should work (I did not tried)
| lxc_checkpoint_SOURCES = lxc_checkpoint.c
| lxc_checkpoint_CFLAGS = -I$(top_srcdir)/src $(USERCR_CFLAGS)

Hmm, I meant that the additional CFLAGS are needed for src/lxc/checkpoint.c
not src/lxc/lxc_checkpoint.c.

Since this checkpoint.c goes into liblxc.so, I tried:

checkpoint_CFLAGS = $(liblxc_so_CFLAGS) $(USERCR_CFLAGS)

but that did not work. Maybe I am missing something still.

| 
| > 
| > Signed-off-by: Sukadev Bhattiprolu <[email protected]>
| > ---
| >  configure.ac        |   19 +++++++++++++++++++
| >  src/lxc/Makefile.am |   10 +++++++++-
| >  2 files changed, 28 insertions(+), 1 deletions(-)
| > 
| > diff --git a/configure.ac b/configure.ac
| > index f82e7df..dd46e4d 100644
| > --- a/configure.ac
| > +++ b/configure.ac
| > @@ -12,6 +12,25 @@ AM_PROG_CC_C_O
| >  AC_GNU_SOURCE
| >  AC_CHECK_PROG(SETCAP, setcap, yes, no, $PATH$PATH_SEPARATOR/sbin)
| >  
| > +AC_ARG_WITH(usercr, [AS_HELP_STRING([--with-usercr=dir], \
| > +           [use the Checkpoint/Restart implementation in 'dir'])], [], \
| > +	   [with_usercr=no])
| > +
| > +if test "x$with_usercr" != "xno"; then
| > +       AS_AC_EXPAND(USERCR_OBJS, "${with_usercr}/checkpoint.o ${with_usercr}/restart.o")
| > +       AS_AC_EXPAND(USERCR_CFLAGS, "-DUSERCR -I${with_usercr}")
| > +
| > +       AC_CHECK_FILE([$with_usercr/app-checkpoint.h], [], \
| > +               AC_MSG_ERROR([--with-usercr specified directory $with_usercr but $with_usercr/app-checkpoint.h was not found]))
| > +
| > +       AC_CHECK_FILE([${with_usercr}/checkpoint.o], [], \
| > +               AC_MSG_ERROR([--with-usercr specified directory $with_usercr but ${with_usercr}/checkpoint.o was not found]))
| > +
| > +       AC_CHECK_FILE([${with_usercr}/restart.o], [], \
| > +               AC_MSG_ERROR([--with-usercr specified directory $with_usercr but ${with_usercr}/restart.o was not found]))
| 
| I do not think the three above AC_CHECK_FILE are a good thing to be done
| as would need to have the user-cr to be already compiled
| at time of configure of lxc.

At this time, we do expect that the external C/R implementation to be compiled - it is not something that is built in LXC right ?

One reason I was trying to do the check is if there is a typo in the path specified by the user, the error would not be reported until the make and it could be confusing error message.

How about I leave the check for 'app-checkpoint.h' and later when the library is implemented, we could check for the existence of the library.

| I think this is better to detect such problem at time of make. |

Sukadev

Ok, let's call it : --with-cr

Hum, no. It would be preferable to integrate them in lxc for now (as experimental code) and later create a glue between lxc and usercr, so usercr interfaces would be callable as a plugin of lxc.

The --with-cr will remains but specifying a CR library. The lxc checkpoint and restart code will then dlopen this library and use the checkpoint / restart functions of this library.

If no library is specified, the lxc-checkpoint / lxc-restart will return ENOSYS.

That will ensure the CR specific code stays in a separate library and will keep things opened to take into account other CR solutions like blcr.

But until we reach this point, we should integrate everything hardcoded in the lxc library in order to let the plugin API drawing to appear in the integration big picture.

Thanks
-- Daniel

Ok, or how about --with-libcr given that it will eventually be a library.

Ok. Sounds good. I will leave the USERCR interfaces in liblxc.so for now.

+1