From 77b861136b6780ce4eabfe9589a0b584e6ed2b43 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, 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 | 16 +++++++++++----- pfr_image_manager.cpp | 2 +- version.cpp | 2 +- version.hpp | 19 +++++++++++++++---- 5 files changed, 29 insertions(+), 12 deletions(-) diff --git a/image_manager.cpp b/image_manager.cpp index 6334704cd980..4fefd221e6d2 100644 --- a/image_manager.cpp +++ b/image_manager.cpp @@ -221,7 +221,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 26b52b3f7846..3f64feb43c55 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, @@ -260,7 +260,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) @@ -336,9 +336,9 @@ 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()) { - error( + info( "Version ({VERSIONID}) is currently running on the BMC; unable to remove.", "VERSIONID", entryId); return; @@ -679,6 +679,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() == @@ -772,7 +778,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 80db63ca4d85..03bc34a3a78b 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 97f3be94b4aa..5410c38887f8 100644 --- a/version.cpp +++ b/version.cpp @@ -208,7 +208,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 8a68cb5f7b1f..afc589c0226c 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