Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

udev/69-dm-lvm-metad.rules.in shouldn't invoke systemd-run from BINDIR #36

Closed
flokli opened this issue Jul 12, 2020 · 11 comments
Closed

Comments

@flokli
Copy link

flokli commented Jul 12, 2020

https://github.com/lvmteam/lvm2/blob/master/udev/69-dm-lvm-metad.rules.in#L111 calls systemd-run from (BINDIR), which is not necessarily where the systemd binaries are installed.

This should probably be discovered from $PATH and made overridable, like THIN_*_CMD and others.

Reading 2c42f60, it seems it's necessary to invoke systemd-run here, and it's not possible to model this differently.

@zkabelac
Copy link

zkabelac commented Feb 1, 2023

metad has been eliminated from lvm2 project
so the rule itself is also no longer installed -> nothing to fix here.

@zkabelac zkabelac closed this as completed Feb 1, 2023
@flokli
Copy link
Author

flokli commented Feb 1, 2023

Thanks! Since which version is this gone?

@zkabelac
Copy link

zkabelac commented Feb 1, 2023

Since commit 67722b3 we have a new way how to 'autoactivate' volume.
So lvmetad has been eliminated.
However looking further at our rule - we have 'systemd-run' executed also from 69-dm-lvm.rules rule and for this moment actually with wrong full path in place (aka /usr/bin/systemd-run - and this should be actually converted to BINDIR in the '.in' file - so it's converted to proper path while running configure at build time.

I'm actually not fully understanding what the you mean by $PATH - since there is 'no path' inside udev rules - everything must go with full explicit path.

So can you more describe what do you want to achieve with this - do you want to see 'configure --system-run-path' option or what is actually wanted ?

@zkabelac zkabelac reopened this Feb 1, 2023
@flokli
Copy link
Author

flokli commented Feb 1, 2023

The problem is that --bindir is not necessarily pointing to the same directory that systemd-run is installed in, so using $(BINDIR)/systemctl might not work. Having a way to specify the path to the systemd-run executable (or the path to where it should look like for it) would work.

I think, long-term lvm shouldn't be shelling out to systemd-run at all, but instead ship systemd (template) service files, which are then activated by adding them via ENV{SYSTEMD_WANTS} in the udev rule: https://systemd.network/systemd.device.html#SYSTEMD_WANTS=

@prajnoha
Copy link
Member

prajnoha commented Feb 2, 2023

We already used SYSTEMD_WANTS before, but there were issues with it - the service is edge-triggered based on flipping the SYSTEMD_READY variable from 0 to 1 at the same time as the SYSTEMD_WANTS is specified - so it requires these to conditions to be met, not just the SYSTEMD_WANTS. Unfortunately, this was not always the case where needed and sometimes it even caused interference (e.g. issue #94).

@zkabelac
Copy link

zkabelac commented Feb 2, 2023

There are also other issues - like basically there is no way to synchronize with 'parallel' stuff executed by systemd services - so it's been causing us basically highly unpredictable device access pattern - while we more prefer execution of scanning inline with udev rule. Also we do plan to store more data inside udevDB - so further daemons like 'udisk2' would not need to scan device again - yet the exact plan is still being analysed for this.

But it's relatively easy to provide a configure option for 'systemd-run' path if that's good for something - although I'd be interesting in which systems/conditions this utility is outside of 'default' /usr/bin directory ?

@flokli
Copy link
Author

flokli commented Feb 2, 2023

This is Nix(OS), and we set BINDIR to a path unique to each build. That path only contains lvm2 outputs, and everything else is referenced similarly by unique paths.

You don't need to really support much of this, but asking for the path to the systemd-run executable (or the "bin" directory in which systemd-run itself is contained (and possibly other systemd binaries)) in the configure script will make this less brittle for us - we currently patch paths manually.

@zkabelac
Copy link

zkabelac commented Feb 2, 2023

This is Nix(OS), and we set BINDIR to a path unique to each build. That path only contains lvm2 outputs, and everything else is referenced similarly by unique paths.

You don't need to really support much of this, but asking for the path to the systemd-run executable (or the "bin" directory in which systemd-run itself is contained (and possibly other systemd binaries)) in the configure script will make this less brittle for us - we currently patch paths manually.

NixOS should be well capable to build 'virtual' /usr/bin for each individual package you have in your system (the whole magic behind NixOS) - so there really should be no reason to configure/tune 'lvm2' specifically for NixOS - so IMHO you are most likely not using it to its full potential here - also note - lvm2 calls number of other binaries (i.e. with lvresize ) and I'm not exactly we provide configurable for every single system's binary tool we use here - we simply rely on /usr/bin...

Referring systemd's binaries in the 'non-standard' directories may actually case number of weird issue on it's own....

Also it's worth to be noted - lvm2 is NOT supposed to be executed inside a 'container' - it's always closely tied to system binaries connected to kernel (udevd) - so just ensuring you are not trying to do something invalid....

@flokli
Copy link
Author

flokli commented Feb 2, 2023

NixOS should be well capable to build 'virtual' /usr/bin for each individual package you have in your system (the whole magic behind NixOS)

No, it's all hardcoded paths in /nix/store/…, that we patch during build time.

I'm not exactly we provide configurable for every single system's binary tool we use here - we simply rely on /usr/bin...

Are you? I was assuming you rely on $PATH for these tools, and NixOS does populate a $PATH set so that tools can be found when invoking lvresize and friends from a shell…

Referring systemd's binaries in the 'non-standard' directories may actually case number of weird issue on it's own....

We already do right now - by patching in these paths ourselves, rather than letting the configure script do it for us. See https://github.com/NixOS/nixpkgs/blob/c11c55dd08a6cb91f92e83adc8522e4fb469d2a9/pkgs/os-specific/linux/lvm2/common.nix#L77-L97

Also it's worth to be noted - lvm2 is NOT supposed to be executed inside a 'container' - it's always closely tied to system binaries connected to kernel (udevd) - so just ensuring you are not trying to do something invalid....

We're not executing things more in a container than other distros - it's just that on NixOS, /usr/bin, /bin and other FHS paths are mostly empty, so things need to refer to other binaries by absolute paths to be able to find them


As I said above, having a configure knob to configure the location to systemd-run would help, but if you don't want to do that, we can still keep patching the different occurences ourselves before build.

@zkabelac
Copy link

zkabelac commented Feb 3, 2023

The way we were using for a while NixOS - we were just using 'packages' as they are and building some 'virtual' 'container-like' environment for testing - so we could test various combination of libraries, without the need to rebuild a single package - but surely the use-case might have been a bit strange.

The issue why we prefer full explicit paths for executed binaries is security reason - since lvm2 is using root privileges we want to lower the risk associated with 'modified' $PATH (although surely we know it's not a big protection) - so if we rely on $PATH somewhere, it's rather an overlook.

But if you provide configure patch for all these paths you need to change - it's not such a big deal to add this support instead of doing local hacking - there could be maybe a fine grained system BINDIR which could differ from lvm2 bindir

@zkabelac
Copy link

Upstream git HEAD now also supports 'configure --with-systemd-run=/path/systemd-run' option.

https://listman.redhat.com/archives/lvm-devel/2023-February/024591.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants