Handle existing sym link with no devTree tag #442
Conversation
branupama
requested review from
SunnySrivastava1984,
jinuthomas,
RekhaAparna01,
PriyangaRamasamy and
souvik1914581
and removed request for
SunnySrivastava1984 and
jinuthomas
vpd-manager application is crashing while coming up when INVENTORY_JSON_SYM_LINK symbolic link exists already and pointing to
the config JSON file having no ‘devTree’ tag in it.
This commit adds the code to handle the above issue in the Worker class
and handles the following scenarios as well,
• If creating sym link fails with “File exists” error message, will try
to delete the existing sym link and create sym link again with the
system config JSON found for that system.
- In case if deleting of sym link fails, will run the application
with the existing configuration file itself.
• getParsedJson API from jsonUtility is being used to parse the system
config JSON file instead of nlohmann::json::parse API. As this API
needs file stream object as an input.
• In case, loaded system config JSON file from the above scenario
doesn’t have ‘devTree’ tag again, application is allowed to run with
that configuration.
Signed-off-by: Anupama B R <[email protected]>
branupama
force-pushed
the
handle_sym_link
branch
from
e84d22e
to
6f5978d
Compare
| @@ -466,31 +469,71 @@ void Worker::setDeviceTreeAndJson() | |||
| ec); | |||
| if (ec) | |||
| { | |||
| throw std::runtime_error( | |||
| "create_symlink system call failed with error" + ec.message()); | |||
| if (ec.message().compare("File exists") == | |||
There was a problem hiding this comment.
instead of doing string compare, can we check on the error code which is 17 for file exists?
https://mariadb.com/kb/en/operating-system-error-codes/
constexpr auto EEXIST = 17;
if(errno == EEXIST)
{
...
}
| else | ||
| { | ||
| logging::logMessage( | ||
| "remove system call failed for file[" + |
There was a problem hiding this comment.
Instead of this statement "remove system call failed for file"
can we have something like - "Unable to delete the existing symlink " -> this will create a relation with the message you logged in line 477.
If there is a continuity in the message statements we log, it will help us in debugging without looking at the code.
| ec.clear(); | ||
| logging::logMessage( | ||
| "Sym link already exists, file [" + | ||
| std::string(INVENTORY_JSON_SYM_LINK) + |
There was a problem hiding this comment.
we know that the symlink path is always /var/lib/vpd/vpd_inventory.json.
what we don't know here is to which target JSON the link is pointing to. You can use filesystem::read_symlink(p) to get the target json path.
Please try to add the target json along with the source json in the below logs as well.
| { | ||
| throw std::runtime_error( | ||
| "create_symlink system call failed with error: " + | ||
| ec.message()); |
There was a problem hiding this comment.
can we have a format to print the error message, something like :
"Error description message. " +
"errno = " +
" Symlink : <source_json_ path> --> < target json path> "
| if (devTreeFromJson.empty()) | ||
| { | ||
| throw JsonException( | ||
| "Mandatory value for device tree missing from JSON", |
There was a problem hiding this comment.
your commit message says that, you will allow the application to run if devTree tag is not found. But here you are throwing the exception. can you please check on this
| } | ||
|
|
||
| auto fitConfigVal = readFitConfigValue(); | ||
|
|
||
| if (fitConfigVal.find(devTreeFromJson) != std::string::npos) | ||
| if (devTreeFromJson.empty() || | ||
| fitConfigVal.find(devTreeFromJson) != std::string::npos) |
There was a problem hiding this comment.
in line 536, why are you using find and not == ?
| @@ -448,6 +448,9 @@ void Worker::setDeviceTreeAndJson() | |||
| types::VPDMapVariant parsedVpdMap; | |||
| fillVPDMap(SYSTEM_VPD_FILE_PATH, parsedVpdMap); | |||
|
|
|||
| // ToDo: Need to check is INVENTORY_JSON_SYM_LINK pointing to correct system | |||
| // JSON or not, if the sym link exists already. | |||
There was a problem hiding this comment.
do we need this todo anymore?
| else | ||
| { | ||
| throw std::runtime_error( | ||
| "create_symlink system call failed with error: " + |
There was a problem hiding this comment.
missing symlink source and target path
There was a problem hiding this comment.
Also the error message in line 504 and line 489 are same. While debugging it is not easy to figure out where we hit the issue. So let's have a description which helps us in debugging.
vpd-manager application is crashing while coming up when INVENTORY_JSON_SYM_LINK symbolic link exists already and pointing to the config JSON file having no ‘devTree’ tag in it.
This commit adds the code to handle the above issue in the Worker class and handles the following scenarios as well,
• If creating sym link fails with “File exists” error message, will try
to delete the existing sym link and create sym link again with the system config JSON found for that system.
• getParsedJson API from jsonUtility is being used to parse the system
config JSON file instead of nlohmann::json::parse API. As this API needs file stream object as an input.
• In case, loaded system config JSON file from the above scenario
doesn’t have ‘devTree’ tag again, application is allowed to run with that configuration.