Add dm-integrity support

Create a linear LV with a dm-integrity layer added
above it.  The dm-integrity layer stores checksums
of the data written to the LV, and returns an error
if data read from the LV does not match the previously
saved checksum.

lvcreate --type integrity --integrity String [options]

The --integrity String specifies if the dm-integrity
metadata (checksums) should be interleaved with data
blocks, or written to a separate external LV:

  --integrity external (default)
  Store integrity metadata on a separate LV.
  Allows removing integrity from the LV later.

  --integrity internal
  Store integrity metadata interleaved with data
  on the same LV.  Around 1% of the LV size will
  be used for integrity metadata.

  --integrity y
  Enable default integrity settings (external).

Command variations:

lvcreate --type integrity -n Name -L Size VG

  [--integrity external is the default]

lvcreate --integrity external -n Name -L Size VG

  [--integrity external implies --type integrity]

lvcreate --integrity y -n Name -L Size VG

  [--integrity y uses default/external, and implies
   --type integrity]

lvcreate --integrity internal -n Name -L Size VG

  [--integrity internal implies --type integrity]

Options:

  --integritymetadata LV
  Use the specified LV for external metadata.
  Allows specific device placement of metadata.
  Without this option, the command creates a
  hidden LV (with an _imeta suffix) to hold the
  metadata.

  --integritysettings String
  set dm-integrity parameters, e.g. to use a journal
  instead of bitmap, --integritysettings "mode=J".

Example:

$ lvcreate --integrity external -n lvex -L1G vg
$ lvs -a vg
  LV           VG Attr       LSize  Origin
  lvex         vg -wi-a-----  1.00g [lvex_iorig]
  [lvex_imeta] vg -wi-ao---- 12.00m
  [lvex_iorig] vg -wi-ao----  1.00g

$ lvcreate --integrity internal -n lvin -L1G vg
$ lvs -a vg
  LV           VG Attr       LSize Origin
  lvin         vg -wi-a----- 1.00g [lvin_iorig]
  [lvin_iorig] vg -wi-ao---- 1.00g

Zeroing:

After a new integrity LV is created, zeroes are written to
the entire LV to initialize integrity metadata (checksums).
Without this initialization, the LV will return read errors
for any unwritten (and uninitialized) data.

A large LV may take a long time to zero.  The -Zn option can
be used to disable the whole-LV zeroing, or the lvcreate
command can be canceled while zeroing the new LV.  In either
case, the user may write to the entire LV to initialize the
integrity metadata themselves.

TODO:

- add command to remove intregrity from an LV (only for external),
  i.e. lvconvert --integrity none LV

- let integrity be used by raid images

lib/label/label.c

@@ -252,8 +252,6 @@ struct label *label_create(struct labeller *labeller)
 /* global variable for accessing the bcache populated by label scan */
 struct bcache *scan_bcache;
 
