[lxc-devel] [PATCH] Use LXCPATH and LOCALSTATEDIR instead of hardcoded /var

Dwight Engen dwight.engen at oracle.com
Wed Dec 5 17:30:22 UTC 2012


On Fri, 30 Nov 2012 17:09:28 -0500
Stéphane Graber <stgraber at ubuntu.com> wrote:

> On 11/30/2012 09:50 AM, Dwight Engen wrote:
> > Signed-off-by: Dwight Engen <dwight.engen at oracle.com>
> 
> It looks like you're mixing LOCALSTATEDIR "/lib/lxc" and LXCPATH
> quite a bit. Shouldn't we use LXCPATH whenever possible?

Sorry, just now noticed your comment: Yes, good catch: in the two cases
in the code, mount_entry_on_absolute_rootfs() and containertests.c, we
should be using LXCPATH as you point out. I think it is correct to use
LOCALSTATEDIR in the templates for the lock files and rootfs cache (ala
lxc-debian.in). Updated patch to follow.

> > ---
> >  src/lxc/Makefile.am           |  3 ++-
> >  src/lxc/conf.c                |  2 +-
> >  src/lxc/lxccontainer.c        |  2 +-
> >  src/tests/Makefile.am         |  3 ++-
> >  src/tests/containertests.c    |  2 +-
> >  src/tests/saveconfig.c        |  2 +-
> >  src/tests/startone.c          | 16 ++++++++--------
> >  templates/lxc-altlinux.in     | 10 +++++-----
> >  templates/lxc-fedora.in       | 12 ++++++------
> >  templates/lxc-opensuse.in     | 10 +++++-----
> >  templates/lxc-ubuntu-cloud.in |  6 +++---
> >  11 files changed, 35 insertions(+), 33 deletions(-)
> > 
> > diff --git a/src/lxc/Makefile.am b/src/lxc/Makefile.am
> > index 02f6855..cb171a3 100644
> > --- a/src/lxc/Makefile.am
> > +++ b/src/lxc/Makefile.am
> > @@ -65,7 +65,8 @@ AM_CFLAGS=-I$(top_srcdir)/src \
> >  	-DLXCROOTFSMOUNT=\"$(LXCROOTFSMOUNT)\" \
> >  	-DLXCPATH=\"$(LXCPATH)\" \
> >  	-DLXCINITDIR=\"$(LXCINITDIR)\" \
> > -	-DLXCTEMPLATEDIR=\"$(LXCTEMPLATEDIR)\"
> > +	-DLXCTEMPLATEDIR=\"$(LXCTEMPLATEDIR)\" \
> > +	-DLOCALSTATEDIR=\"$(LOCALSTATEDIR)\"
> >  
> >  if ENABLE_APPARMOR
> >  AM_CFLAGS += -DHAVE_APPARMOR
> > diff --git a/src/lxc/conf.c b/src/lxc/conf.c
> > index c02e109..d9f1bde 100644
> > --- a/src/lxc/conf.c
> > +++ b/src/lxc/conf.c
> > @@ -1369,7 +1369,7 @@ static int
> > mount_entry_on_absolute_rootfs(struct mntent *mntent, 
> >  	/* if rootfs->path is a blockdev path, allow container
> > fstab to
> >  	 * use /var/lib/lxc/CN/rootfs as the target prefix */
> > -	r = snprintf(path, MAXPATHLEN, "/var/lib/lxc/%s/rootfs",
> > lxc_name);
> > +	r = snprintf(path, MAXPATHLEN, LOCALSTATEDIR
> > "/lib/lxc/%s/rootfs", lxc_name); if (r < 0 || r >= MAXPATHLEN)
> >  		goto skipvarlib;
> >  
> > diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> > index e41fd0d..72ad1b8 100644
> > --- a/src/lxc/lxccontainer.c
> > +++ b/src/lxc/lxccontainer.c
> > @@ -922,7 +922,7 @@ struct lxc_container *lxc_container_new(const
> > char *name) c->get_config_item = lxcapi_get_config_item;
> >  
> >  	/* we'll allow the caller to update these later */
> > -	if (lxc_log_init("/var/log/lxccontainer.log", "trace",
> > "lxc_container", 0)) {
> > +	if (lxc_log_init(LOCALSTATEDIR "/log/lxccontainer.log",
> > "trace", "lxc_container", 0)) { fprintf(stderr, "failed to open
> > log\n"); goto err;
> >  	}
> > diff --git a/src/tests/Makefile.am b/src/tests/Makefile.am
> > index fa61f70..01d7001 100644
> > --- a/src/tests/Makefile.am
> > +++ b/src/tests/Makefile.am
> > @@ -14,7 +14,8 @@ lxc_test_getkeys_SOURCES = getkeys.c
> >  AM_CFLAGS=-I$(top_srcdir)/src \
> >  	-DLXCROOTFSMOUNT=\"$(LXCROOTFSMOUNT)\" \
> >  	-DLXCPATH=\"$(LXCPATH)\" \
> > -	-DLXCINITDIR=\"$(LXCINITDIR)\"
> > +	-DLXCINITDIR=\"$(LXCINITDIR)\" \
> > +	-DLOCALSTATEDIR=\"$(LOCALSTATEDIR)\"
> >  
> >  bin_PROGRAMS = lxc-test-containertests lxc-test-locktests
> > lxc-test-startone \ lxc-test-destroytest lxc-test-saveconfig
> > lxc-test-createtest \ diff --git a/src/tests/containertests.c
> > b/src/tests/containertests.c index db6942f..485529d 100644
> > --- a/src/tests/containertests.c
> > +++ b/src/tests/containertests.c
> > @@ -122,7 +122,7 @@ int main(int argc, char *argv[])
> >  		goto out;
> >  	}
> >  	str = c->config_file_name(c);
> > -#define CONFIGFNAM "/var/lib/lxc/" MYNAME "/config"
> > +#define CONFIGFNAM LOCALSTATEDIR "/lib/lxc/" MYNAME "/config"
> >  	if (!str || strcmp(str, CONFIGFNAM)) {
> >  		fprintf(stderr, "%d: got wrong config file name
> > (%s, not %s)\n", __LINE__, str, CONFIGFNAM); goto out;
> > diff --git a/src/tests/saveconfig.c b/src/tests/saveconfig.c
> > index 20afdc5..21450b2 100644
> > --- a/src/tests/saveconfig.c
> > +++ b/src/tests/saveconfig.c
> > @@ -92,7 +92,7 @@ int main(int argc, char *argv[])
> >  		fprintf(stderr, "%d: failed writing config
> > file /tmp/lxctest1\n", __LINE__); goto out;
> >  	}
> > -	rename("/var/lib/lxc/" MYNAME "/config", "/var/lib/lxc/"
> > MYNAME "/config.bak");
> > +	rename(LXCPATH "/" MYNAME "/config", LXCPATH "/" MYNAME
> > "/config.bak"); if (!c->save_config(c, NULL)) {
> >  		fprintf(stderr, "%d: failed writing config
> > file\n", __LINE__); goto out;
> > diff --git a/src/tests/startone.c b/src/tests/startone.c
> > index 73c8f00..81a6bfb 100644
> > --- a/src/tests/startone.c
> > +++ b/src/tests/startone.c
> > @@ -178,21 +178,21 @@ int main(int argc, char *argv[])
> >  		goto out;
> >  	c->stop(c);
> >  
> > -	ret = system("mkdir
> > -p /var/lib/lxc/lxctest1/rootfs//usr/local/libexec/lxc");
> > +	ret = system("mkdir -p " LXCPATH
> > "/lxctest1/rootfs//usr/local/libexec/lxc"); if (!ret)
> > -		ret = system("mkdir
> > -p /var/lib/lxc/lxctest1/rootfs/usr/lib/lxc/");
> > +		ret = system("mkdir -p " LXCPATH
> > "/lxctest1/rootfs/usr/lib/lxc/"); if (!ret)
> > -		ret = system("cp
> > src/lxc/lxc-init /var/lib/lxc/lxctest1/rootfs//usr/local/libexec/lxc");
> > +		ret = system("cp src/lxc/lxc-init " LXCPATH
> > "/lxctest1/rootfs//usr/local/libexec/lxc"); if (!ret)
> > -		ret = system("cp
> > src/lxc/liblxc.so /var/lib/lxc/lxctest1/rootfs/usr/lib/lxc");
> > +		ret = system("cp src/lxc/liblxc.so " LXCPATH
> > "/lxctest1/rootfs/usr/lib/lxc"); if (!ret)
> > -		ret = system("cp
> > src/lxc/liblxc.so /var/lib/lxc/lxctest1/rootfs/usr/lib/lxc/liblxc.so.0");
> > +		ret = system("cp src/lxc/liblxc.so " LXCPATH
> > "/lxctest1/rootfs/usr/lib/lxc/liblxc.so.0"); if (!ret)
> > -		ret = system("cp
> > src/lxc/liblxc.so /var/lib/lxc/lxctest1/rootfs/usr/lib");
> > +		ret = system("cp src/lxc/liblxc.so " LXCPATH
> > "/lxctest1/rootfs/usr/lib"); if (!ret)
> > -		ret = system("mkdir
> > -p /var/lib/lxc/lxctest1/rootfs/dev/shm");
> > +		ret = system("mkdir -p " LXCPATH
> > "/lxctest1/rootfs/dev/shm"); if (!ret)
> > -		ret = system("chroot /var/lib/lxc/lxctest1/rootfs
> > apt-get install --no-install-recommends lxc");
> > +		ret = system("chroot " LXCPATH "/lxctest1/rootfs
> > apt-get install --no-install-recommends lxc"); if (ret) {
> >  		fprintf(stderr, "%d: failed to installing lxc-init
> > in container\n", __LINE__); goto out;
> > diff --git a/templates/lxc-altlinux.in b/templates/lxc-altlinux.in
> > index 0bf9735..668b3eb 100644
> > --- a/templates/lxc-altlinux.in
> > +++ b/templates/lxc-altlinux.in
> > @@ -26,7 +26,7 @@
> >  
> >  #Configurations
> >  arch=$(arch)
> > -cache_base=/var/cache/lxc/altlinux/$arch
> > +cache_base=@LOCALSTATEDIR@/cache/lxc/altlinux/$arch
> >  default_path=@LXCPATH@
> >  default_profile=default
> >  profile_dir=/etc/lxc/profiles
> > @@ -196,7 +196,7 @@ update_altlinux()
> >  
> >  install_altlinux()
> >  {
> > -    mkdir -p /var/lock/subsys/
> > +    mkdir -p @LOCALSTATEDIR@/lock/subsys/
> >      (
> >  	flock -x 200
> >  	if [ $? -ne 0 ]; then
> > @@ -230,7 +230,7 @@ install_altlinux()
> >  
> >  	return 0
> >  
> > -	) 200>/var/lock/subsys/lxc
> > +	) 200>@LOCALSTATEDIR@/lock/subsys/lxc
> >  
> >      return $?
> >  }
> > @@ -328,7 +328,7 @@ clean()
> >  	rm --preserve-root --one-file-system -rf $cache && echo
> > "Done." || exit 1 exit 0
> >  
> > -    ) 200>/var/lock/subsys/lxc
> > +    ) 200>@LOCALSTATEDIR@/lock/subsys/lxc
> >  }
> >  
> >  usage()
> > @@ -345,7 +345,7 @@ usage:
> >  Mandatory args:
> >    -n,--name         container name, used to as an identifier for
> > that container from now on Optional args:
> > -  -p,--path         path to where the container rootfs will be
> > created, defaults to /var/lib/lxc. The container config will go
> > under /var/lib/lxc in and case
> > +  -p,--path         path to where the container rootfs will be
> > created, defaults to @LXCPATH at . The container config will go under
> > @LXCPATH@ in that case -c,--clean        clean the cache
> > -R,--release      ALTLinux release for the new container. if the
> > host is ALTLinux, then it will defaultto the host's release.
> > -4,--ipv4         specify the ipv4 address to assign to the
> > virtualized interface, eg. 192.168.1.123/24 diff --git
> > a/templates/lxc-fedora.in b/templates/lxc-fedora.in index
> > edd4b9d..c2fb3a0 100644 --- a/templates/lxc-fedora.in +++
> > b/templates/lxc-fedora.in @@ -27,8 +27,8 @@ 
> >  #Configurations
> >  arch=$(arch)
> > -cache_base=/var/cache/lxc/fedora/$arch
> > -default_path=/var/lib/lxc
> > +cache_base=@LOCALSTATEDIR@/cache/lxc/fedora/$arch
> > +default_path=@LXCPATH@
> >  root_password=root
> >  
> >  # is this fedora?
> > @@ -200,7 +200,7 @@ update_fedora()
> >  
> >  install_fedora()
> >  {
> > -    mkdir -p /var/lock/subsys/
> > +    mkdir -p @LOCALSTATEDIR@/lock/subsys/
> >      (
> >  	flock -x 200
> >  	if [ $? -ne 0 ]; then
> > @@ -234,7 +234,7 @@ install_fedora()
> >  
> >  	return 0
> >  
> > -	) 200>/var/lock/subsys/lxc
> > +	) 200>@LOCALSTATEDIR@/lock/subsys/lxc
> >  
> >      return $?
> >  }
> > @@ -303,7 +303,7 @@ clean()
> >  	rm --preserve-root --one-file-system -rf $cache && echo
> > "Done." || exit 1 exit 0
> >  
> > -    ) 200>/var/lock/subsys/lxc
> > +    ) 200>@LOCALSTATEDIR@/lock/subsys/lxc
> >  }
> >  
> >  usage()
> > @@ -316,7 +316,7 @@ usage:
> >  Mandatory args:
> >    -n,--name         container name, used to as an identifier for
> > that container from now on Optional args:
> > -  -p,--path         path to where the container rootfs will be
> > created, defaults to /var/lib/lxc. The container config will go
> > under /var/lib/lxc in that case
> > +  -p,--path         path to where the container rootfs will be
> > created, defaults to @LXCPATH at . The container config will go under
> > @LXCPATH@ in that case -c,--clean        clean the cache
> > -R,--release      Fedora release for the new container. if the host
> > is Fedora, then it will defaultto the host's release.
> > -A,--arch         NOT USED YET. Define what arch the container will
> > be [i686,x86_64] diff --git a/templates/lxc-opensuse.in
> > b/templates/lxc-opensuse.in index 3242451..01e8f5e 100644 ---
> > a/templates/lxc-opensuse.in +++ b/templates/lxc-opensuse.in @@
> > -213,9 +213,9 @@ copy_opensuse() 
> >  install_opensuse()
> >  {
> > -    cache="/var/cache/lxc/opensuse"
> > +    cache="@LOCALSTATEDIR@/cache/lxc/opensuse"
> >      rootfs=$1
> > -    mkdir -p /var/lock/subsys/
> > +    mkdir -p @LOCALSTATEDIR@/lock/subsys/
> >      (
> >  	flock -x 200
> >  	if [ $? -ne 0 ]; then
> > @@ -243,7 +243,7 @@ install_opensuse()
> >  
> >  	return 0
> >  
> > -	) 200>/var/lock/subsys/lxc
> > +	) 200>@LOCALSTATEDIR@/lock/subsys/lxc
> >  
> >      return $?
> >  }
> > @@ -298,7 +298,7 @@ EOF
> >  
> >  clean()
> >  {
> > -    cache="/var/cache/lxc/opensuse"
> > +    cache="@LOCALSTATEDIR@/cache/lxc/opensuse"
> >  
> >      if [ ! -e $cache ]; then
> >  	exit 0
> > @@ -316,7 +316,7 @@ clean()
> >  	rm --preserve-root --one-file-system -rf $cache && echo
> > "Done." || exit 1 exit 0
> >  
> > -    ) 200>/var/lock/subsys/lxc
> > +    ) 200>@LOCALSTATEDIR@/lock/subsys/lxc
> >  }
> >  
> >  usage()
> > diff --git a/templates/lxc-ubuntu-cloud.in
> > b/templates/lxc-ubuntu-cloud.in index 300b47d..e1e7431 100644
> > --- a/templates/lxc-ubuntu-cloud.in
> > +++ b/templates/lxc-ubuntu-cloud.in
> > @@ -262,7 +262,7 @@ type wget
> >  
> >  # determine the url, tarball, and directory names
> >  # download if needed
> > -cache="/var/cache/lxc/cloud-$release"
> > +cache="@LOCALSTATEDIR@/cache/lxc/cloud-$release"
> >  
> >  mkdir -p $cache
> >  
> > @@ -318,7 +318,7 @@ build_root_tgz()
> >      trap SIGTERM
> >  }
> >  
> > -mkdir -p /var/lock/subsys/
> > +mkdir -p @LOCALSTATEDIR@/lock/subsys/
> >  (
> >      flock -x 200
> >  
> > @@ -395,7 +395,7 @@ EOF
> >          echo "If you do not have a meta-data service, this
> > container will likely be useless." 
> >      fi
> > -) 200>/var/lock/subsys/lxc-ubucloud
> > +) 200>@LOCALSTATEDIR@/lock/subsys/lxc-ubucloud
> >  
> >  copy_configuration $path $rootfs $name $arch $release
> >  
> > 
> 
> 





More information about the lxc-devel mailing list