Conversation
| @@ -118,6 +118,34 @@ int guardNext(GuardFile& file, int pos, GuardRecord& guard) | |||
| return pos; | |||
There was a problem hiding this comment.
Commit message:
Please elaborate on the problem and add test case details.
we could split this commit as 2 commit
- New flag and API (it can talk about the actual issue and solution)
- libguard using that API to support BMC flow too (The reason for updating the flag in the libguard too)
libguard/guard_interface.cpp
Outdated
| if (set) | ||
| { | ||
| // Setting write bit in guard header | ||
| guardRecord.iv_flags = GARD_FLAG_MASK_PNOR_WRITE_IN_PROGRESS; |
There was a problem hiding this comment.
I think we will hit the endianness issue here. Can please confirm with the HB team?
BMC is little-endian and HB is big-endian (pnor), HB uses the 7th bit from the flag but, BMC will set in the 0th bit in the flag.
The same comment is applicable to the else block.
There was a problem hiding this comment.
This is unit8_t no endianness issue for 1 byte data.
libguard/guard_interface.cpp
Outdated
| memset(&existGuard, 0xff, sizeOfGuard); | ||
|
|
||
| GuardFile file(guardFilePath); | ||
| toggleWriteFlag(file, true); |
There was a problem hiding this comment.
Why cannot just use it inside the GuardFile::write() instead of calling many places? any specific reason?
There was a problem hiding this comment.
To keep the guard_file.cpp only do basic read and write operation, any external operation should be done in interface.cpp i.e. writing to header etc.
Change: Adding GardFlags and GardFlagPnorWriteInProgress enums to handle GUARD partition write. Signed-off-by: Chirag Sharma <chirshar@in.ibm.com>
Change: Adding new function to set/reset the write flag. While creating guard record or re-writing a guard record write flag will be set to 0x00 and once the write completes, set the flag to 0x01. Tested: While writing the guard data the flag is set as 0x00 and once the write is finished its set back as 0x01. Created guard records to check if data is coming as expected or not. Signed-off-by: Chirag Sharma <chirshar@in.ibm.com>
Change: Adding function checkWriteflag, which will read the write flag. Tested: In ipl code calling this function and its working as expected i.e. if flag is set to 0x00 return true else false. Signed-off-by: Chirag Sharma <chirshar@in.ibm.com>
| (existGuard.targetId == entityPath)) && | ||
| (existGuard.recordId != GUARD_RESOLVED)) | ||
| { | ||
| toggleWriteFlag(file); |
There was a problem hiding this comment.
this is not correct, these chain operatoins are dangerous you look for CustoFD class you need to follow that, constructor of CustomFd opens the file and destructor closes the fd.
{
CustomFD fd;
.
.
}
when it reaches here fd will be closed
|
Please include summary in the PR why these chnages introduced. PR name "Huard" is not making any sense here. |
RameshIyyar
requested review from
RameshIyyar
and removed request for
RameshIyyar
4 participants
No description provided.