[pve-devel] [PATCH manager 1/2] postinst: set custom LVM settings

[pve-devel] [PATCH manager 1/2] postinst: set custom LVM settings

[Fabian Grünbichler]

now that we no longer ship our own LVM packages, set the relevant
filtering options here if they are missing.

for an upgrade from PVE 6.x, the following two scenarios are likely:

A: user edited config provided by our old lvm2 package. it likely
contains our (or a modified) global_filter, but the old scan_lvs
default. in this case we ignore global_filter as long as it contains our
'don't scan zvols' entry, and set scan_lvs to false.

B: config provided by our old lvm2 package was taken over by default
config from stock lvm2 package. scan_lvs defaults to false already, but
global_filter is unset (scan everything), so we need to set our own
global_filter excluding zvols.

other combinations should be handled fine as well.

for new installs (installer, install on top of Debian Bullseye) we are
always in scenario B.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    once other difference between our old config and the stock one is that we had
    'issue_discards' enabled. we could either put this in the release notes, or
    also enable it here automatically - but it is less straight-forward since the
    default is not "almost certainly wrong" like for the filtering options..
    
    we could drop the "check for marker" and just do this once on initial install
    and upgrades from 6.x, but since the fallout from not having these in place can
    be data corruption (activating multiple VGs with same name, using one from a
    guest on the host!) I'd rather play it safe..

 debian/postinst | 55 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/debian/postinst b/debian/postinst
index dcf77d24..241aac08 100755
--- a/debian/postinst
+++ b/debian/postinst
@@ -8,6 +8,58 @@ set -e
 # done its automatic conffile handling, and all the packages we depend
 # of are already fully installed and configured.
 
+LVM_CONF_MARKER="# added by pve-manager to avoid scanning"
+
+set_lvm_conf() {
+    OLD_VALUE="$(lvmconfig --typeconfig full devices/global_filter)"
+    NEW_VALUE='global_filter=["r|/dev/zd.*|"]'
+
+    # only do these changes once
+    # keep user changes afterwards provided marker is still there..
+    if grep -qv "$LVM_CONF_MARKER" /etc/lvm/lvm.conf; then
+	# check global_filter
+	# keep previous setting from our custom packaging if it is still there
+	if echo "$OLD_VALUE" | grep -qvF 'r|/dev/zd.*|'; then
+	    SET_FILTER=1
+	    BACKUP=1
+	fi
+	# should be the default since bullseye
+	if lvmconfig --typeconfig full devices/scan_lvs | grep -qv 'scan_lvs=0'; then
+	    SET_SCAN_LVS=1
+	    BACKUP=1
+	fi
+	if test -n "$BACKUP"; then
+	    echo "Backing up lvm.conf before setting pve-manager specific settings.."
+	    cp -vb /etc/lvm/lvm.conf /etc/lvm/lvm.conf.bak
+	fi
+	if test -n "$SET_FILTER"; then
+	    echo "Setting 'global_filter' in /etc/lvm/lvm.conf to prevent zvols from being scanned:"
+	    echo "$OLD_VALUE => $NEW_VALUE"
+	    # comment out existing setting
+	    sed -i -e 's/^\([[:space:]]*global_filter[[:space:]]*=\)/#\1/' /etc/lvm/lvm.conf
+	    # add new section with our setting
+	    cat >> /etc/lvm/lvm.conf <<EOF
+devices {
+	 $LVM_CONF_MARKER ZFS zvols
+	 global_filter=$NEW_VALUE
+}
+EOF
+	fi
+	if test -n "$SET_SCAN_LVS"; then
+	    echo "Adding scan_lvs=0 setting to /etc/lvm/lvm.conf to prevent LVs from being scanned."
+	    # comment out existing setting
+	    sed -i -e 's/^\([[:space:]]*scan_lvs[[:space:]]*=\)/#\1/' /etc/lvm/lvm.conf
+	    # add new section with our setting
+	    cat >> /etc/lvm/lvm.conf <<EOF
+devices {
+	 $LVM_CONF_MARKER LVM volumes
+	 scan_lvs=0
+}
+EOF
+	fi
+    fi
+}
+
 case "$1" in
   triggered)
     # We don't print a status message here, as dpkg already said
