[pve-devel] applied: [PATCH pve-manager v4] postinst: filter rbds in lvm

[Fabian Grünbichler]

thanks to both of you!

On December 22, 2023 10:58 am, Stefan Hanreich wrote:
> Since LVM 2.03.15 RBD devices are also scanned by default [1]. This
> can lead to guest volumes being recognized and displayed on the host
> when using KRBD for RBD-backed disks. In order to prevent this we add
> an additional filter to the LVM config to avoid scanning rbds.
> 
> This also prevents a bug where LVM created a very high amount of
> archive entries when there were logical volumes with the same path
> available. This could happen when two guests with RBD disks had the
> same LVM layout or a guest and host had the same layout.
> 
> previous behavior:
> If there is no marker in the LVM conf and global_filter does not
> contain '/dev/zd.*': replace the global_filter with our version
> 
> new behavior:
> Replace the global_filter iff:
> - There is no marker and global_filter is empty
> - The global_filter is exactly the old default
> 
> If we don't replace the filter and it is a non-default value: We print
> a warning. Addtionally we force this function to run once when
> upgrading from older versions.
> 
> The previous versions could replace custom global_filters where the
> comment had been removed and the zvol directive removed. The new
> behavior is slightly more conservative, but works the same in other
> cases.
> 
> [1] https://gitlab.com/lvmteam/lvm2/-/commit/6a431eb24241caf2277d3e5b4718782d92650a2a
> 
> Signed-off-by: Stefan Hanreich <s.hanreich at proxmox.com>
> ---
> 
> Changes from v3 -> v4:
> - Move LVM_SUPPRESS_FD_WARNINGS=1 in order to prevent fd warnings from
>   the lvmconfig invocation
> 
> Changes from v2 -> v3:
> - Additionally only change empty values if there is no marker
> - Print a warning when encountering a non-default value
> - Check the LVM config for validity afterwards and restore it from
>   backup if it is invalid
> 
> Changes from v1 -> v2:
> - changed replacement logic:
>   - if there is an existing global_filter, we replace the line
>   - if there is no existing global_filter we add a whole 'devices' block
> - we only rewrite if there is no global_filter set or if it is the value
> we set in versions <= 8.1.3
> 
>  debian/postinst | 51 +++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 37 insertions(+), 14 deletions(-)
> 
> diff --git a/debian/postinst b/debian/postinst
> index 4c9a1f250..8028e39ee 100755
> --- a/debian/postinst
> +++ b/debian/postinst
> @@ -9,23 +9,33 @@ set -e
>  # installed and configured.
>  
>  set_lvm_conf() {
> +    local FORCE="$1"
>      LVM_CONF_MARKER="# added by pve-manager to avoid scanning"
>  
>      # keep user changes afterwards provided marker is still there..
> -    if grep -qLF "$LVM_CONF_MARKER" /etc/lvm/lvm.conf; then
> +    if grep -qLF "$LVM_CONF_MARKER" /etc/lvm/lvm.conf && test -z "$FORCE"; then
>          return 0 # only do these changes once
>      fi
>  
> -    OLD_VALUE="$(lvmconfig --typeconfig full devices/global_filter)"
> -    NEW_VALUE='global_filter=["r|/dev/zd.*|"]'
> -
>      export LVM_SUPPRESS_FD_WARNINGS=1
>  
> -    # 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
> +    OLD_VALUE="$(lvmconfig --typeconfig diff devices/global_filter || true)"
> +    NEW_VALUE='global_filter=["r|/dev/zd.*|","r|/dev/rbd.*|"]'
> +
> +    # update global_filter if:
> +    # it is empty and there is no marker OR exactly the one we set before 8.1.4
> +    if (! grep -qF "$LVM_CONF_MARKER" /etc/lvm/lvm.conf && test -z "$OLD_VALUE")\
> +	|| (echo "$OLD_VALUE" | grep -qF '="r|/dev/zd.*|"');
> +    then
>          SET_FILTER=1
>          BACKUP=1
> +    # print warning if global_filter is set but not our old/new default
> +    elif test -n "$OLD_VALUE"\
> +	&& ! echo "$OLD_VALUE" | grep -qF '="r|/dev/zd.*|"'\
> +	&& ! echo "$OLD_VALUE" | grep -qF "$NEW_VALUE";
> +    then
> +	echo "non-default 'global_filter' value '$OLD_VALUE' in /etc/lvm/lvm.conf, not setting '$NEW_VALUE' automatically"
> +	echo "consider adapting your 'global_filter' manually."
>      fi
>      # should be the default since bullseye
>      if lvmconfig --typeconfig full devices/scan_lvs | grep -qv 'scan_lvs=0'; then
> @@ -37,17 +47,19 @@ set_lvm_conf() {
>          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 "Setting 'global_filter' in /etc/lvm/lvm.conf to prevent zvols and rbds 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
> +        if test -n "$OLD_VALUE"; then
> +            sed -i -e "s/$LVM_CONF_MARKER ZFS zvols/$LVM_CONF_MARKER ZFS zvols and Ceph rbds/" /etc/lvm/lvm.conf
> +            sed -i -e "s!^\([[:space:]]*\)\(global_filter[[:space:]]*=.*\)\$!\1# \2\n\1$NEW_VALUE!" /etc/lvm/lvm.conf
> +        else
> +            cat >> /etc/lvm/lvm.conf <<EOF
>  devices {
> -     $LVM_CONF_MARKER ZFS zvols
> +     $LVM_CONF_MARKER ZFS zvols and Ceph rbds
>       $NEW_VALUE
> - }
> +}
>  EOF
> +        fi
>      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."
> @@ -61,6 +73,11 @@ devices {
>   }
>  EOF
>      fi
> +
> +    if ! lvmconfig --validate; then
> +	echo "Invalid LVM config detected - restoring from /etc/lvm/lvm.conf.bak"
> +	mv /etc/lvm/lvm.conf.bak /etc/lvm/lvm.conf
> +    fi
>  }
>  
>  migrate_apt_auth_conf() {
> @@ -165,6 +182,12 @@ case "$1" in
>          rm -v "$BETA_SOURCES" || true
>      fi
>  
> +    if test ! -e /proxmox_install_mode && test -n "$2" && dpkg --compare-versions "$2" 'lt' '8.1.4~'; then
> +        if test -e /etc/lvm/lvm.conf ; then
> +            set_lvm_conf 1
> +        fi
> +    fi
> +
>      set_lvm_conf
>  
>      if test ! -e /proxmox_install_mode; then
> -- 
> 2.39.2
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
>