Add dm-integrity support

Create a linear LV with integrity added to it:

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

or

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

When creating, --zero y will zero the entire LV.
This is suggested to initialize integrity checksums
and avoid causing read errors in unwritten portions
of the new LV.

Options:

  --integrity internal
  integrity checksum metadata is interleaved with data.

  --integrity external
  integrity checksum metadata on separate LV, allows
  adding integrity to existing LV, or removing integrity.

  --integritymetadata LV
  use the specified LV for external metadata.

  --integritysettings String
  set dm-integrity parameters.

TODO:

- for internal, zero start of origin LV

- increase allocated LV size by an extra extent, and then
  round provided_data_sectors down to be a multiple of extent size

- 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,6 +252,8 @@ 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,14 +118,6 @@ 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,15 +29,6 @@
 
 #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
@@ -48,7 +39,7 @@ int lv_create_integrity_metadata(struct cmd_context *cmd,
 				struct lvcreate_params *lp)
 {
 	char metaname[NAME_LEN];
-	uint64_t lv_size_bytes, meta_bytes, meta_sectors;
+	uint32_t extent_bytes;
 	struct logical_volume *lv;
 	struct lvcreate_params lp_meta = {
 		.activate = CHANGE_AN,
@@ -72,15 +63,12 @@ int lv_create_integrity_metadata(struct cmd_context *cmd,
 	if (!(lp_meta.lv_name = strdup(metaname)))
 		return_0;
 
-	lp_meta.pvh = lp->pvh;
+	/* 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;
 
-	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 %u extents, %s",
-		  metaname, lp_meta.extents, display_size(cmd, meta_sectors));
+	log_debug("Creating integrity metadata LV with %u extents", lp_meta.extents);
 
 	dm_list_init(&lp_meta.tags);
 
@@ -99,7 +87,6 @@ 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;
@@ -128,15 +115,7 @@ static int _get_provided_data_sectors(struct logical_volume *lv, uint64_t *provi
 		goto fail;
 	}
 
-	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;
+	*provided_data_sectors = status.seg_status.integrity->provided_data_sectors;
 
 	dm_pool_destroy(status.seg_status.mem);
 	return 1;
@@ -152,18 +131,13 @@ 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;
 
@@ -213,19 +187,10 @@ 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(vg, meta_name))) {
+		if (!(meta_lv = find_lv(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;
-		}
 	}
 
 	/*
@@ -243,37 +208,16 @@ 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;
 		}
 
-		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 = 1;
-
-		if (!wipe_lv(meta_lv, wipe)) {
-			log_error("Failed to zero LV for integrity metadata %s", display_lvname(meta_lv));
+		if (!activate_and_wipe_lv(meta_lv, 0)) {
+			log_error("LV %s could not be zeroed.", display_lvname(meta_lv));
 			return 0;
 		}
-
-		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_data_sectors = seg->len;
 		seg->integrity_meta_dev = meta_lv;
 		lv_set_hidden(meta_lv);
 		/* TODO: give meta_lv a suffix? e.g. _imeta */
@@ -313,11 +257,6 @@ 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

@@ -55,8 +55,6 @@ 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.
  */
@@ -7426,72 +7424,11 @@ int insert_layer_for_segments_on_pv(struct cmd_context *cmd,
 	return 1;
 }
 
-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 ...", name, display_size(cmd, zero_bytes/512));
-	log_print("Cancel command to zero device manually.");
-
-	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.
-	 */
-
-	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 (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);
-		}
-	}
-
-	/*
-	 * 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);
-
-	label_scan_invalidate(dev);
-	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 */
 
 /*
  * Initialize the LV with 'value'.
@@ -7551,7 +7488,44 @@ int wipe_lv(struct logical_volume *lv, struct wipe_params wp)
 			stack;
 	}
 
-	if (wp.do_zero) {
+	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) {
 		zero_sectors = wp.zero_sectors ? : UINT64_C(4096) >> SECTOR_SHIFT;
 
 		if (zero_sectors > lv->size)
@@ -7762,14 +7736,10 @@ static int _should_wipe_lv(struct lvcreate_params *lp,
 	     first_seg(first_seg(lv)->pool_lv)->zero_new_blocks))
 		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;
+	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;
+	}
 
 	/* Cannot zero read-only volume */
 	if ((lv->status & LVM_WRITE) &&
@@ -8028,6 +7998,13 @@ 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;
 
@@ -8255,6 +8232,12 @@ 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);
 	/*
@@ -8374,25 +8357,27 @@ 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 (_should_wipe_lv(lp, lv, !lp->suppress_zero_warn)) {
+	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 (!wipe_lv(lv, (struct wipe_params)
 			     {
 				     .do_zero = lp->zero,
@@ -8406,39 +8391,6 @@ 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 */
-		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,7 +998,6 @@ 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 */
@@ -1405,6 +1404,5 @@ 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,17 +1800,5 @@ 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)
-		zero_lv_name(cmd, lp.vg_name, lp.lv_name, lp.post_zero_bytes);
-
 	return ret;
 }