@@ -86,6 +138,9 @@ case "$1" in
 	    newaliases || true
 	fi
     fi
+
+    set_lvm_conf
+
     ;;
 
   abort-upgrade|abort-remove|abort-deconfigure)
-- 
2.30.2

[pve-devel] [PATCH manager 2/2] postinst: remove outdated calls

[Fabian Grünbichler]

any system upgrading to 7.x was either installed with >= 6.4 in the
first place, or upgraded to >= 6.4 and thus fixed those issues already.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 debian/postinst | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/debian/postinst b/debian/postinst
index 241aac08..d845b082 100755
--- a/debian/postinst
+++ b/debian/postinst
@@ -132,13 +132,6 @@ case "$1" in
 	done
     fi
 
-    # TODO: remove once PVE 7.0 is released
-    if test -n "$2"; then
-	if dpkg --compare-versions "$2" 'lt' '6.0-11'; then
-	    newaliases || true
-	fi
-    fi
-
     set_lvm_conf
 
     ;;
@@ -150,7 +143,4 @@ case "$1" in
      exit 0;;
 esac
 
-# FIXME: remove in 7.0
-dpkg-maintscript-helper rm_conffile /etc/apt/apt.conf.d/75pveconf 6.0-0\+3 pve-manager -- "$@"
-
 exit 0
-- 
2.30.2

Re: [pve-devel] [PATCH manager 1/2] postinst: set custom LVM settings

[Fabian Grünbichler]

