bmcdump: ignore dump request when a dump is in progress
At present generating a user dump is an asynchronous call
and does not block the user to generate another dump.
This can cause out-of-memory issues if dumps are
generated in a loop.
Adding a check to see if the user-initiated dump is
already in progress before starting another user
dump request.
Return "Unavailable" error if generate user dump request is
received while a user dump generation is in progress.
Tested:
'''
root@p10bmc:~# busctl --verbose call xyz.openbmc_project.Dump.Manager
/xyz/openbmc_project/dump/bmc xyz.openbmc_project.Dump.Create CreateDump a{sv} 0
MESSAGE "o" {
OBJECT_PATH "/xyz/openbmc_project/dump/bmc/entry/3";
};
root@p10bmc:~# busctl --verbose call xyz.openbmc_project.Dump.Manager
/xyz/openbmc_project/dump/bmc xyz.openbmc_project.Dump.Create CreateDump a{sv} 0
Call failed: The operation is not allowed
root@p10bmc:~# busctl --verbose call xyz.openbmc_project.Dump.Manager
/xyz/openbmc_project/dump/bmc xyz.openbmc_project.Dump.Create CreateDump a{sv} 0
Call failed: The operation is not allowed
root@p10bmc:~# busctl --verbose call xyz.openbmc_project.Dump.Manager
/xyz/openbmc_project/dump/bmc xyz.openbmc_project.Dump.Create CreateDump a{sv} 0
MESSAGE "o" {
OBJECT_PATH "/xyz/openbmc_project/dump/bmc/entry/4";
};
'''
Signed-off-by: Marri Devender Rao <devenrao@in.ibm.com>
Change-Id: Ic9434f34c040405f8664f7dc71109e7cb67a80c2
diff --git a/dump_manager_bmc.cpp b/dump_manager_bmc.cpp
index 94f8f40..e06c1f6 100644
--- a/dump_manager_bmc.cpp
+++ b/dump_manager_bmc.cpp
@@ -28,6 +28,8 @@
using namespace sdbusplus::xyz::openbmc_project::Common::Error;
using namespace phosphor::logging;
+bool Manager::fUserDumpInProgress = false;
+
namespace internal
{
@@ -47,6 +49,11 @@
"BMC dump accepts not more than 2 additional parameters");
}
+ if (Manager::fUserDumpInProgress == true)
+ {
+ elog<sdbusplus::xyz::openbmc_project::Common::Error::Unavailable>();
+ }
+
// Get the originator id and type from params
std::string originatorId;
originatorTypes originatorType;
@@ -82,6 +89,7 @@
elog<InternalFailure>();
}
+ Manager::fUserDumpInProgress = true;
return objPath.string();
}
@@ -101,7 +109,6 @@
// get dreport type map entry
auto tempType = TypeMap.find(type);
-
execl("/usr/bin/dreport", "dreport", "-d", dumpPath.c_str(), "-i",
id.c_str(), "-s", std::to_string(size).c_str(), "-q", "-v", "-p",
fullPaths.empty() ? "" : fullPaths.front().c_str(), "-t",
@@ -109,25 +116,28 @@
// dreport script execution is failed.
auto error = errno;
- log<level::ERR>(
- fmt::format(
- "Error occurred during dreport function execution, errno({})",
- error)
- .c_str());
+ log<level::ERR>(fmt::format("Error occurred during dreport "
+ "function execution, errno({})",
+ error)
+ .c_str());
elog<InternalFailure>();
}
else if (pid > 0)
{
- auto rc = sd_event_add_child(eventLoop.get(), nullptr, pid,
- WEXITED | WSTOPPED, callback, nullptr);
+ // local variable goes out of scope using pointer, callback method
+ // need to dellocate the pointer
+ Type* typePtr = new Type();
+ *typePtr = type;
+ int rc = sd_event_add_child(eventLoop.get(), nullptr, pid,
+ WEXITED | WSTOPPED, callback,
+ reinterpret_cast<void*>(typePtr));
if (0 > rc)
{
// Failed to add to event loop
- log<level::ERR>(
- fmt::format(
- "Error occurred during the sd_event_add_child call, rc({})",
- rc)
- .c_str());
+ log<level::ERR>(fmt::format("Error occurred during the "
+ "sd_event_add_child call, rc({})",
+ rc)
+ .c_str());
elog<InternalFailure>();
}
}
@@ -139,7 +149,6 @@
.c_str());
elog<InternalFailure>();
}
-
return ++lastEntryId;
}
@@ -192,11 +201,12 @@
catch (const std::invalid_argument& e)
{
log<level::ERR>(
- fmt::format(
- "Error in creating dump entry, errormsg({}), OBJECTPATH({}), "
- "ID({}), TIMESTAMP({}), SIZE({}), FILENAME({})",
- e.what(), objPath.c_str(), id, timestamp,
- std::filesystem::file_size(file), file.filename().c_str())
+ fmt::format("Error in creating dump entry, errormsg({}), "
+ "OBJECTPATH({}), "
+ "ID({}), TIMESTAMP({}), SIZE({}), FILENAME({})",
+ e.what(), objPath.c_str(), id, timestamp,
+ std::filesystem::file_size(file),
+ file.filename().c_str())
.c_str());
return;
}
diff --git a/dump_manager_bmc.hpp b/dump_manager_bmc.hpp
index 6c1910f..9297f6e 100644
--- a/dump_manager_bmc.hpp
+++ b/dump_manager_bmc.hpp
@@ -8,7 +8,6 @@
#include <xyz/openbmc_project/Dump/Create/server.hpp>
#include <filesystem>
-
namespace phosphor
{
namespace dump
@@ -119,10 +118,14 @@
*
* @returns 0 on success, -1 on fail
*/
- static int callback(sd_event_source*, const siginfo_t*, void*)
+ static int callback(sd_event_source*, const siginfo_t*, void* type)
{
- // No specific action required in
- // the sd_event_add_child callback.
+ Type* ptr = reinterpret_cast<Type*>(type);
+ if (*ptr == Type::UserRequested)
+ {
+ fUserDumpInProgress = false;
+ }
+ delete ptr;
return 0;
}
/** @brief Remove specified watch object pointer from the
@@ -146,6 +149,10 @@
/** @brief Path to the dump file*/
std::string dumpDir;
+ /** @brief Flag to reject user intiated dump if a dump is in progress*/
+ // TODO: https://github.com/openbmc/phosphor-debug-collector/issues/19
+ static bool fUserDumpInProgress;
+
/** @brief Child directory path and its associated watch object map
* [path:watch object]
*/