Support uploading same image for multiple times
The code was unable to upload the same image for multiple times because
it has the same version ID.
Add a salt when generating the version ID so that we could get different
version IDs for the same image, and thus it is possible to upload the
images for multiple times. Use a random generator with the time as seed
to ensure uniqueness.
When the image updater starts up, use the mount directory name or
functional suffix as seeds to make the version ids unique.
The version ID is then necessary in the version object because the Delete
interface needs it when the version object is to be deleted.
With this change, it is possible to upload and update the same image for
multiple times, and systems with dual BMC chips could have the same image
as well.
Tested: Upload a same image for multiple times and verify all the
uploads are OK and getting different DBus objects;
Verify it is OK to delete the uploaded images by deleting the
DBus objects.
Verify the code update works on all layouts and that the flash
id (Path property) was set.
- static
root@romulus:~# busctl --no-pager introspect \
xyz.openbmc_project.Software.BMC.Updater \
/xyz/openbmc_project/software/79139bc0
...
.Path property s "9e3b868e"
root@romulus:~# ls -l /run/media/rofs-9e3b868e-functional/etc/
lrwxrwxrwx 1 root root 15 Jan 22 20:11 os-release ->
/etc/os-release
- ubi
root@witherspoon:~# busctl --no-pager introspect \
xyz.openbmc_project.Software.BMC.Updater \
/xyz/openbmc_project/software/151bc3d8
...
.Path property s "cfb85943"
root@witherspoon:~# df
/dev/ubiblock0_0 18816 18816 0 100% /media/rofs-cfb85943-functional
/dev/ubiblock4_0 18816 18816 0 100% /media/rofs-26085328
root@witherspoon:~# cat /var/lib/phosphor-bmc-code-mgmt/cfb85943/priority
{
"priority": 0
}
- mmc
root@p10bmc:~# busctl --no-pager introspect \
xyz.openbmc_project.Software.BMC.Updater \
/xyz/openbmc_project/software/cb5f99e2
...
.Path property s "b"
root@p10bmc:~# df
/dev/mmcblk0p5 202095 153782 33107 82% /media/rofs-b-functional
root@p10bmc:~# cat /var/lib/phosphor-bmc-code-mgmt/b/priority
{
"priority": 0
}
Signed-off-by: Lei YU <yulei.sh@bytedance.com>
Signed-off-by: Adriana Kobylak <anoo@us.ibm.com>
Change-Id: I76842de4d5b7c5175acc43ed7c57e0deb4057be5
diff --git a/image_manager.cpp b/image_manager.cpp
index 922575b..93fac66 100644
--- a/image_manager.cpp
+++ b/image_manager.cpp
@@ -18,7 +18,9 @@
#include <algorithm>
#include <cstring>
+#include <ctime>
#include <filesystem>
+#include <random>
#include <string>
namespace phosphor
@@ -187,7 +189,8 @@
Version::getValue(manifestPath.string(), "ExtendedVersion");
// Compute id
- auto id = Version::getId(version);
+ auto salt = std::to_string(randomGen());
+ auto id = Version::getId(version + salt);
fs::path imageDirPath = std::string{IMG_UPLOAD_DIR};
imageDirPath /= id;
@@ -211,7 +214,7 @@
auto versionPtr = std::make_unique<Version>(
bus, objPath, version, purpose, extendedVersion,
imageDirPath.string(),
- std::bind(&Manager::erase, this, std::placeholders::_1));
+ std::bind(&Manager::erase, this, std::placeholders::_1), id);
versionPtr->deleteObject =
std::make_unique<phosphor::software::manager::Delete>(bus, objPath,
*versionPtr);
diff --git a/image_manager.hpp b/image_manager.hpp
index acd56b6..25f0ced 100644
--- a/image_manager.hpp
+++ b/image_manager.hpp
@@ -3,6 +3,8 @@
#include <sdbusplus/server.hpp>
+#include <chrono>
+#include <random>
#include <string>
namespace phosphor
@@ -51,6 +53,10 @@
/** @brief Persistent sdbusplus DBus bus connection. */
sdbusplus::bus::bus& bus;
+ /** @brief The random generator to get the version salt */
+ std::mt19937 randomGen{static_cast<unsigned>(
+ std::chrono::system_clock::now().time_since_epoch().count())};
+
/**
* @brief Untar the tarball.
*
diff --git a/item_updater.cpp b/item_updater.cpp
index 9309a3f..e694ee0 100644
--- a/item_updater.cpp
+++ b/item_updater.cpp
@@ -145,7 +145,8 @@
auto versionPtr = std::make_unique<VersionClass>(
bus, path, version, purpose, extendedVersion, filePath,
- std::bind(&ItemUpdater::erase, this, std::placeholders::_1));
+ std::bind(&ItemUpdater::erase, this, std::placeholders::_1),
+ versionId);
versionPtr->deleteObject =
std::make_unique<phosphor::software::manager::Delete>(bus, path,
*versionPtr);
@@ -217,7 +218,10 @@
continue;
}
- auto id = VersionClass::getId(version);
+ // The flash location is part of the mount name: rofs-<location>
+ auto flashId = iter.path().native().substr(BMC_RO_PREFIX_LEN);
+
+ auto id = VersionClass::getId(version + flashId);
// Check if the id has already been added. This can happen if the
// BMC partitions / devices were manually flashed with the same
@@ -227,8 +231,6 @@
continue;
}
- // The flash location is part of the mount name: rofs-<location>
- auto flashId = iter.path().native().substr(BMC_RO_PREFIX_LEN);
auto functional = false;
if (iter.path().native().find(functionalSuffix) !=
std::string::npos)
@@ -274,7 +276,8 @@
// Create Version instance for this version.
auto versionPtr = std::make_unique<VersionClass>(
bus, path, version, purpose, extendedVersion, flashId,
- std::bind(&ItemUpdater::erase, this, std::placeholders::_1));
+ std::bind(&ItemUpdater::erase, this, std::placeholders::_1),
+ id);
if (functional)
{
versionPtr->setFunctional(true);
@@ -324,7 +327,8 @@
if (activations.size() == 0)
{
auto version = VersionClass::getBMCVersion(OS_RELEASE_FILE);
- auto id = phosphor::software::manager::Version::getId(version);
+ auto id = phosphor::software::manager::Version::getId(version +
+ functionalSuffix);
auto versionFileDir = BMC_ROFS_PREFIX + id + functionalSuffix + "/etc/";
try
{
@@ -787,7 +791,7 @@
};
biosVersion = std::make_unique<VersionClass>(
bus, path, version, VersionPurpose::Host, "", "",
- std::bind(dummyErase, std::placeholders::_1));
+ std::bind(dummyErase, std::placeholders::_1), "");
biosVersion->deleteObject =
std::make_unique<phosphor::software::manager::Delete>(bus, path,
*biosVersion);
diff --git a/version.cpp b/version.cpp
index 11d4201..250c221 100644
--- a/version.cpp
+++ b/version.cpp
@@ -206,7 +206,7 @@
{
if (parent.eraseCallback)
{
- parent.eraseCallback(parent.getId(parent.version()));
+ parent.eraseCallback(parent.id);
}
}
diff --git a/version.hpp b/version.hpp
index 73cc137..b6269e0 100644
--- a/version.hpp
+++ b/version.hpp
@@ -79,9 +79,9 @@
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) :
+ eraseFunc callback, const std::string& id) :
VersionInherit(bus, (objPath).c_str(), true),
- eraseCallback(callback), versionStr(versionString)
+ eraseCallback(callback), id(id), versionStr(versionString)
{
// Set properties.
extendedVersion(extVersion);
@@ -106,11 +106,13 @@
* @details The version id is a unique 8 hexadecimal digit id
* calculated from the version string.
*
- * @param[in] version - The image's version string (e.g. v1.99.10-19).
+ * @param[in] versionWithSalt - The image's version string
+ * (e.g. v1.99.10-19) plus an optional salt
+ * string.
*
* @return The id.
*/
- static std::string getId(const std::string& version);
+ static std::string getId(const std::string& versionWithSalt);
/**
* @brief Get the active BMC machine name string.
@@ -166,6 +168,9 @@
/** @brief The parent's erase callback. */
eraseFunc eraseCallback;
+ /** @brief The version ID of the object */
+ const std::string id;
+
private:
/** @brief This Version's version string */
const std::string versionStr;