OpenPOWER: Ignore duplicate system dump entry
This commit introduces changes to handle scenarios where
the host might report the same system dump entry more than once.
The changes have been made to address two main scenarios:
When the host generates a system dump having the same dump id
and size as an existing entry, the new notification is ignored
if the existing entry is marked as 'completed'. A trace is added
if the existing entry is incomplete.
In cases where the dump id matches an existing entry, but the size
is different, the old entry is considered stale and gets deleted.
A new entry will be created subsequently.
This issue arises particularly during internal failure recovery
in the host, where it notifies all the pending dumps again.
In cases where entries are already available for such dumps on the BMC,
duplicate entries may be created, potentially leading to confusion
for users. This change prevents the creation of new entries if an entry
already exists for the same dump.
Tests:
Create a disruptive system dump and offload
Create a non-disruptive system dump and offload
Signed-off-by: Dhruvaraj Subhashchandran <dhruvaraj@in.ibm.com>
Change-Id: I31d92df43476a19880d084a68e5db17c5207bd81
diff --git a/dump-extensions/openpower-dumps/dump_manager_system.cpp b/dump-extensions/openpower-dumps/dump_manager_system.cpp
index af8972a..fad4947 100644
--- a/dump-extensions/openpower-dumps/dump_manager_system.cpp
+++ b/dump-extensions/openpower-dumps/dump_manager_system.cpp
@@ -29,21 +29,69 @@
std::chrono::system_clock::now().time_since_epoch())
.count();
- // System dump can get created due to a fault in server
- // or by request from user. A system dump by fault is
- // first reported here, but for a user requested dump an
- // entry will be created first with invalid source id.
- // Since there can be only one system dump creation at a time,
- // if there is an entry with invalid sourceId update that.
+ // A system dump can be created due to a fault in the server or by a user
+ // request. A system dump by fault is first reported here, but for a
+ // user-requested dump, an entry will be created first with an invalid
+ // source id. Since only one system dump creation is allowed at a time, if
+ // there's an entry with an invalid sourceId, we will update that entry.
+ openpower::dump::system::Entry* upEntry = nullptr;
for (auto& entry : entries)
{
openpower::dump::system::Entry* sysEntry =
dynamic_cast<openpower::dump::system::Entry*>(entry.second.get());
- if (sysEntry->sourceDumpId() == INVALID_SOURCE_ID)
+
+ // If there's already a completed entry with the input source id and
+ // size, ignore this notification
+ if ((sysEntry->sourceDumpId() == dumpId) && (sysEntry->size() == size))
{
- sysEntry->update(timeStamp, size, dumpId);
- return;
+ if (sysEntry->status() ==
+ phosphor::dump::OperationStatus::Completed)
+ {
+ lg2::info(
+ "System dump entry with source dump id:{SOURCE_ID} and "
+ "size: {SIZE} is already present with entry id:{ID}",
+ "SOURCE_ID", dumpId, "SIZE", size, "ID",
+ sysEntry->getDumpId());
+ return;
+ }
+ else
+ {
+ lg2::error("A duplicate notification for an incomplete dump "
+ "dump id: {SOURCE_ID} entry id: {ID}",
+ "SOURCE_D", dumpId, "ID", sysEntry->getDumpId());
+ upEntry = sysEntry;
+ break;
+ }
}
+ else if (sysEntry->sourceDumpId() == dumpId)
+ {
+ // If the dump id is the same but the size is different, then this
+ // is a new dump. So, delete the stale entry and prepare to create a
+ // new one.
+ lg2::info("A previous dump entry found with same source id: "
+ "{SOURCE_ID}, deleting it, entry id: {DUMP_ID}",
+ "SOURCE_ID", dumpId, "DUMP_ID", sysEntry->getDumpId());
+ sysEntry->delete_();
+ // No 'break' here, as we need to continue checking other entries.
+ }
+
+ // Save the first entry with INVALID_SOURCE_ID, but continue in the loop
+ // to ensure the new entry is not a duplicate.
+ if ((sysEntry->sourceDumpId() == INVALID_SOURCE_ID) &&
+ (upEntry == nullptr))
+ {
+ upEntry = sysEntry;
+ }
+ }
+
+ if (upEntry != nullptr)
+ {
+ lg2::info(
+ "System Dump Notify: Updating dumpId:{ID} Source Id:{SOURCE_ID} "
+ "Size:{SIZE}",
+ "ID", upEntry->getDumpId(), "SOURCE_ID", dumpId, "SIZE", size);
+ upEntry->update(timeStamp, size, dumpId);
+ return;
}
// Get the id
@@ -55,6 +103,9 @@
// For now replacing it with null
try
{
+ lg2::info("System Dump Notify: creating new dump "
+ "entry dumpId:{ID} Source Id:{SOURCE_ID} Size:{SIZE}",
+ "ID", id, "SOURCE_ID", dumpId, "SIZE", size);
entries.insert(std::make_pair(
id, std::make_unique<system::Entry>(
bus, objPath.c_str(), id, timeStamp, size, dumpId,