nixos/filesystems: mount-pstore.service improvements

[ghost]

Motivation for this change

The previous implementation was not functioning correctly for configurations other than CONFIG_PSTORE m (the NixOS default). This replaces the modprobe@.service dependency with a script (for n) and waits for the backend to be registered even if the persistent storage is already mounted (for y).

See also #123902 (comment).

Things done

• Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
• Built on platform(s)
  • NixOS
  • macOS
  • other Linux distributions
• Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
• Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
• Tested execution of all binary files (usually in ./result/bin/)
• Added a release notes entry if the change is major or breaking
• Fits CONTRIBUTING.md.

[ajs124]

Works on my machine.

[ajs124]

Anything I can do to move this forward? I have a bunch of machines running the same kernel without pstore and while I could fix it in the config for them, this PR seems like a more appropriate fix.

[ghost]

Sorry, I'll get on this today. I want to rewrite the service because I noticed systemd-pstore can get started too early with CONFIG_PSTORE=y.

[nixos-discourse]

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/387

[ju1m]

My system has no pstore backend (neither efi_pstore because this system, Lenovo X201, does not support EFI, nor ramoops because it requires parameters like mem_address=0x30000000 mem_size=0x100000 'memmap=0x100000$0x30000000' to work) and thus mount-pstore.service now fails with:

-- Boot aa39f8583cf04d349906b9060d09f98d --
juil. 26 15:11:59 oignon jdxyhkqa3q589ji0y3y98aq4kppbfi01-mount-pstore.sh[679]: Persistent Storage backend was not registered in time.
juil. 26 15:11:59 oignon systemd[1]: mount-pstore.service: Main process exited, code=exited, status=1/FAILURE
juil. 26 15:11:59 oignon systemd[1]: mount-pstore.service: Failed with result 'exit-code'.
juil. 26 15:11:59 oignon systemd[1]: Failed to start mount-pstore.service.

My unknowledgeable guess is that some condition should be added somewhere to disable mount-pstore.service in my case, maybe using the fact that efiSupport = false, or maybe something more clever.

I'm using boot.kernelPackages = pkgs.linuxPackages_hardened:

$ zgrep -i pstore /proc/config.gz 
CONFIG_EFI_VARS_PSTORE=m
# CONFIG_EFI_VARS_PSTORE_DEFAULT_DISABLE is not set
CONFIG_PSTORE=m
CONFIG_PSTORE_DEFLATE_COMPRESS=m
CONFIG_PSTORE_LZO_COMPRESS=m
CONFIG_PSTORE_LZ4_COMPRESS=m
CONFIG_PSTORE_LZ4HC_COMPRESS=m
# CONFIG_PSTORE_842_COMPRESS is not set
# CONFIG_PSTORE_ZSTD_COMPRESS is not set
CONFIG_PSTORE_COMPRESS=y
CONFIG_PSTORE_DEFLATE_COMPRESS_DEFAULT=y
# CONFIG_PSTORE_LZO_COMPRESS_DEFAULT is not set
# CONFIG_PSTORE_LZ4_COMPRESS_DEFAULT is not set
# CONFIG_PSTORE_LZ4HC_COMPRESS_DEFAULT is not set
CONFIG_PSTORE_COMPRESS_DEFAULT="deflate"
# CONFIG_PSTORE_CONSOLE is not set
# CONFIG_PSTORE_PMSG is not set
# CONFIG_PSTORE_FTRACE is not set
CONFIG_PSTORE_RAM=m

[ghost]

As I understand it there isn't really any way good way to tell whether a persistent store is available (since there are a number of possibilities) until the kernel does or doesn't manage to use it.

You will want to simply add systemd.services.mount-pstore.enable = false to your configuration to suppress this.

[ajs124]

Turns out, this breaks the unstable-small channel: https://hydra.nixos.org/eval/1689382 -> https://nix-cache.s3.amazonaws.com/log/2wi0635565fzq9pjx410cipmfqggj4qk-vm-test-run-installer-simple.drv

[ghost]

I see, these virtual machines come without a pstore backend and the failure to activate all services is treated as an unsuccessful test, which makes sense - and so the failure is a much bigger issue than when a human can judge it to be correct or not. Considering other automated tools will probably relying on the absence of failures, adding some config to make this succeed seems inappropriate, since it would need to be replicated everywhere.
Though it is unfortunate that some people will not be noticeably alerted to a real failure of their backend this way, the cost of that seems smaller to me.

[ghost]

Should be fixed in #131587, currently running the test locally to confirm.