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

[Eric DeVolder]

Hi, thought I would try again...
eric

On 3/11/20 2:37 PM, Eric DeVolder wrote:
> 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;