[systemd-devel] [RFC] pstore: options to enable kernel writing into the pstore

[Eric DeVolder]

Systemd-devel,

Below is a proposal for adding a couple of settings to the systemd pstore
service so that it can enable the kernel parameters that allow the
kernel to write into the pstore.

Regards,
eric


 From 837d716c6e7ed02518a399356df95bf7c47e1772 Mon Sep 17 00:00:00 2001
From: Eric DeVolder <eric.devolder at oracle.com>
Date: Wed, 11 Mar 2020 14:11:03 -0500
Subject: [RFC] pstore: options to enable kernel writing into the pstore

The systemd pstore service archives the contents of /sys/fs/pstore
upon boot so that there is room for a subsequent dump. The pstore is
usually backed by flash memory typically in the vicinity of 64KB.  The
pstore can contain post-mortem debug information even if kdump fails
or is not enabld.

The issue is that while the service is present, the kernel still needs
to be configured to write data into the pstore. It has two parameters,
crash_kexec_post_notifiers and printk.always_kmsg_dump, that control
writes into pstore.

The crash_kexec_post_notifiers parameter enables the kernel to write
dmesg (including stack trace) into pstore upon a panic, and
printk.always_kmsg_dump parameter enables the kernel to write dmesg upon
a shutdown (shutdown, reboot, halt).

As it stands today, these parameters are not managed/manipulated by the
systemd pstore service, and are solely reliant upon the user [to have
the foresight] to set them on the kernel command line at boot, or post
boot via sysfs. Furthermore, the user would need to set these parameters
in a persistent fashion so that that they are enabled on subsequent
reboots.

This patch allows the user to set these parameters via the systemd
pstore service, and forget about it. This patch introduces two new
settings in the pstore.conf, 'kmsg' and 'crash'. If either of these
is set to true, then the corresponding parameter is enabled in the
kernel. If the setting is false, then the parameter is not touched,
thus preserving whatever behavior the user may have previously
chosen.
---
  src/pstore/pstore.c | 36 +++++++++++++++++++++++++++++++++++-
  1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/src/pstore/pstore.c b/src/pstore/pstore.c
index 5c812b5d5b..02bd94751f 100644
--- a/src/pstore/pstore.c
+++ b/src/pstore/pstore.c
@@ -68,6 +68,10 @@ static DEFINE_CONFIG_PARSE_ENUM(config_parse_pstore_storage, pstore_storage, PSt
  static PStoreStorage arg_storage = PSTORE_STORAGE_EXTERNAL;

  static bool arg_unlink = true;
+static bool arg_kmsg = false;
+static bool arg_crash = false;
+static const char *arg_kmsg_path = "/sys/module/printk/parameters/always_kmsg_dump";
+static const char *arg_crash_path = "/sys/module/kernel/parameters/crash_kexec_post_notifiers";
  static const char *arg_sourcedir = "/sys/fs/pstore";
  static const char *arg_archivedir = "/var/lib/systemd/pstore";

@@ -75,6 +79,8 @@ static int parse_config(void) {
          static const ConfigTableItem items[] = {
                  { "PStore", "Unlink",  config_parse_bool,           0, &arg_unlink },
                  { "PStore", "Storage", config_parse_pstore_storage, 0, &arg_storage },
+                { "PStore", "kmsg",    config_parse_bool,           0, &arg_kmsg },
+                { "PStore", "crash",   config_parse_bool,           0, &arg_crash },
                  {}
          };

@@ -363,7 +369,7 @@ static int list_files(PStoreList *list, const char *sourcepath) {

  static int run(int argc, char *argv[]) {
          _cleanup_(pstore_entries_reset) PStoreList list = {};
-        int r;
+        int fd, r;

          log_setup_service();

@@ -380,6 +386,34 @@ static int run(int argc, char *argv[]) {
          log_debug("Selected storage: %s.", pstore_storage_to_string(arg_storage));
          log_debug("Selected unlink: %s.", yes_no(arg_unlink));

+        if (arg_kmsg) {
+                /* Only enable if requested; otherwise do not touch the parameter */
+                /* NOTE: These errors are not fatal */
+                fd = open(arg_kmsg_path, O_WRONLY|O_CLOEXEC);
+                if (fd < 0)
+                        log_error_errno(r, "Failed to open %s: %m", arg_kmsg_path);
+                r = write(fd, "Y", 1);
+                if (r != 1)
+                        log_error_errno(r, "Failed to write: %m");
+                else
+                        log_debug("Set printk.always_kmsg_dump.");
+                close(fd);
+        }
+
+        if (arg_crash) {
+                /* Only enable if requested; otherwise do not touch the parameter */
+                /* NOTE: These errors are not fatal */
+                fd = open(arg_crash_path, O_WRONLY|O_CLOEXEC);
+                if (fd < 0)
+                        log_error_errno(r, "Failed to open %s: %m", arg_crash_path);
+                r = write(fd, "Y", 1);
+                if (r != 1)
+                        log_error_errno(r, "Failed to write: %m");
+                else
+                        log_debug("Set crash_kexec_post_notifiers.");
+                close(fd);
+        }
+
          if (arg_storage == PSTORE_STORAGE_NONE)
                  /* Do nothing, intentionally, leaving pstore untouched */
                  return 0;
-- 
2.20.1