[lxc-devel] [PATCH] introduce lxc_config

[Dwight Engen]

On Mon, 29 Apr 2013 16:30:00 -0500
Serge Hallyn <serge.hallyn at ubuntu.com> wrote:

> Quoting Serge Hallyn (serge.hallyn at ubuntu.com):
> > Quoting Serge Hallyn (serge.hallyn at ubuntu.com):
> > > add vg and zfsroot options to lxc.functions and use in lxc-create
> > > 
> > > also make sure to drop spaces between = and variable in lxc.conf
> > 
> > Hm, a hunk is missing from this patch.  Ignore this one for now, I'm
> > getting rid of the duplication of reading global configs in
> > lxc.functions.in and replacing it with a wrapper program
> > 'lxc_config', will merge that with this when I send.
> 
> [[
> Eh, on second thought, the dropped part disappears anyway, so here is
> the last bit as a separate patch.
> 
> Dwight or Stéphane, if one of you could review and/or ack the set at
> https://github.com/hallyn/lxc/commits/s.b.clone2 (or in this thread),
> I'd like to address any needed changes and push to staging.

Couple of minor comments below

Acked-by: Dwight Engen <dwight.engen at oracle.com>

> ]]
> 
> Subject: [PATCH 1/1] introduce lxc_config
> 
> It's a tiny program (exported through the api) wrapping the util.c
> helpers for reading /etc/lxc/lxc.conf variables, and replaces
> the kludgy shell duplication in lxc.functions.in
> 
> Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
> ---
>  src/lxc/Makefile.am      |  4 +++-
>  src/lxc/lxc.functions.in | 33 +++-----------------------------
>  src/lxc/lxc_config.c     | 50
> ++++++++++++++++++++++++++++++++++++++++++++++++
> src/lxc/lxccontainer.c   | 10 ++++++++++ src/lxc/lxccontainer.h   |
> 2 ++ 5 files changed, 68 insertions(+), 31 deletions(-)
>  create mode 100644 src/lxc/lxc_config.c
> 
> diff --git a/src/lxc/Makefile.am b/src/lxc/Makefile.am
> index fee6103..e3ff4a9 100644
> --- a/src/lxc/Makefile.am
> +++ b/src/lxc/Makefile.am
> @@ -161,7 +161,8 @@ bin_PROGRAMS = \
>  	lxc-unfreeze \
>  	lxc-checkpoint \
>  	lxc-restart \
> -	lxc-kill
> +	lxc-kill \
> +	lxc-config
>  
>  pkglibexec_PROGRAMS = \
>  	lxc-init
> @@ -178,6 +179,7 @@ LDADD=liblxc.so @CAP_LIBS@ @APPARMOR_LIBS@
> @SECCOMP_LIBS@ lxc_attach_SOURCES = lxc_attach.c
>  lxc_cgroup_SOURCES = lxc_cgroup.c
>  lxc_checkpoint_SOURCES = lxc_checkpoint.c
> +lxc_config_SOURCES = lxc_config.c
>  lxc_console_SOURCES = lxc_console.c
>  lxc_execute_SOURCES = lxc_execute.c
>  lxc_freeze_SOURCES = lxc_freeze.c
> diff --git a/src/lxc/lxc.functions.in b/src/lxc/lxc.functions.in
> index 416267f..ad3d42f 100644
> --- a/src/lxc/lxc.functions.in
> +++ b/src/lxc/lxc.functions.in
> @@ -25,33 +25,6 @@ bindir=@BINDIR@
>  templatedir=@LXCTEMPLATEDIR@
>  lxcinitdir=@LXCINITDIR@
>  
> -get_default_lxcpath() {
> -	LXC_PATH=$(grep -v "^#" "$globalconf" 2>/dev/null | grep
> "[ \t]*lxcpath[ \t]*=") || true
> -	if [ -n "$LXC_PATH" ]; then
> -		echo $LXC_PATH | awk -F= '{ print $2 }'
> -	else
> -		echo @LXCPATH@
> -	fi
> -}
> -
> -get_default_vg() {
> -	LXC_VG=$(grep -v "^#" "$globalconf" 2>/dev/null | grep
> "[ \t]*lvm_vg[ \t]*=") || true
> -	if [ -n "$LXC_VG" ]; then
> -		echo $LXC_VG | awk -F= '{ print $2 }'
> -	else
> -		echo "lxc"
> -	fi
> -}
> -
> -get_default_zfsroot() {
> -	LXC_ZFSROOT=$(grep -v "^#" "$globalconf" 2>/dev/null | grep
> "[ \t]*zfsroot[ \t]*=") || true
> -	if [ -n "$LXC_ZFSROOT" ]; then
> -		echo $LXC_ZFSROOT | awk -F= '{ print $2 }'
> -	else
> -		echo "tank/lxc"
> -	fi
> -}
> -
> -lxc_path=`get_default_lxcpath`
> -lxc_vg=`get_default_vg`
> -lxc_zfsroot=`get_default_zfsroot`
> +lxc_path=`lxc-config default_path`
> +lxc_vg=`lxc-config lvm_vg`
> +lxc_zfsroot=`lxc-config zfsroot`
> diff --git a/src/lxc/lxc_config.c b/src/lxc/lxc_config.c
> new file mode 100644
> index 0000000..83621ec
> --- /dev/null
> +++ b/src/lxc/lxc_config.c
> @@ -0,0 +1,50 @@
> +#include <stdio.h>
> +#include "config.h"
> +#include "lxccontainer.h"
> +
> +struct lxc_config_items {
> +	char *name;
> +	const char *(*fn)(void);
> +};
> +
> +struct lxc_config_items items[] =
> +{
> +	{ .name = "default_path", .fn =

Should this be "lxcpath" instead of "default_path" so it matches the
field in the file? (or change default_lxc_path() to "default_path" but
that would break users' files).

> &lxc_get_default_config_path, },
> +	{ .name = "lvm_vg", .fn = &lxc_get_default_lvm_vg, },
> +	{ .name = "zfsroot", .fn = &lxc_get_default_zfs_root, },
> +	{ .name = NULL, },
> +};
> +
> +void usage(char *me)
> +{
> +	printf("Usage: %s -l: list all available configuration
> items\n", me);
> +	printf("       %s item: print configuration item\n", me);
> +	exit(1);
> +}
> +
> +void list_config_items(void)
> +{
> +	struct lxc_config_items *i;
> +
> +	for (i = &items[0]; i->name; i++)
> +		printf("%s\n", i->name);
> +	exit(0);
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +	struct lxc_config_items *i;
> +
> +	if (argc < 2)
> +		usage(argv[0]);
> +	if (strcmp(argv[1], "-l") == 0)
> +		list_config_items();
> +	for (i = &items[0]; i->name; i++) {
> +		if (strcmp(argv[1], i->name) == 0) {
> +			printf("%s\n", i->fn());
> +			exit(0);
> +		}
> +	}
> +	printf("Unknown configuration item: %s\n", argv[1]);
> +	return 0;

Should we return 1 or EXIT_FAILURE here?

> +}
> diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> index ef111c0..e218407 100644
> --- a/src/lxc/lxccontainer.c
> +++ b/src/lxc/lxccontainer.c
> @@ -1007,6 +1007,16 @@ const char *lxc_get_default_config_path(void)
>  	return default_lxc_path();
>  }
>  
> +const char *lxc_get_default_lvm_vg(void)
> +{
> +	return default_lvm_vg();
> +}
> +
> +const char *lxc_get_default_zfs_root(void)
> +{
> +	return default_zfs_root();
> +}
> +
>  const char *lxc_get_version(void)
>  {
>  	return lxc_version();
> diff --git a/src/lxc/lxccontainer.h b/src/lxc/lxccontainer.h
> index a4be753..b6bd97c 100644
> --- a/src/lxc/lxccontainer.h
> +++ b/src/lxc/lxccontainer.h
> @@ -122,6 +122,8 @@ int lxc_container_get(struct lxc_container *c);
>  int lxc_container_put(struct lxc_container *c);
>  int lxc_get_wait_states(const char **states);
>  const char *lxc_get_default_config_path(void);
> +const char *lxc_get_default_lvm_vg(void);
> +const char *lxc_get_default_zfs_root(void);
>  const char *lxc_get_version(void);
>  
>  #if 0