[RFC][PATCH][lxc]: Add --with-usercr configure option

[RFC][PATCH][lxc]: Add --with-usercr configure option

[Sukadev Bhattiprolu]

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

---
From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
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.

Signed-off-by: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
 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]))
+fi
+
+
 AC_ARG_ENABLE([doc],
 	[AC_HELP_STRING([--enable-doc], [make mans (require docbook2man installed) [default=auto]])],
 	[], [enable_doc=auto])
diff --git a/src/lxc/Makefile.am b/src/lxc/Makefile.am
index be64b2a..e8748b5 100644
--- a/src/lxc/Makefile.am
+++ b/src/lxc/Makefile.am
@@ -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

Re: [RFC][PATCH][lxc]: Add --with-usercr configure option

[Michel Normand]

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

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 <sukadev@linux.vnet.ibm.com>
> 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.

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

> 
> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> ---
>  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.
I think this is better to detect such problem at time of make.

> +fi
> +
> +
>  AC_ARG_ENABLE([doc],
>  	[AC_HELP_STRING([--enable-doc], [make mans (require docbook2man installed) [default=auto]])],
>  	[], [enable_doc=auto])
> diff --git a/src/lxc/Makefile.am b/src/lxc/Makefile.am
> index be64b2a..e8748b5 100644
> --- a/src/lxc/Makefile.am
> +++ b/src/lxc/Makefile.am
> @@ -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


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

Re: [RFC][PATCH][lxc]: Add --with-usercr configure option

[Sukadev Bhattiprolu]

Michel Normand [normand-NmTC/0ZBporQT0dZR+AlfA@public.gmane.org] 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 <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
| > 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 <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
| > ---
| >  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

Re: [RFC][PATCH][lxc]: Add --with-usercr configure option

[Daniel Lezcano]

Sukadev Bhattiprolu wrote:
> Pls let me know if this will work. Will address your other comments
> and repost entire patchset.
> 
> ---
> From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> 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'

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

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

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

Re: [RFC][PATCH][lxc]: Add --with-usercr configure option

[Sukadev Bhattiprolu]

Daniel Lezcano [dlezcano-NmTC/0ZBporQT0dZR+AlfA@public.gmane.org] wrote:
> Sukadev Bhattiprolu wrote:
>> Pls let me know if this will work. Will address your other comments
>> and repost entire patchset.
>>
>> ---
>> From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
>> 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'
>
> Ok, let's call it : --with-cr

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

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

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

Re: [RFC][PATCH][lxc]: Add --with-usercr configure option

[Daniel Lezcano]

Sukadev Bhattiprolu wrote:
> Daniel Lezcano [dlezcano-NmTC/0ZBporQT0dZR+AlfA@public.gmane.org] wrote:
>> Sukadev Bhattiprolu wrote:
>>> Pls let me know if this will work. Will address your other comments
>>> and repost entire patchset.
>>>
>>> ---
>>> From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
>>> 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'
>> Ok, let's call it : --with-cr
> 
> Ok, or how about --with-libcr given that it will eventually be a library.

+1