kernel_collector: Add efi_pstore driver pattern
With the upstream commit 893c5f1de620fb0134ddef37dfd850f6c639162b, the
efi pstore driver has been renamed from "efi" to "efi_pstore". This
driver name is used as a pattern when collecting and processing kernel
crash files, so we must try to use both patterns in order to be
compatible with both newer and older kernel versions.
Specifically, when collecting files we collect all files matching either
name. When deleting, reading, and checking the type of files, we try
both driver patterns and continue on success, only failing if both
patterns fail the operation.
BUG=b/316205830
TEST=presubmit, build lakitu and lakitu_next images with patch and verify that both collect crash logs correctly
RELEASE_NOTE=None
Change-Id: I34000a7e382c8319ba1fd8fd827b599728ac9acf
Reviewed-on: https://cos-review.googlesource.com/c/third_party/overlays/chromiumos-overlay/+/71351
Tested-by: RBE-prod-presubmit <service-269995278450@remotebuildexecution.iam.gserviceaccount.com>
Reviewed-by: Oleksandr Tymoshenko <ovt@google.com>
Tested-by: Cusky Presubmit Bot <presubmit@cos-infra-prod.iam.gserviceaccount.com>
diff --git a/chromeos-base/crash-reporter/crash-reporter-0.0.1-r4254.ebuild b/chromeos-base/crash-reporter/crash-reporter-0.0.1-r4255.ebuild
similarity index 98%
rename from chromeos-base/crash-reporter/crash-reporter-0.0.1-r4254.ebuild
rename to chromeos-base/crash-reporter/crash-reporter-0.0.1-r4255.ebuild
index b746a42..1975c54 100644
--- a/chromeos-base/crash-reporter/crash-reporter-0.0.1-r4254.ebuild
+++ b/chromeos-base/crash-reporter/crash-reporter-0.0.1-r4255.ebuild
@@ -64,6 +64,10 @@
dev-libs/protobuf
"
+PATCHES=(
+ "${FILESDIR}/0001-kernel_collector-Add-efi_pstore-driver-pattern.patch"
+)
+
src_configure() {
platform_src_configure
use arcpp && use_i686 && platform_src_configure_i686
diff --git a/chromeos-base/crash-reporter/crash-reporter-9999.ebuild b/chromeos-base/crash-reporter/crash-reporter-9999.ebuild
index 4157cb1..605ad1b 100644
--- a/chromeos-base/crash-reporter/crash-reporter-9999.ebuild
+++ b/chromeos-base/crash-reporter/crash-reporter-9999.ebuild
@@ -62,6 +62,10 @@
dev-libs/protobuf
"
+PATCHES=(
+ "${FILESDIR}/0001-kernel_collector-Add-efi_pstore-driver-pattern.patch"
+)
+
src_configure() {
platform_src_configure
use arcpp && use_i686 && platform_src_configure_i686
diff --git a/chromeos-base/crash-reporter/files/0001-kernel_collector-Add-efi_pstore-driver-pattern.patch b/chromeos-base/crash-reporter/files/0001-kernel_collector-Add-efi_pstore-driver-pattern.patch
new file mode 100644
index 0000000..80acb66
--- /dev/null
+++ b/chromeos-base/crash-reporter/files/0001-kernel_collector-Add-efi_pstore-driver-pattern.patch
@@ -0,0 +1,144 @@
+From 81a9af002215a0f8230c6f1eb967982a5d67043c Mon Sep 17 00:00:00 2001
+From: Kevin Berry <kpberry@google.com>
+Date: Wed, 8 May 2024 09:18:12 +0000
+Subject: [PATCH] kernel_collector: Add efi_pstore driver pattern
+
+With the upstream commit 893c5f1de620fb0134ddef37dfd850f6c639162b, the
+efi pstore driver has been renamed from "efi" to "efi_pstore". This
+driver name is used as a pattern when collecting and processing kernel
+crash files, so we must try to use both patterns in order to be
+compatible with both newer and older kernel versions.
+
+Specifically, when collecting files we collect all files matching either
+name. When deleting, reading, and checking the type of files, we try
+both driver patterns and continue on success, only failing if both
+patterns fail the operation.
+
+Change-Id: I0bb6a0dac632404ba683c2153b9fb587352916b4
+---
+ crash-reporter/kernel_collector.cc | 91 ++++++++++++++++++++++++++++-----------------
+ 1 file changed, 56 insertions(+), 35 deletions(-)
+
+diff --git a/kernel_collector.cc b/kernel_collector.cc
+index 231879b456..d560394652 100644
+--- a/kernel_collector.cc
++++ b/kernel_collector.cc
+@@ -47,7 +47,11 @@ constexpr char kDumpPath[] = "/sys/fs/pstore";
+ constexpr char kDumpRecordDmesgName[] = "dmesg";
+ constexpr char kDumpRecordConsoleName[] = "console";
+ constexpr char kDumpDriverRamoopsName[] = "ramoops";
+-constexpr char kDumpDriverEfiName[] = "efi";
++// Older kernel versions use "efi" as the driver name, while newer ones use
++// "efi_pstore".
++constexpr char const* kDumpDriverEfiNames[] = {"efi", "efi_pstore"};
++constexpr size_t numKDumpDriverEfiNames =
++ sizeof(kDumpDriverEfiNames) / sizeof(char const*);
+ // The files take the form <record type>-<driver name>-<record id>.
+ // e.g. console-ramoops-0 or dmesg-ramoops-0.
+ constexpr char kDumpNameFormat[] = "%s-%s-%zu";
+@@ -481,9 +485,20 @@ CrashCollector::ComputedCrashSeverity KernelCollector::ComputeSeverity(
+
+ // Returns file path for corresponding efi crash part.
+ base::FilePath KernelCollector::EfiCrash::GetFilePath(uint32_t part) const {
+- return collector_.dump_path_.Append(
+- StringPrintf("%s-%s-%" PRIu64, kDumpRecordDmesgName, kDumpDriverEfiName,
+- GetIdForPart(part)));
++ base::FilePath partPath;
++ char const* kDumpDriverEfiName;
++
++ for (int i = 0; i < numKDumpDriverEfiNames; i++) {
++ kDumpDriverEfiName = kDumpDriverEfiNames[i];
++ partPath = collector_.dump_path_.Append(
++ StringPrintf("%s-%s-%" PRIu64, kDumpRecordDmesgName, kDumpDriverEfiName,
++ GetIdForPart(part)));
++ if (base::PathExists(partPath)) {
++ break;
++ }
++ }
++
++ return partPath;
+ }
+
+ // Get type of crash.
+@@ -542,41 +557,47 @@ std::vector<KernelCollector::EfiCrash> KernelCollector::FindEfiCrashes() const {
+ return efi_crashes;
+ }
+
+- // Scan /sys/fs/pstore/.
+- std::string efi_crash_pattern =
+- StringPrintf("%s-%s-*", kDumpRecordDmesgName, kDumpDriverEfiName);
+- base::FileEnumerator efi_file_iter(
+- pstore_dir, false, base::FileEnumerator::FILES, efi_crash_pattern);
+-
+- for (auto efi_file = efi_file_iter.Next(); !efi_file.empty();
+- efi_file = efi_file_iter.Next()) {
+- uint64_t crash_id;
+- if (!base::StringToUint64(
+- efi_file.BaseName().value().substr(efi_crash_pattern.length() - 1),
+- &crash_id)) {
+- // This should not ever happen.
+- LOG(ERROR) << "Failed to parse efi file name:"
+- << efi_file.BaseName().value();
+- continue;
+- }
++ char const* kDumpDriverEfiName;
++ for (size_t i = 0; i < numKDumpDriverEfiNames; i++) {
++ kDumpDriverEfiName = kDumpDriverEfiNames[i];
++
++ // Scan /sys/fs/pstore/.
++ std::string efi_crash_pattern =
++ StringPrintf("%s-%s-*", kDumpRecordDmesgName, kDumpDriverEfiName);
++ base::FileEnumerator efi_file_iter(
++ pstore_dir, false, base::FileEnumerator::FILES, efi_crash_pattern);
++
++ for (auto efi_file = efi_file_iter.Next(); !efi_file.empty();
++ efi_file = efi_file_iter.Next()) {
++ uint64_t crash_id;
++ if (!base::StringToUint64(efi_file.BaseName().value().substr(
++ efi_crash_pattern.length() - 1),
++ &crash_id)) {
++ // This should not ever happen.
++ LOG(ERROR) << "Failed to parse efi file name:"
++ << efi_file.BaseName().value();
++ continue;
++ }
+
+- const uint64_t keyed_crash_id = EfiCrash::GetIdForPart(crash_id, 1);
+- std::vector<EfiCrash>::iterator it =
+- std::find_if(efi_crashes.begin(), efi_crashes.end(),
+- [keyed_crash_id](const EfiCrash& efi_crash) -> bool {
+- return efi_crash.GetId() == keyed_crash_id;
+- });
+- if (it != efi_crashes.end()) {
+- // Update part number if its greater.
+- it->UpdateMaxPart(crash_id);
++ const uint64_t keyed_crash_id = EfiCrash::GetIdForPart(crash_id, 1);
++ std::vector<EfiCrash>::iterator it =
++ std::find_if(efi_crashes.begin(), efi_crashes.end(),
++ [keyed_crash_id](const EfiCrash& efi_crash) -> bool {
++ return efi_crash.GetId() == keyed_crash_id;
++ });
++ if (it != efi_crashes.end()) {
++ // Update part number if its greater.
++ it->UpdateMaxPart(crash_id);
+
+- } else {
+- // New crash detected.
+- EfiCrash efi_crash(keyed_crash_id, *this);
+- efi_crash.UpdateMaxPart(crash_id);
+- efi_crashes.push_back(efi_crash);
++ } else {
++ // New crash detected.
++ EfiCrash efi_crash(keyed_crash_id, *this);
++ efi_crash.UpdateMaxPart(crash_id);
++ efi_crashes.push_back(efi_crash);
++ }
+ }
+ }
++
+ return efi_crashes;
+ }
+
+--
+2.45.0.rc1.225.g2a3ae87e7f-goog
+
