On December 29, 2023 1:41 pm, Friedrich Weber wrote:
> I started testing this and will send a complete mail later, just wanted
> to mention one thing I've stumbled upon.
>
> Consider this pre-upgrade lvm.conf:
>
> devices {
> # added by pve-manager to avoid scanning ZFS zvols
> global_filter=[
> "r|/dev/zd.*|"]
> }
>
> As `lvmconfig` normalizes the linebreak, SET_FILTER is 1 but apparently
> the `sed` command produces a malformed config (I think it comments out
> only the first line, but I didn't check). The validity check fails so
> the pre-upgrade lvm.conf is restored, according to the logs:
>
> '/etc/lvm/lvm.conf' -> '/etc/lvm/lvm.conf.bak' (backup:
> '/etc/lvm/lvm.conf.bak~')
> Setting 'global_filter' in /etc/lvm/lvm.conf to prevent zvols and rbds
> from being scanned:
> global_filter="r|/dev/zd.*|" =>
> global_filter=["r|/dev/zd.*|","r|/dev/rbd.*|"]
> Parse error at byte 103604 (line 2307): unexpected token
> Failed to load config file /etc/lvm/lvm.conf
> Invalid LVM config detected - restoring from /etc/lvm/lvm.conf.bak
> Setting up proxmox-ve (8.1.0) ...
>
> This is quite the edge case. So I'm not sure if it worth the hassle to
> change the logic to handle it properly (especially as the validity check
> handles it somewhat gracefully)?
IMHO this is okay, and the alternative would require basically
re-implementing the LVM config parser and monitoring it for changes
(e.g., quoting, comments, whitespace handling, ...)
the error/warning is also clear enough w.r.t. what the intention is, so
any user with a weirdly formatted config should be able to manually do
the change if desired.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
