bmc: allowing packing transport flags densely
Future transport backends can densely fill in the upper 5 bits of the
transport flag bitfield.
Signed-off-by: Benjamin Fair <benjaminfair@google.com>
Change-Id: Ie4ee59e0581e458a9020775e18270100f9a1de4e
diff --git a/bmc/firmware_handler.cpp b/bmc/firmware_handler.cpp
index b995671..9b35e5f 100644
--- a/bmc/firmware_handler.cpp
+++ b/bmc/firmware_handler.cpp
@@ -68,16 +68,8 @@
return nullptr;
}
- std::uint16_t bitmask = 0;
- for (const auto& item : transports)
- {
- /* TODO: can use std::accumulate() unless I'm mistaken. :D */
- bitmask |= item.bitmask;
- }
-
- return std::make_unique<FirmwareBlobHandler>(std::move(firmwares), blobs,
- transports, bitmask,
- std::move(actionPacks));
+ return std::make_unique<FirmwareBlobHandler>(
+ std::move(firmwares), blobs, transports, std::move(actionPacks));
}
/* Check if the path is in our supported list (or active list). */
@@ -146,12 +138,13 @@
}
/* They are requesting information about the generic blob_id. */
- meta->blobState = bitmask;
- meta->size = 0;
- /* The generic blob_ids state is only the bits related to the transport
- * mechanisms.
+ /* Older host tools expect the blobState to contain a bitmask of available
+ * transport backends, so report that we support all of them in order to
+ * preserve backwards compatibility.
*/
+ meta->blobState = transportMask;
+ meta->size = 0;
return true;
}
@@ -424,26 +417,19 @@
* layout flash update or a UBI tarball.
*/
- /* Check the flags for the transport mechanism: if none match we don't
- * support what they request.
- */
- if ((flags & bitmask) == 0)
- {
- return false;
- }
+ std::uint16_t transportFlag = flags & transportMask;
/* How are they expecting to copy this data? */
- auto d = std::find_if(
- transports.begin(), transports.end(),
- [&flags](const auto& iter) { return (iter.bitmask & flags); });
+ auto d = std::find_if(transports.begin(), transports.end(),
+ [&transportFlag](const auto& iter) {
+ return (iter.bitmask == transportFlag);
+ });
if (d == transports.end())
{
return false;
}
- /* We found the transport handler they requested, no surprise since
- * above we verify they selected at least one we wanted.
- */
+ /* We found the transport handler they requested */
/* Elsewhere I do this check by checking "if ::ipmi" because that's the
* only non-external data pathway -- but this is just a more generic
diff --git a/bmc/firmware_handler.hpp b/bmc/firmware_handler.hpp
index 3a4b4be..6275843 100644
--- a/bmc/firmware_handler.hpp
+++ b/bmc/firmware_handler.hpp
@@ -117,19 +117,18 @@
* @param[in] firmwares - list of firmware types and their handlers
* @param[in] blobs - list of blobs_ids to support
* @param[in] transports - list of transport types and their handlers
- * @param[in] bitmask - bitmask of transports to support
* @param[in] verification - pointer to object for triggering verification
* @param[in] update - point to object for triggering the update
*/
FirmwareBlobHandler(std::vector<HandlerPack>&& firmwares,
const std::vector<std::string>& blobs,
const std::vector<DataHandlerPack>& transports,
- std::uint16_t bitmask, ActionMap&& actionPacks) :
+ ActionMap&& actionPacks) :
handlers(std::move(firmwares)),
- blobIDs(blobs), transports(transports), bitmask(bitmask),
- activeImage(activeImageBlobId), activeHash(activeHashBlobId),
- verifyImage(verifyBlobId), updateImage(updateBlobId), lookup(),
- state(UpdateState::notYetStarted), actionPacks(std::move(actionPacks))
+ blobIDs(blobs), transports(transports), activeImage(activeImageBlobId),
+ activeHash(activeHashBlobId), verifyImage(verifyBlobId),
+ updateImage(updateBlobId), lookup(), state(UpdateState::notYetStarted),
+ actionPacks(std::move(actionPacks))
{
}
~FirmwareBlobHandler() = default;
@@ -227,9 +226,6 @@
/** List of handlers by transport type. */
std::vector<DataHandlerPack> transports;
- /** The bits set indicate what transport mechanisms are supported. */
- std::uint16_t bitmask;
-
/** Active image session. */
Session activeImage;
@@ -260,6 +256,11 @@
ActionStatus lastVerificationStatus = ActionStatus::unknown;
ActionStatus lastUpdateStatus = ActionStatus::unknown;
+
+ /** Portion of "flags" argument to open() which specifies the desired
+ * transport type
+ */
+ static constexpr std::uint16_t transportMask = 0xff00;
};
} // namespace ipmi_flash
diff --git a/bmc/test/firmware_stat_unittest.cpp b/bmc/test/firmware_stat_unittest.cpp
index 6e97cc8..4eb2e99 100644
--- a/bmc/test/firmware_stat_unittest.cpp
+++ b/bmc/test/firmware_stat_unittest.cpp
@@ -14,7 +14,9 @@
namespace
{
-TEST(FirmwareHandlerStatTest, StatOnInactiveBlobIDReturnsTransport)
+/* This test ensures the stat() method preserves compatibility with older host
+ * tools by reporting that all transports are supported. */
+TEST(FirmwareHandlerStatTest, StatOnInactiveBlobIDReturnsAllTransports)
{
/* Test that the metadata information returned matches expectations for this
* case.
@@ -38,7 +40,8 @@
blobs::BlobMeta meta;
EXPECT_TRUE(handler->stat("asdf", &meta));
- EXPECT_EQ(FirmwareFlags::UpdateFlags::ipmi, meta.blobState);
+ /* All transport flags are set */
+ EXPECT_EQ(0xff00, meta.blobState);
}
} // namespace
diff --git a/bmc/test/firmware_state_notyetstarted_unittest.cpp b/bmc/test/firmware_state_notyetstarted_unittest.cpp
index 8aa811f..2a100bd 100644
--- a/bmc/test/firmware_state_notyetstarted_unittest.cpp
+++ b/bmc/test/firmware_state_notyetstarted_unittest.cpp
@@ -73,18 +73,15 @@
TEST_F(FirmwareHandlerNotYetStartedTest, StatEachBlobIdVerifyResults)
{
/* In this original state, calling stat() on the blob ids will return the
- * transported supported.
+ * idle status
*/
- blobs::BlobMeta expected;
- expected.blobState = FirmwareFlags::UpdateFlags::ipmi;
- expected.size = 0;
auto blobs = handler->getBlobIds();
for (const auto& blob : blobs)
{
blobs::BlobMeta meta = {};
EXPECT_TRUE(handler->stat(blob, &meta));
- EXPECT_EQ(expected, meta);
+ EXPECT_EQ(expectedIdleMeta, meta);
}
}
diff --git a/bmc/test/firmware_state_updatecompleted_unittest.cpp b/bmc/test/firmware_state_updatecompleted_unittest.cpp
index fd7f583..320d09e 100644
--- a/bmc/test/firmware_state_updatecompleted_unittest.cpp
+++ b/bmc/test/firmware_state_updatecompleted_unittest.cpp
@@ -122,10 +122,6 @@
{
getToUpdateCompleted(ActionStatus::success);
- blobs::BlobMeta expected;
- expected.blobState = FirmwareFlags::UpdateFlags::ipmi;
- expected.size = 0;
-
std::vector<std::string> testBlobs = {staticLayoutBlobId, hashBlobId};
for (const auto& blob : testBlobs)
{
@@ -133,7 +129,7 @@
blobs::BlobMeta meta = {};
EXPECT_TRUE(handler->stat(blob, &meta));
- EXPECT_EQ(expected, meta);
+ EXPECT_EQ(expectedIdleMeta, meta);
}
}
diff --git a/bmc/test/firmware_state_updatepending_unittest.cpp b/bmc/test/firmware_state_updatepending_unittest.cpp
index 6f5162f..d757b3c 100644
--- a/bmc/test/firmware_state_updatepending_unittest.cpp
+++ b/bmc/test/firmware_state_updatepending_unittest.cpp
@@ -163,17 +163,13 @@
{
getToUpdatePending();
- blobs::BlobMeta expected;
- expected.blobState = FirmwareFlags::UpdateFlags::ipmi;
- expected.size = 0;
-
for (const auto& blob : startingBlobs)
{
ASSERT_TRUE(handler->canHandleBlob(blob));
blobs::BlobMeta meta = {};
EXPECT_TRUE(handler->stat(blob, &meta));
- EXPECT_EQ(expected, meta);
+ EXPECT_EQ(expectedIdleMeta, meta);
}
}
diff --git a/bmc/test/firmware_state_updatestarted_unittest.cpp b/bmc/test/firmware_state_updatestarted_unittest.cpp
index 2e4f020..086dcfa 100644
--- a/bmc/test/firmware_state_updatestarted_unittest.cpp
+++ b/bmc/test/firmware_state_updatestarted_unittest.cpp
@@ -113,10 +113,6 @@
{
getToUpdateStarted();
- blobs::BlobMeta expected;
- expected.blobState = FirmwareFlags::UpdateFlags::ipmi;
- expected.size = 0;
-
std::vector<std::string> testBlobs = {staticLayoutBlobId, hashBlobId};
for (const auto& blob : testBlobs)
{
@@ -124,7 +120,7 @@
blobs::BlobMeta meta = {};
EXPECT_TRUE(handler->stat(blob, &meta));
- EXPECT_EQ(expected, meta);
+ EXPECT_EQ(expectedIdleMeta, meta);
}
}
diff --git a/bmc/test/firmware_state_uploadinprogress_unittest.cpp b/bmc/test/firmware_state_uploadinprogress_unittest.cpp
index 72da8bb..366435b 100644
--- a/bmc/test/firmware_state_uploadinprogress_unittest.cpp
+++ b/bmc/test/firmware_state_uploadinprogress_unittest.cpp
@@ -95,10 +95,6 @@
/* Calling stat() on the normal blobs (not the active) ones will work and
* return the same information as in the notYetStarted state.
*/
- blobs::BlobMeta expected;
- expected.blobState = FirmwareFlags::UpdateFlags::ipmi;
- expected.size = 0;
-
openToInProgress(staticLayoutBlobId);
std::vector<std::string> testBlobs = {staticLayoutBlobId, hashBlobId};
@@ -106,7 +102,7 @@
{
blobs::BlobMeta meta = {};
EXPECT_TRUE(handler->stat(blob, &meta));
- EXPECT_EQ(expected, meta);
+ EXPECT_EQ(expectedIdleMeta, meta);
}
}
diff --git a/bmc/test/firmware_state_verificationcompleted_unittest.cpp b/bmc/test/firmware_state_verificationcompleted_unittest.cpp
index 85f1895..0c6bae7 100644
--- a/bmc/test/firmware_state_verificationcompleted_unittest.cpp
+++ b/bmc/test/firmware_state_verificationcompleted_unittest.cpp
@@ -141,10 +141,6 @@
{
getToVerificationCompleted(ActionStatus::success);
- blobs::BlobMeta expected;
- expected.blobState = FirmwareFlags::UpdateFlags::ipmi;
- expected.size = 0;
-
std::vector<std::string> testBlobs = {staticLayoutBlobId, hashBlobId};
for (const auto& blob : testBlobs)
{
@@ -152,7 +148,7 @@
blobs::BlobMeta meta = {};
EXPECT_TRUE(handler->stat(blob, &meta));
- EXPECT_EQ(expected, meta);
+ EXPECT_EQ(expectedIdleMeta, meta);
}
}
diff --git a/bmc/test/firmware_state_verificationpending_unittest.cpp b/bmc/test/firmware_state_verificationpending_unittest.cpp
index 92d5c6b..c9b8e92 100644
--- a/bmc/test/firmware_state_verificationpending_unittest.cpp
+++ b/bmc/test/firmware_state_verificationpending_unittest.cpp
@@ -174,17 +174,13 @@
{
getToVerificationPending(staticLayoutBlobId);
- blobs::BlobMeta expected;
- expected.blobState = FirmwareFlags::UpdateFlags::ipmi;
- expected.size = 0;
-
std::vector<std::string> testBlobs = {staticLayoutBlobId, hashBlobId};
for (const auto& blob : testBlobs)
{
ASSERT_TRUE(handler->canHandleBlob(blob));
blobs::BlobMeta meta = {};
EXPECT_TRUE(handler->stat(blob, &meta));
- EXPECT_EQ(expected, meta);
+ EXPECT_EQ(expectedIdleMeta, meta);
}
}
diff --git a/bmc/test/firmware_state_verificationstarted_unittest.cpp b/bmc/test/firmware_state_verificationstarted_unittest.cpp
index 8c0d476..a974c2b 100644
--- a/bmc/test/firmware_state_verificationstarted_unittest.cpp
+++ b/bmc/test/firmware_state_verificationstarted_unittest.cpp
@@ -194,10 +194,6 @@
{
getToVerificationStarted(staticLayoutBlobId);
- blobs::BlobMeta expected;
- expected.blobState = FirmwareFlags::UpdateFlags::ipmi;
- expected.size = 0;
-
std::vector<std::string> testBlobs = {staticLayoutBlobId, hashBlobId};
for (const auto& blob : testBlobs)
{
@@ -205,7 +201,7 @@
blobs::BlobMeta meta = {};
EXPECT_TRUE(handler->stat(blob, &meta));
- EXPECT_EQ(expected, meta);
+ EXPECT_EQ(expectedIdleMeta, meta);
}
}
diff --git a/bmc/test/firmware_unittest.hpp b/bmc/test/firmware_unittest.hpp
index 6bf515d..3d7fa4e 100644
--- a/bmc/test/firmware_unittest.hpp
+++ b/bmc/test/firmware_unittest.hpp
@@ -187,6 +187,8 @@
std::uint16_t flags =
blobs::OpenFlags::write | FirmwareFlags::UpdateFlags::ipmi;
+ blobs::BlobMeta expectedIdleMeta = {0xff00, 0, {}};
+
std::vector<std::string> startingBlobs = {staticLayoutBlobId, hashBlobId};
};