From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
Stefan Hanreich <s.hanreich@proxmox.com>
Subject: Re: [pve-devel] [PATCH pve-manager v4] postinst: filter rbds in lvm
Date: Tue, 09 Jan 2024 10:23:26 +0100 [thread overview]
Message-ID: <1704792084.y7kfxyvj2i.astroid@yuna.none> (raw)
In-Reply-To: <a2719cf9-8e48-476c-bb73-f4aa0d9da32b@proxmox.com>
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.
next prev parent reply other threads:[~2024-01-09 9:24 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-22 9:58 Stefan Hanreich
2023-12-29 12:41 ` Friedrich Weber
2024-01-09 9:23 ` Fabian Grünbichler [this message]
2024-01-09 9:34 ` [pve-devel] applied: " Fabian Grünbichler
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1704792084.y7kfxyvj2i.astroid@yuna.none \
--to=f.gruenbichler@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
--cc=s.hanreich@proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox