Keyword VPD:Write keyword's value on hardware #459
Conversation
This commit implements API to write keyword's value on hardware for keyword VPD types. Test: busctl call com.ibm.VPD.Manager /com/ibm/VPD/Manager com.ibm.VPD.Manager WriteKeyword sv "/tmp/keyword.dat" "(say)" "FN" 2 0x65 0x66 hexdump -C keyword.dat 00000010 53 41 53 84 9b 00 46 4e 08 65 66 31 4b 55 37 32 |SAS...FN.ef1KU72| busctl get-property xyz.openbmc_project.Inventory.Manager /xyz/openbmc_project/inventory/system/chassis/motherboard /keyword_vpd_type com.ibm.ipzvpd.VINI FN ay 8 101 102 49 75 85 55 50 52 Change-Id: I203c7d6def40830459f90adf9c0e3c88560956e5 Signed-off-by: Priyanga Ramasamy <[email protected]>
PriyangaRamasamy
requested review from
jinuthomas,
souvik1914581,
SunnySrivastava1984,
RekhaAparna01 and
branupama
| @@ -120,7 +120,8 @@ std::shared_ptr<ParserInterface> | |||
| { | |||
| logging::logMessage("KWD vpd parser selected for VPD file path " + | |||
| i_vpdFilePath); | |||
| return std::make_shared<KeywordVpdParser>(i_vpdVector); | |||
| return std::make_shared<KeywordVpdParser>(i_vpdVector, | |||
There was a problem hiding this comment.
Is this file path validated before using ?
There was a problem hiding this comment.
validated at the caller's place here https://github.com/ibm-openbmc/openpower-vpd-parser/blob/P11_Dev/src/manager.cpp#L218 . But yeah we have to validate in this API as well so that we are safe even if anyother caller failed to validate.
There was a problem hiding this comment.
we will create a new issue to address such validations across the repo.
| } | ||
|
|
||
| // Iterate through VPD vector to find the keyword | ||
| if (findKeyword(l_keywordName) != 0) |
There was a problem hiding this comment.
Where is this defined? Can this be done soon after line 161, so that you don't need to check the keyword data size if this keyword is not a valid one
There was a problem hiding this comment.
The l_keywordData is the user given keyword's data to update on EEPROM.
The l_keywordName is the user given keyword's name which we are finding in the EEPROM at line 169.
|
|
||
| if (l_keywordData.size() == 0) | ||
| { | ||
| logging::logMessage("Given keyword's data is of length 0"); |
There was a problem hiding this comment.
What are the possibilities of a keyword value being size 0?
There was a problem hiding this comment.
User by mistake sending an empty buffer to update on VPD.
| { | ||
| // Resize the given array to the right size | ||
| l_keywordData.resize(l_keywordActualSize); | ||
| l_keywordData.shrink_to_fit(); |
There was a problem hiding this comment.
How to ensure the shrinking will get the correct value as a result? Will this not corrupt it? Should you throw here as invalid?
There was a problem hiding this comment.
resize will erase the extra contents and resizes the vector to l_keywordActualSize. Here only the contents are erased but the vector capacity doesn't change.
So we use shrink_to_fit to remove the unused capacity. So it won't corrupt the vpd.
|
This commit implements API to write keyword's value on hardware for keyword VPD types.
Test:
busctl call com.ibm.VPD.Manager /com/ibm/VPD/Manager com.ibm.VPD.Manager WriteKeyword sv "/tmp/keyword.dat" "(say)" "FN" 2 0x65 0x66
hexdump -C keyword.dat
00000010 53 41 53 84 9b 00 46 4e 08 65 66 31 4b 55 37 32 |SAS...FN.ef1KU72|
busctl get-property xyz.openbmc_project.Inventory.Manager /xyz/openbmc_project/inventory/system/chassis/motherboard /keyword_vpd_type com.ibm.ipzvpd.VINI FN
ay 8 101 102 49 75 85 55 50 52
Change-Id: I203c7d6def40830459f90adf9c0e3c88560956e5