Merge ~paelzer/ubuntu/+source/lvm2:fix-1657646-thin-provisioning-tools-MIR-1828887-complete into ubuntu/+source/lvm2:ubuntu/eoan-devel

[Christian Ehrhardt ]

[Christian Ehrhardt  (paelzer)]

IMHO no FF violation as it already was a dependency before.
But we could sort out the promotion in Eoan which would be great (some 20.04 work done already).

[Christian Ehrhardt  (paelzer)]

MIR at https://bugs.launchpad.net/ubuntu/+source/thin-provisioning-tools/+bug/1828887

[Christian Ehrhardt  (paelzer)]

To ensure this is not a crazy to fix late gcc9/glibc build issue I have a PPA at
https://launchpad.net/~paelzer/+archive/ubuntu/eoan-test-build-lvm2

[Christian Ehrhardt  (paelzer)]

@Reviewer - the change is minimal please sponsor right away once (if) you approve

[Dimitri John Ledkov (xnox)]

This is all good, but i would also want to take in a cherrypick from upstream to fix this: https://bugs.launchpad.net/ubuntu/+source/lvm2/+bug/1842436

[Christian Ehrhardt  (paelzer)]

Thank Dimitri, ack to the extra fix that should go on top.
I'll add that somewhen later today.

903e636... by Christian Ehrhardt  on 2019-09-09

