[lxc-devel] [PATCH] [RFC] snapshots: move snapshot directory (v3)

Serge Hallyn serge.hallyn at ubuntu.com
Fri May 23 03:46:49 UTC 2014


Originally we kept snapshots under /var/lib/lxcsnaps.  If a
separate btrfs is mounted at /var/lib/lxc, then we can't
make btrfs snapshots under /var/lib/lxcsnaps.

This patch moves the default directory to /var/lib/lxc/c/snaps.
If /var/lib/lxcsnaps already exists, then use that.

If we are deleting a container which has snapshots, we currently
will delete the container itself and its rootfs, but not its
snapshots.  This could be confusing for the user, and there is
no option to c->destroy() to ask for different behavior.  So
currently a user would have to delete all snapshots first, then
delete the container.  Ideas for better handling this would be
welcome, but we don't want to change the current api, so while
adding a new c->destroy_full() would be ok, adding a flags
argument to c->destroy(c, flags) is not.

lxclock: use ".$lxcname" for container lock files

that way we can use /run/lock/lxc/$lxcpath/$lxcname/snaps as a
directory when locking snapshots without having to worry about
/run/lock//lxc/$lxcpath/$lxcname being a file.

snapshot: update paths in testcase

TODO: either change how we handle snapshots when deleting a
container, or better clean up in src/tests/snapshot.c, so that
lxc-test-snapshot can succeed twice in a row.  (Currently it
cannot, bc snapxxx1 has two snapshots and snapxxx3 has one.

Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
---
 src/lxc/bdev.c         |  6 +++---
 src/lxc/lxccontainer.c | 52 +++++++++++++++++++++++++++++++++++++-------------
 src/lxc/lxccontainer.h |  4 ++--
 src/lxc/lxclock.c      | 10 +++++-----
 src/lxc/utils.c        | 20 ++++++++++++-------
 src/lxc/utils.h        |  2 +-
 src/tests/snapshot.c   |  6 +++---
 7 files changed, 66 insertions(+), 34 deletions(-)

diff --git a/src/lxc/bdev.c b/src/lxc/bdev.c
index 751fa9f..efdbecc 100644
--- a/src/lxc/bdev.c
+++ b/src/lxc/bdev.c
@@ -482,7 +482,7 @@ static int dir_clonepaths(struct bdev *orig, struct bdev *new, const char *oldna
 
 static int dir_destroy(struct bdev *orig)
 {
-	if (lxc_rmdir_onedev(orig->src) < 0)
+	if (lxc_rmdir_onedev(orig->src, NULL) < 0)
 		return -1;
 	return 0;
 }
@@ -2073,7 +2073,7 @@ static int overlayfs_destroy(struct bdev *orig)
 	if (!upper)
 		return -22;
 	upper++;
-	return lxc_rmdir_onedev(upper);
+	return lxc_rmdir_onedev(upper, NULL);
 }
 
 /*
@@ -2350,7 +2350,7 @@ static int aufs_destroy(struct bdev *orig)
 	if (!upper)
 		return -22;
 	upper++;
-	return lxc_rmdir_onedev(upper);
+	return lxc_rmdir_onedev(upper, NULL);
 }
 
 /*
diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
index fdac433..e796240 100644
--- a/src/lxc/lxccontainer.c
+++ b/src/lxc/lxccontainer.c
@@ -1977,7 +1977,7 @@ out:
 static int lxc_rmdir_onedev_wrapper(void *data)
 {
 	char *arg = (char *) data;
-	return lxc_rmdir_onedev(arg);
+	return lxc_rmdir_onedev(arg, "snaps");
 }
 
 static int do_bdev_destroy(struct lxc_conf *conf)
@@ -2054,7 +2054,7 @@ static bool lxcapi_destroy(struct lxc_container *c)
 	if (am_unpriv())
 		ret = userns_exec_1(c->lxc_conf, lxc_rmdir_onedev_wrapper, path);
 	else
-		ret = lxc_rmdir_onedev(path);
+		ret = lxc_rmdir_onedev(path, "snaps");
 	if (ret < 0) {
 		ERROR("Error destroying container directory for %s", c->name);
 		goto out;
@@ -2846,16 +2846,42 @@ static int get_next_index(const char *lxcpath, char *cname)
 	}
 }
 
+static bool get_snappath_dir(struct lxc_container *c, char *snappath)
+{
+	int ret;
+	/*
+	 * If the old style snapshot path exists, use it
+	 * /var/lib/lxc -> /var/lib/lxcsnaps
+	 */
+	ret = snprintf(snappath, MAXPATHLEN, "%ssnaps", c->config_path);
+	if (ret < 0 || ret >= MAXPATHLEN)
+		return false;
+	if (dir_exists(snappath)) {
+		ret = snprintf(snappath, MAXPATHLEN, "%ssnaps/%s", c->config_path, c->name);
+		if (ret < 0 || ret >= MAXPATHLEN)
+			return false;
+		return true;
+	}
+
+	/*
+	 * Use the new style path
+	 * /var/lib/lxc -> /var/lib/lxc + c->name + /snaps + \0
+	 */
+	ret = snprintf(snappath, MAXPATHLEN, "%s/%s/snaps", c->config_path, c->name);
+	if (ret < 0 || ret >= MAXPATHLEN)
+		return false;
+	return true;
+}
+
 static int lxcapi_snapshot(struct lxc_container *c, const char *commentfile)
 {
 	int i, flags, ret;
 	struct lxc_container *c2;
 	char snappath[MAXPATHLEN], newname[20];
 
-	// /var/lib/lxc -> /var/lib/lxcsnaps \0
-	ret = snprintf(snappath, MAXPATHLEN, "%ssnaps/%s", c->config_path, c->name);
-	if (ret < 0 || ret >= MAXPATHLEN)
+	if (!get_snappath_dir(c, snappath)) {
 		return -1;
+	}
 	i = get_next_index(snappath, c->name);
 
 	if (mkdir_p(snappath, 0755) < 0) {
@@ -2989,7 +3015,7 @@ static char *get_timestamp(char* snappath, char *name)
 static int lxcapi_snapshot_list(struct lxc_container *c, struct lxc_snapshot **ret_snaps)
 {
 	char snappath[MAXPATHLEN], path2[MAXPATHLEN];
-	int dirlen, count = 0, ret;
+	int count = 0, ret;
 	struct dirent dirent, *direntp;
 	struct lxc_snapshot *snaps =NULL, *nsnaps;
 	DIR *dir;
@@ -2997,9 +3023,7 @@ static int lxcapi_snapshot_list(struct lxc_container *c, struct lxc_snapshot **r
 	if (!c || !lxcapi_is_defined(c))
 		return -1;
 
-	// snappath is ${lxcpath}snaps/${lxcname}/
-	dirlen = snprintf(snappath, MAXPATHLEN, "%ssnaps/%s", c->config_path, c->name);
-	if (dirlen < 0 || dirlen >= MAXPATHLEN) {
+	if (!get_snappath_dir(c, snappath)) {
 		ERROR("path name too long");
 		return -1;
 	}
@@ -3067,7 +3091,7 @@ out_free:
 static bool lxcapi_snapshot_restore(struct lxc_container *c, const char *snapname, const char *newname)
 {
 	char clonelxcpath[MAXPATHLEN];
-	int flags = 0,ret;
+	int flags = 0;
 	struct lxc_container *snap, *rest;
 	struct bdev *bdev;
 	bool b = false;
@@ -3090,8 +3114,7 @@ static bool lxcapi_snapshot_restore(struct lxc_container *c, const char *snapnam
 			return false;
 		}
 	}
-	ret = snprintf(clonelxcpath, MAXPATHLEN, "%ssnaps/%s", c->config_path, c->name);
-	if (ret < 0 || ret >= MAXPATHLEN) {
+	if (!get_snappath_dir(c, clonelxcpath)) {
 		bdev_put(bdev);
 		return false;
 	}
@@ -3127,7 +3150,7 @@ static bool lxcapi_snapshot_destroy(struct lxc_container *c, const char *snapnam
 	if (!c || !c->name || !c->config_path)
 		return false;
 
-	ret = snprintf(clonelxcpath, MAXPATHLEN, "%ssnaps/%s", c->config_path, c->name);
+	ret = snprintf(clonelxcpath, MAXPATHLEN, "%s/%s/snaps", c->config_path, c->name);
 	if (ret < 0 || ret >= MAXPATHLEN)
 		goto err;
 
@@ -3307,6 +3330,9 @@ struct lxc_container *lxc_container_new(const char *name, const char *configpath
 {
 	struct lxc_container *c;
 
+	if (!name)
+		return NULL;
+
 	c = malloc(sizeof(*c));
 	if (!c) {
 		fprintf(stderr, "failed to malloc lxc_container\n");
diff --git a/src/lxc/lxccontainer.h b/src/lxc/lxccontainer.h
index 1d0628a..2418cb8 100644
--- a/src/lxc/lxccontainer.h
+++ b/src/lxc/lxccontainer.h
@@ -649,7 +649,7 @@ struct lxc_container {
 	 * \brief Create a container snapshot.
 	 *
 	 * Assuming default paths, snapshots will be created as
-	 * \c /var/lib/lxcsnaps/\<c\>/snap\<n\>
+	 * \c /var/lib/lxc/\<c\>/snaps/snap\<n\>
 	 * where \c \<c\> represents the container name and \c \<n\>
 	 * represents the zero-based snapshot number.
 	 *
@@ -691,7 +691,7 @@ struct lxc_container {
 	 *  fail if the  snapshot is overlay-based, since the snapshots
 	 *  will pin the original container.
 	 * \note As an example, if the container exists as \c /var/lib/lxc/c1, snapname might be \c 'snap0'
-	 *  (representing \c /var/lib/lxcsnaps/c1/snap0). If \p newname is \p c2,
+	 *  (representing \c /var/lib/lxc/c1/snaps/snap0). If \p newname is \p c2,
 	 *  then \c snap0 will be copied to \c /var/lib/lxc/c2.
 	 */
 	bool (*snapshot_restore)(struct lxc_container *c, const char *snapname, const char *newname);
diff --git a/src/lxc/lxclock.c b/src/lxc/lxclock.c
index 1d62729..9d1c6f4 100644
--- a/src/lxc/lxclock.c
+++ b/src/lxc/lxclock.c
@@ -108,8 +108,8 @@ static char *lxclock_name(const char *p, const char *n)
 	 * $XDG_RUNTIME_DIR + "/lock/lxc/$lxcpath/$lxcname + '\0' if non-root
 	 */
 
-	/* length of "/lock/lxc/" + $lxcpath + "/" + $lxcname + '\0' */
-	len = strlen("/lock/lxc/") + strlen(n) + strlen(p) + 2;
+	/* length of "/lock/lxc/" + $lxcpath + "/" + "." + $lxcname + '\0' */
+	len = strlen("/lock/lxc/") + strlen(n) + strlen(p) + 3;
 	rundir = get_rundir();
 	if (!rundir)
 		return NULL;
@@ -129,7 +129,7 @@ static char *lxclock_name(const char *p, const char *n)
 	ret = mkdir_p(dest, 0755);
 	if (ret < 0) {
 		/* fall back to "/tmp/" $(id -u) "/lxc/" $lxcpath / $lxcname + '\0' */
-		int l2 = 33 + strlen(n) + strlen(p);
+		int l2 = 34 + strlen(n) + strlen(p);
 		if (l2 > len) {
 			char *d;
 			d = realloc(dest, l2);
@@ -147,9 +147,9 @@ static char *lxclock_name(const char *p, const char *n)
 			free(rundir);
 			return NULL;
 		}
-		ret = snprintf(dest, len, "/tmp/%d/lxc/%s/%s", geteuid(), p, n);
+		ret = snprintf(dest, len, "/tmp/%d/lxc/%s/.%s", geteuid(), p, n);
 	} else
-		ret = snprintf(dest, len, "%s/lock/lxc/%s/%s", rundir, p, n);
+		ret = snprintf(dest, len, "%s/lock/lxc/%s/.%s", rundir, p, n);
 
 	free(rundir);
 
diff --git a/src/lxc/utils.c b/src/lxc/utils.c
index b076ce7..b740756 100644
--- a/src/lxc/utils.c
+++ b/src/lxc/utils.c
@@ -46,7 +46,8 @@
 
 lxc_log_define(lxc_utils, lxc);
 
-static int _recursive_rmdir_onedev(char *dirname, dev_t pdev)
+static int _recursive_rmdir_onedev(char *dirname, dev_t pdev,
+				   const char *exclude, int level)
 {
 	struct dirent dirent, *direntp;
 	DIR *dir;
@@ -70,6 +71,9 @@ static int _recursive_rmdir_onedev(char *dirname, dev_t pdev)
 		    !strcmp(direntp->d_name, ".."))
 			continue;
 
+		if (!level && exclude && !strcmp(direntp->d_name, exclude))
+			continue;
+
 		rc = snprintf(pathname, MAXPATHLEN, "%s/%s", dirname, direntp->d_name);
 		if (rc < 0 || rc >= MAXPATHLEN) {
 			ERROR("pathname too long");
@@ -85,7 +89,7 @@ static int _recursive_rmdir_onedev(char *dirname, dev_t pdev)
 		if (mystat.st_dev != pdev)
 			continue;
 		if (S_ISDIR(mystat.st_mode)) {
-			if (_recursive_rmdir_onedev(pathname, pdev) < 0)
+			if (_recursive_rmdir_onedev(pathname, pdev, exclude, level+1) < 0)
 				failed=1;
 		} else {
 			if (unlink(pathname) < 0) {
@@ -95,9 +99,11 @@ static int _recursive_rmdir_onedev(char *dirname, dev_t pdev)
 		}
 	}
 
-	if (rmdir(dirname) < 0) {
-		ERROR("%s: failed to delete %s", __func__, dirname);
-		failed=1;
+	if (!exclude || level != 0) {
+		if (rmdir(dirname) < 0) {
+			ERROR("%s: failed to delete %s", __func__, dirname);
+			failed=1;
+		}
 	}
 
 	ret = closedir(dir);
@@ -110,7 +116,7 @@ static int _recursive_rmdir_onedev(char *dirname, dev_t pdev)
 }
 
 /* returns 0 on success, -1 if there were any failures */
-extern int lxc_rmdir_onedev(char *path)
+extern int lxc_rmdir_onedev(char *path, const char *exclude)
 {
 	struct stat mystat;
 
@@ -119,7 +125,7 @@ extern int lxc_rmdir_onedev(char *path)
 		return -1;
 	}
 
-	return _recursive_rmdir_onedev(path, mystat.st_dev);
+	return _recursive_rmdir_onedev(path, mystat.st_dev, exclude, 0);
 }
 
 static int mount_fs(const char *source, const char *target, const char *type)
diff --git a/src/lxc/utils.h b/src/lxc/utils.h
index 691e56c..8516fcf 100644
--- a/src/lxc/utils.h
+++ b/src/lxc/utils.h
@@ -34,7 +34,7 @@
 #include "config.h"
 
 /* returns 1 on success, 0 if there were any failures */
-extern int lxc_rmdir_onedev(char *path);
+extern int lxc_rmdir_onedev(char *path, const char *exclude);
 extern void lxc_setup_fs(void);
 extern int get_u16(unsigned short *val, const char *arg, int base);
 extern int mkdir_p(const char *dir, mode_t mode);
diff --git a/src/tests/snapshot.c b/src/tests/snapshot.c
index 09cee1a..179b6a5 100644
--- a/src/tests/snapshot.c
+++ b/src/tests/snapshot.c
@@ -39,7 +39,7 @@ static void try_to_remove(void)
 			c->destroy(c);
 		lxc_container_put(c);
 	}
-	snprintf(snappath, 1024, "%ssnaps/%s", lxc_get_global_config_item("lxc.lxcpath"), MYNAME);
+	snprintf(snappath, 1024, "%s/%s/snaps", lxc_get_global_config_item("lxc.lxcpath"), MYNAME);
 	c = lxc_container_new("snap0", snappath);
 	if (c) {
 		if (c->is_defined(c))
@@ -95,11 +95,11 @@ int main(int argc, char *argv[])
 		goto err;
 	}
 
-	// rootfs should be ${lxcpath}snaps/${lxcname}/snap0/rootfs
+	// rootfs should be ${lxcpath}${lxcname}/snaps/snap0/rootfs
 	struct stat sb;
 	int ret;
 	char path[1024];
-	snprintf(path, 1024, "%ssnaps/%s/snap0/rootfs", lxc_get_global_config_item("lxc.lxcpath"), MYNAME);
+	snprintf(path, 1024, "%s/%s/snaps/snap0/rootfs", lxc_get_global_config_item("lxc.lxcpath"), MYNAME);
 	ret = stat(path, &sb);
 	if (ret != 0) {
 		fprintf(stderr, "%s: %d: snapshot was not actually created\n", __FILE__, __LINE__);
-- 
2.0.0.rc0



More information about the lxc-devel mailing list