storage: rework lvm backend

Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>

src/lxc/bdev/bdev.c

@@ -138,8 +138,8 @@ static const struct bdev_ops lvm_ops = {
     .clone_paths = &lvm_clonepaths,
     .destroy = &lvm_destroy,
     .create = &lvm_create,
-    .create_clone = NULL,
-    .create_snapshot = NULL,
+    .create_clone = &lvm_create_clone,
+    .create_snapshot = &lvm_create_snapshot,
     .can_snapshot = true,
     .can_backup = false,
 };
@@ -444,6 +444,19 @@ struct bdev *bdev_copy(struct lxc_container *c0, const char *cname,
 		return new;
 	}
 
+	if (!strcmp(orig->type, "lvm") && !strcmp(new->type, "lvm")) {
+		bool bret = false;
+		if (snap)
+			bret = new->ops->create_snapshot(c0->lxc_conf, orig,
+							 new, newsize);
+		else
+			bret = new->ops->create_clone(c0->lxc_conf, orig, new,
+						      newsize);
+		if (!bret)
+			return NULL;
+		return new;
+	}
+
 	if (strcmp(bdevtype, "btrfs")) {
 		if (!strcmp(new->type, "overlay") ||
 		    !strcmp(new->type, "overlayfs"))

src/lxc/bdev/lxclvm.c

@@ -22,8 +22,8 @@
  */
 
 #define _GNU_SOURCE
-#define __STDC_FORMAT_MACROS /* Required for PRIu64 to work. */
-#include <inttypes.h> /* Required for PRIu64 to work. */
+#define __STDC_FORMAT_MACROS
+#include <inttypes.h>
 #include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -36,118 +36,199 @@
 #include "config.h"
 #include "log.h"
 #include "lxclvm.h"
+#include "lxcrsync.h"
 #include "storage_utils.h"
 #include "utils.h"
 
-/* major()/minor() */
 #ifdef MAJOR_IN_MKDEV
-#    include <sys/mkdev.h>
+#include <sys/mkdev.h>
 #endif
 
 lxc_log_define(lxclvm, lxc);
 
-extern char *dir_new_path(char *src, const char *oldname, const char *name,
-			  const char *oldpath, const char *lxcpath);
+struct lvcreate_args {
+	const char *size;
+	const char *vg;
+	const char *lv;
+	const char *thinpool;
 
-/* Path must be '/dev/$vg/$lv', $vg must be an existing VG, and $lv must not yet
- * exist.  This function will attempt to create /dev/$vg/$lv of size $size. If
+	/* snapshot specific arguments */
+	const char *source_lv;
+};
+
+static int lvm_destroy_exec_wrapper(void *data)
+{
+	struct lvcreate_args *args = data;
+
+	(void)setenv("LVM_SUPPRESS_FD_WARNINGS", "1", 1);
+	execlp("lvremove", "lvremove", "-f", args->lv, (char *)NULL);
+
+	return -1;
+}
+
+static int lvm_create_exec_wrapper(void *data)
+{
+	struct lvcreate_args *args = data;
+
+	(void)setenv("LVM_SUPPRESS_FD_WARNINGS", "1", 1);
+	if (args->thinpool)
+		execlp("lvcreate", "lvcreate", "--thinpool", args->thinpool,
+		       "-V", args->size, args->vg, "-n", args->lv,
+		       (char *)NULL);
+	else
+		execlp("lvcreate", "lvcreate", "-L", args->size, args->vg, "-n",
+		       args->lv, (char *)NULL);
+
+	return -1;
+}
+
+static int lvm_snapshot_exec_wrapper(void *data)
+{
+	struct lvcreate_args *args = data;
+
+	(void)setenv("LVM_SUPPRESS_FD_WARNINGS", "1", 1);
+	if (args->thinpool)
+		execlp("lvcreate", "lvcreate", "-s", "-n", args->lv,
+		       args->source_lv, (char *)NULL);
+	else
+		execlp("lvcreate", "lvcreate", "-s", "-L", args->size, "-n",
+		       args->lv, args->source_lv, (char *)NULL);
+
+	return -1;
+}
+
+/* The path must be "/dev/<vg>/<lv>". The volume group <vg> must be an existing
+ * volume group, and the logical volume <lv> must not yet exist.
+ * This function will attempt to create "/dev/<vg>/<lv> of size <size>. If
  * thinpool is specified, we'll check for it's existence and if it's a valid
- * thin pool, and if so, we'll create the requested lv from that thin pool.
+ * thin pool, and if so, we'll create the requested logical volume from that
+ * thin pool.
  */
 static int do_lvm_create(const char *path, uint64_t size, const char *thinpool)
 {
-	int ret, pid, len;
-	char sz[24], *pathdup, *vg, *lv, *tp = NULL;
+	int len, ret;
+	char *pathdup, *vg, *lv;
+	char cmd_output[MAXPATHLEN];
+	char sz[24];
+	char *tp = NULL;
+	struct lvcreate_args cmd_args = {0};
 
-	if ((pid = fork()) < 0) {
-		SYSERROR("failed fork");
+	ret = snprintf(sz, 24, "%" PRIu64 "b", size);
+	if (ret < 0 || ret >= 24) {
+		ERROR("Failed to create string: %d", ret);
 		return -1;
 	}
-	if (pid > 0)
-		return wait_for_pid(pid);
-
-	// specify bytes to lvcreate
-	ret = snprintf(sz, 24, "%"PRIu64"b", size);
-	if (ret < 0 || ret >= 24)
-		exit(EXIT_FAILURE);
 
 	pathdup = strdup(path);
-	if (!pathdup)
-		exit(EXIT_FAILURE);
+	if (!pathdup) {
+		ERROR("Failed to duplicate string \"%s\"", path);
+		return -1;
+	}
 
 	lv = strrchr(pathdup, '/');
-	if (!lv)
-		exit(EXIT_FAILURE);
-
+	if (!lv) {
+		ERROR("Failed to detect \"/\" in string \"%s\"", pathdup);
+		free(pathdup);
+		return -1;
+	}
 	*lv = '\0';
 	lv++;
+	TRACE("Parsed logical volume \"%s\"", lv);
 
 	vg = strrchr(pathdup, '/');
-	if (!vg)
-		exit(EXIT_FAILURE);
+	if (!vg) {
+		ERROR("Failed to detect \"/\" in string \"%s\"", pathdup);
+		free(pathdup);
+		return -1;
+	}
 	vg++;
+	TRACE("Parsed volume group \"%s\"", vg);
 
 	if (thinpool) {
 		len = strlen(pathdup) + strlen(thinpool) + 2;
 		tp = alloca(len);
 
 		ret = snprintf(tp, len, "%s/%s", pathdup, thinpool);
-		if (ret < 0 || ret >= len)
-			exit(EXIT_FAILURE);
+		if (ret < 0 || ret >= len) {
+			ERROR("Failed to create string: %d", ret);
+			free(pathdup);
+			return -1;
+		}
 
 		ret = lvm_is_thin_pool(tp);
-		INFO("got %d for thin pool at path: %s", ret, tp);
-		if (ret < 0)
-			exit(EXIT_FAILURE);
-
-		if (!ret)
+		TRACE("got %d for thin pool at path: %s", ret, tp);
+		if (ret < 0) {
+			ERROR("Failed to detect whether \"%s\" is a thinpool", tp);
+			free(pathdup);
+			return -1;
+		} else if (!ret) {
+			TRACE("Detected that \"%s\" is not a thinpool", tp);
 			tp = NULL;
+		} else {
+			TRACE("Detected \"%s\" is a thinpool", tp);
+		}
 	}
 
-	(void)setenv("LVM_SUPPRESS_FD_WARNINGS", "1", 1);
-	if (!tp)
-	    execlp("lvcreate", "lvcreate", "-L", sz, vg, "-n", lv, (char *)NULL);
-	else
-	    execlp("lvcreate", "lvcreate", "--thinpool", tp, "-V", sz, vg, "-n", lv, (char *)NULL);
+	cmd_args.thinpool = tp;
+	cmd_args.vg = vg;
+	cmd_args.lv = lv;
+	cmd_args.size = sz;
+	TRACE("Creating new lvm storage volume \"%s\" on volume group \"%s\" "
+	      "of size \"%s\"", lv, vg, sz);
+	ret = run_command(cmd_output, sizeof(cmd_output),
+			  lvm_create_exec_wrapper, (void *)&cmd_args);
+	if (ret < 0) {
+		ERROR("Failed to create logical volume \"%s\": %s", lv,
+		      cmd_output);
+		free(pathdup);
+		return -1;
+	}
+	TRACE("Created new lvm storage volume \"%s\" on volume group \"%s\" "
+	      "of size \"%s\"", lv, vg, sz);
 
-	SYSERROR("execlp");
-	exit(EXIT_FAILURE);
+	free(pathdup);
+	return ret;
 }
 
-
-/*
- * Look at /sys/dev/block/maj:min/dm/uuid.  If it contains the hardcoded LVM
- * prefix "LVM-", then this is an lvm2 LV
+/* Look at "/sys/dev/block/maj:min/dm/uuid". If it contains the hardcoded LVM
+ * prefix "LVM-" then this is an lvm2 LV.
  */
 int lvm_detect(const char *path)
 {
-	char devp[MAXPATHLEN], buf[4];
-	FILE *fout;
-	int ret;
+	int fd;
+	ssize_t ret;
 	struct stat statbuf;
+	char devp[MAXPATHLEN], buf[4];
 
 	if (!strncmp(path, "lvm:", 4))
 		return 1;
 
 	ret = stat(path, &statbuf);
-	if (ret != 0)
+	if (ret < 0)
 		return 0;
+
 	if (!S_ISBLK(statbuf.st_mode))
 		return 0;
 
 	ret = snprintf(devp, MAXPATHLEN, "/sys/dev/block/%d:%d/dm/uuid",
-			major(statbuf.st_rdev), minor(statbuf.st_rdev));
+		       major(statbuf.st_rdev), minor(statbuf.st_rdev));
 	if (ret < 0 || ret >= MAXPATHLEN) {
-		ERROR("lvm uuid pathname too long");
+		ERROR("Failed to create string");
 		return 0;
 	}
-	fout = fopen(devp, "r");
-	if (!fout)
+
+	fd = open(devp, O_RDONLY);
+	if (fd < 0)
 		return 0;
-	ret = fread(buf, 1, 4, fout);
-	fclose(fout);
-	if (ret != 4 || strncmp(buf, "LVM-", 4) != 0)
+
+	ret = read(fd, buf, sizeof(buf));
+	close(fd);
+	if (ret != sizeof(buf))
 		return 0;
+
+	if (strncmp(buf, "LVM-", 4))
+		return 0;
+
 	return 1;
 }
 
@@ -183,8 +264,10 @@ int lvm_umount(struct bdev *bdev)
 int lvm_compare_lv_attr(const char *path, int pos, const char expected)
 {
 	struct lxc_popen_FILE *f;
-	int ret, len, status, start=0;
-	char *cmd, output[12];
+	int ret, len, status;
+	char *cmd;
+	char output[12];
+	int start=0;
 	const char *lvscmd = "lvs --unbuffered --noheadings -o lv_attr %s 2>/dev/null";
 
 	len = strlen(lvscmd) + strlen(path) - 1;
@@ -195,23 +278,22 @@ int lvm_compare_lv_attr(const char *path, int pos, const char expected)
 		return -1;
 
 	f = lxc_popen(cmd);
-
-	if (f == NULL) {
+	if (!f) {
 		SYSERROR("popen failed");
 		return -1;
 	}
 
-	ret = fgets(output, 12, f->f) == NULL;
+	if (!fgets(output, 12, f->f))
+		ret = 1;
 
 	status = lxc_pclose(f);
-
+	/* Assume either vg or lvs do not exist, default comparison to false. */
 	if (ret || WEXITSTATUS(status))
-		// Assume either vg or lvs do not exist, default
-		// comparison to false.
 		return 0;
 
 	len = strlen(output);
-	while(start < len && output[start] == ' ') start++;
+	while (start < len && output[start] == ' ')
+		start++;
 
 	if (start + pos < len && output[start + pos] == expected)
 		return 1;
@@ -231,175 +313,261 @@ int lvm_is_thin_pool(const char *path)
 
 int lvm_snapshot(const char *orig, const char *path, uint64_t size)
 {
-	int ret, pid;
-	char sz[24], *pathdup, *lv;
+	int ret;
+	char *pathdup, *lv;
+	char sz[24];
+	char cmd_output[MAXPATHLEN];
+	struct lvcreate_args cmd_args = {0};
 
-	if ((pid = fork()) < 0) {
-		SYSERROR("failed fork");
+	ret = snprintf(sz, 24, "%" PRIu64 "b", size);
+	if (ret < 0 || ret >= 24) {
+		ERROR("Failed to create string");
 		return -1;
 	}
-	if (pid > 0)
-		return wait_for_pid(pid);
-
-	// specify bytes to lvcreate
-	ret = snprintf(sz, 24, "%"PRIu64"b", size);
-	if (ret < 0 || ret >= 24)
-		exit(EXIT_FAILURE);
 
 	pathdup = strdup(path);
-	if (!pathdup)
-		exit(EXIT_FAILURE);
+	if (!pathdup) {
+		ERROR("Failed to duplicate string \"%s\"", path);
+		return -1;
+	}
+
 	lv = strrchr(pathdup, '/');
 	if (!lv) {
+		ERROR("Failed to detect \"/\" in string \"%s\"", pathdup);
 		free(pathdup);
-		exit(EXIT_FAILURE);
+		return -1;
 	}
 	*lv = '\0';
 	lv++;
+	TRACE("Parsed logical volume \"%s\"", lv);
 
-	// check if the original lv is backed by a thin pool, in which case we
-	// cannot specify a size that's different from the original size.
+	/* Check if the original logical volume is backed by a thinpool, in
+	 * which case we cannot specify a size that's different from the
+	 * original size.
+	 */
 	ret = lvm_is_thin_volume(orig);
-	if (ret == -1) {
+	if (ret < 0) {
+		free(pathdup);
+		return -1;
+	} else if (ret) {
+		cmd_args.thinpool = orig;
+	}
+
+	cmd_args.lv = lv;
+	cmd_args.source_lv = orig;
+	cmd_args.size = sz;
+	TRACE("Creating new lvm snapshot \"%s\" of \"%s\" with size \"%s\"", lv,
+	      orig, sz);
+	ret = run_command(cmd_output, sizeof(cmd_output),
+			  lvm_snapshot_exec_wrapper, (void *)&cmd_args);
+	if (ret < 0) {
+		ERROR("Failed to create logical volume \"%s\": %s", orig,
+		      cmd_output);
 		free(pathdup);
 		return -1;
 	}
 
-	(void)setenv("LVM_SUPPRESS_FD_WARNINGS", "1", 1);
-	if (!ret) {
-		ret = execlp("lvcreate", "lvcreate", "-s", "-L", sz, "-n", lv, orig, (char *)NULL);
-	} else {
-		ret = execlp("lvcreate", "lvcreate", "-s", "-n", lv, orig, (char *)NULL);
-	}
-
 	free(pathdup);
-	exit(EXIT_FAILURE);
+	return 0;
 }
 
 int lvm_clonepaths(struct bdev *orig, struct bdev *new, const char *oldname,
 		const char *cname, const char *oldpath, const char *lxcpath, int snap,
 		uint64_t newsize, struct lxc_conf *conf)
 {
-	char fstype[100];
-	uint64_t size = newsize;
 	int len, ret;
-	const char *cmd_args[2];
-	char cmd_output[MAXPATHLEN];
+	const char *vg;
 
 	if (!orig->src || !orig->dest)
 		return -1;
 
+	if (strcmp(orig->type, "lvm") && snap) {
+		ERROR("LVM snapshot from \"%s\" storage driver is not supported",
+		      orig->type);
+		return -1;
+	}
+
 	if (strcmp(orig->type, "lvm")) {
-		const char *vg;
-
-		if (snap) {
-			ERROR("LVM snapshot from %s backing store is not supported",
-				orig->type);
-			return -1;
-		}
 		vg = lxc_global_config_value("lxc.bdev.lvm.vg");
-		if (!vg) {
-			ERROR("The \"lxc.bdev.lvm.vg\" key is not set");
+		new->src = lxc_string_join(
+		    "/",
+		    (const char *[]){"lvm:", "dev", vg, cname, NULL},
+		    false);
+	} else {
+		char *dup, *slider, *src;
+
+		src = lxc_storage_get_path(orig->src, orig->type);
+
+		dup = strdup(src);
+		if (!dup) {
+			ERROR("Failed to duplicate string \"%s\"", src);
 			return -1;
 		}
 
-		len = strlen("/dev/") + strlen(vg) + strlen(cname) + 4 + 2;
-		new->src = malloc(len);
-		if (!new->src)
+		slider = strrchr(dup, '/');
+		if (!slider) {
+			ERROR("Failed to detect \"/\" in string \"%s\"", dup);
+			free(dup);
 			return -1;
+		}
+		*slider = '\0';
+		slider = dup;
 
-		ret = snprintf(new->src, len, "lvm:/dev/%s/%s", vg, cname);
-		if (ret < 0 || ret >= len)
-			return -1;
-	} else {
-		new->src = dir_new_path(orig->src, oldname, cname, oldpath, lxcpath);
-		if (!new->src)
-			return -1;
+		new->src = lxc_string_join(
+		    "/",
+		    (const char *[]){"lvm:", *slider == '/' ? ++slider : slider,
+				     cname, NULL},
+		    false);
+		free(dup);
+	}
+	if (!new->src) {
+		ERROR("Failed to create string");
+		return -1;
 	}
 
 	if (orig->mntopts) {
 		new->mntopts = strdup(orig->mntopts);
-		if (!new->mntopts)
+		if (!new->mntopts) {
+			ERROR("Failed to duplicate string \"%s\"", orig->mntopts);
 			return -1;
+		}
 	}
 
 	len = strlen(lxcpath) + strlen(cname) + strlen("rootfs") + 3;
 	new->dest = malloc(len);
-	if (!new->dest)
+	if (!new->dest) {
+		ERROR("Failed to allocate memory");
 		return -1;
-	ret = snprintf(new->dest, len, "%s/%s/rootfs", lxcpath, cname);
-	if (ret < 0 || ret >= len)
-		return -1;
-	if (mkdir_p(new->dest, 0755) < 0)
-		return -1;
-
-	if (is_blktype(orig)) {
-		if (!newsize && blk_getsize(orig, &size) < 0) {
-			ERROR("Error getting size of %s", orig->src);
-			return -1;
-		}
-		if (detect_fs(orig, fstype, 100) < 0) {
-			INFO("could not find fstype for %s, using ext3", orig->src);
-			return -1;
-		}
-	} else {
-		sprintf(fstype, "ext3");
-		if (!newsize)
-			size = DEFAULT_FS_SIZE;
 	}
 
-	if (snap) {
-		char *newsrc, *origsrc;
+	ret = snprintf(new->dest, len, "%s/%s/rootfs", lxcpath, cname);
+	if (ret < 0 || ret >= len) {
+		ERROR("Failed to create string");
+		return -1;
+	}
 
-		origsrc = lxc_storage_get_path(orig->src, "lvm");
-		newsrc = lxc_storage_get_path(new->src, "lvm");
-
-		if (lvm_snapshot(origsrc, newsrc, size) < 0) {
-			ERROR("could not create %s snapshot of %s", new->src, orig->src);
-			return -1;
-		}
-	} else {
-		char *src;
-
-		src = lxc_storage_get_path(new->src, "lvm");
-		if (do_lvm_create(src, size, lxc_global_config_value("lxc.bdev.lvm.thin_pool")) < 0) {
-			ERROR("Error creating new lvm blockdev");
-			return -1;
-		}
-
-		cmd_args[0] = fstype;
-		cmd_args[1] = src;
-		// create an fs in the loopback file
-		ret = run_command(cmd_output, sizeof(cmd_output),
-				  do_mkfs_exec_wrapper, (void *)cmd_args);
-		if (ret < 0)
-			return -1;
+	ret = mkdir_p(new->dest, 0755);
+	if (ret < 0) {
+		SYSERROR("Failed to create directory \"%s\"", new->dest);
+		return -1;
 	}
 
 	return 0;
 }
 
-int lvm_destroy(struct bdev *orig)
+bool lvm_create_clone(struct lxc_conf *conf, struct bdev *orig,
+		      struct bdev *new, uint64_t newsize)
 {
 	char *src;
+	const char *thinpool;
+	int ret;
+	struct rsync_data data;
+	char *cmd_args[2];
+	char cmd_output[MAXPATHLEN];
+	char fstype[100] = "ext4";
+	uint64_t size = newsize;
 
-	pid_t pid;
+	if (is_blktype(orig)) {
+		/* detect size */
+		if (!newsize && blk_getsize(orig, &size) < 0) {
+			ERROR("Failed to detect size of logical volume \"%s\"",
+			      orig->src);
+			return -1;
+		}
 
-	if ((pid = fork()) < 0)
-		return -1;
-
-	if (!pid) {
-		(void)setenv("LVM_SUPPRESS_FD_WARNINGS", "1", 1);
-		src = lxc_storage_get_path(orig->src, "lvm");
-		execlp("lvremove", "lvremove", "-f", src, (char *)NULL);
-		exit(EXIT_FAILURE);
+		/* detect filesystem */
+		if (detect_fs(orig, fstype, 100) < 0) {
+			INFO("Failed to detect filesystem type for \"%s\"", orig->src);
+			return -1;
+		}
+	} else if (!newsize) {
+			size = DEFAULT_FS_SIZE;
 	}
 
-	return wait_for_pid(pid);
+	src = lxc_storage_get_path(new->src, "lvm");
+	thinpool = lxc_global_config_value("lxc.bdev.lvm.thin_pool");
+
+	ret = do_lvm_create(src, size, thinpool);
+	if (ret < 0) {
+		ERROR("Failed to create lvm storage volume \"%s\"", src);
+		return -1;
+	}
+
+	cmd_args[0] = fstype;
+	cmd_args[1] = src;
+	ret = run_command(cmd_output, sizeof(cmd_output),
+			do_mkfs_exec_wrapper, (void *)cmd_args);
+	if (ret < 0) {
+		ERROR("Failed to create new filesystem \"%s\" for lvm storage "
+		      "volume \"%s\": %s", fstype, src, cmd_output);
+		return -1;
+	}
+
+	data.orig = orig;
+	data.new = new;
+	ret = rsync_rootfs(&data);
+	if (ret < 0) {
+		ERROR("Failed to rsync from \"%s\" to \"%s\"", orig->dest,
+		      new->dest);
+		return false;
+	}
+
+	TRACE("Created lvm storage volume \"%s\"", new->dest);
+	return true;
+}
+
+bool lvm_create_snapshot(struct lxc_conf *conf, struct bdev *orig,
+			 struct bdev *new, uint64_t newsize)
+{
+	int ret;
+	char *newsrc, *origsrc;
+	uint64_t size = newsize;
+
+	if (is_blktype(orig)) {
+		if (!newsize && blk_getsize(orig, &size) < 0) {
+			ERROR("Failed to detect size of logical volume \"%s\"",
+			      orig->src);
+			return -1;
+		}
+	} else if (!newsize) {
+			size = DEFAULT_FS_SIZE;
+	}
+
+	origsrc = lxc_storage_get_path(orig->src, "lvm");
+	newsrc = lxc_storage_get_path(new->src, "lvm");
+
+	ret = lvm_snapshot(origsrc, newsrc, size);
+	if (ret < 0) {
+		ERROR("Failed to create lvm \"%s\" snapshot of \"%s\"",
+		      new->src, orig->src);
+		return false;
+	}
+
+	TRACE("Created lvm snapshot \"%s\" from \"%s\"", new->dest, orig->dest);
+	return true;
+}
+
+int lvm_destroy(struct bdev *orig)
+{
+	int ret;
+	char cmd_output[MAXPATHLEN];
+	struct lvcreate_args cmd_args = {0};
+
+	cmd_args.lv = lxc_storage_get_path(orig->src, "lvm");
+	ret = run_command(cmd_output, sizeof(cmd_output),
+			  lvm_destroy_exec_wrapper, (void *)&cmd_args);
+	if (ret < 0) {
+		ERROR("Failed to destroy logical volume \"%s\": %s", orig->src,
+		      cmd_output);
+		return -1;
+	}
+
+	TRACE("Destroyed logical volume \"%s\"", orig->src);
+	return 0;
 }
 
 int lvm_create(struct bdev *bdev, const char *dest, const char *n,
-		struct bdev_specs *specs)
+	       struct bdev_specs *specs)
 {
 	const char *vg, *thinpool, *fstype, *lv = n;
 	uint64_t sz;
@@ -424,20 +592,26 @@ int lvm_create(struct bdev *bdev, const char *dest, const char *n,
 
 	len = strlen(vg) + strlen(lv) + 4 + 7;
 	bdev->src = malloc(len);
-	if (!bdev->src)
+	if (!bdev->src) {
+		ERROR("Failed to allocate memory");
 		return -1;
+	}
 
 	ret = snprintf(bdev->src, len, "lvm:/dev/%s/%s", vg, lv);
-	if (ret < 0 || ret >= len)
+	if (ret < 0 || ret >= len) {
+		ERROR("Failed to create string");
 		return -1;
+	}
 
-	// fssize is in bytes.
+	/* size is in bytes */
 	sz = specs->fssize;
 	if (!sz)
 		sz = DEFAULT_FS_SIZE;
 
-	if (do_lvm_create(bdev->src + 4, sz, thinpool) < 0) {
-		ERROR("Error creating new lvm blockdev %s size %"PRIu64" bytes", bdev->src, sz);
+	ret = do_lvm_create(bdev->src + 4, sz, thinpool);
+	if (ret < 0) {
+		ERROR("Error creating new logical volume \"%s\" of size "
+		      "\"%" PRIu64 " bytes\"", bdev->src, sz);
 		return -1;
 	}
 
@@ -446,19 +620,27 @@ int lvm_create(struct bdev *bdev, const char *dest, const char *n,
 		fstype = DEFAULT_FSTYPE;
 
 	cmd_args[0] = fstype;
-	cmd_args[1] = bdev->src + 4;
+	cmd_args[1] = lxc_storage_get_path(bdev->src, bdev->type);
 	ret = run_command(cmd_output, sizeof(cmd_output), do_mkfs_exec_wrapper,
 			  (void *)cmd_args);
-	if (ret < 0)
-		return -1;
-
-	if (!(bdev->dest = strdup(dest)))
-		return -1;
-
-	if (mkdir_p(bdev->dest, 0755) < 0) {
-		ERROR("Error creating %s", bdev->dest);
+	if (ret < 0) {
+		ERROR("Failed to create new logical volume \"%s\": %s",
+		      bdev->src, cmd_output);
 		return -1;
 	}
 
+	bdev->dest = strdup(dest);
+	if (!bdev->dest) {
+		ERROR("Failed to duplicate string \"%s\"", dest);
+		return -1;
+	}
+
+	ret = mkdir_p(bdev->dest, 0755);
+	if (ret < 0) {
+		SYSERROR("Failed to create directory \"%s\"", bdev->dest);
+		return -1;
+	}
+
+	TRACE("Created new logical volume \"%s\"", bdev->dest);
 	return 0;
 }

src/lxc/bdev/lxclvm.h

@@ -25,6 +25,7 @@
 #define __LXC_LVM_H
 
 #define _GNU_SOURCE
+#include <stdbool.h>
 #include <stdint.h>
 
 /* defined in bdev.h */
@@ -52,5 +53,9 @@ int lvm_clonepaths(struct bdev *orig, struct bdev *new, const char *oldname,
 int lvm_destroy(struct bdev *orig);
 int lvm_create(struct bdev *bdev, const char *dest, const char *n,
 		struct bdev_specs *specs);
+bool lvm_create_clone(struct lxc_conf *conf, struct bdev *orig,
+		      struct bdev *new, uint64_t newsize);
+bool lvm_create_snapshot(struct lxc_conf *conf, struct bdev *orig,
+			 struct bdev *new, uint64_t newsize);
 
 #endif /* __LXC_LVM_H */