Unable to set minimum ship level
The software-manager-tool internally calls WriteKeyword d-bus api to
set firmware version in hardware (in VSYS FV).
The software-manager-tool is unable to set current firmware level
as minimum ship level due to a bug in vpd-manager "WriteKeyword"
d-bus api.
In WriteKeyword, when we try to read from filestream and write into
vector, the maximum possible vpd size(65504) is given as the total
bytes to be read (n). But the actual hardware data is lesser than
65504 in size.
If the filestream read operation runs out of characters to extract
before n characters have been successfully read, it reads the
available characters and stores it in vector and internally it sets
both bad bit and fail bit.
And with recent changes since we catch all filestream exceptions, this bug
has been caught and code flow is broken.
Fix:
Query the size of the VPD and perform filestream read based on the
given VPD size. If the given VPD size exceeds the maximum possible size
(65504), then read till 65504 only.
The same issue has been addressed in vpd-tool read keyword from hardware.
Test:
vpd-tool -r -H -O /sys/bus/i2c/drivers/at24/8-0050/eeprom -R VSYS -K FV
{
"/sys/bus/i2c/drivers/at24/8-0050/eeprom": {
"FV": "fw1020.00-00 "
}
}
vpd-tool -r -O /system/chassis/motherboard -R VSYS -K FV
{
"/system/chassis/motherboard": {
"FV": "fw1020.00-00 "
}
}
software-manager-tool --setminlevel
<6> Current version: fw1050.00-4.16-dirty. Setting Minimum Ship Level to: fw1050.00-4
vpd-tool -r -O /system/chassis/motherboard -R VSYS -K FV
{
"/system/chassis/motherboard": {
"FV": "fw1050.00-4 "
}
}
vpd-tool -r -H -O /sys/bus/i2c/drivers/at24/8-0050/eeprom -R VSYS -K FV
{
"/sys/bus/i2c/drivers/at24/8-0050/eeprom": {
"FV": "fw1050.00-4 "
}
}
Signed-off-by: Priyanga Ramasamy <priyanga24@in.ibm.com>
Change-Id: I6e24d744542f0a67a872605c7793d73ee27c541c
diff --git a/const.hpp b/const.hpp
index 2b227b1..c3d4c03 100644
--- a/const.hpp
+++ b/const.hpp
@@ -1,6 +1,7 @@
#pragma once
#include <cstdint>
+#include <filesystem>
#include <iostream>
namespace openpower
@@ -114,13 +115,14 @@
constexpr auto errIntfForGpioError = "com.ibm.VPD.Error.GPIOError";
constexpr auto motherBoardInterface =
"xyz.openbmc_project.Inventory.Item.Board.Motherboard";
-constexpr auto invItemIntf = "xyz.openbmc_project.Inventory.Item";
constexpr auto systemVpdFilePath = "/sys/bus/i2c/drivers/at24/8-0050/eeprom";
constexpr auto i2cPathPrefix = "/sys/bus/i2c/drivers/";
constexpr auto spiPathPrefix = "/sys/bus/spi/drivers/";
constexpr auto at24driver = "at24";
constexpr auto at25driver = "at25";
constexpr auto ee1004driver = "ee1004";
+constexpr auto invItemIntf = "xyz.openbmc_project.Inventory.Item";
+static constexpr std::uintmax_t MAX_VPD_SIZE = 65504;
namespace lengths
{
diff --git a/vpd-manager/editor_impl.cpp b/vpd-manager/editor_impl.cpp
index 6015d7c..898a925 100644
--- a/vpd-manager/editor_impl.cpp
+++ b/vpd-manager/editor_impl.cpp
@@ -601,22 +601,29 @@
// prevent any data/ECC corruption in case BMC reboots while VPD update.
enableRebootGuard();
- // TODO: Figure out a better way to get max possible VPD size.
Binary completeVPDFile;
- completeVPDFile.resize(65504);
vpdFileStream.exceptions(std::ifstream::badbit |
std::ifstream::failbit);
try
{
vpdFileStream.open(vpdFilePath,
std::ios::in | std::ios::out | std::ios::binary);
+
+ auto vpdFileSize = std::min(std::filesystem::file_size(vpdFilePath),
+ MAX_VPD_SIZE);
+ if (vpdFileSize == 0)
+ {
+ std::cerr << "File size is 0 for " << vpdFilePath << std::endl;
+ throw std::runtime_error("File size is 0.");
+ }
+
+ completeVPDFile.resize(vpdFileSize);
vpdFileStream.seekg(startOffset, std::ios_base::cur);
vpdFileStream.read(reinterpret_cast<char*>(&completeVPDFile[0]),
- 65504);
- completeVPDFile.resize(vpdFileStream.gcount());
+ vpdFileSize);
vpdFileStream.clear(std::ios_base::eofbit);
}
- catch (const std::ifstream::failure& fail)
+ catch (const std::system_error& fail)
{
std::cerr << "Exception in file handling [" << vpdFilePath
<< "] error : " << fail.what();
diff --git a/vpd_tool.cpp b/vpd_tool.cpp
index ca48a39..9bad631 100644
--- a/vpd_tool.cpp
+++ b/vpd_tool.cpp
@@ -230,15 +230,6 @@
catch (const exception& e)
{
cerr << e.what();
-
- if (*Hardware)
- {
- std::cerr << std::endl
- << "Did you provide a valid offset? By"
- " default VPD offset is taken as 0. To input offset,"
- " use --seek. Refer vpd-tool help."
- << std::endl;
- }
rc = -1;
}
diff --git a/vpd_tool_impl.cpp b/vpd_tool_impl.cpp
index 5610ced..2ba5398 100644
--- a/vpd_tool_impl.cpp
+++ b/vpd_tool_impl.cpp
@@ -655,10 +655,6 @@
{
ifstream inventoryJson(INVENTORY_JSON_SYM_LINK);
auto jsonFile = nlohmann::json::parse(inventoryJson);
-
- Binary completeVPDFile;
- completeVPDFile.resize(65504);
- fstream vpdFileStream;
std::string inventoryPath;
if (jsonFile["frus"].contains(fruPath))
@@ -683,17 +679,30 @@
inventoryPath = jsonFile["frus"][fruPath][0]["inventoryPath"];
}
+ Binary completeVPDFile;
+ fstream vpdFileStream;
+
vpdFileStream.exceptions(std::ifstream::badbit | std::ifstream::failbit);
try
{
vpdFileStream.open(fruPath,
std::ios::in | std::ios::out | std::ios::binary);
+
+ auto vpdFileSize = std::min(std::filesystem::file_size(fruPath),
+ constants::MAX_VPD_SIZE);
+ if (vpdFileSize == 0)
+ {
+ std::cerr << "File size is 0 for " << fruPath << std::endl;
+ throw std::runtime_error("File size is 0.");
+ }
+
+ completeVPDFile.resize(vpdFileSize);
vpdFileStream.seekg(startOffset, ios_base::cur);
- vpdFileStream.read(reinterpret_cast<char*>(&completeVPDFile[0]), 65504);
- completeVPDFile.resize(vpdFileStream.gcount());
+ vpdFileStream.read(reinterpret_cast<char*>(&completeVPDFile[0]),
+ vpdFileSize);
vpdFileStream.clear(std::ios_base::eofbit);
}
- catch (const std::ifstream::failure& fail)
+ catch (const std::system_error& fail)
{
std::cerr << "Exception in file handling [" << fruPath
<< "] error : " << fail.what();