From 1fe10a14c38e9bcc6bf8c74b30885ed1d0ba6129 Mon Sep 17 00:00:00 2001 From: Adriana Kobylak Date: Wed, 22 Sep 2021 15:52:00 +0000 Subject: [PATCH] oem: ibm: Assemble the image in a forked process The assembling of the tarball image file is a long running process, causing pldm to not reply to any other requests until it's done, causing errors such as when the hypervisor tries to ping the bmc to determine if it's running. Move the assembling of the image to a forked process so that it runs in the background. Move the assembling function to the code update class so that it has access to the code update sensor to set it to error if an failure occurs, since no parent process will wait for it. Use double fork to avoid zombie tasks, this allows the grandchild to be reparented to init. Tested: Verified the hypervisor did not reset the BMC during an inband update because it didn't receive a reply to its ping request. Change-Id: I47d5fde1a1311e94d5b03f7fcf4c0ab15ba054e3 Signed-off-by: Adriana Kobylak --- .../libpldmresponder/inband_code_update.cpp | 134 +++++++++++++----- .../libpldmresponder/inband_code_update.hpp | 12 +- oem/ibm/libpldmresponder/oem_ibm_handler.cpp | 2 +- 3 files changed, 108 insertions(+), 40 deletions(-) diff --git a/oem/ibm/libpldmresponder/inband_code_update.cpp b/oem/ibm/libpldmresponder/inband_code_update.cpp index c474c25ed..dc83af864 100644 --- a/oem/ibm/libpldmresponder/inband_code_update.cpp +++ b/oem/ibm/libpldmresponder/inband_code_update.cpp @@ -523,49 +523,117 @@ int processCodeUpdateLid(const std::string& filePath) return PLDM_SUCCESS; } -int assembleCodeUpdateImage() +int CodeUpdate::assembleCodeUpdateImage() { - // Create the hostfw squashfs image from the LID files without header - auto rc = executeCmd("/usr/sbin/mksquashfs", lidDirPath.c_str(), - hostfwImagePath.c_str(), "-all-root", "-no-recovery"); - if (rc < 0) + pid_t pid = fork(); + + if (pid == 0) { - return PLDM_ERROR; - } + pid_t nextPid = fork(); + if (nextPid == 0) + { + // Create the hostfw squashfs image from the LID files without + // header + auto rc = executeCmd("/usr/sbin/mksquashfs", lidDirPath.c_str(), + hostfwImagePath.c_str(), "-all-root", + "-no-recovery"); + if (rc < 0) + { + std::cerr << "Error occurred during the mksqusquashfs call" + << std::endl; + setCodeUpdateProgress(false); + auto sensorId = getFirmwareUpdateSensor(); + sendStateSensorEvent(sensorId, PLDM_STATE_SENSOR_STATE, 0, + uint8_t(CodeUpdateState::FAIL), + uint8_t(CodeUpdateState::START)); + exit(EXIT_FAILURE); + } - fs::create_directories(updateDirPath); + fs::create_directories(updateDirPath); - // Extract the BMC tarball content - rc = executeCmd("/bin/tar", "-xf", tarImagePath.c_str(), "-C", - updateDirPath); - if (rc < 0) - { - return PLDM_ERROR; - } + // Extract the BMC tarball content + rc = executeCmd("/bin/tar", "-xf", tarImagePath.c_str(), "-C", + updateDirPath); + if (rc < 0) + { + setCodeUpdateProgress(false); + auto sensorId = getFirmwareUpdateSensor(); + sendStateSensorEvent(sensorId, PLDM_STATE_SENSOR_STATE, 0, + uint8_t(CodeUpdateState::FAIL), + uint8_t(CodeUpdateState::START)); + exit(EXIT_FAILURE); + } + + // Add the hostfw image to the directory where the contents were + // extracted + fs::copy_file(hostfwImagePath, + fs::path(updateDirPath) / hostfwImageName, + fs::copy_options::overwrite_existing); + + // Remove the tarball file, then re-generate it with so that the + // hostfw image becomes part of the tarball + fs::remove(tarImagePath); + rc = executeCmd("/bin/tar", "-cf", tarImagePath, ".", "-C", + updateDirPath); + if (rc < 0) + { + std::cerr + << "Error occurred during the generation of the tarball" + << std::endl; + setCodeUpdateProgress(false); + auto sensorId = getFirmwareUpdateSensor(); + sendStateSensorEvent(sensorId, PLDM_STATE_SENSOR_STATE, 0, + uint8_t(CodeUpdateState::FAIL), + uint8_t(CodeUpdateState::START)); + exit(EXIT_FAILURE); + } + + // Copy the tarball to the update directory to trigger the phosphor + // software manager to create a version interface + fs::copy_file(tarImagePath, updateImagePath, + fs::copy_options::overwrite_existing); - // Add the hostfw image to the directory where the contents were extracted - fs::copy_file(hostfwImagePath, fs::path(updateDirPath) / hostfwImageName, - fs::copy_options::overwrite_existing); + // Cleanup + fs::remove_all(updateDirPath); + fs::remove_all(lidDirPath); + fs::remove_all(imageDirPath); - // Remove the tarball file, then re-generate it with so that the hostfw - // image becomes part of the tarball - fs::remove(tarImagePath); - rc = executeCmd("/bin/tar", "-cf", tarImagePath, ".", "-C", updateDirPath); - if (rc < 0) + exit(EXIT_SUCCESS); + } + else if (nextPid < 0) + { + std::cerr << "Error occurred during fork. ERROR=" << errno + << std::endl; + exit(EXIT_FAILURE); + } + + // Do nothing as parent. When parent exits, child will be reparented + // under init and be reaped properly. + exit(0); + } + else if (pid > 0) + { + int status; + if (waitpid(pid, &status, 0) < 0) + { + std::cerr << "Error occurred during waitpid. ERROR=" << errno + << std::endl; + return PLDM_ERROR; + } + else if (WEXITSTATUS(status) != 0) + { + std::cerr + << "Failed to execute the assembling of the image. STATUS=" + << status << std::endl; + return PLDM_ERROR; + } + } + else { + std::cerr << "Error occurred during fork. ERROR=" << errno << std::endl; return PLDM_ERROR; } - // Copy the tarball to the update directory to trigger the phosphor software - // manager to create a version interface - fs::copy_file(tarImagePath, updateImagePath, - fs::copy_options::overwrite_existing); - - // Cleanup - fs::remove_all(updateDirPath); - fs::remove_all(lidDirPath); - fs::remove_all(imageDirPath); - return PLDM_SUCCESS; } diff --git a/oem/ibm/libpldmresponder/inband_code_update.hpp b/oem/ibm/libpldmresponder/inband_code_update.hpp index 7dbc735bb..7832579be 100644 --- a/oem/ibm/libpldmresponder/inband_code_update.hpp +++ b/oem/ibm/libpldmresponder/inband_code_update.hpp @@ -168,6 +168,12 @@ class CodeUpdate */ void deleteImage(); + /** @brief Method to assemble the code update tarball and trigger the + * phosphor software manager to create a version interface + * @return - PLDM_SUCCESS codes + */ + int assembleCodeUpdateImage(); + virtual ~CodeUpdate() {} @@ -228,11 +234,5 @@ int setBootSide(uint16_t entityInstance, uint8_t currState, */ int processCodeUpdateLid(const std::string& filePath); -/** @brief Method to assemble the code update tarball and trigger the - * phosphor software manager to create a version interface - * @return - PLDM_SUCCESS codes - */ -int assembleCodeUpdateImage(); - } // namespace responder } // namespace pldm diff --git a/oem/ibm/libpldmresponder/oem_ibm_handler.cpp b/oem/ibm/libpldmresponder/oem_ibm_handler.cpp index 4ec745e2f..a58c9bd7d 100644 --- a/oem/ibm/libpldmresponder/oem_ibm_handler.cpp +++ b/oem/ibm/libpldmresponder/oem_ibm_handler.cpp @@ -555,7 +555,7 @@ void pldm::responder::oem_ibm_platform::Handler::_processEndUpdate( sdeventplus::source::EventBase& /*source */) { assembleImageEvent.reset(); - int retc = assembleCodeUpdateImage(); + int retc = codeUpdate->assembleCodeUpdateImage(); if (retc != PLDM_SUCCESS) { codeUpdate->setCodeUpdateProgress(false);