From 19661c1173d9d82dfbb879f8cc89c05a0883dffa Mon Sep 17 00:00:00 2001 From: AppaRao Puli Date: Wed, 21 Apr 2021 21:16:47 +0000 Subject: [PATCH] Fix delete image by ID and inhibit removal of bmc_active Delete image by ID was broken because when hitting the delete dbus interface, it recalculated the ID from the parent version, which then does not match because of the random number addition that was added to the ID when the parent interface was created. This saves away the parent interface ID and recalls it rather than recalculating it. Also, there was a logic error in deleting images that would delete the active BMC image. This fixes up that error. Tested: run multiple back-to back updates and see that when the fwupd script calls delete on the seamless images, the interfaces are deleted and that the bmc_active interface is not deleted. Signed-off-by: Vernon Mauery Signed-off-by: AppaRao Puli --- image_manager.cpp | 2 +- item_updater.cpp | 20 +++++++++++++------- pfr_image_manager.cpp | 2 +- version.cpp | 2 +- version.hpp | 19 +++++++++++++++---- 5 files changed, 31 insertions(+), 14 deletions(-) diff --git a/image_manager.cpp b/image_manager.cpp index 4be357f..311a63f 100644 --- a/image_manager.cpp +++ b/image_manager.cpp @@ -219,7 +219,7 @@ int Manager::processImage(const std::string& tarFilePath) { // Create Version object auto versionPtr = std::make_unique( - bus, objPath, version, purpose, extendedVersion, + bus, objPath, id, version, purpose, extendedVersion, imageDirPath.string(), std::bind(&Manager::erase, this, std::placeholders::_1)); versionPtr->deleteObject = diff --git a/item_updater.cpp b/item_updater.cpp index 6efd519..6685bf5 100644 --- a/item_updater.cpp +++ b/item_updater.cpp @@ -145,7 +145,7 @@ void ItemUpdater::createActivation(sdbusplus::message::message& msg) activationState, associations))); auto versionPtr = std::make_unique( - bus, path, version, purpose, extendedVersion, filePath, + bus, path, versionId, version, purpose, extendedVersion, filePath, std::bind(&ItemUpdater::erase, this, std::placeholders::_1)); versionPtr->deleteObject = std::make_unique(bus, path, @@ -263,7 +263,7 @@ void ItemUpdater::processBMCImage() // Create Version instance for this version. auto versionPtr = std::make_unique( - bus, path, version, purpose, extendedVersion, "", + bus, path, id, version, purpose, extendedVersion, "", std::bind(&ItemUpdater::erase, this, std::placeholders::_1)); auto isVersionFunctional = versionPtr->isFunctional(); if (!isVersionFunctional) @@ -338,11 +338,11 @@ void ItemUpdater::erase(std::string entryId) auto it = versions.find(entryId); if (it != versions.end()) { - if (it->second->isFunctional() && ACTIVE_BMC_MAX_ALLOWED > 1) + if (it->second->isFunctional()) { - log("Error: Version is currently running on the BMC. " - "Unable to remove.", - entry("VERSIONID=%s", entryId.c_str())); + log("Error: Version is currently running on the BMC. " + "Unable to remove.", + entry("VERSIONID=%s", entryId.c_str())); return; } } @@ -681,6 +681,12 @@ void ItemUpdater::freeSpace(Activation& caller) std::size_t count = 0; for (const auto& iter : activations) { + if (versions.find(iter.second->versionId)->second->isFunctional()) + { + // don't bother with function versions + continue; + } + if ((iter.second.get()->activation() == server::Activation::Activations::Active) || (iter.second.get()->activation() == @@ -775,7 +781,7 @@ void ItemUpdater::createBIOSObject() // Do nothing; }; biosVersion = std::make_unique( - bus, path, version, VersionPurpose::Host, "", "", + bus, path, versionId, version, VersionPurpose::Host, "", "", std::bind(dummyErase, std::placeholders::_1)); biosVersion->deleteObject = std::make_unique(bus, path, diff --git a/pfr_image_manager.cpp b/pfr_image_manager.cpp index 55ad21f..e2dcc80 100644 --- a/pfr_image_manager.cpp +++ b/pfr_image_manager.cpp @@ -399,7 +399,7 @@ int Manager::processImage(const std::string& imgFilePath) std::string objPath = std::string{SOFTWARE_OBJPATH} + '/' + id; auto versionPtr = std::make_unique( - bus, objPath, ver, purpose, extVer, imageDirPath.string(), + bus, objPath, id, ver, purpose, extVer, imageDirPath.string(), std::bind(&Manager::erase, this, std::placeholders::_1)); versionPtr->deleteObject = std::make_unique(bus, objPath, diff --git a/version.cpp b/version.cpp index 04f40c4..44ef571 100644 --- a/version.cpp +++ b/version.cpp @@ -206,7 +206,7 @@ void Delete::delete_() { if (parent.eraseCallback) { - parent.eraseCallback(parent.getId(parent.version())); + parent.eraseCallback(parent.getExtId()); } } diff --git a/version.hpp b/version.hpp index 8a68cb5..afc589c 100644 --- a/version.hpp +++ b/version.hpp @@ -77,11 +77,11 @@ class Version : public VersionInherit * @param[in] callback - The eraseFunc callback */ Version(sdbusplus::bus::bus& bus, const std::string& objPath, - const std::string& versionString, VersionPurpose versionPurpose, - const std::string& extVersion, const std::string& filePath, - eraseFunc callback) : + const std::string& extId, const std::string& versionString, + VersionPurpose versionPurpose, const std::string& extVersion, + const std::string& filePath, eraseFunc callback) : VersionInherit(bus, (objPath).c_str(), true), - eraseCallback(callback), versionStr(versionString) + eraseCallback(callback), versionStr(versionString), extId(extId) { // Set properties. extendedVersion(extVersion); @@ -150,6 +150,15 @@ class Version : public VersionInherit */ bool isFunctional(); + /* @brief Return the extended ID of this version object + * + * @ return - returns the extended ID string + */ + std::string getExtId() + { + return extId; + } + /** @brief Persistent Delete D-Bus object */ std::unique_ptr deleteObject; @@ -159,6 +168,8 @@ class Version : public VersionInherit private: /** @brief This Version's version string */ const std::string versionStr; + /** @brief This is extended version string */ + const std::string extId; }; } // namespace manager -- 2.17.1