-#define BCACHE_BLOCK_SIZE_IN_SECTORS 256 /* 256*512 = 128K */
-
 static bool _in_bcache(struct device *dev)
 {
 	if (!dev)

lib/label/label.h

@@ -118,6 +118,14 @@ int label_scan_open(struct device *dev);
 int label_scan_open_excl(struct device *dev);
 int label_scan_open_rw(struct device *dev);
 
+/*
+ * These are the sizes the label.c uses to set up
+ * and use bcache (they are not bcache restrictions
+ * or defs.)
+ */
+#define BCACHE_BLOCK_SIZE_IN_SECTORS 256 /* 256*512 = 128K */
+#define BCACHE_BLOCK_SIZE_IN_BYTES 131072
+
 /*
  * Wrappers around bcache equivalents.
  * (these make it easier to disable bcache and revert to direct rw if needed)

lib/metadata/integrity_manip.c

@@ -29,6 +29,15 @@
 
 #define ONE_MB_IN_BYTES 1048576
 
+/*
+ * Every 500M of data needs 4M of metadata.
+ * (From trial and error testing.)
+ */
+static uint64_t _lv_size_bytes_to_integrity_meta_bytes(uint64_t lv_size_bytes)
+{
+	return ((lv_size_bytes / (500 * ONE_MB_IN_BYTES)) + 1) * (4 * ONE_MB_IN_BYTES);
+}
+
 /*
  * The user wants external metadata, but did not specify an existing
  * LV to hold metadata, so create an LV for metadata.  Save it in
@@ -39,7 +48,7 @@ int lv_create_integrity_metadata(struct cmd_context *cmd,
 				struct lvcreate_params *lp)
 {
 	char metaname[NAME_LEN];
-	uint32_t extent_bytes;
+	uint64_t lv_size_bytes, meta_bytes, meta_sectors;
 	struct logical_volume *lv;
 	struct lvcreate_params lp_meta = {
 		.activate = CHANGE_AN,
@@ -53,6 +62,7 @@ int lv_create_integrity_metadata(struct cmd_context *cmd,
 		.vg_name = vg->name,
 		.zero = 0,
 		.wipe_signatures = 0,
+		.suppress_zero_warn = 1,
 	};
 
 	if (dm_snprintf(metaname, NAME_LEN, "%s_imeta", lp->lv_name) < 0) {
@@ -63,12 +73,15 @@ int lv_create_integrity_metadata(struct cmd_context *cmd,
 	if (!(lp_meta.lv_name = strdup(metaname)))
 		return_0;
 
-	/* TODO: set pvh per command line */
-	/* TODO: scale size according to LV size. */
-	extent_bytes = vg->extent_size * SECTOR_SIZE;
-	lp_meta.extents = (4 * ONE_MB_IN_BYTES) / extent_bytes;
+	lp_meta.pvh = lp->pvh;
 
-	log_debug("Creating integrity metadata LV with %u extents", lp_meta.extents);
+	lv_size_bytes = lp->extents * vg->extent_size * 512;
+	meta_bytes = _lv_size_bytes_to_integrity_meta_bytes(lv_size_bytes);
+	meta_sectors = meta_bytes / 512;
+	lp_meta.extents = meta_sectors / vg->extent_size;
+
+	log_print("Creating integrity metadata LV %s with size %s.",
+		  metaname, display_size(cmd, meta_sectors));
 
 	dm_list_init(&lp_meta.tags);
 
@@ -87,6 +100,7 @@ int lv_create_integrity_metadata(struct cmd_context *cmd,
 static int _get_provided_data_sectors(struct logical_volume *lv, uint64_t *provided_data_sectors)
 {
 	struct lv_with_info_and_seg_status status;
+	uint64_t data_sectors, extra_sectors;
 
 	memset(&status, 0, sizeof(status));
 	status.seg_status.type = SEG_STATUS_NONE;
@@ -115,7 +129,15 @@ static int _get_provided_data_sectors(struct logical_volume *lv, uint64_t *provi
 		goto fail;
 	}
 
-	*provided_data_sectors = status.seg_status.integrity->provided_data_sectors;
+	data_sectors = status.seg_status.integrity->provided_data_sectors;
+
+	if ((extra_sectors = (data_sectors % lv->vg->extent_size))) {
+		data_sectors -= extra_sectors;
+		log_debug("Reduce provided_data_sectors by %llu to %llu for extent alignment",
+			  (unsigned long long)extra_sectors, (unsigned long long)data_sectors);
+	}
+
+	*provided_data_sectors = data_sectors;
 
 	dm_pool_destroy(status.seg_status.mem);
 	return 1;
@@ -131,13 +153,18 @@ int lv_add_integrity(struct logical_volume *lv, const char *arg,
 		     struct integrity_settings *settings)
 {
 	struct cmd_context *cmd = lv->vg->cmd;
+	struct volume_group *vg = lv->vg;
 	struct integrity_settings *set;
 	struct logical_volume *lv_orig;
 	struct logical_volume *meta_lv = NULL;
 	const struct segment_type *segtype;
 	struct lv_segment *seg;
+	uint64_t meta_bytes, meta_sectors;
+	uint64_t lv_size_sectors;
 	int ret;
 
+	lv_size_sectors = lv->size;
+
 	if (!(segtype = get_segtype_from_string(cmd, SEG_TYPE_NAME_INTEGRITY)))
 		return_0;
 
@@ -187,10 +214,19 @@ int lv_add_integrity(struct logical_volume *lv, const char *arg,
 	if (meta_lv_created)
 		meta_lv = meta_lv_created;
 	else if (meta_name) {
-		if (!(meta_lv = find_lv(lv->vg, meta_name))) {
+		if (!(meta_lv = find_lv(vg, meta_name))) {
 			log_error("LV %s not found.", meta_name);
 			return_0;
 		}
+
+		meta_bytes = _lv_size_bytes_to_integrity_meta_bytes(lv->size * 512);
+		meta_sectors = meta_bytes / 512;
+
+		if (meta_lv->size < meta_sectors) {
+			log_error("Integrity metadata needs %s, metadata LV is only %s.",
+				 display_size(cmd, meta_sectors), display_size(cmd, meta_lv->size));
+			return 0;
+		}
 	}
 
 	/*
@@ -208,16 +244,37 @@ int lv_add_integrity(struct logical_volume *lv, const char *arg,
 	 */
 
 	if (meta_lv) {
+		struct wipe_params wipe;
+
 		if (!sync_local_dev_names(cmd)) {
 			log_error("Failed to sync local devices.");
 			return 0;
 		}
 
-		if (!activate_and_wipe_lv(meta_lv, 0)) {
-			log_error("LV %s could not be zeroed.", display_lvname(meta_lv));
+		log_verbose("Zeroing LV for integrity metadata");
+
+		if (!lv_is_active(meta_lv)) {
+			if (!activate_lv(cmd, meta_lv)) {
+				log_error("Failed to activate LV %s to zero", display_lvname(meta_lv));
+				return 0;
+			}
+		}
+
+		memset(&wipe, 0, sizeof(wipe));
+		wipe.do_zero = 1;
+		wipe.zero_sectors = 8;
+
+		if (!wipe_lv(meta_lv, wipe)) {
+			log_error("Failed to zero LV for integrity metadata %s", display_lvname(meta_lv));
 			return 0;
 		}
-		seg->integrity_data_sectors = seg->len;
+
+		if (!deactivate_lv(cmd, meta_lv)) {
+			log_error("Failed to deactivate LV %s after zero", display_lvname(meta_lv));
+			return 0;
+		}
+
+		seg->integrity_data_sectors = lv_size_sectors;
 		seg->integrity_meta_dev = meta_lv;
 		lv_set_hidden(meta_lv);
 		/* TODO: give meta_lv a suffix? e.g. _imeta */
@@ -246,6 +303,11 @@ int lv_add_integrity(struct logical_volume *lv, const char *arg,
 			ret = 1;
 		}
 
+		if (!seg->integrity_data_sectors) {
+			log_error("LV size too small to include metadata.");
+			ret = 0;
+		}
+
 		lv->status |= LV_UNCOMMITTED;
 
 		if (!deactivate_lv(cmd, lv)) {
@@ -257,6 +319,11 @@ int lv_add_integrity(struct logical_volume *lv, const char *arg,
 		lv->status &= ~LV_NOSCAN;
 		lv->status &= ~LV_TEMPORARY;
 	}
+
+	log_debug("Write VG with integrity added to LV");
+
+	if (!vg_write(vg) || !vg_commit(vg))
+		ret = 0;
  out:
 	return ret;
 }

lib/metadata/lv_manip.c

@@ -28,6 +28,7 @@
 #include "lib/datastruct/str_list.h"
 #include "lib/config/defaults.h"
 #include "lib/misc/lvm-exec.h"
+#include "lib/misc/lvm-signal.h"
 #include "lib/mm/memlock.h"
 #include "lib/locking/lvmlockd.h"
 #include "lib/label/label.h"
@@ -55,6 +56,8 @@ typedef enum {
 #define A_POSITIONAL_FILL	0x40	/* Slots are positional and filled using PREFERRED */
 #define A_PARTITION_BY_TAGS	0x80	/* No allocated area may share any tag with any other */
 
+#define ONE_MB_IN_BYTES 1048576
+
 /*
  * Constant parameters during a single allocation attempt.
  */
@@ -7424,11 +7427,88 @@ int insert_layer_for_segments_on_pv(struct cmd_context *cmd,
 	return 1;
 }
 
-/* FIXME: copied from label.c */
-#define BCACHE_BLOCK_SIZE_IN_SECTORS 256 /* 256*512 = 128K */
-#define BCACHE_BLOCK_SIZE_IN_BYTES 131072
-#define ONE_MB_IN_BYTES 1048576
-#define ONE_MB_IN_SECTORS 2048   /* 2048 * 512 = 1048576 */
+int zero_lv_name(struct cmd_context *cmd, const char *vg_name, const char *lv_name, uint64_t lv_size_bytes)
+{
+	char name[PATH_MAX];
+	struct device *dev;
+	uint64_t off = 0, i = 0, j = 0;
+	uint64_t zero_bytes;
+	uint32_t extra_bytes;
+
+	if (dm_snprintf(name, sizeof(name), "%s%s/%s", cmd->dev_dir, vg_name, lv_name) < 0) {
+		log_error("Device path name too long, device not zeroed (%s).", lv_name);
+		return 0;
+	}
+
+	if (!(dev = dev_cache_get(cmd, name, NULL))) {
+		log_error("Failed to find device %s: device not zeroed.", name);
+		return 0;
+	}
+
+	if (!label_scan_open_rw(dev)) {
+		log_error("Failed to open %s: device not zeroed.", name);
+		return 0;
+	}
+
+	zero_bytes = lv_size_bytes;
+
+	log_print("Zeroing %s %s... (cancel command to zero manually)",
+		  name, display_size(cmd, zero_bytes/512));
+
+	if ((extra_bytes = (zero_bytes % ONE_MB_IN_BYTES)))
+		zero_bytes -= extra_bytes;
+
+	/*
+	 * Write 1MiB at a time to avoid going over bcache size.
+	 * Then write 128KiB (bcache block sizes) at a time to
+	 * cover remaining dev size.
+	 */
+
+	sigint_allow();
+
+	for (i = 0; i < (zero_bytes / ONE_MB_IN_BYTES); i++) {
+		off = i * ONE_MB_IN_BYTES;
+		if (!dev_write_zeros(dev, off, (size_t)ONE_MB_IN_BYTES))
+			log_error("Failed to zero LV at offset %llu.", (unsigned long long)off);
+
+		if (off && !(off % (1024 * ONE_MB_IN_BYTES)))
+			log_print("Zeroed %s...", display_size(cmd, off/512));
+
+		if (sigint_caught()) {
+			log_print("Zeroing canceled.");
+			goto out;
+		}
+
+	}
+
+	if (extra_bytes) {
+		log_debug("Zeroing final %u bytes at %llu.", extra_bytes, (unsigned long long)off);
+
+		for (j = 0; j < (extra_bytes / BCACHE_BLOCK_SIZE_IN_BYTES); j++) {
+			off = i * ONE_MB_IN_BYTES + j * BCACHE_BLOCK_SIZE_IN_BYTES;
+			if (!dev_write_zeros(dev, off, (size_t)BCACHE_BLOCK_SIZE_IN_BYTES))
+				log_error("Failed to zero LV at offset %llu.", (unsigned long long)off);
+
+			if (sigint_caught()) {
+				log_print("Zeroing canceled.");
+				goto out;
+			}
+		}
+	}
+
+	/*
+	 * FIXME: bcache can't write partial blocks yet.
+	 * This shouldn't actually happen given current
+	 * usage where LV size is a multiple of extents.
+	 */
+	if ((extra_bytes = lv_size_bytes - (i * ONE_MB_IN_BYTES + j * BCACHE_BLOCK_SIZE_IN_BYTES)))
+		log_warn("WARNING: last %llu bytes not zeroed.", (unsigned long long)extra_bytes);
+
+out:
+	sigint_restore();
+	label_scan_invalidate(dev);
+	return 1;
+}
 
 /*
  * Initialize the LV with 'value'.
@@ -7488,44 +7568,7 @@ int wipe_lv(struct logical_volume *lv, struct wipe_params wp)
 			stack;
 	}
 
-	if (wp.do_zero && !wp.zero_value && (wp.zero_sectors >= ONE_MB_IN_SECTORS)) {
-		uint64_t off = 0, i = 0, j = 0;
-		uint64_t zero_bytes;
-		uint32_t extra_bytes;
-
-		zero_bytes = wp.zero_sectors * 512;
-
-		if ((extra_bytes = (zero_bytes % ONE_MB_IN_BYTES)))
-			zero_bytes -= extra_bytes;
-
-		log_print("Zeroing %llu MiB...", (unsigned long long)(zero_bytes / ONE_MB_IN_BYTES));
-
-		/*
-		 * Write 1MiB at a time to avoid going over bcache size.
-		 * Then write 128KiB at a time to cover remaining dev size.
-		 */
-
-		for (i = 0; i < (zero_bytes / ONE_MB_IN_BYTES); i++) {
-			off = i * ONE_MB_IN_BYTES;
-			if (!dev_write_zeros(dev, off, (size_t)ONE_MB_IN_BYTES))
-				log_error("Failed to zero LV at offset %llu.", (unsigned long long)off);
-		}
-
-		if (extra_bytes) {
-			log_warn("Zeroing %u bytes at %llu...", extra_bytes, (unsigned long long)off);
-
-			for (j = 0; j < (extra_bytes / BCACHE_BLOCK_SIZE_IN_BYTES); j++) {
-				off = i * ONE_MB_IN_BYTES + j * BCACHE_BLOCK_SIZE_IN_BYTES;
-				if (!dev_write_zeros(dev, off, (size_t)BCACHE_BLOCK_SIZE_IN_BYTES))
-					log_error("Failed to zero LV at offset %llu.", (unsigned long long)off);
-			}
-		}
-
-		/* FIXME: bcache can't write partial block yet */
-		if ((extra_bytes = (wp.zero_sectors * 512) - (i * ONE_MB_IN_BYTES + j * BCACHE_BLOCK_SIZE_IN_BYTES)))
-			log_warn("WARNING: last %llu bytes not zeroed.", (unsigned long long)extra_bytes);
-
-	} else if (wp.do_zero) {
+	if (wp.do_zero) {
 		zero_sectors = wp.zero_sectors ? : UINT64_C(4096) >> SECTOR_SHIFT;
 
 		if (zero_sectors > lv->size)
@@ -7736,10 +7779,14 @@ static int _should_wipe_lv(struct lvcreate_params *lp,
 	     first_seg(first_seg(lv)->pool_lv)->zero_new_blocks))
 		return 0;
 
-	if (seg_is_integrity(lp) && (!lp->zero || !(lv->status & LVM_WRITE))) {
-		log_warn("WARNING: --zero not enabled, integrity will not be initialized and may cause read errors.");
-		return 0;
-	}
+	/*
+	 * dm-integrity requires the first sector (integrity superblock) to be
+	 * zero when creating a new integrity device.
+	 * TODO: print a warning or error if the user specifically
+	 * asks for no wiping or zeroing?
+	 */
+	if (seg_is_integrity(lp))
+		return 1;
 
 	/* Cannot zero read-only volume */
 	if ((lv->status & LVM_WRITE) &&
@@ -7998,13 +8045,6 @@ static struct logical_volume *_lv_create_an_lv(struct volume_group *vg,
 			return_0;
 
 	} else if (seg_is_integrity(lp)) {
-		/*
-		 * TODO: if using internal metadata, estimate the amount of metadata
-		 * that will be needed, and add this to the amount of PV space being
-		 * allocated so that the usable LV size is what the user requested.
-		 * Or, just request an extra extent_size bytes, then round the
-		 * provided_data_sectors down to be an extent_size multiple.
-		 */
 		if (!(create_segtype = get_segtype_from_string(vg->cmd, SEG_TYPE_NAME_STRIPED)))
 			return_0;
 
@@ -8232,12 +8272,6 @@ static struct logical_volume *_lv_create_an_lv(struct volume_group *vg,
 		}
 	}
 
-	if (seg_is_integrity(lp)) {
-		if (!lv_add_integrity(lv, lp->integrity_arg, lp->integrity_meta_lv,
-				      lp->integrity_meta_name, &lp->integrity_settings))
-			return_NULL;
-	}
-
 	lv_set_activation_skip(lv, lp->activation_skip & ACTIVATION_SKIP_SET,
 			       lp->activation_skip & ACTIVATION_SKIP_SET_ENABLED);
 	/*
@@ -8357,27 +8391,25 @@ static struct logical_volume *_lv_create_an_lv(struct volume_group *vg,
 			goto revert_new_lv;
 		}
 		lv->status &= ~LV_TEMPORARY;
+
+	} else if (seg_is_integrity(lp)) {
+		/*
+		 * Activate the new origin LV so it can be zeroed/wiped
+		 * below before adding integrity.
+		 */
+		lv->status |= LV_TEMPORARY;
+		if (!activate_lv(cmd, lv)) {
+			log_error("Failed to activate new LV for wiping.");
+			goto revert_new_lv;
+		}
+		lv->status &= ~LV_TEMPORARY;
+
 	} else if (!lv_active_change(cmd, lv, lp->activate)) {
 		log_error("Failed to activate new LV %s.", display_lvname(lv));
 		goto deactivate_and_revert_new_lv;
 	}
 
-	if (seg_is_integrity(lp)) {
-		struct wipe_params wipe;
-
-		memset(&wipe, 0, sizeof(wipe));
-		wipe.do_zero = 1;
-		wipe.zero_sectors = first_seg(lv)->integrity_data_sectors;
-
-		if (!_should_wipe_lv(lp, lv, 1))
-			goto_out;
-
-		if (!wipe_lv(lv, wipe))
-			log_error("Failed to zero LV.");
-
-		goto out;
-
-	} else if (_should_wipe_lv(lp, lv, !lp->suppress_zero_warn)) {
+	if (_should_wipe_lv(lp, lv, !lp->suppress_zero_warn)) {
 		if (!wipe_lv(lv, (struct wipe_params)
 			     {
 				     .do_zero = lp->zero,
@@ -8391,6 +8423,42 @@ static struct logical_volume *_lv_create_an_lv(struct volume_group *vg,
 		}
 	}
 
+	if (seg_is_integrity(lp)) {
+		log_verbose("Adding integrity to new LV");
+
+		/* Origin is active from zeroing, deactivate to add integrity. */
+
+		if (!deactivate_lv(cmd, lv)) {
+			log_error("Failed to deactivate LV to add integrity");
+			goto revert_new_lv;
+		}
+
+		if (!lv_add_integrity(lv, lp->integrity_arg, lp->integrity_meta_lv,
+				      lp->integrity_meta_name, &lp->integrity_settings))
+			goto revert_new_lv;
+
+		backup(vg);
+
+		/*
+		 * Activate for zeroing
+		 * TODO: activate in zero_lv_name() if zeroing is enabled.
+		 */
+		if (!activate_lv(cmd, lv)) {
+			log_error("Failed to activate LV to zero integrity.");
+			goto out;
+		}
+
+		/*
+		 * The entire LV is zeroed, which can take a long time,
+		 * so defer this to the end of the command when no locks
+		 * are held, and the command can be canceled without
+		 * problems (if the user doesn't want to wait, or wants
+		 * to do the zeroing themselves.)
+		 */
+		lp->post_zero_bytes = first_seg(lv)->integrity_data_sectors * 512;
+		goto out;
+	}
+
 	if (seg_is_vdo_pool(lp)) {
 		if (!convert_vdo_pool_lv(lv, &lp->vdo_params, &lp->virtual_extents)) {
 			stack;

lib/metadata/metadata-exported.h

@@ -998,6 +998,7 @@ struct lvcreate_params {
 	int approx_alloc;     /* all */
 	alloc_policy_t alloc; /* all */
 	struct dm_vdo_target_params vdo_params; /* vdo */
+	uint64_t post_zero_bytes; /* write zeros to LV after it's created */
 
 	const char *integrity_arg;
 	const char *integrity_meta_name; /* external LV is user-specified */
@@ -1404,5 +1405,6 @@ int lv_create_integrity_metadata(struct cmd_context *cmd,
 			struct volume_group *vg,
 			struct lvcreate_params *lp);
 
+int zero_lv_name(struct cmd_context *cmd, const char *vg_name, const char *lv_name, uint64_t zero_bytes);
 
 #endif

tools/lvcreate.c

@@ -1800,5 +1800,21 @@ int lvcreate(struct cmd_context *cmd, int argc, char **argv)
 
 	_destroy_lvcreate_params(&lp);
 	destroy_processing_handle(cmd, handle);
+
+	/*
+	 * FIXME: make this interruptible. If the user cancels the zeroing,
+	 * they can either use the LV without it being zeroed, or can finish
+	 * zeroing it themselves, e.g. with dd.
+	 *
+	 * FIXME: if the user asked for the lv to be created in inactive
+	 * (-an) then deactivate the LV after zeroing.
+	 */
+	if (lp.post_zero_bytes) {
+		if (!lp.zero)
+			log_warn("WARNING: not zeroing LV, integrity read errors may occur.");
+		else
+			zero_lv_name(cmd, lp.vg_name, lp.lv_name, lp.post_zero_bytes);
+	}
+
 	return ret;
 }