Waiting for PSUs to be available during startup
Modified the Software PSU Updater Service to wait until inventory
service available. Updated ItemUpdater::scanDirectory to search
for the installed PSU model directory. For example, the PSU model
2B1E should have a directory named /usr/share/obmc/psu/2B1E. The
firmware file and the MANIFEST reside in the model directory.
Added a new function, `onInterfacesAdded`, processes D-Bus messages
to update the internal state of PSU devices. It performs the following
steps:
- Read D-Bus message to extract the object path and interfaces.
- Check for PSU Interface.
- Retrieve all PSUs inventory paths from the D-Bus.
- Update PSU present status and model. If the PSU is present and has
valid version, process FW update to latest version.
Fixed the issue with version comparison when there is no firmware to
compare with.
Tested:
Tested the code in simulation module and verified it waits for all PSUs
to be discovered. Verified phosphor-psu-code-manager invokes psutils to
get the PSU version, compare and update.
Change-Id: Ic26e215baffd56fc070cc0cf6d3fff92fdfb914c
Signed-off-by: Faisal Awada <faisal@us.ibm.com>
diff --git a/services/xyz.openbmc_project.Software.Psu.Updater.service b/services/xyz.openbmc_project.Software.Psu.Updater.service
index 24de16c..12d23f4 100644
--- a/services/xyz.openbmc_project.Software.Psu.Updater.service
+++ b/services/xyz.openbmc_project.Software.Psu.Updater.service
@@ -1,7 +1,9 @@
[Unit]
Description=OpenBMC PSU Code Manager
Wants=xyz.openbmc_project.Software.Version.service
+After=mapper-wait@-xyz-openbmc_project-inventory.service
After=xyz.openbmc_project.Software.Version.service
+After=phosphor-psu-monitor.service
[Service]
ExecStart=/usr/bin/phosphor-psu-code-manager
diff --git a/src/item_updater.cpp b/src/item_updater.cpp
index 6f774cb..dd6ffd4 100644
--- a/src/item_updater.cpp
+++ b/src/item_updater.cpp
@@ -15,6 +15,7 @@
{
constexpr auto MANIFEST_VERSION = "version";
constexpr auto MANIFEST_EXTENDED_VERSION = "extended_version";
+constexpr auto TIMEOUT = 10;
} // namespace
namespace phosphor
@@ -358,7 +359,11 @@
auto version = utils::getVersion(psuPath);
if (!version.empty())
{
- createPsuObject(psuPath, version);
+ if (psuPathActivationMap.find(psuPath) ==
+ psuPathActivationMap.end())
+ {
+ createPsuObject(psuPath, version);
+ }
// Check if there is new PSU images to update
syncToLatestImage();
}
@@ -391,31 +396,37 @@
auto service = utils::getService(bus, p.c_str(), ITEM_IFACE);
auto present = utils::getProperty<bool>(bus, service.c_str(), p.c_str(),
ITEM_IFACE, PRESENT);
+ psuStatusMap[p].model = utils::getProperty<std::string>(
+ bus, service.c_str(), p.c_str(), ASSET_IFACE, MODEL);
auto version = utils::getVersion(p);
- if (present && !version.empty())
+ if ((psuPathActivationMap.find(p) == psuPathActivationMap.end()) &&
+ present && !version.empty())
{
createPsuObject(p, version);
+ // Add matches for PSU Inventory's property changes
+ psuMatches.emplace_back(
+ bus, MatchRules::propertiesChanged(p, ITEM_IFACE),
+ std::bind(&ItemUpdater::onPsuInventoryChangedMsg, this,
+ std::placeholders::_1)); // For present
+ psuMatches.emplace_back(
+ bus, MatchRules::propertiesChanged(p, ASSET_IFACE),
+ std::bind(&ItemUpdater::onPsuInventoryChangedMsg, this,
+ std::placeholders::_1)); // For model
}
- // Add matches for PSU Inventory's property changes
- psuMatches.emplace_back(
- bus, MatchRules::propertiesChanged(p, ITEM_IFACE),
- std::bind(&ItemUpdater::onPsuInventoryChangedMsg, this,
- std::placeholders::_1)); // For present
- psuMatches.emplace_back(
- bus, MatchRules::propertiesChanged(p, ASSET_IFACE),
- std::bind(&ItemUpdater::onPsuInventoryChangedMsg, this,
- std::placeholders::_1)); // For model
}
}
void ItemUpdater::processStoredImage()
{
scanDirectory(IMG_DIR_BUILTIN);
+
scanDirectory(IMG_DIR_PERSIST);
}
void ItemUpdater::scanDirectory(const fs::path& dir)
{
+ auto manifest = dir;
+ auto path = dir;
// The directory shall put PSU images in directories named with model
if (!fs::exists(dir))
{
@@ -428,28 +439,52 @@
entry("PATH=%s", dir.c_str()));
return;
}
- for (const auto& d : fs::directory_iterator(dir))
+
+ for (const auto& [key, item] : psuStatusMap)
{
- // If the model in manifest does not match the dir name
- // Log a warning and skip it
- auto path = d.path();
- auto manifest = path / MANIFEST_FILE;
- if (fs::exists(manifest))
+ if (!item.model.empty())
{
- auto ret = Version::getValues(
- manifest.string(),
- {MANIFEST_VERSION, MANIFEST_EXTENDED_VERSION});
- auto version = ret[MANIFEST_VERSION];
- auto extVersion = ret[MANIFEST_EXTENDED_VERSION];
- auto info = Version::getExtVersionInfo(extVersion);
- auto model = info["model"];
- if (path.stem() != model)
- {
- log<level::ERR>("Unmatched model",
- entry("PATH=%s", path.c_str()),
- entry("MODEL=%s", model.c_str()));
- continue;
- }
+ path = path / item.model;
+ manifest = dir / item.model / MANIFEST_FILE;
+ break;
+ }
+ }
+ if (path == dir)
+ {
+ log<level::ERR>("Model directory not found");
+ return;
+ }
+
+ if (!fs::is_directory(path))
+ {
+ log<level::ERR>("The path is not a directory",
+ entry("PATH=%s", path.c_str()));
+ return;
+ }
+
+ if (!fs::exists(manifest))
+ {
+ log<level::ERR>("No MANIFEST found",
+ entry("PATH=%s", manifest.c_str()));
+ return;
+ }
+ // If the model in manifest does not match the dir name
+ // Log a warning
+ if (fs::is_regular_file(manifest))
+ {
+ auto ret = Version::getValues(
+ manifest.string(), {MANIFEST_VERSION, MANIFEST_EXTENDED_VERSION});
+ auto version = ret[MANIFEST_VERSION];
+ auto extVersion = ret[MANIFEST_EXTENDED_VERSION];
+ auto info = Version::getExtVersionInfo(extVersion);
+ auto model = info["model"];
+ if (path.stem() != model)
+ {
+ log<level::ERR>("Unmatched model", entry("PATH=%s", path.c_str()),
+ entry("MODEL=%s", model.c_str()));
+ }
+ else
+ {
auto versionId = utils::getVersionId(version);
auto it = activations.find(versionId);
if (it == activations.end())
@@ -474,11 +509,11 @@
it->second->path(path);
}
}
- else
- {
- log<level::ERR>("No MANIFEST found",
- entry("PATH=%s", path.c_str()));
- }
+ }
+ else
+ {
+ log<level::ERR>("MANIFEST is not a file",
+ entry("PATH=%s", manifest.c_str()));
}
}
@@ -518,9 +553,9 @@
auto paths = utils::getPSUInventoryPath(bus);
for (const auto& p : paths)
{
- // As long as there is a PSU is not associated with the latest image,
- // run the activation so that all PSUs are running the same latest
- // image.
+ // As long as there is a PSU is not associated with the latest
+ // image, run the activation so that all PSUs are running the same
+ // latest image.
if (!utils::isAssociated(p, assocs))
{
log<level::INFO>("Automatically update PSU",
@@ -537,6 +572,80 @@
activation->requestedActivation(Activation::RequestedActivations::Active);
}
+void ItemUpdater::onPSUInterfaceAdded(sdbusplus::message_t& msg)
+{
+ sdbusplus::message::object_path objPath;
+ std::map<std::string,
+ std::map<std::string, std::variant<bool, std::string>>>
+ interfaces;
+ msg.read(objPath, interfaces);
+ std::string path = objPath.str;
+
+ if (interfaces.find(PSU_INVENTORY_IFACE) == interfaces.end() ||
+ (psuStatusMap[path].present && !psuStatusMap[path].model.empty()))
+ {
+ return;
+ }
+
+ auto timeout = std::chrono::steady_clock::now() +
+ std::chrono::seconds(TIMEOUT);
+
+ // Poll the inventory item until it gets the present property
+ // or the timeout is reached
+ while (std::chrono::steady_clock::now() < timeout)
+ {
+ try
+ {
+ psuStatusMap[path].present = utils::getProperty<bool>(
+ bus, msg.get_sender(), path.c_str(), ITEM_IFACE, PRESENT);
+ break;
+ }
+ catch (const std::exception& e)
+ {
+ auto err = errno;
+ log<level::INFO>(
+ std::format("Failed to get Inventory Item Present. errno={}",
+ err)
+ .c_str());
+ sleep(1);
+ }
+ }
+
+ // Poll the inventory item until it retrieves model or the timeout is
+ // reached. The model is the path trail of the firmware's and manifest
+ // subdirectory. If the model not found the firmware and manifest
+ // cannot be located.
+ timeout = std::chrono::steady_clock::now() + std::chrono::seconds(TIMEOUT);
+ while (std::chrono::steady_clock::now() < timeout &&
+ psuStatusMap[path].present)
+ {
+ try
+ {
+ psuStatusMap[path].model = utils::getProperty<std::string>(
+ bus, msg.get_sender(), path.c_str(), ASSET_IFACE, MODEL);
+ processPSUImageAndSyncToLatest();
+ break;
+ }
+ catch (const std::exception& e)
+ {
+ auto err = errno;
+ log<level::INFO>(
+ std::format(
+ "Failed to get Inventory Decorator Asset model. errno={}",
+ err)
+ .c_str());
+ sleep(1);
+ }
+ }
+}
+
+void ItemUpdater::processPSUImageAndSyncToLatest()
+{
+ processPSUImage();
+ processStoredImage();
+ syncToLatestImage();
+}
+
} // namespace updater
} // namespace software
} // namespace phosphor
diff --git a/src/item_updater.hpp b/src/item_updater.hpp
index 0ff2600..61ff666 100644
--- a/src/item_updater.hpp
+++ b/src/item_updater.hpp
@@ -55,11 +55,16 @@
MatchRules::interfacesAdded() +
MatchRules::path(SOFTWARE_OBJPATH),
std::bind(std::mem_fn(&ItemUpdater::createActivation),
- this, std::placeholders::_1))
+ this, std::placeholders::_1)),
+ psuInterfaceMatch(
+ bus,
+ MatchRules::interfacesAdded() +
+ MatchRules::path("/xyz/openbmc_project/inventory") +
+ MatchRules::sender("xyz.openbmc_project.Inventory.Manager"),
+ std::bind(std::mem_fn(&ItemUpdater::onPSUInterfaceAdded), this,
+ std::placeholders::_1))
{
- processPSUImage();
- processStoredImage();
- syncToLatestImage();
+ processPSUImageAndSyncToLatest();
}
/** @brief Deletes version
@@ -179,6 +184,24 @@
/** @brief Invoke the activation via DBus */
void invokeActivation(const std::unique_ptr<Activation>& activation);
+ /** @brief Callback function for interfaces added signal.
+ *
+ * This method is called when a new interface is added. It updates the
+ * internal status map and process the new PSU if it's present.
+ *
+ * @param[in] msg - Data associated with subscribed signal
+ */
+ void onPSUInterfaceAdded(sdbusplus::message_t& msg);
+
+ /**
+ * @brief Handles the processing of PSU images.
+ *
+ * This function responsible for invoking the sequence of processing PSU
+ * images, processing stored images, and syncing to the latest firmware
+ * image.
+ */
+ void processPSUImageAndSyncToLatest();
+
/** @brief Persistent sdbusplus D-Bus bus connection. */
sdbusplus::bus_t& bus;
@@ -219,6 +242,14 @@
* It is used to handle psu inventory changed event, that only create psu
* software object when a PSU is present and the model is retrieved */
std::map<std::string, psuStatus> psuStatusMap;
+
+ /** @brief Signal match for PSU interfaces added.
+ *
+ * This match listens for D-Bus signals indicating new interface has been
+ * added. When such a signal received, it triggers the
+ * `onInterfacesAdded` method to handle the new PSU.
+ */
+ sdbusplus::bus::match_t psuInterfaceMatch;
};
} // namespace updater
diff --git a/test/test_item_updater.cpp b/test/test_item_updater.cpp
index e62b2a8..6a0a08e 100644
--- a/test/test_item_updater.cpp
+++ b/test/test_item_updater.cpp
@@ -88,6 +88,9 @@
EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psuPath),
_, StrEq(PRESENT)))
.WillOnce(Return(any(PropertyType(false)))); // not present
+ EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psuPath),
+ _, StrEq(MODEL)))
+ .WillOnce(Return(any(PropertyType(std::string("")))));
// The item updater itself
EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(dBusPath)))
@@ -114,6 +117,9 @@
EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psuPath),
_, StrEq(PRESENT)))
.WillOnce(Return(any(PropertyType(true)))); // present
+ EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psuPath),
+ _, StrEq(MODEL)))
+ .WillOnce(Return(any(PropertyType(std::string("dummyModel")))));
// The item updater itself
EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(dBusPath)))
@@ -146,11 +152,17 @@
EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu0), _,
StrEq(PRESENT)))
.WillOnce(Return(any(PropertyType(true)))); // present
+ EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu0), _,
+ StrEq(MODEL)))
+ .WillOnce(Return(any(PropertyType(std::string("dummyModel0")))));
EXPECT_CALL(mockedUtils, getVersion(StrEq(psu1)))
.WillOnce(Return(std::string(version1)));
EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu1), _,
StrEq(PRESENT)))
.WillOnce(Return(any(PropertyType(true)))); // present
+ EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu1), _,
+ StrEq(MODEL)))
+ .WillOnce(Return(any(PropertyType(std::string("dummyModel1")))));
// The item updater itself
EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(dBusPath)))
@@ -192,11 +204,17 @@
EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu0), _,
StrEq(PRESENT)))
.WillOnce(Return(any(PropertyType(true)))); // present
+ EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu0), _,
+ StrEq(MODEL)))
+ .WillOnce(Return(any(PropertyType(std::string("dummyModel0")))));
EXPECT_CALL(mockedUtils, getVersion(StrEq(psu1)))
.WillOnce(Return(std::string(version1)));
EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu1), _,
StrEq(PRESENT)))
.WillOnce(Return(any(PropertyType(true)))); // present
+ EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu1), _,
+ StrEq(MODEL)))
+ .WillOnce(Return(any(PropertyType(std::string("dummyModel1")))));
// The item updater itself
EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(dBusPath)))
@@ -238,6 +256,9 @@
EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psuPath),
_, StrEq(PRESENT)))
.WillOnce(Return(any(PropertyType(true)))); // present
+ EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psuPath),
+ _, StrEq(MODEL)))
+ .WillOnce(Return(any(PropertyType(std::string("dummyModel")))));
// The item updater itself
EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(dBusPath)))
@@ -274,6 +295,9 @@
EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psuPath),
_, StrEq(PRESENT)))
.WillOnce(Return(any(PropertyType(false)))); // not present
+ EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psuPath),
+ _, StrEq(MODEL)))
+ .WillOnce(Return(any(PropertyType(std::string("")))));
// The item updater itself
EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(dBusPath)))
@@ -310,6 +334,9 @@
EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psuPath),
_, StrEq(PRESENT)))
.WillOnce(Return(any(PropertyType(true)))); // present
+ EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psuPath),
+ _, StrEq(MODEL)))
+ .WillOnce(Return(any(PropertyType(std::string("dummyModel")))));
// The item updater itself
EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(dBusPath)))
@@ -371,11 +398,17 @@
EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu0), _,
StrEq(PRESENT)))
.WillOnce(Return(any(PropertyType(true)))); // present
+ EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu0), _,
+ StrEq(MODEL)))
+ .WillOnce(Return(any(PropertyType(std::string("dummyModel0")))));
EXPECT_CALL(mockedUtils, getVersion(StrEq(psu1)))
.WillOnce(Return(std::string(version1)));
EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu1), _,
StrEq(PRESENT)))
.WillOnce(Return(any(PropertyType(true)))); // present
+ EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu1), _,
+ StrEq(MODEL)))
+ .WillOnce(Return(any(PropertyType(std::string("dummyModel1")))));
// The item updater itself
EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(dBusPath)))
@@ -446,6 +479,9 @@
EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psuPath),
_, StrEq(PRESENT)))
.WillOnce(Return(any(PropertyType(false)))); // not present
+ EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psuPath),
+ _, StrEq(MODEL)))
+ .WillOnce(Return(any(PropertyType(std::string("")))));
// The item updater itself
EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(dBusPath)))
@@ -461,7 +497,7 @@
auto objPathInvalid = getObjPath("psu-test.v0.5");
// activation and version object will be added on scan dir
EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(objPathValid)))
- .Times(2);
+ .Times(0);
EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(objPathInvalid)))
.Times(0);
scanDirectory("./psu-images-one-valid-one-invalid");
@@ -482,6 +518,9 @@
EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psuPath),
_, StrEq(PRESENT)))
.WillOnce(Return(any(PropertyType(true)))); // present
+ EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psuPath),
+ _, StrEq(MODEL)))
+ .WillOnce(Return(any(PropertyType(std::string("dummyModel")))));
// The item updater itself
EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(dBusPath)))
@@ -501,7 +540,7 @@
_, StrEq(objPath),
StrEq("xyz.openbmc_project.Common.FilePath"),
Pointee(StrEq("Path"))))
- .Times(1);
+ .Times(0);
scanDirectory("./psu-images-valid-version0");
}
@@ -528,11 +567,17 @@
EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu0), _,
StrEq(PRESENT)))
.WillOnce(Return(any(PropertyType(true)))); // present
+ EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu0), _,
+ StrEq(MODEL)))
+ .WillOnce(Return(any(PropertyType(std::string("dummyModel0")))));
EXPECT_CALL(mockedUtils, getVersion(StrEq(psu1)))
.WillOnce(Return(std::string(version1)));
EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu1), _,
StrEq(PRESENT)))
.WillOnce(Return(any(PropertyType(true)))); // present
+ EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu1), _,
+ StrEq(MODEL)))
+ .WillOnce(Return(any(PropertyType(std::string("dummyModel1")))));
itemUpdater = std::make_unique<ItemUpdater>(mockedBus, dBusPath);
@@ -592,11 +637,17 @@
EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu0), _,
StrEq(PRESENT)))
.WillOnce(Return(any(PropertyType(true)))); // present
+ EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu0), _,
+ StrEq(MODEL)))
+ .WillOnce(Return(any(PropertyType(std::string("dummyModel0")))));
EXPECT_CALL(mockedUtils, getVersion(StrEq(psu1)))
.WillOnce(Return(std::string(version1)));
EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu1), _,
StrEq(PRESENT)))
.WillOnce(Return(any(PropertyType(true)))); // present
+ EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu1), _,
+ StrEq(MODEL)))
+ .WillOnce(Return(any(PropertyType(std::string("dummyModel1")))));
itemUpdater = std::make_unique<ItemUpdater>(mockedBus, dBusPath);
@@ -638,14 +689,17 @@
std::string objPath = getObjPath(version);
ON_CALL(mockedUtils, getPSUInventoryPath(_))
.WillByDefault(Return(std::vector<std::string>({psuPath})));
- ON_CALL(mockedUtils, getService(_, StrEq(psuPath), _))
- .WillByDefault(Return(service));
+ EXPECT_CALL(mockedUtils, getService(_, StrEq(psuPath), _))
+ .WillOnce(Return(service))
+ .WillOnce(Return(service));
EXPECT_CALL(mockedUtils, getVersion(StrEq(psuPath)))
.WillOnce(Return(std::string(version)));
ON_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psuPath), _,
StrEq(PRESENT)))
.WillByDefault(Return(any(PropertyType(true)))); // present
-
+ ON_CALL(mockedUtils,
+ getPropertyImpl(_, StrEq(service), StrEq(psuPath), _, StrEq(MODEL)))
+ .WillByDefault(Return(any(PropertyType(std::string("dummyModel")))));
itemUpdater = std::make_unique<ItemUpdater>(mockedBus, dBusPath);
// Add an association to simulate that it has image in BMC filesystem