d/p/lp-1842436-restrict-PVs-with-mixed-block-size.patch: Avoid creation of mixed-blocksize PV on LVM volume groups (LP: #1842436)

Signed-off-by: Christian Ehrhardt <email address hidden>

94beead... by Christian Ehrhardt  on 2019-09-09

changelog: Avoid creation of mixed-blocksize PV on LVM volume groups (LP: #1842436)

Signed-off-by: Christian Ehrhardt <email address hidden>

[Christian Ehrhardt  (paelzer)]

Preliminary changes for these extra changes are in the MP now and building in the PPA.
Need to test it later on ...

6ee59e1... by Christian Ehrhardt  on 2019-09-09

d/p/lp-1842436-restrict-PVs-with-mixed-block-size.patch: backporting changes

Signed-off-by: Christian Ehrhardt <email address hidden>

[Christian Ehrhardt  (paelzer)]

Needs to use the older dev_get_block_size, WIP and iterating on builds.

[Christian Ehrhardt  (paelzer)]

Functionally this works, I'd love to see a man page or default conf entry for it.
Will take a look for it tomorrow.

405b29b... by Christian Ehrhardt  on 2019-09-10

d/p/lp-1842436-*: add self tests

Signed-off-by: Christian Ehrhardt <email address hidden>

5aef8fa... by Christian Ehrhardt  on 2019-09-10

d/p/lp-1842436-*: add example config entry

Signed-off-by: Christian Ehrhardt <email address hidden>

139f961... by Christian Ehrhardt  on 2019-09-10

changelog: extend 1842436 fix to multiple patches

Signed-off-by: Christian Ehrhardt <email address hidden>

[Christian Ehrhardt  (paelzer)]

Upstream generates these config snippets - lets take the respective snippet from 08396b4bce45fb8311979250623f04ec0ddb628c
Tests are in cd8a0133fa28a0dff2714c80f822e8c665fe6237 and while I don't see them running (in general) it might be safer to add this as well.

On install I see the (expected) conffile prompt and the example I want in the config
        # Configuration option devices/allow_mixed_block_sizes.
        # Allow PVs in the same VG with different logical block sizes.
        # When allowed, the user is responsible to ensure that an LV is
        # using PVs with matching block sizes when necessary.
        allow_mixed_block_sizes = 0

e0ff7df... by Christian Ehrhardt  on 2019-09-10

d/control: add thin-provisioning-tools build-dep as configure wants it around for some checks

Signed-off-by: Christian Ehrhardt <email address hidden>

4d20b23... by Christian Ehrhardt  on 2019-09-10

changelog: mention thin-provisioning-tools build-dep addition

Signed-off-by: Christian Ehrhardt <email address hidden>

[Christian Ehrhardt  (paelzer)]

Finally there were plenty of warnings (working fine eventually, but could easily become more required) from the configure about thin-provisioning-tools.
Since we now properly enable it lets add it as build dep as well, as configure would like to check some detaily (paths, version, ...) about it.

This gets the warnings down by 19 which won't hurt to find actual issues.

[Christian Ehrhardt  (paelzer)]

Ok, all new changes pushed.
Ready for review and sponsoring.

[Dimitri John Ledkov (xnox)]

review: Approve

[Christian Ehrhardt  (paelzer)]

This is on the bring, but I think it needs no FFe as it is really much more a fix than a feature.
Thanks for the review xnox!

Tagged and uploaded to Eoan.

[Christian Ehrhardt  (paelzer)]

This migrated, closing forgotten MP

diff --git a/debian/changelog b/debian/changelog
index ff71edc..c22d661 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,15 @@
+lvm2 (2.03.02-2ubuntu6) eoan; urgency=medium
+
+  * d/control: stop dropping thin-provisioning-tools to Suggests as it
+    is ready to be promoted via MIR LP 1828887. Fixes usability issues
+    of thin-provisioning-tools not being installed by default (LP: #1657646).
+    - d/control: also add thin-provisioning-tools build-dep as configure
+      wants it around for some checks at build time.
+  * d/p/lp-1842436-*: Avoid creation of mixed-blocksize PV on LVM
+    volume groups as it can cause FS corruption (LP: #1842436)
+
+ -- Christian Ehrhardt <[e-mail]>  Fri, 06 Sep 2019 08:23:10 +0200
+
 lvm2 (2.03.02-2ubuntu5) eoan; urgency=medium
 
   * Detect systemd in udev rule at runtime.
diff --git a/debian/control b/debian/control
index 3d0ef7f..8dd64a8 100644
--- a/debian/control
+++ b/debian/control
@@ -25,7 +25,8 @@ Build-Depends:
  python3-dbus,
  python3-pyudev,
  pkg-config,
- systemd
+ systemd,
+ thin-provisioning-tools
 Standards-Version: 4.1.1
 Homepage: http://sources.redhat.com/lvm2/
 Vcs-Git: https://salsa.debian.org/lvm-team/lvm2.git
@@ -34,7 +35,7 @@ Vcs-Browser: https://salsa.debian.org/lvm-team/lvm2
 Package: lvm2
 Architecture: linux-any
 Depends: ${shlibs:Depends}, ${misc:Depends}, lsb-base, dmsetup (>= 1.02.145-2~), dmeventd
-Suggests: thin-provisioning-tools
+Recommends: thin-provisioning-tools
 Conflicts: clvm
 Multi-Arch: foreign
 Description: Linux Logical Volume Manager
diff --git a/debian/patches/lp-1842436-make-generate-example-config.patch b/debian/patches/lp-1842436-make-generate-example-config.patch
new file mode 100644
index 0000000..b814788
--- /dev/null
+++ b/debian/patches/lp-1842436-make-generate-example-config.patch
@@ -0,0 +1,37 @@
+From 08396b4bce45fb8311979250623f04ec0ddb628c Mon Sep 17 00:00:00 2001
+From: Zdenek Kabelac <[e-mail]>
+Date: Fri, 9 Aug 2019 12:56:28 +0200
+Subject: [PATCH] make: generate
+
+Run make generate.
+
+Backport-Note: only take the snippet related to allow_mixed_block_sizes
+Origin: backport, https://sourceware.org/git/?p=lvm2.git;a=commit;h=08396b4bce45fb8311979250623f04ec0ddb628c
+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1842436
+Last-Update: 2019-09-10
+
+---
+ conf/example.conf.in   | 18 ++++++-----
+ man/lvconvert.8_pregen | 70 +++++++++++++++++++++++-------------------
+ 2 files changed, 48 insertions(+), 40 deletions(-)
+
+diff --git a/conf/example.conf.in b/conf/example.conf.in
+index 154f621bc..91ce5bec6 100644
+--- a/conf/example.conf.in
++++ b/conf/example.conf.in
+@@ -347,6 +347,12 @@ devices {
+ 	# Enabling this setting allows the VG to be used as usual even with
+ 	# uncertain devices.
+ 	allow_changes_with_duplicate_pvs = 0
++
++	# Configuration option devices/allow_mixed_block_sizes.
++	# Allow PVs in the same VG with different logical block sizes.
++	# When allowed, the user is responsible to ensure that an LV is
++	# using PVs with matching block sizes when necessary.
++	allow_mixed_block_sizes = 0
+ }
+ 
+ # Configuration section allocation.
+-- 
+2.23.0
+
diff --git a/debian/patches/lp-1842436-restrict-PVs-with-mixed-block-size.patch b/debian/patches/lp-1842436-restrict-PVs-with-mixed-block-size.patch
new file mode 100644
index 0000000..06fd1ae
--- /dev/null
+++ b/debian/patches/lp-1842436-restrict-PVs-with-mixed-block-size.patch
@@ -0,0 +1,215 @@
+From 0404539edb25e4a9d3456bb3e6b402aa2767af6b Mon Sep 17 00:00:00 2001
+From: David Teigland <[e-mail]>
+Date: Thu, 1 Aug 2019 10:06:47 -0500
+Subject: [PATCH] vgcreate/vgextend: restrict PVs with mixed block sizes
+
+Avoid having PVs with different logical block sizes in the same VG.
+This prevents LVs from having mixed block sizes, which can produce
+file system errors.
+
+The new config setting devices/allow_mixed_block_sizes (default 0)
+can be changed to 1 to return to the unrestricted mode.
+
+Backport-Notes:
+- tools/lvmcmdline.c misses the context of the new change
+- dev_get_direct_block_sizes didn't exist, use dev_get_block_size
+
+Origin: backport, https://sourceware.org/git/?p=lvm2.git;a=commit;h=0404539edb25e4a9d3456bb3e6b402aa2767af6b
+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1842436
+Last-Update: 2019-09-09
+
+---
+ lib/commands/toolcontext.h       |  1 +
+ lib/config/config_settings.h     |  5 ++++
+ lib/metadata/metadata-exported.h |  1 +
+ lib/metadata/metadata.c          | 44 ++++++++++++++++++++++++++++++
+ tools/lvmcmdline.c               |  2 ++
+ tools/toollib.c                  | 47 ++++++++++++++++++++++++++++++++
+ tools/vgcreate.c                 |  2 ++
+ 7 files changed, 102 insertions(+)
+
+--- a/lib/commands/toolcontext.h
++++ b/lib/commands/toolcontext.h
+@@ -153,6 +153,7 @@ struct cmd_context {
+ 	unsigned include_shared_vgs:1;		/* report/display cmds can reveal lockd VGs */
+ 	unsigned include_active_foreign_vgs:1;	/* cmd should process foreign VGs with active LVs */
+ 	unsigned vg_read_print_access_error:1;	/* print access errors from vg_read */
++	unsigned allow_mixed_block_sizes:1;
+ 	unsigned force_access_clustered:1;
+ 	unsigned lockd_gl_disable:1;
+ 	unsigned lockd_vg_disable:1;
+--- a/lib/config/config_settings.h
++++ b/lib/config/config_settings.h
+@@ -448,6 +448,11 @@ cfg(devices_allow_changes_with_duplicate
+ 	"Enabling this setting allows the VG to be used as usual even with\n"
+ 	"uncertain devices.\n")
+ 
++cfg(devices_allow_mixed_block_sizes_CFG, "allow_mixed_block_sizes", devices_CFG_SECTION, 0, CFG_TYPE_BOOL, 0, vsn(2, 3, 6), NULL, 0, NULL,
++	"Allow PVs in the same VG with different logical block sizes.\n"
++	"When allowed, the user is responsible to ensure that an LV is\n"
++	"using PVs with matching block sizes when necessary.\n")
++
+ cfg_array(allocation_cling_tag_list_CFG, "cling_tag_list", allocation_CFG_SECTION, CFG_DEFAULT_UNDEFINED, CFG_TYPE_STRING, NULL, vsn(2, 2, 77), NULL, 0, NULL,
+ 	"Advise LVM which PVs to use when searching for new space.\n"
+ 	"When searching for free space to extend an LV, the 'cling' allocation\n"
+--- a/lib/metadata/metadata-exported.h
++++ b/lib/metadata/metadata-exported.h
+@@ -608,6 +608,7 @@ struct pvcreate_params {
+ 	unsigned is_remove : 1;         /* is removing PVs, not creating */
+ 	unsigned preserve_existing : 1;
+ 	unsigned check_failed : 1;
++	unsigned check_consistent_block_size : 1;
+ };
+ 
+ struct lvresize_params {
+--- a/lib/metadata/metadata.c
++++ b/lib/metadata/metadata.c
+@@ -669,12 +669,40 @@ int vg_extend_each_pv(struct volume_grou
+ {
+ 	struct pv_list *pvl;
+ 	unsigned int max_phys_block_size = 0;
++	unsigned int physical_block_size, logical_block_size;
++	unsigned int prev_lbs = 0;
++	int inconsistent_existing_lbs = 0;
+ 
+ 	log_debug_metadata("Adding PVs to VG %s.", vg->name);
+ 
+ 	if (vg_bad_status_bits(vg, RESIZEABLE_VG))
+ 		return_0;
+ 
++	/*
++	 * Check if existing PVs have inconsistent block sizes.
++	 * If so, do not enforce new devices to be consistent.
++	 */
++	dm_list_iterate_items(pvl, &vg->pvs) {
++		logical_block_size = 0;
++		physical_block_size = 0;
++
++		if (!dev_get_block_size(pvl->pv->dev, &physical_block_size, &logical_block_size))
++			continue;
++
++		if (!logical_block_size)
++			continue;
++
++		if (!prev_lbs) {
++			prev_lbs = logical_block_size;
++			continue;
++		}
++		
++		if (prev_lbs != logical_block_size) {
++			inconsistent_existing_lbs = 1;
++			break;
++		}
++	}
++
+ 	dm_list_iterate_items(pvl, &pp->pvs) {
+ 		log_debug_metadata("Adding PV %s to VG %s.", pv_dev_name(pvl->pv), vg->name);
+ 
+@@ -685,6 +713,22 @@ int vg_extend_each_pv(struct volume_grou
+ 			return 0;
+ 		}
+ 
++		logical_block_size = 0;
++		physical_block_size = 0;
++
++		if (!dev_get_block_size(pvl->pv->dev, &physical_block_size, &logical_block_size))
++			log_warn("WARNING: PV %s has unknown block size.", pv_dev_name(pvl->pv));
++
++		else if (prev_lbs && logical_block_size && (logical_block_size != prev_lbs)) {
++			if (vg->cmd->allow_mixed_block_sizes || inconsistent_existing_lbs)
++				log_debug("Devices have inconsistent block sizes (%u and %u)", prev_lbs, logical_block_size);
++			else {
++				log_error("Devices have inconsistent logical block sizes (%u and %u).",
++					  prev_lbs, logical_block_size);
++				return 0;
++			}
++		}
++
+ 		if (!add_pv_to_vg(vg, pv_dev_name(pvl->pv), pvl->pv, 0)) {
+ 			log_error("PV %s cannot be added to VG %s.",
+ 				  pv_dev_name(pvl->pv), vg->name);
+--- a/tools/lvmcmdline.c
++++ b/tools/lvmcmdline.c
+@@ -2312,6 +2312,8 @@ static int _get_current_settings(struct
+ 	if (cmd->cname->flags & CAN_USE_ONE_SCAN)
+ 		cmd->can_use_one_scan = 1;
+ 
++	cmd->allow_mixed_block_sizes = find_config_tree_bool(cmd, devices_allow_mixed_block_sizes_CFG, NULL);
++
+ 	cmd->partial_activation = 0;
+ 	cmd->degraded_activation = 0;
+ 	activation_mode = find_config_tree_str(cmd, activation_mode_CFG, NULL);
+--- a/tools/toollib.c
++++ b/tools/toollib.c
+@@ -5363,6 +5363,8 @@ int pvcreate_each_device(struct cmd_cont
+ 	struct pv_list *vgpvl;
+ 	struct device_list *devl;
+ 	const char *pv_name;
++	unsigned int physical_block_size, logical_block_size;
++	unsigned int prev_pbs = 0, prev_lbs = 0;
+ 	int must_use_all = (cmd->cname->flags & MUST_USE_ALL_ARGS);
+ 	int found;
+ 	unsigned i;
+@@ -5415,6 +5417,51 @@ int pvcreate_each_device(struct cmd_cont
+ 		pd->dev = dev_cache_get(cmd, pd->name, cmd->filter);
+ 
+ 	/*
++	 * Check for consistent block sizes.
++	 */
++	if (pp->check_consistent_block_size) {
++		dm_list_iterate_items(pd, &pp->arg_devices) {
++			if (!pd->dev)
++				continue;
++
++			logical_block_size = 0;
++			physical_block_size = 0;
++
++			if (!dev_get_block_size(pd->dev, &physical_block_size, &logical_block_size)) {
++				log_warn("WARNING: Unknown block size for device %s.", dev_name(pd->dev));
++				continue;
++			}
++
++			if (!logical_block_size) {
++				log_warn("WARNING: Unknown logical_block_size for device %s.", dev_name(pd->dev));
++				continue;
++			}
++
++			if (!prev_lbs) {
++				prev_lbs = logical_block_size;
++				prev_pbs = physical_block_size;
++				continue;
++			}
++
++			if (prev_lbs == logical_block_size) {
++				/* Require lbs to match, just warn about unmatching pbs. */
++				if (!cmd->allow_mixed_block_sizes && prev_pbs && physical_block_size &&
++				    (prev_pbs != physical_block_size))
++					log_warn("WARNING: Devices have inconsistent physical block sizes (%u and %u).",
++						  prev_pbs, physical_block_size);
++				continue;
++			}
++
++			if (!cmd->allow_mixed_block_sizes) {
++				log_error("Devices have inconsistent logical block sizes (%u and %u).",
++					  prev_lbs, logical_block_size);
++				log_print("See lvm.conf allow_mixed_block_sizes.");
++				return 0;
++			}
++		}
++	}
++
++	/*
+ 	 * Use process_each_pv to search all existing PVs and devices.
+ 	 *
+ 	 * This is a slightly different way to use process_each_pv, because the
+--- a/tools/vgcreate.c
++++ b/tools/vgcreate.c
+@@ -47,6 +47,8 @@ int vgcreate(struct cmd_context *cmd, in
+ 	/* Don't create a new PV on top of an existing PV like pvcreate does. */
+ 	pp.preserve_existing = 1;
+ 
++	pp.check_consistent_block_size = 1;
++
+ 	if (!vgcreate_params_set_defaults(cmd, &vp_def, NULL))
+ 		return EINVALID_CMD_LINE;
+ 	vp_def.vg_name = vg_name;
diff --git a/debian/patches/lp-1842436-tests-allow-mixed-block-sizes.patch b/debian/patches/lp-1842436-tests-allow-mixed-block-sizes.patch
new file mode 100644
index 0000000..5494984
--- /dev/null
+++ b/debian/patches/lp-1842436-tests-allow-mixed-block-sizes.patch
@@ -0,0 +1,68 @@
+From cd8a0133fa28a0dff2714c80f822e8c665fe6237 Mon Sep 17 00:00:00 2001
+From: David Teigland <[e-mail]>
+Date: Thu, 1 Aug 2019 10:13:41 -0500
+Subject: [PATCH] tests: allow-mixed-block-sizes
+
+
+Origin: upstream, https://sourceware.org/git/?p=lvm2.git;a=commit;h=cd8a0133fa28a0dff2714c80f822e8c665fe6237
+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1842436
+Last-Update: 2019-09-10
+
+---
+ test/shell/allow-mixed-block-sizes.sh | 44 +++++++++++++++++++++++++++
+ 1 file changed, 44 insertions(+)
+ create mode 100644 test/shell/allow-mixed-block-sizes.sh
+
+diff --git a/test/shell/allow-mixed-block-sizes.sh b/test/shell/allow-mixed-block-sizes.sh
+new file mode 100644
+index 000000000..18032561f
+--- /dev/null
++++ b/test/shell/allow-mixed-block-sizes.sh
+@@ -0,0 +1,44 @@
++#!/usr/bin/env bash
++
++# Copyright (C) 2019 Red Hat, Inc. All rights reserved.
++#
++# This copyrighted material is made available to anyone wishing to use,
++# modify, copy, or redistribute it subject to the terms and conditions
++# of the GNU General Public License v.2.
++#
++# You should have received a copy of the GNU General Public License
++# along with this program; if not, write to the Free Software Foundation,
++# Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
++
++SKIP_WITH_LVMPOLLD=1
++
++. lib/inittest
++
++dd if=/dev/zero of=loopa bs=$((1024*1024)) count=2 2> /dev/null
++dd if=/dev/zero of=loopb bs=$((1024*1024)) count=2 2> /dev/null
++LOOP1=$(losetup -f loopa --sector-size 4096 --show)
++LOOP2=$(losetup -f loopb --show)
++
++echo $LOOP1
++echo $LOOP2
++
++aux extend_filter "a|$LOOP1|"
++aux extend_filter "a|$LOOP2|"
++
++not vgcreate --config 'devices {allow_mixed_block_sizes=0 scan="/dev"}' $vg $LOOP1 $LOOP2
++vgcreate --config 'devices {allow_mixed_block_sizes=1 scan="/dev"}' $vg $LOOP1 $LOOP2
++vgs --config 'devices {allow_mixed_block_sizes=1 scan="/dev"}' $vg
++
++wipefs -a $LOOP1
++wipefs -a $LOOP2
++
++vgcreate --config 'devices {allow_mixed_block_sizes=1 scan="/dev"}' $vg $LOOP1
++vgs --config 'devices {allow_mixed_block_sizes=1 scan="/dev"}' $vg
++not vgextend --config 'devices {allow_mixed_block_sizes=0 scan="/dev"}' $vg $LOOP2
++vgextend --config 'devices {allow_mixed_block_sizes=1 scan="/dev"}' $vg $LOOP2
++
++losetup -d $LOOP1
++losetup -d $LOOP2
++rm loopa
++rm loopb
++
+-- 
+2.23.0
+
diff --git a/debian/patches/series b/debian/patches/series
index b8eaedc..ce362dc 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -13,3 +13,6 @@ blkdeactivate-use-usr-bin-sort.patch
 0010-make-generate-config-update.patch
 fix-systemd-run-location.patch
 Detect-systemd-at-run-time-in-69-dm-lvm-metad.rules.patch
+lp-1842436-restrict-PVs-with-mixed-block-size.patch
+lp-1842436-tests-allow-mixed-block-sizes.patch
+lp-1842436-make-generate-example-config.patch