On June 22, 2021 12:16 pm, Fabian Grünbichler wrote:
> now that we no longer ship our own LVM packages, set the relevant
> filtering options here if they are missing.
> 
> for an upgrade from PVE 6.x, the following two scenarios are likely:
> 
> A: user edited config provided by our old lvm2 package. it likely
> contains our (or a modified) global_filter, but the old scan_lvs
> default. in this case we ignore global_filter as long as it contains our
> 'don't scan zvols' entry, and set scan_lvs to false.
> 
> B: config provided by our old lvm2 package was taken over by default
> config from stock lvm2 package. scan_lvs defaults to false already, but
> global_filter is unset (scan everything), so we need to set our own
> global_filter excluding zvols.
> 
> other combinations should be handled fine as well.
> 
> for new installs (installer, install on top of Debian Bullseye) we are
> always in scenario B.
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
> 
> Notes:
>     once other difference between our old config and the stock one is that we had
>     'issue_discards' enabled. we could either put this in the release notes, or
>     also enable it here automatically - but it is less straight-forward since the
>     default is not "almost certainly wrong" like for the filtering options..
>     
>     we could drop the "check for marker" and just do this once on initial install
>     and upgrades from 6.x, but since the fallout from not having these in place can
>     be data corruption (activating multiple VGs with same name, using one from a
>     guest on the host!) I'd rather play it safe..
> 
>  debian/postinst | 55 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
> 
> diff --git a/debian/postinst b/debian/postinst
> index dcf77d24..241aac08 100755
> --- a/debian/postinst
> +++ b/debian/postinst
> @@ -8,6 +8,58 @@ set -e
>  # done its automatic conffile handling, and all the packages we depend
>  # of are already fully installed and configured.
>  
> +LVM_CONF_MARKER="# added by pve-manager to avoid scanning"
> +
> +set_lvm_conf() {
> +    OLD_VALUE="$(lvmconfig --typeconfig full devices/global_filter)"
> +    NEW_VALUE='global_filter=["r|/dev/zd.*|"]'
> +
> +    # only do these changes once
> +    # keep user changes afterwards provided marker is still there..
> +    if grep -qv "$LVM_CONF_MARKER" /etc/lvm/lvm.conf; then
> +	# check global_filter
> +	# keep previous setting from our custom packaging if it is still there
> +	if echo "$OLD_VALUE" | grep -qvF 'r|/dev/zd.*|'; then
> +	    SET_FILTER=1
> +	    BACKUP=1
> +	fi
> +	# should be the default since bullseye
> +	if lvmconfig --typeconfig full devices/scan_lvs | grep -qv 'scan_lvs=0'; then
> +	    SET_SCAN_LVS=1
> +	    BACKUP=1
> +	fi
> +	if test -n "$BACKUP"; then
> +	    echo "Backing up lvm.conf before setting pve-manager specific settings.."
> +	    cp -vb /etc/lvm/lvm.conf /etc/lvm/lvm.conf.bak
> +	fi
> +	if test -n "$SET_FILTER"; then
> +	    echo "Setting 'global_filter' in /etc/lvm/lvm.conf to prevent zvols from being scanned:"
> +	    echo "$OLD_VALUE => $NEW_VALUE"
> +	    # comment out existing setting
> +	    sed -i -e 's/^\([[:space:]]*global_filter[[:space:]]*=\)/#\1/' /etc/lvm/lvm.conf
> +	    # add new section with our setting
> +	    cat >> /etc/lvm/lvm.conf <<EOF
> +devices {
> +	 $LVM_CONF_MARKER ZFS zvols
> +	 global_filter=$NEW_VALUE

this should of course not have the 'global_filter=' prefix (moved that 
into NEW_VALUE before sending and forgot to update this line :-/), but 
rather be just

+	 $NEW_VALUE

> +}
> +EOF
> +	fi
> +	if test -n "$SET_SCAN_LVS"; then
> +	    echo "Adding scan_lvs=0 setting to /etc/lvm/lvm.conf to prevent LVs from being scanned."
> +	    # comment out existing setting
> +	    sed -i -e 's/^\([[:space:]]*scan_lvs[[:space:]]*=\)/#\1/' /etc/lvm/lvm.conf
> +	    # add new section with our setting
> +	    cat >> /etc/lvm/lvm.conf <<EOF
> +devices {
> +	 $LVM_CONF_MARKER LVM volumes
> +	 scan_lvs=0
> +}
> +EOF
> +	fi
> +    fi
> +}
> +
>  case "$1" in
>    triggered)
>      # We don't print a status message here, as dpkg already said
> @@ -86,6 +138,9 @@ case "$1" in
>  	    newaliases || true
>  	fi
>      fi
> +
> +    set_lvm_conf
> +
>      ;;
>  
>    abort-upgrade|abort-remove|abort-deconfigure)
> -- 
> 2.30.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 

Re: [pve-devel] [PATCH manager 1/2] postinst: set custom LVM settings

[Thomas Lamprecht]

On 22.06.21 12:16, Fabian Grünbichler wrote:
> now that we no longer ship our own LVM packages, set the relevant
> filtering options here if they are missing.
> 
> for an upgrade from PVE 6.x, the following two scenarios are likely:
> 
> A: user edited config provided by our old lvm2 package. it likely
> contains our (or a modified) global_filter, but the old scan_lvs
> default. in this case we ignore global_filter as long as it contains our
> 'don't scan zvols' entry, and set scan_lvs to false.
> 
> B: config provided by our old lvm2 package was taken over by default
> config from stock lvm2 package. scan_lvs defaults to false already, but
> global_filter is unset (scan everything), so we need to set our own
> global_filter excluding zvols.
> 
> other combinations should be handled fine as well.
> 
> for new installs (installer, install on top of Debian Bullseye) we are
> always in scenario B.
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
> 
> Notes:
>     once other difference between our old config and the stock one is that we had
>     'issue_discards' enabled. we could either put this in the release notes, or
>     also enable it here automatically - but it is less straight-forward since the
>     default is not "almost certainly wrong" like for the filtering options..
>     
>     we could drop the "check for marker" and just do this once on initial install
>     and upgrades from 6.x, but since the fallout from not having these in place can
>     be data corruption (activating multiple VGs with same name, using one from a
>     guest on the host!) I'd rather play it safe..
> 
>  debian/postinst | 55 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
> 
> diff --git a/debian/postinst b/debian/postinst
> index dcf77d24..241aac08 100755
> --- a/debian/postinst
> +++ b/debian/postinst
> @@ -8,6 +8,58 @@ set -e
>  # done its automatic conffile handling, and all the packages we depend
>  # of are already fully installed and configured.
>  
> +LVM_CONF_MARKER="# added by pve-manager to avoid scanning"
> +
> +set_lvm_conf() {

I'd maybe set the "don't whine about open FDs other than 0, 1, 2" flag here (weird thing
to check for anyway):

