[lxc-devel] [PATCH RFC] snapshot: enforce keeping same backing store type

[Serge Hallyn]

(only compile-tested so far, see below why)

Stéphane noticed that lxc-snapshot of a dir-backed container
created an overlayfs container.  The expectation is that the
user can continue to modify the original container and later make
a new snapshot, but this doesn't work with the existing behavior -
the overlayfs clone will end up with the modified contents.

So add a 'LXC_CLONE_KEEPBDEVTYPE' flag, which c->snapshot()
passes to c->clone().  Note - this will make lxc-snapshot of a
directory backed container fail.  I agree with Stéphane that it
would be nicer to have it automatically do a copy snapshot,
but doing so would require more shenanigans - i.e. another
clone flag LXC_CLONE_MAYBSNAPSHOT - and is also not ideal imo.

So just sending this out now for feedback and ideas - what do
you think is the best behavior?

Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
---
 src/lxc/bdev.c         | 6 ++++--
 src/lxc/bdev.h         | 2 +-
 src/lxc/lxccontainer.c | 5 +++--
 src/lxc/lxccontainer.h | 3 ++-
 4 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/src/lxc/bdev.c b/src/lxc/bdev.c
index b737eff..a975545 100644
--- a/src/lxc/bdev.c
+++ b/src/lxc/bdev.c
@@ -2011,11 +2011,13 @@ struct bdev *bdev_init(const char *src, const char *dst, const char *data)
  */
 struct bdev *bdev_copy(const char *src, const char *oldname, const char *cname,
 			const char *oldpath, const char *lxcpath, const char *bdevtype,
-			int snap, const char *bdevdata, unsigned long newsize,
+			int flags, const char *bdevdata, unsigned long newsize,
 			int *needs_rdep)
 {
 	struct bdev *orig, *new;
 	pid_t pid;
+	bool snap = flags & LXC_CLONE_SNAPSHOT;
+	bool keepbdevtype = flags & LXC_CLONE_KEEPBDEVTYPE;
 
 	/* if the container name doesn't show up in the rootfs path, then
 	 * we don't know how to come up with a new name
@@ -2051,7 +2053,7 @@ struct bdev *bdev_copy(const char *src, const char *oldname, const char *cname,
 	/*
 	 * If newtype is NULL and snapshot is set, then use overlayfs
 	 */
-	if (!bdevtype && snap && strcmp(orig->type , "dir") == 0)
+	if (!bdevtype && !keepbdevtype && snap && strcmp(orig->type , "dir") == 0)
 		bdevtype = "overlayfs";
 
 	*needs_rdep = 0;
diff --git a/src/lxc/bdev.h b/src/lxc/bdev.h
index 8c17117..bdf7751 100644
--- a/src/lxc/bdev.h
+++ b/src/lxc/bdev.h
@@ -99,7 +99,7 @@ struct bdev *bdev_init(const char *src, const char *dst, const char *data);
 
 struct bdev *bdev_copy(const char *src, const char *oldname, const char *cname,
 			const char *oldpath, const char *lxcpath, const char *bdevtype,
-			int snap, const char *bdevdata, unsigned long newsize,
+			int flags, const char *bdevdata, unsigned long newsize,
 			int *needs_rdep);
 struct bdev *bdev_create(const char *dest, const char *type,
 			const char *cname, struct bdev_specs *specs);
diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
index e1d004f..a51d611 100644
--- a/src/lxc/lxccontainer.c
+++ b/src/lxc/lxccontainer.c
@@ -2280,7 +2280,7 @@ static int copy_storage(struct lxc_container *c0, struct lxc_container *c,
 	int need_rdep;
 
 	bdev = bdev_copy(c0->lxc_conf->rootfs.path, c0->name, c->name,
-			c0->config_path, c->config_path, newtype, !!(flags & LXC_CLONE_SNAPSHOT),
+			c0->config_path, c->config_path, newtype, flags,
 			bdevdata, newsize, &need_rdep);
 	if (!bdev) {
 		ERROR("Error copying storage");
@@ -2622,7 +2622,8 @@ static int lxcapi_snapshot(struct lxc_container *c, const char *commentfile)
 	if (ret < 0 || ret >= 20)
 		return -1;
 
-	flags = LXC_CLONE_SNAPSHOT | LXC_CLONE_KEEPMACADDR | LXC_CLONE_KEEPNAME;
+	flags = LXC_CLONE_SNAPSHOT | LXC_CLONE_KEEPMACADDR | LXC_CLONE_KEEPNAME |
+		LXC_CLONE_KEEPBDEVTYPE;
 	c2 = c->clone(c, newname, snappath, flags, NULL, NULL, 0, NULL);
 	if (!c2) {
 		ERROR("clone of %s:%s failed\n", c->config_path, c->name);
diff --git a/src/lxc/lxccontainer.h b/src/lxc/lxccontainer.h
index 797ad91..2cb0217 100644
--- a/src/lxc/lxccontainer.h
+++ b/src/lxc/lxccontainer.h
@@ -31,7 +31,8 @@
 #define LXC_CLONE_KEEPNAME        (1 << 0) /*!< Do not edit the rootfs to change the hostname */
 #define LXC_CLONE_KEEPMACADDR     (1 << 1) /*!< Do not change the MAC address on network interfaces */
 #define LXC_CLONE_SNAPSHOT        (1 << 2) /*!< Snapshot the original filesystem(s) */
-#define LXC_CLONE_MAXFLAGS        (1 << 3) /*!< Number of \c LXC_CLONE_* flags */
+#define LXC_CLONE_KEEPBDEVTYPE    (1 << 3) /*!< Use the same bdev type */
+#define LXC_CLONE_MAXFLAGS        (1 << 4) /*!< Number of \c LXC_CLONE_* flags */
 #define LXC_CREATE_QUIET          (1 << 0) /*!< Redirect \c stdin to \c /dev/zero and \c stdout and \c stderr to \c /dev/null */
 #define LXC_CREATE_MAXFLAGS       (1 << 1) /*!< Number of \c LXC_CREATE* flags */
 
-- 
1.8.5.2