Re: [pve-devel] [PATCH pve-manager] postinst: Filter RADOS block devices

[Stefan Hanreich <s.hanreich@proxmox.com>]

On 12/13/23 16:35, 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 RADOS block
> devices.
> 
> 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.
> 
> The following cases can happen where postinst gets executed:
> 
> Upgrading from < 8.1.4: We force the new global_filter to be set in
> the LVM config
> 
> Upgrading from >= 8.1.4: do nothing
> 
> New Installation: Run the function as before, just with the new
> global_filter value
> 
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
> I have tested this for all cases by executing the script manually in a
> VM.
> 
> Just in the case of upgrading from < 8.1.4 the resulting LVM config is
> quite ugly:
> 
> ```
> devices {
>      # added by pve-manager to avoid scanning ZFS zvols
> #     global_filter=["r|/dev/zd.*|"]
>  }
> devices {
>      # added by pve-manager to avoid scanning ZFS zvols and RADOS block devices
>      global_filter=["r|/dev/zd.*|","r|/dev/rbd.*|"]
>  }
> ```
> 
> Trying to find and delete the existing, enclosing devices {} part also
> seemed a bit brittle to me, particularly since users could be adding
> custom values to this section as well - which we would have to handle
> then as well.
> 
> Does anyone maybe have a better idea on how to handle this without
> generating such an ugly config?
> 
> 
> 
>  debian/postinst | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/debian/postinst b/debian/postinst
> index 4c9a1f250..59d88105c 100755
> --- a/debian/postinst
> +++ b/debian/postinst
> @@ -9,21 +9,25 @@ 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
>  
> +    FILTER_VALUE='"r|/dev/zd.*|","r|/dev/rbd.*|"'
> +
>      OLD_VALUE="$(lvmconfig --typeconfig full devices/global_filter)"
> -    NEW_VALUE='global_filter=["r|/dev/zd.*|"]'
> +    NEW_VALUE="global_filter=[$FILTER_VALUE]"
>  
>      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
> +    if echo "$OLD_VALUE" | grep -qvF "$FILTER_VALUE"; then

This would also replace global_filter directives where
`"r|/dev/zd.*|","r|/dev/rbd.*|"` is only a part of the global_filter. In
order to prevent this I would have to include the square brackets as well.

Friedrich volunteered for testing, so I'll wait for his review with a v2
(and maybe some possible suggestions on how to prevent the resulting
ugly config)

>          SET_FILTER=1
>          BACKUP=1
>      fi
> @@ -37,14 +41,14 @@ 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
>  devices {
> -     $LVM_CONF_MARKER ZFS zvols
> +     $LVM_CONF_MARKER ZFS zvols and RADOS block devices
>       $NEW_VALUE
>   }
>  EOF
> @@ -165,6 +169,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