fw-update : Optimize passing infra parameters
Refactor UpdateManager and DeviceUpdater to optimize passing the D-Bus event
loop handle, handler for requester and instance ID requester.
Signed-off-by: Tom Joseph <rushtotom@gmail.com>
Change-Id: I1860ffc49513d941af372423a197b1d42eda6e36
diff --git a/fw-update/device_updater.cpp b/fw-update/device_updater.cpp
index 63e4f73..9a46a15 100644
--- a/fw-update/device_updater.cpp
+++ b/fw-update/device_updater.cpp
@@ -15,7 +15,7 @@
void DeviceUpdater::startFwUpdateFlow()
{
- auto instanceId = requester.getInstanceId(eid);
+ auto instanceId = updateManager->requester.getInstanceId(eid);
// NumberOfComponents
const auto& applicableComponents =
std::get<ApplicableComponents>(fwDeviceIDRecord);
@@ -44,13 +44,13 @@
sizeof(struct pldm_request_update_req) + compImgSetVerStrInfo.length);
if (rc)
{
- requester.markFree(eid, instanceId);
+ updateManager->requester.markFree(eid, instanceId);
std::cerr << "encode_request_update_req failed, EID=" << unsigned(eid)
<< ", RC=" << rc << "\n";
// Handle error scenario
}
- rc = handler.registerRequest(
+ rc = updateManager->handler.registerRequest(
eid, instanceId, PLDM_FWUP, PLDM_REQUEST_UPDATE, std::move(request),
std::move(std::bind_front(&DeviceUpdater::requestUpdate, this)));
if (rc)
@@ -95,15 +95,16 @@
// Optional fields DeviceMetaData and GetPackageData not handled
pldmRequest = std::make_unique<sdeventplus::source::Defer>(
- event, std::bind(&DeviceUpdater::sendPassCompTableRequest, this,
- componentIndex));
+ updateManager->event,
+ std::bind(&DeviceUpdater::sendPassCompTableRequest, this,
+ componentIndex));
}
void DeviceUpdater::sendPassCompTableRequest(size_t offset)
{
pldmRequest.reset();
- auto instanceId = requester.getInstanceId(eid);
+ auto instanceId = updateManager->requester.getInstanceId(eid);
// TransferFlag
const auto& applicableComponents =
std::get<ApplicableComponents>(fwDeviceIDRecord);
@@ -166,13 +167,13 @@
sizeof(pldm_pass_component_table_req) + compVerStrInfo.length);
if (rc)
{
- requester.markFree(eid, instanceId);
+ updateManager->requester.markFree(eid, instanceId);
std::cerr << "encode_pass_component_table_req failed, EID="
<< unsigned(eid) << ", RC=" << rc << "\n";
// Handle error scenario
}
- rc = handler.registerRequest(
+ rc = updateManager->handler.registerRequest(
eid, instanceId, PLDM_FWUP, PLDM_PASS_COMPONENT_TABLE,
std::move(request),
std::move(std::bind_front(&DeviceUpdater::passCompTable, this)));
@@ -226,15 +227,17 @@
{
componentIndex = 0;
pldmRequest = std::make_unique<sdeventplus::source::Defer>(
- event, std::bind(&DeviceUpdater::sendUpdateComponentRequest, this,
- componentIndex));
+ updateManager->event,
+ std::bind(&DeviceUpdater::sendUpdateComponentRequest, this,
+ componentIndex));
}
else
{
componentIndex++;
pldmRequest = std::make_unique<sdeventplus::source::Defer>(
- event, std::bind(&DeviceUpdater::sendPassCompTableRequest, this,
- componentIndex));
+ updateManager->event,
+ std::bind(&DeviceUpdater::sendPassCompTableRequest, this,
+ componentIndex));
}
}
@@ -242,7 +245,7 @@
{
pldmRequest.reset();
- auto instanceId = requester.getInstanceId(eid);
+ auto instanceId = updateManager->requester.getInstanceId(eid);
const auto& applicableComponents =
std::get<ApplicableComponents>(fwDeviceIDRecord);
const auto& comp = compImageInfos[applicableComponents[offset]];
@@ -290,13 +293,13 @@
sizeof(pldm_update_component_req) + compVerStrInfo.length);
if (rc)
{
- requester.markFree(eid, instanceId);
+ updateManager->requester.markFree(eid, instanceId);
std::cerr << "encode_update_component_req failed, EID=" << unsigned(eid)
<< ", RC=" << rc << "\n";
// Handle error scenario
}
- rc = handler.registerRequest(
+ rc = updateManager->handler.registerRequest(
eid, instanceId, PLDM_FWUP, PLDM_UPDATE_COMPONENT, std::move(request),
std::move(std::bind_front(&DeviceUpdater::updateComponent, this)));
if (rc)
@@ -594,15 +597,16 @@
{
componentIndex = 0;
pldmRequest = std::make_unique<sdeventplus::source::Defer>(
- event,
+ updateManager->event,
std::bind(&DeviceUpdater::sendActivateFirmwareRequest, this));
}
else
{
componentIndex++;
pldmRequest = std::make_unique<sdeventplus::source::Defer>(
- event, std::bind(&DeviceUpdater::sendUpdateComponentRequest, this,
- componentIndex));
+ updateManager->event,
+ std::bind(&DeviceUpdater::sendUpdateComponentRequest, this,
+ componentIndex));
}
return response;
@@ -611,7 +615,7 @@
void DeviceUpdater::sendActivateFirmwareRequest()
{
pldmRequest.reset();
- auto instanceId = requester.getInstanceId(eid);
+ auto instanceId = updateManager->requester.getInstanceId(eid);
Request request(sizeof(pldm_msg_hdr) +
sizeof(struct pldm_activate_firmware_req));
auto requestMsg = reinterpret_cast<pldm_msg*>(request.data());
@@ -621,12 +625,12 @@
sizeof(pldm_activate_firmware_req));
if (rc)
{
- requester.markFree(eid, instanceId);
+ updateManager->requester.markFree(eid, instanceId);
std::cerr << "encode_activate_firmware_req failed, EID="
<< unsigned(eid) << ", RC=" << rc << "\n";
}
- rc = handler.registerRequest(
+ rc = updateManager->handler.registerRequest(
eid, instanceId, PLDM_FWUP, PLDM_ACTIVATE_FIRMWARE, std::move(request),
std::move(std::bind_front(&DeviceUpdater::activateFirmware, this)));
if (rc)
diff --git a/fw-update/device_updater.hpp b/fw-update/device_updater.hpp
index 95f8adf..03ee0c3 100644
--- a/fw-update/device_updater.hpp
+++ b/fw-update/device_updater.hpp
@@ -35,9 +35,6 @@
/** @brief Constructor
*
* @param[in] eid - Endpoint ID of the firmware device
- * @param[in] event - PLDM daemon's main event loop
- * @param[in] requester - Instance ID manager for PLDM requests
- * @param[in] handler - PLDM request handler
* @param[in] package - File stream for firmware update package
* @param[in] fwDeviceIDRecord - FirmwareDeviceIDRecord in the fw update
* package that matches this firmware device
@@ -50,19 +47,16 @@
* @param[in] updateManager - To update the status of fw update of the
* device
*/
- explicit DeviceUpdater(
- mctp_eid_t eid, sdeventplus::Event& event,
- pldm::dbus_api::Requester& requester,
- pldm::requester::Handler<pldm::requester::Request>& handler,
- std::ifstream& package, const FirmwareDeviceIDRecord& fwDeviceIDRecord,
- const ComponentImageInfos& compImageInfos,
- const ComponentInfo& compInfo, uint32_t maxTransferSize,
- UpdateManager* updateManager) :
+ explicit DeviceUpdater(mctp_eid_t eid, std::ifstream& package,
+ const FirmwareDeviceIDRecord& fwDeviceIDRecord,
+ const ComponentImageInfos& compImageInfos,
+ const ComponentInfo& compInfo,
+ uint32_t maxTransferSize,
+ UpdateManager* updateManager) :
eid(eid),
- event(event), requester(requester), handler(handler), package(package),
- fwDeviceIDRecord(fwDeviceIDRecord), compImageInfos(compImageInfos),
- compInfo(compInfo), maxTransferSize(maxTransferSize),
- updateManager(updateManager)
+ package(package), fwDeviceIDRecord(fwDeviceIDRecord),
+ compImageInfos(compImageInfos), compInfo(compInfo),
+ maxTransferSize(maxTransferSize), updateManager(updateManager)
{}
/** @brief Start the firmware update flow for the FD
@@ -179,15 +173,6 @@
/** @brief Endpoint ID of the firmware device */
mctp_eid_t eid;
- /** @brief PLDM daemon's main event loop */
- sdeventplus::Event& event;
-
- /** @brief Instance ID manager for PLDM requests */
- pldm::dbus_api::Requester& requester;
-
- /** @brief PLDM request handler */
- pldm::requester::Handler<pldm::requester::Request>& handler;
-
/** @brief File stream for firmware update package */
std::ifstream& package;
diff --git a/fw-update/test/device_updater_test.cpp b/fw-update/test/device_updater_test.cpp
index 24f85e2..1469516 100644
--- a/fw-update/test/device_updater_test.cpp
+++ b/fw-update/test/device_updater_test.cpp
@@ -10,18 +10,12 @@
#include <gtest/gtest.h>
using namespace pldm;
-using namespace std::chrono;
using namespace pldm::fw_update;
class DeviceUpdaterTest : public testing::Test
{
protected:
DeviceUpdaterTest() :
- event(sdeventplus::Event::get_default()),
- requester(pldm::utils::DBusHandler::getBus(),
- "/xyz/openbmc_project/pldm"),
- reqHandler(fd, event, requester, false, seconds(1), 2,
- milliseconds(100)),
package("./test_pkg", std::ios::binary | std::ios::in | std::ios::ate)
{
fwDeviceIDRecord = {
@@ -39,9 +33,6 @@
}
int fd = -1;
- sdeventplus::Event event;
- pldm::dbus_api::Requester requester;
- requester::Handler<requester::Request> reqHandler;
std::ifstream package;
FirmwareDeviceIDRecord fwDeviceIDRecord;
ComponentImageInfos compImageInfos;
@@ -90,9 +81,8 @@
TEST_F(DeviceUpdaterTest, ReadPackage512B)
{
- DeviceUpdater deviceUpdater(0, event, requester, reqHandler, package,
- fwDeviceIDRecord, compImageInfos, compInfo, 512,
- nullptr);
+ DeviceUpdater deviceUpdater(0, package, fwDeviceIDRecord, compImageInfos,
+ compInfo, 512, nullptr);
constexpr std::array<uint8_t, sizeof(pldm_msg_hdr) +
sizeof(pldm_request_firmware_data_req)>
diff --git a/fw-update/update_manager.cpp b/fw-update/update_manager.cpp
index 42e2156..5e9e98a 100644
--- a/fw-update/update_manager.cpp
+++ b/fw-update/update_manager.cpp
@@ -145,12 +145,11 @@
const auto& fwDeviceIDRecord =
fwDeviceIDRecords[deviceUpdaterInfo.second];
auto search = componentInfoMap.find(deviceUpdaterInfo.first);
- deviceUpdaterMap.emplace(deviceUpdaterInfo.first,
- std::make_unique<DeviceUpdater>(
- deviceUpdaterInfo.first, event, requester,
- handler, package, fwDeviceIDRecord,
- compImageInfos, search->second,
- MAXIMUM_TRANSFER_SIZE, this));
+ deviceUpdaterMap.emplace(
+ deviceUpdaterInfo.first,
+ std::make_unique<DeviceUpdater>(
+ deviceUpdaterInfo.first, package, fwDeviceIDRecord,
+ compImageInfos, search->second, MAXIMUM_TRANSFER_SIZE, this));
}
fwPackageFilePath = packageFilePath;
diff --git a/fw-update/update_manager.hpp b/fw-update/update_manager.hpp
index c942e38..5d4fe47 100644
--- a/fw-update/update_manager.hpp
+++ b/fw-update/update_manager.hpp
@@ -93,12 +93,12 @@
TotalComponentUpdates& totalNumComponentUpdates);
const std::string swRootPath{"/xyz/openbmc_project/software/"};
-
- private:
Event& event; //!< reference to PLDM daemon's main event loop
/** @brief PLDM request handler */
pldm::requester::Handler<pldm::requester::Request>& handler;
Requester& requester; //!< reference to Requester object
+
+ private:
/** @brief Device identifiers of the managed FDs */
const DescriptorMap& descriptorMap;
/** @brief Component information needed for the update of the managed FDs */