all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Stefan Hanreich <s.hanreich@proxmox.com>
To: "Proxmox VE development discussion" <pve-devel@lists.proxmox.com>,
	"Fabian Grünbichler" <f.gruenbichler@proxmox.com>
Subject: Re: [pve-devel] [PATCH pve-manager v2] postinst: filter rbds in lvm
Date: Tue, 19 Dec 2023 14:13:15 +0100	[thread overview]
Message-ID: <61f7d1e6-d1cb-4076-b67f-9a15ee9bee41@proxmox.com> (raw)
In-Reply-To: <1702986417.rjiew8jrm6.astroid@yuna.none>

Although already shortly discussed off-list, here the summary of the
discussion. v3 coming soon.

On 12/19/23 12:54, Fabian Grünbichler wrote:
> this part is now a lot stricter then before (e.g., if the user has
> added multipath devices or something else to the filter for whatever
> reason, the filter won't be extended).

Yes indeed, although if I stuck with the previous logic the filter would
have just gotten replaced which is arguably worse (imo). That's what I
wanted to prevent with the change.

> should we at least print a warning in that case?

> iff
> - the config is not default (OLD_VALUE is set)
> - the old value is neither our expected old value nor our new value
> 
> 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."

Yes, that sounds sensible - I'll add printing a warning as else

> also, the combination of marker found, but no $OLD_VALUE would indicate
> that the user explicitly disabled/commented our previously set value -
> maybe in that case we also should just print a warning instead of
> overriding that choice?


Yes indeed I hadn't thought of that - so we need to change the logic to
this:

$MARKER_FOUND = grep -qLF "$LVM_CONF_MARKER" /etc/lvm/lvm.conf

(!$MARKER_FOUND && test -z "$OLD_VALUE")
|| (echo "$OLD_VALUE" | grep -qF '"r|/dev/zd.*|"')




  reply	other threads:[~2023-12-19 13:13 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-15 13:51 Stefan Hanreich
2023-12-15 14:10 ` Stefan Hanreich
2023-12-19 11:54 ` Fabian Grünbichler
2023-12-19 13:13   ` Stefan Hanreich [this message]
2023-12-19 12:10 ` Friedrich Weber

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=61f7d1e6-d1cb-4076-b67f-9a15ee9bee41@proxmox.com \
    --to=s.hanreich@proxmox.com \
    --cc=f.gruenbichler@proxmox.com \
    --cc=pve-devel@lists.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal