Refactor: Make Activation inherit FilePath interface
Previously Version inherits FilePath interface to provide the
information of the file path property of a software, and Activation was
using pre-defined location to find the image path (IMG_DIR / versionId).
Now the code is going to support pre-built image and stored image during
update, the Activation class needs to know the file path to perform the
update.
So this commit "moves" the FilePath from Version to Activation.
Tested: Verify the "Path" property on FilePath still exists on the DBus
object after uploading a tarball.
Signed-off-by: Lei YU <mine260309@gmail.com>
Change-Id: I37b41eaa93cd239ab7732f6fac9400410995ca71
diff --git a/src/activation.cpp b/src/activation.cpp
index 5023712..2ca7d86 100644
--- a/src/activation.cpp
+++ b/src/activation.cpp
@@ -166,12 +166,12 @@
{
if (!activationProgress)
{
- activationProgress = std::make_unique<ActivationProgress>(bus, path);
+ activationProgress = std::make_unique<ActivationProgress>(bus, objPath);
}
if (!activationBlocksTransition)
{
activationBlocksTransition =
- std::make_unique<ActivationBlocksTransition>(bus, path);
+ std::make_unique<ActivationBlocksTransition>(bus, objPath);
}
auto psuPaths = utils::getPSUInventoryPath(bus);
@@ -226,8 +226,8 @@
// TODO: delete the old software object
deleteImageManagerObject();
- associationInterface->createActiveAssociation(path);
- associationInterface->addFunctionalAssociation(path);
+ associationInterface->createActiveAssociation(objPath);
+ associationInterface->addFunctionalAssociation(objPath);
activation(Status::Active);
}
@@ -238,7 +238,7 @@
constexpr auto versionServiceStr = "xyz.openbmc_project.Software.Version";
constexpr auto deleteInterface = "xyz.openbmc_project.Object.Delete";
std::string versionService;
- auto services = utils::getServices(bus, path.c_str(), deleteInterface);
+ auto services = utils::getServices(bus, objPath.c_str(), deleteInterface);
// We need to find the phosphor-version-software-manager's version service
// to invoke the delete interface
@@ -257,7 +257,7 @@
}
// Call the Delete object for <versionID> inside image_manager
- auto method = bus.new_method_call(versionService.c_str(), path.c_str(),
+ auto method = bus.new_method_call(versionService.c_str(), objPath.c_str(),
deleteInterface, "Delete");
try
{
@@ -267,7 +267,7 @@
{
log<level::ERR>("Error performing call to Delete object path",
entry("ERROR=%s", e.what()),
- entry("PATH=%s", path.c_str()));
+ entry("PATH=%s", objPath.c_str()));
}
}
diff --git a/src/activation.hpp b/src/activation.hpp
index 7ad03c9..7a9e464 100644
--- a/src/activation.hpp
+++ b/src/activation.hpp
@@ -9,6 +9,7 @@
#include <queue>
#include <sdbusplus/server.hpp>
#include <xyz/openbmc_project/Association/Definitions/server.hpp>
+#include <xyz/openbmc_project/Common/FilePath/server.hpp>
#include <xyz/openbmc_project/Software/Activation/server.hpp>
#include <xyz/openbmc_project/Software/ActivationBlocksTransition/server.hpp>
#include <xyz/openbmc_project/Software/ActivationProgress/server.hpp>
@@ -101,7 +102,8 @@
using ActivationInherit = sdbusplus::server::object::object<
sdbusplus::xyz::openbmc_project::Software::server::ExtendedVersion,
sdbusplus::xyz::openbmc_project::Software::server::Activation,
- sdbusplus::xyz::openbmc_project::Association::server::Definitions>;
+ sdbusplus::xyz::openbmc_project::Association::server::Definitions,
+ sdbusplus::xyz::openbmc_project::Common::server::FilePath>;
/** @class Activation
* @brief OpenBMC activation software management implementation.
@@ -122,13 +124,15 @@
* @param[in] extVersion - The extended version
* @param[in] activationStatus - The status of Activation
* @param[in] assocs - Association objects
+ * @param[in] filePath - The image filesystem path
*/
- Activation(sdbusplus::bus::bus& bus, const std::string& path,
+ Activation(sdbusplus::bus::bus& bus, const std::string& objPath,
const std::string& versionId, const std::string& extVersion,
Status activationStatus, const AssociationList& assocs,
- AssociationInterface* associationInterface) :
- ActivationInherit(bus, path.c_str(), true),
- bus(bus), path(path), versionId(versionId),
+ AssociationInterface* associationInterface,
+ const std::string& filePath) :
+ ActivationInherit(bus, objPath.c_str(), true),
+ bus(bus), objPath(objPath), versionId(versionId),
systemdSignals(
bus,
sdbusRule::type::signal() + sdbusRule::member("JobRemoved") +
@@ -142,6 +146,7 @@
extendedVersion(extVersion);
activation(activationStatus);
associations(assocs);
+ path(filePath);
auto info = Version::getExtVersionInfo(extVersion);
manufacturer = info["manufacturer"];
@@ -227,7 +232,7 @@
sdbusplus::bus::bus& bus;
/** @brief Persistent DBus object path */
- std::string path;
+ std::string objPath;
/** @brief Version id */
std::string versionId;
diff --git a/src/item_updater.cpp b/src/item_updater.cpp
index 49b95df..8d5695c 100644
--- a/src/item_updater.cpp
+++ b/src/item_updater.cpp
@@ -112,12 +112,13 @@
extendedVersion = it->second;
}
- auto activation = createActivationObject(
- path, versionId, extendedVersion, activationState, associations);
+ auto activation =
+ createActivationObject(path, versionId, extendedVersion,
+ activationState, associations, filePath);
activations.emplace(versionId, std::move(activation));
auto versionPtr =
- createVersionObject(path, versionId, version, purpose, filePath);
+ createVersionObject(path, versionId, version, purpose);
versions.emplace(versionId, std::move(versionPtr));
}
return;
@@ -188,10 +189,11 @@
const std::string& extVersion,
sdbusplus::xyz::openbmc_project::Software::server::Activation::Activations
activationStatus,
- const AssociationList& assocs)
+ const AssociationList& assocs, const std::string& filePath)
{
return std::make_unique<Activation>(bus, path, versionId, extVersion,
- activationStatus, assocs, this);
+ activationStatus, assocs, this,
+ filePath);
}
void ItemUpdater::createPsuObject(const std::string& psuInventoryPath,
@@ -221,13 +223,13 @@
ACTIVATION_REV_ASSOCIATION,
psuInventoryPath));
- auto activation = createActivationObject(path, versionId, "",
- activationState, associations);
+ auto activation = createActivationObject(
+ path, versionId, "", activationState, associations, "");
activations.emplace(versionId, std::move(activation));
psuPathActivationMap.emplace(psuInventoryPath, activations[versionId]);
auto versionPtr = createVersionObject(path, versionId, psuVersion,
- VersionPurpose::PSU, "");
+ VersionPurpose::PSU);
versions.emplace(versionId, std::move(versionPtr));
createActiveAssociation(path);
@@ -275,11 +277,10 @@
const std::string& objPath, const std::string& versionId,
const std::string& versionString,
sdbusplus::xyz::openbmc_project::Software::server::Version::VersionPurpose
- versionPurpose,
- const std::string& filePath)
+ versionPurpose)
{
auto version = std::make_unique<Version>(
- bus, objPath, versionId, versionString, versionPurpose, filePath,
+ bus, objPath, versionId, versionString, versionPurpose,
std::bind(&ItemUpdater::erase, this, std::placeholders::_1));
return version;
}
diff --git a/src/item_updater.hpp b/src/item_updater.hpp
index 67f6e57..33db8bc 100644
--- a/src/item_updater.hpp
+++ b/src/item_updater.hpp
@@ -112,7 +112,7 @@
const std::string& extVersion,
sdbusplus::xyz::openbmc_project::Software::server::Activation::
Activations activationStatus,
- const AssociationList& assocs);
+ const AssociationList& assocs, const std::string& filePath);
/** @brief Create Version object */
std::unique_ptr<Version>
@@ -120,8 +120,7 @@
const std::string& versionId,
const std::string& versionString,
sdbusplus::xyz::openbmc_project::Software::server::
- Version::VersionPurpose versionPurpose,
- const std::string& filePath);
+ Version::VersionPurpose versionPurpose);
/** @brief Create Activation and Version object for PSU inventory
* @details If the same version exists for multiple PSUs, just add
diff --git a/src/version.hpp b/src/version.hpp
index d4106e6..89db260 100644
--- a/src/version.hpp
+++ b/src/version.hpp
@@ -3,7 +3,6 @@
#include "config.h"
#include <sdbusplus/bus.hpp>
-#include <xyz/openbmc_project/Common/FilePath/server.hpp>
#include <xyz/openbmc_project/Object/Delete/server.hpp>
#include <xyz/openbmc_project/Software/Version/server.hpp>
@@ -17,8 +16,7 @@
using eraseFunc = std::function<void(std::string)>;
using VersionInherit = sdbusplus::server::object::object<
- sdbusplus::xyz::openbmc_project::Software::server::Version,
- sdbusplus::xyz::openbmc_project::Common::server::FilePath>;
+ sdbusplus::xyz::openbmc_project::Software::server::Version>;
using DeleteInherit = sdbusplus::server::object::object<
sdbusplus::xyz::openbmc_project::Object::server::Delete>;
@@ -86,13 +84,11 @@
* @param[in] versionId - The version Id
* @param[in] versionString - The version string
* @param[in] versionPurpose - The version purpose
- * @param[in] filePath - The image filesystem path
* @param[in] callback - The eraseFunc callback
*/
Version(sdbusplus::bus::bus& bus, const std::string& objPath,
const std::string& versionId, const std::string& versionString,
- VersionPurpose versionPurpose, const std::string& filePath,
- eraseFunc callback) :
+ VersionPurpose versionPurpose, eraseFunc callback) :
VersionInherit(bus, (objPath).c_str(), true),
eraseCallback(callback), bus(bus), objPath(objPath),
versionId(versionId), versionStr(versionString)
@@ -100,7 +96,6 @@
// Set properties.
purpose(versionPurpose);
version(versionString);
- path(filePath);
deleteObject = std::make_unique<Delete>(bus, objPath, *this);
diff --git a/test/test_activation.cpp b/test/test_activation.cpp
index 0c40d97..088e3ba 100644
--- a/test/test_activation.cpp
+++ b/test/test_activation.cpp
@@ -59,6 +59,7 @@
std::unique_ptr<Activation> activation;
std::string versionId = "abcdefgh";
std::string extVersion = "manufacturer=TestManu,model=TestModel";
+ std::string filePath = "";
std::string dBusPath = std::string(SOFTWARE_OBJPATH) + "/" + versionId;
Status status = Status::Ready;
AssociationList associations;
@@ -66,9 +67,9 @@
TEST_F(TestActivation, ctordtor)
{
- activation = std::make_unique<Activation>(mockedBus, dBusPath, versionId,
- extVersion, status, associations,
- &mockedAssociationInterface);
+ activation = std::make_unique<Activation>(
+ mockedBus, dBusPath, versionId, extVersion, status, associations,
+ &mockedAssociationInterface, filePath);
}
namespace phosphor::software::updater::internal
@@ -91,9 +92,9 @@
TEST_F(TestActivation, doUpdateWhenNoPSU)
{
- activation = std::make_unique<Activation>(mockedBus, dBusPath, versionId,
- extVersion, status, associations,
- &mockedAssociationInterface);
+ activation = std::make_unique<Activation>(
+ mockedBus, dBusPath, versionId, extVersion, status, associations,
+ &mockedAssociationInterface, filePath);
ON_CALL(mockedUtils, getPSUInventoryPath(_))
.WillByDefault(
Return(std::vector<std::string>({}))); // No PSU inventory
@@ -109,9 +110,9 @@
TEST_F(TestActivation, doUpdateOnePSUOK)
{
constexpr auto psu0 = "/com/example/inventory/psu0";
- activation = std::make_unique<Activation>(mockedBus, dBusPath, versionId,
- extVersion, status, associations,
- &mockedAssociationInterface);
+ activation = std::make_unique<Activation>(
+ mockedBus, dBusPath, versionId, extVersion, status, associations,
+ &mockedAssociationInterface, filePath);
ON_CALL(mockedUtils, getPSUInventoryPath(_))
.WillByDefault(
Return(std::vector<std::string>({psu0}))); // One PSU inventory
@@ -133,9 +134,9 @@
constexpr auto psu1 = "/com/example/inventory/psu1";
constexpr auto psu2 = "/com/example/inventory/psu2";
constexpr auto psu3 = "/com/example/inventory/psu3";
- activation = std::make_unique<Activation>(mockedBus, dBusPath, versionId,
- extVersion, status, associations,
- &mockedAssociationInterface);
+ activation = std::make_unique<Activation>(
+ mockedBus, dBusPath, versionId, extVersion, status, associations,
+ &mockedAssociationInterface, filePath);
ON_CALL(mockedUtils, getPSUInventoryPath(_))
.WillByDefault(Return(
std::vector<std::string>({psu0, psu1, psu2, psu3}))); // 4 PSUs
@@ -171,9 +172,9 @@
constexpr auto psu1 = "/com/example/inventory/psu1";
constexpr auto psu2 = "/com/example/inventory/psu2";
constexpr auto psu3 = "/com/example/inventory/psu3";
- activation = std::make_unique<Activation>(mockedBus, dBusPath, versionId,
- extVersion, status, associations,
- &mockedAssociationInterface);
+ activation = std::make_unique<Activation>(
+ mockedBus, dBusPath, versionId, extVersion, status, associations,
+ &mockedAssociationInterface, filePath);
ON_CALL(mockedUtils, getPSUInventoryPath(_))
.WillByDefault(Return(
std::vector<std::string>({psu0, psu1, psu2, psu3}))); // 4 PSUs
@@ -197,9 +198,9 @@
TEST_F(TestActivation, doUpdateOnExceptionFromDbus)
{
constexpr auto psu0 = "/com/example/inventory/psu0";
- activation = std::make_unique<Activation>(mockedBus, dBusPath, versionId,
- extVersion, status, associations,
- &mockedAssociationInterface);
+ activation = std::make_unique<Activation>(
+ mockedBus, dBusPath, versionId, extVersion, status, associations,
+ &mockedAssociationInterface, filePath);
ON_CALL(mockedUtils, getPSUInventoryPath(_))
.WillByDefault(
Return(std::vector<std::string>({psu0}))); // One PSU inventory
@@ -214,9 +215,9 @@
{
constexpr auto psu0 = "/com/example/inventory/psu0";
extVersion = "manufacturer=TestManu,model=DifferentModel";
- activation = std::make_unique<Activation>(mockedBus, dBusPath, versionId,
- extVersion, status, associations,
- &mockedAssociationInterface);
+ activation = std::make_unique<Activation>(
+ mockedBus, dBusPath, versionId, extVersion, status, associations,
+ &mockedAssociationInterface, filePath);
ON_CALL(mockedUtils, getPSUInventoryPath(_))
.WillByDefault(Return(std::vector<std::string>({psu0})));
activation->requestedActivation(RequestedStatus::Active);
@@ -228,9 +229,9 @@
{
constexpr auto psu0 = "/com/example/inventory/psu0";
extVersion = "manufacturer=DifferentManu,model=TestModel";
- activation = std::make_unique<Activation>(mockedBus, dBusPath, versionId,
- extVersion, status, associations,
- &mockedAssociationInterface);
+ activation = std::make_unique<Activation>(
+ mockedBus, dBusPath, versionId, extVersion, status, associations,
+ &mockedAssociationInterface, filePath);
ON_CALL(mockedUtils, getPSUInventoryPath(_))
.WillByDefault(Return(std::vector<std::string>({psu0})));
activation->requestedActivation(RequestedStatus::Active);
@@ -245,9 +246,9 @@
extVersion = "manufacturer=AnyManu,model=TestModel";
// Below is the same as doUpdateOnePSUOK case
constexpr auto psu0 = "/com/example/inventory/psu0";
- activation = std::make_unique<Activation>(mockedBus, dBusPath, versionId,
- extVersion, status, associations,
- &mockedAssociationInterface);
+ activation = std::make_unique<Activation>(
+ mockedBus, dBusPath, versionId, extVersion, status, associations,
+ &mockedAssociationInterface, filePath);
ON_CALL(mockedUtils, getPSUInventoryPath(_))
.WillByDefault(
Return(std::vector<std::string>({psu0}))); // One PSU inventory
@@ -272,9 +273,9 @@
ON_CALL(mockedUtils, getPropertyImpl(_, _, StrEq(psu1), _, StrEq(MODEL)))
.WillByDefault(
Return(any(PropertyType(std::string("DifferentModel")))));
- activation = std::make_unique<Activation>(mockedBus, dBusPath, versionId,
- extVersion, status, associations,
- &mockedAssociationInterface);
+ activation = std::make_unique<Activation>(
+ mockedBus, dBusPath, versionId, extVersion, status, associations,
+ &mockedAssociationInterface, filePath);
ON_CALL(mockedUtils, getPSUInventoryPath(_))
.WillByDefault(Return(
std::vector<std::string>({psu0, psu1, psu2, psu3}))); // 4 PSUs