export LVM_SUPPRESS_FD_WARNINGS=1

> +    OLD_VALUE="$(lvmconfig --typeconfig full devices/global_filter)"
> +    NEW_VALUE='global_filter=["r|/dev/zd.*|"]'
> +
> +    # only do these changes once
> +    # keep user changes afterwards provided marker is still there..
> +    if grep -qv "$LVM_CONF_MARKER" /etc/lvm/lvm.conf; then

the FD leak notice was always shown twice to me, so I checked more closely and
even though my lvm.conf has the marker intact and a manual call of the grep
exits as expected the code still takes this branch, quite weird IMO & surely
something stupid on my side, but did not yet found the cause..

> +	# check global_filter
> +	# keep previous setting from our custom packaging if it is still there
> +	if echo "$OLD_VALUE" | grep -qvF 'r|/dev/zd.*|'; then
> +	    SET_FILTER=1
> +	    BACKUP=1
> +	fi
> +	# should be the default since bullseye
> +	if lvmconfig --typeconfig full devices/scan_lvs | grep -qv 'scan_lvs=0'; then
> +	    SET_SCAN_LVS=1
> +	    BACKUP=1
> +	fi
> +	if test -n "$BACKUP"; then
> +	    echo "Backing up lvm.conf before setting pve-manager specific settings.."
> +	    cp -vb /etc/lvm/lvm.conf /etc/lvm/lvm.conf.bak
> +	fi
> +	if test -n "$SET_FILTER"; then
> +	    echo "Setting 'global_filter' in /etc/lvm/lvm.conf to prevent zvols from being scanned:"
> +	    echo "$OLD_VALUE => $NEW_VALUE"
> +	    # comment out existing setting
> +	    sed -i -e 's/^\([[:space:]]*global_filter[[:space:]]*=\)/#\1/' /etc/lvm/lvm.conf
> +	    # add new section with our setting
> +	    cat >> /etc/lvm/lvm.conf <<EOF
> +devices {
> +	 $LVM_CONF_MARKER ZFS zvols
> +	 global_filter=$NEW_VALUE
> +}
> +EOF
> +	fi
> +	if test -n "$SET_SCAN_LVS"; then
> +	    echo "Adding scan_lvs=0 setting to /etc/lvm/lvm.conf to prevent LVs from being scanned."
> +	    # comment out existing setting
> +	    sed -i -e 's/^\([[:space:]]*scan_lvs[[:space:]]*=\)/#\1/' /etc/lvm/lvm.conf
> +	    # add new section with our setting
> +	    cat >> /etc/lvm/lvm.conf <<EOF
> +devices {
> +	 $LVM_CONF_MARKER LVM volumes
> +	 scan_lvs=0
> +}
> +EOF
> +	fi
> +    fi
> +}
> +
>  case "$1" in
>    triggered)
>      # We don't print a status message here, as dpkg already said
> @@ -86,6 +138,9 @@ case "$1" in
>  	    newaliases || true
>  	fi
>      fi
> +
> +    set_lvm_conf
> +
>      ;;
>  
>    abort-upgrade|abort-remove|abort-deconfigure)
> 

[pve-devel] applied: [PATCH manager 2/2] postinst: remove outdated calls

[Thomas Lamprecht]

On 22.06.21 12:16, Fabian Grünbichler wrote:
> any system upgrading to 7.x was either installed with >= 6.4 in the
> first place, or upgraded to >= 6.4 and thus fixed those issues already.
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
>  debian/postinst | 10 ----------
>  1 file changed, 10 deletions(-)
> 
>

applied this one, thanks!