[lxc-devel] [PATCH] Refactor lxc-snapshot

Christian Brauner christianvanbrauner at gmail.com
Fri Aug 7 21:57:21 UTC 2015


Updated version as PR on github.

On Thu, Aug 06, 2015 at 03:47:04PM +0200, Christian Brauner wrote:
> - lxc_snapshot.c lacked necessary members in the associated lxc_arguments struct
>   in arguments.h. This commit extends the lxc_arguments struct to include
>   several parameters used by lxc-snapshot which allows a rewrite that is more
>   consistent with the rest of the lxc-* executables.
> - All tests have been moved beyond the call to lxc_log_init() to allow for the
>   messages to be printed or saved.
> - Some small changes to the my_args struct. (The enum task is set to
>   SNAP (for snapshot) per default and variables illustrating the usage of the
>   command line flags are written in all caps.)
> 
> Signed-off-by: Christian Brauner <christianvanbrauner at gmail.com>
> ---
>  src/lxc/arguments.h    |   8 ++
>  src/lxc/lxc_snapshot.c | 317 +++++++++++++++++++++++++++----------------------
>  2 files changed, 182 insertions(+), 143 deletions(-)
> 
> diff --git a/src/lxc/arguments.h b/src/lxc/arguments.h
> index cc85f86..86fd02e 100644
> --- a/src/lxc/arguments.h
> +++ b/src/lxc/arguments.h
> @@ -94,12 +94,20 @@ struct lxc_arguments {
>  	int list;
>  	char *groups;
>  
> +	/* lxc-snapshot and lxc-clone */
> +	enum task {DESTROY, LIST, RESTORE, SNAP, } task;
> +	int print_comments;
> +	char *commentfile;
> +	char *newname;
> +	char *snapname;
> +
>  	/* remaining arguments */
>  	char *const *argv;
>  	int argc;
>  
>  	/* private arguments */
>  	void *data;
> +
>  };
>  
>  #define LXC_COMMON_OPTIONS \
> diff --git a/src/lxc/lxc_snapshot.c b/src/lxc/lxc_snapshot.c
> index a03c0c0..99cbe48 100644
> --- a/src/lxc/lxc_snapshot.c
> +++ b/src/lxc/lxc_snapshot.c
> @@ -16,8 +16,8 @@
>   * with this program; if not, write to the Free Software Foundation, Inc.,
>   * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
>   */
> -#include "config.h"
>  
> +#include "confile.h"
>  #include <stdio.h>
>  #include <libgen.h>
>  #include <unistd.h>
> @@ -35,18 +35,148 @@
>  
>  lxc_log_define(lxc_snapshot_ui, lxc);
>  
> -static char *newname;
> -static char *snapshot;
> +static int my_parser(struct lxc_arguments *args, int c, char *arg);
>  
> -#define DO_SNAP 0
> -#define DO_LIST 1
> -#define DO_RESTORE 2
> -#define DO_DESTROY 3
> -static int action;
> -static int print_comments;
> -static char *commentfile;
> +static const struct option my_longopts[] = {
> +	{"list", no_argument, 0, 'L'},
> +	{"restore", required_argument, 0, 'r'},
> +	{"destroy", required_argument, 0, 'd'},
> +	{"comment", required_argument, 0, 'c'},
> +	{"showcomments", no_argument, 0, 'C'},
> +	LXC_COMMON_OPTIONS};
> +
> +static struct lxc_arguments my_args = {
> +	.progname = "lxc-snapshot",
> +	.help = "\
> +--name=NAME [-P lxcpath] [-L [-C]] [-c commentfile] [-r snapname [newname]]\n\
> +\n\
> +lxc-snapshot snapshots a container\n\
> +\n\
> +Options :\n\
> +  -n, --name=NAME     NAME for name of the container\n\
> +  -L, --list          list all snapshots\n\
> +  -C, --showcomments  show snapshot comments\n\
> +  -c, --comment=FILE  add FILE as a comment\n\
> +  -r, --restore=NAME  restore snapshot NAME, e.g. 'snap0'\n\
> +  -d, --destroy=NAME  destroy snapshot NAME, e.g. 'snap0'\n\
> +                      use ALL to destroy all snapshots\n",
> +	.options = my_longopts,
> +	.parser = my_parser,
> +	.checker = NULL,
> +	.task = SNAP,
> +};
> +
> +static int do_destroy_snapshots(struct lxc_container *c, char *snapname);
> +static int do_list_snapshots(struct lxc_container *c, int print_comments);
> +static int do_restore_snapshots(struct lxc_container *c, char *snapname,
> +				char *newname);
> +static int do_snapshot(struct lxc_container *c, char *commentfile);
> +static int do_snapshot_task(struct lxc_container *c, enum task task);
> +static void print_file(char *path);
> +
> +int main(int argc, char *argv[])
> +{
> +	struct lxc_container *c;
> +	int ret;
> +
> +	if (lxc_arguments_parse(&my_args, argc, argv))
> +		exit(EXIT_FAILURE);
> +
> +	if (!my_args.log_file)
> +		my_args.log_file = "none";
> +
> +	if (lxc_log_init(my_args.name, my_args.log_file, my_args.log_priority,
> +			 my_args.progname, my_args.quiet, my_args.lxcpath[0]))
> +		exit(EXIT_FAILURE);
> +	lxc_log_options_no_override();
> +
> +	if (geteuid()) {
> +		if (access(my_args.lxcpath[0], O_RDWR) < 0) {
> +			fprintf(stderr, "You lack access to %s\n",
> +				my_args.lxcpath[0]);
> +			exit(EXIT_FAILURE);
> +		}
> +	}
> +
> +	if (my_args.argc > 1) {
> +		ERROR("Too many arguments");
> +		exit(EXIT_FAILURE);
> +	}
> +
> +	if (my_args.argc == 1)
> +		my_args.newname = my_args.argv[0];
> +
> +	c = lxc_container_new(my_args.name, my_args.lxcpath[0]);
> +	if (!c) {
> +		fprintf(stderr, "System error loading container\n");
> +		exit(EXIT_FAILURE);
> +	}
> +
> +	if (!c->may_control(c)) {
> +		fprintf(stderr, "Insufficent privileges to control %s\n",
> +			my_args.name);
> +		lxc_container_put(c);
> +		exit(EXIT_FAILURE);
> +	}
> +
> +	ret = do_snapshot_task(c, my_args.task);
> +
> +	lxc_container_put(c);
> +
> +	if (ret == 0)
> +		exit(EXIT_SUCCESS);
> +	exit(EXIT_FAILURE);
> +}
> +
> +static int do_snapshot_task(struct lxc_container *c, enum task task)
> +{
> +	int ret = 0;
> +
> +	switch (task) {
> +	case DESTROY:
> +		ret = do_destroy_snapshots(c, my_args.snapname);
> +		break;
> +	case LIST:
> +		ret = do_list_snapshots(c, my_args.print_comments);
> +		break;
> +	case RESTORE:
> +		ret =
> +		    do_restore_snapshots(c, my_args.snapname, my_args.newname);
> +		break;
> +	case SNAP:
> +		ret = do_snapshot(c, my_args.commentfile);
> +		break;
> +	}
> +
> +	return ret;
> +}
>  
> -static int do_snapshot(struct lxc_container *c)
> +static int my_parser(struct lxc_arguments *args, int c, char *arg)
> +{
> +	switch (c) {
> +	case 'L':
> +		args->task = LIST;
> +		break;
> +	case 'r':
> +		args->task = RESTORE;
> +		args->snapname = arg;
> +		break;
> +	case 'd':
> +		args->task = DESTROY;
> +		args->snapname = arg;
> +		break;
> +	case 'c':
> +		args->commentfile = arg;
> +		break;
> +	case 'C':
> +		args->print_comments = 1;
> +		break;
> +	}
> +
> +	return 0;
> +}
> +
> +static int do_snapshot(struct lxc_container *c, char *commentfile)
>  {
>  	int ret;
>  
> @@ -57,26 +187,11 @@ static int do_snapshot(struct lxc_container *c)
>  	}
>  
>  	INFO("Created snapshot snap%d", ret);
> -	return 0;
> -}
>  
> -static void print_file(char *path)
> -{
> -	if (!path)
> -		return;
> -	FILE *f = fopen(path, "r");
> -	char *line = NULL;
> -	size_t sz = 0;
> -	if (!f)
> -		return;
> -	while (getline(&line, &sz, f) != -1) {
> -		printf("%s", line);
> -	}
> -	free(line);
> -	fclose(f);
> +	return 0;
>  }
>  
> -static int do_list_snapshots(struct lxc_container *c)
> +static int do_list_snapshots(struct lxc_container *c, int print_comments)
>  {
>  	struct lxc_snapshot *s;
>  	int i, n;
> @@ -90,148 +205,64 @@ static int do_list_snapshots(struct lxc_container *c)
>  		printf("No snapshots\n");
>  		return 0;
>  	}
> -	for (i=0; i<n; i++) {
> +
> +	for (i = 0; i < n; i++) {
>  		printf("%s (%s) %s\n", s[i].name, s[i].lxcpath, s[i].timestamp);
>  		if (print_comments)
>  			print_file(s[i].comment_pathname);
>  		s[i].free(&s[i]);
>  	}
> +
>  	free(s);
> +
>  	return 0;
>  }
>  
> -static int do_restore_snapshots(struct lxc_container *c)
> +static int do_restore_snapshots(struct lxc_container *c, char *snapname,
> +				char *newname)
>  {
> -	if (c->snapshot_restore(c, snapshot, newname))
> +	if (c->snapshot_restore(c, snapname, newname))
>  		return 0;
>  
> -	ERROR("Error restoring snapshot %s", snapshot);
> +	ERROR("Error restoring snapshot %s", snapname);
> +
>  	return -1;
>  }
>  
> -static int do_destroy_snapshots(struct lxc_container *c)
> +static int do_destroy_snapshots(struct lxc_container *c, char *snapname)
>  {
> -	bool bret;
> -	if (strcmp(snapshot, "ALL") == 0)
> -		bret = c->snapshot_destroy_all(c);
> +	bool ret;
> +
> +	if (strcmp(snapname, "ALL") == 0)
> +		ret = c->snapshot_destroy_all(c);
>  	else
> -		bret = c->snapshot_destroy(c, snapshot);
> +		ret = c->snapshot_destroy(c, snapname);
>  
> -	if (bret)
> +	if (ret)
>  		return 0;
>  
> -	ERROR("Error destroying snapshot %s", snapshot);
> -	return -1;
> -}
> +	ERROR("Error destroying snapshot %s", snapname);
>  
> -static int my_parser(struct lxc_arguments* args, int c, char* arg)
> -{
> -	switch (c) {
> -	case 'L': action = DO_LIST; break;
> -	case 'r': snapshot = arg; action = DO_RESTORE; break;
> -	case 'd': snapshot = arg; action = DO_DESTROY; break;
> -	case 'c': commentfile = arg; break;
> -	case 'C': print_comments = true; break;
> -	}
> -	return 0;
> +	return -1;
>  }
>  
> -static const struct option my_longopts[] = {
> -	{"list", no_argument, 0, 'L'},
> -	{"restore", required_argument, 0, 'r'},
> -	{"destroy", required_argument, 0, 'd'},
> -	{"comment", required_argument, 0, 'c'},
> -	{"showcomments", no_argument, 0, 'C'},
> -	LXC_COMMON_OPTIONS
> -};
> -
> -
> -static struct lxc_arguments my_args = {
> -	.progname = "lxc-snapshot",
> -	.help     = "\
> ---name=NAME [-P lxcpath] [-L [-C]] [-c commentfile] [-r snapname [newname]]\n\
> -\n\
> -lxc-snapshot snapshots a container\n\
> -\n\
> -Options :\n\
> -  -n, --name=NAME   NAME for name of the container\n\
> -  -L, --list          list snapshots\n\
> -  -C, --showcomments  show snapshot comments in list\n\
> -  -c, --comment=file  add file as a comment\n\
> -  -r, --restore=name  restore snapshot name, i.e. 'snap0'\n\
> -  -d, --destroy=name  destroy snapshot name, i.e. 'snap0'\n\
> -                      use ALL to destroy all snapshots\n",
> -	.options  = my_longopts,
> -	.parser   = my_parser,
> -	.checker  = NULL,
> -};
> -
> -/*
> - * lxc-snapshot -P lxcpath -n container
> - * lxc-snapshot -P lxcpath -n container -l
> - * lxc-snapshot -P lxcpath -n container -r snap3 recovered_1
> - */
> -
> -int main(int argc, char *argv[])
> +static void print_file(char *path)
>  {
> -	struct lxc_container *c;
> -	int ret = 0;
> -
> -	if (lxc_arguments_parse(&my_args, argc, argv))
> -		exit(1);
> -
> -	if (!my_args.log_file)
> -		my_args.log_file = "none";
> -
> -	if (my_args.argc > 1) {
> -		ERROR("Too many arguments");
> -		exit(1);
> -	}
> -	if (my_args.argc == 1)
> -		newname = my_args.argv[0];
> -
> -	if (lxc_log_init(my_args.name, my_args.log_file, my_args.log_priority,
> -			 my_args.progname, my_args.quiet, my_args.lxcpath[0]))
> -		exit(1);
> -	lxc_log_options_no_override();
> -
> -	if (geteuid()) {
> -		if (access(my_args.lxcpath[0], O_RDWR) < 0) {
> -			fprintf(stderr, "You lack access to %s\n", my_args.lxcpath[0]);
> -			exit(1);
> -		}
> -	}
> +	if (!path)
> +		return;
>  
> -	c = lxc_container_new(my_args.name, my_args.lxcpath[0]);
> -	if (!c) {
> -		fprintf(stderr, "System error loading container\n");
> -		exit(1);
> -	}
> +	FILE *f = fopen(path, "r");
> +	char *line = NULL;
> +	size_t sz = 0;
>  
> -	if (!c->may_control(c)) {
> -		fprintf(stderr, "Insufficent privileges to control %s\n", my_args.name);
> -		lxc_container_put(c);
> -		exit(1);
> -	}
> +	if (!f)
> +		return;
>  
> -	switch(action) {
> -	case DO_SNAP:
> -		ret = do_snapshot(c);
> -		break;
> -	case DO_LIST:
> -		ret = do_list_snapshots(c);
> -		break;
> -	case DO_RESTORE:
> -		ret = do_restore_snapshots(c);
> -		break;
> -	case DO_DESTROY:
> -		ret = do_destroy_snapshots(c);
> -		break;
> +	while (getline(&line, &sz, f) != -1) {
> +		printf("%s", line);
>  	}
>  
> -	lxc_container_put(c);
> -
> -	if (ret == 0)
> -		exit(EXIT_SUCCESS);
> -	exit(EXIT_FAILURE);
> +	free(line);
> +	fclose(f);
>  }
> +
> -- 
> 2.5.0
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20150807/843bd462/attachment.sig>


More information about the lxc-devel mailing list