Make enableUnit and disableUnit files as optional
Disable Unit Files has no effect if the service is already masked.
Here are the logs:
root:~# systemctl mask xyz.openbmc_project.PCIe
Created symlink /etc/systemd/system/xyz.openbmc_project.PCIe.service ->
/dev/null.
root:~# systemctl disable xyz.openbmc_project.PCIe
Unit /etc/systemd/system/xyz.openbmc_project.PCIe.service is masked,
ignoring.
Thus disabling after masking is not really required.
As masking creates a symlink of the unit file in /etc/systemd/system to
/dev/null and unmasking removes it. Due to symlink being directed to
/dev/null, it would not be possible to start a masked service at all.
Disabling removes an existing (proper) symlink in /etc/systemd/system/
<multi-user.target.wants>or<insert-other-relevant-directory>/* to the
unit file, and enabling it inserts back the symlink in /etc/systemd/
system/<multi-user.target.wants>or<insert-other-relevant-directory>/*.
In case of mctpd, the "enable unit" created an unnecessary symlink in
/etc/systemd/system, thus it caused systemd to start mctp service but
they are designed to be started by a different service than systemd.
Thus, enabling after unmasking is not required for mctp.
However, there is an issue here with the service which has its unit
file's symlink defined in /etc/systemd/system directory, then,
unmasking will remove the symlink altogether. Although no service
today has its symlink in /etc/systemd/system, but to work with such
possibility we made an optional parameter disableOrEnableUnitFiles i.e.
pass it as an argument for the startOrStopService. Also, the correct
order needs to be disable the service first and mask it. In the other
way around - it needs to be unmasked first and enable it.
Tested:
Regression testing has been done and all the functionalities are working
as intended for all the below tests:
1) Stop,start,enable and disable functionalities for all the MCTP
services and PCIe service.
2) For disable test, we disable the service, restart the BMC and check
is it still disabled or not. Same with AC cycle.
3) For enable test, we re-enable the service, restart BMC and check
whether it is enabled or not and same with
AC cycle.
Signed-off-by: ANJALI RAY <anjali.ray@intel.com>
Change-Id: I9ebd20a4f0ba8c11d6f08a54078550172cb382ea
diff --git a/src/manufacturingcommands.cpp b/src/manufacturingcommands.cpp
index 7ed7506..9f16d95 100644
--- a/src/manufacturingcommands.cpp
+++ b/src/manufacturingcommands.cpp
@@ -1033,7 +1033,8 @@
static ipmi::RspType<> startOrStopService(ipmi::Context::ptr& ctx,
const uint8_t enable,
- const std::string& serviceName)
+ const std::string& serviceName,
+ bool disableOrEnableUnitFiles = true)
{
constexpr bool runtimeOnly = false;
constexpr bool force = false;
@@ -1052,26 +1053,33 @@
"StartUnit", serviceName, "replace");
break;
case ipmi::SupportedFeatureActions::disable:
+ if (disableOrEnableUnitFiles == true)
+ {
+ ctx->bus->yield_method_call(
+ ctx->yield, ec, systemDService, systemDObjPath,
+ systemDMgrIntf, "DisableUnitFiles",
+ std::array<const char*, 1>{serviceName.c_str()},
+ runtimeOnly);
+ }
ctx->bus->yield_method_call(
ctx->yield, ec, systemDService, systemDObjPath, systemDMgrIntf,
"MaskUnitFiles",
std::array<const char*, 1>{serviceName.c_str()}, runtimeOnly,
force);
- ctx->bus->yield_method_call(
- ctx->yield, ec, systemDService, systemDObjPath, systemDMgrIntf,
- "DisableUnitFiles",
- std::array<const char*, 1>{serviceName.c_str()}, runtimeOnly);
break;
case ipmi::SupportedFeatureActions::enable:
ctx->bus->yield_method_call(
ctx->yield, ec, systemDService, systemDObjPath, systemDMgrIntf,
"UnmaskUnitFiles",
std::array<const char*, 1>{serviceName.c_str()}, runtimeOnly);
- ctx->bus->yield_method_call(
- ctx->yield, ec, systemDService, systemDObjPath, systemDMgrIntf,
- "EnableUnitFiles",
- std::array<const char*, 1>{serviceName.c_str()}, runtimeOnly,
- force);
+ if (disableOrEnableUnitFiles == true)
+ {
+ ctx->bus->yield_method_call(
+ ctx->yield, ec, systemDService, systemDObjPath,
+ systemDMgrIntf, "EnableUnitFiles",
+ std::array<const char*, 1>{serviceName.c_str()},
+ runtimeOnly, force);
+ }
break;
default:
phosphor::logging::log<phosphor::logging::level::WARNING>(
@@ -1121,7 +1129,7 @@
if (binding == objectPath.substr(pos + 1))
{
return startOrStopService(ctx, enable,
- getMCTPServiceName(objectPath));
+ getMCTPServiceName(objectPath), false);
}
}
return ipmi::responseSuccess();