srvcfg-manager State change issue fix.
srvcfg-manager State change is not working as
it need to act on both .service and .socket files.
Also used MaskUnitFiles and UnmaskUnitFiles to
make changes persistent.
Tested By:
Disabled/Enabled the state and checked the service
is stopping/starting properly. Also rebooted system
and checked persistent behaviour. Checked changing
port/channel and validated functionality.
Change-Id: Ib0dce95db3926f5d700fb729aae7bdac1caf64e4
Signed-off-by: AppaRao Puli <apparao.puli@linux.intel.com>
diff --git a/inc/utils.hpp b/inc/utils.hpp
index 67ced4c..27e66f3 100644
--- a/inc/utils.hpp
+++ b/inc/utils.hpp
@@ -33,8 +33,8 @@
const std::string &unitName,
const std::string &actionMethod);
-void systemdUnitFileStateChange(
+void systemdUnitFilesStateChange(
const std::shared_ptr<sdbusplus::asio::connection> &conn,
- const std::string &unitName, const std::string &unitState);
+ const std::vector<std::string> &unitFiles, const std::string &unitState);
bool checkSystemdUnitExist(const std::string &unitName);
diff --git a/src/srvcfg_manager.cpp b/src/srvcfg_manager.cpp
index 04d41ca..333c001 100644
--- a/src/srvcfg_manager.cpp
+++ b/src/srvcfg_manager.cpp
@@ -126,6 +126,7 @@
std::string srvUnitName(sysDUnitName);
if (srvUnitName == "dropbear")
{
+ // Dropbear service expects template arguments.
srvUnitName.append("@");
}
srvUnitName.append(".service");
@@ -137,7 +138,15 @@
"async_method_call error: Failed to get property");
return;
}
- stateValue = pValue;
+ if ((pValue == "enabled") || (pValue == "static") ||
+ (pValue == "unmasked"))
+ {
+ stateValue = "enabled";
+ }
+ else if ((pValue == "disabled") || (pValue == "masked"))
+ {
+ stateValue = "disabled";
+ }
conn->async_method_call(
[](boost::system::error_code ec) {
if (ec)
@@ -212,6 +221,12 @@
void ServiceConfig::applySystemDServiceConfig()
{
+ if (updatedFlag)
+ {
+ // No updates. Just return.
+ return;
+ }
+
phosphor::logging::log<phosphor::logging::level::INFO>(
"Applying new settings.",
phosphor::logging::entry("OBJPATH=%s", objPath.c_str()));
@@ -261,28 +276,46 @@
phosphor::logging::elog<sdbusplus::xyz::openbmc_project::Common::
Error::InternalFailure>();
}
+ }
- // Systemd forcing explicit socket stop before reload...!
- std::string socketUnitName(sysDUnitName + ".socket");
- systemdUnitAction(conn, socketUnitName, sysdActionStopUnit);
-
- std::string srvUnitName(sysDUnitName + ".service");
- systemdUnitAction(conn, srvUnitName, sysdActionStopUnit);
-
- // Perform daemon reload to read new settings
- systemdDaemonReload(conn);
-
- // Restart the unit
- systemdUnitAction(conn, socketUnitName, sysdActionStartUnit);
- systemdUnitAction(conn, srvUnitName, sysdActionStartUnit);
+ std::string socketUnitName(sysDUnitName + ".socket");
+ std::string srvUnitName(sysDUnitName);
+ if (srvUnitName == "dropbear")
+ {
+ // Dropbear service expects template arguments.
+ // Todo: Unit action for service, fails with error
+ // "missing the instance name". Needs to implement
+ // getting all running instances and use it. This
+ // impact runtime but works fine during reboot.
+ srvUnitName.append("@");
+ }
+ srvUnitName.append(".service");
+ // Stop the running service in below scenarios.
+ // 1. State changed from "enabled" to "disabled"
+ // 2. No change in state and existing stateValue is
+ // "enabled" and there is change in other properties.
+ if (((stateValue == "disabled") &&
+ (updatedFlag & (1 << static_cast<uint8_t>(UpdatedProp::state)))) ||
+ (!(updatedFlag & (1 << static_cast<uint8_t>(UpdatedProp::state))) &&
+ (stateValue == "enabled") && (updatedFlag)))
+ {
+ systemdUnitAction(conn, socketUnitName, "StopUnit");
+ systemdUnitAction(conn, srvUnitName, "StopUnit");
}
if (updatedFlag & (1 << static_cast<uint8_t>(UpdatedProp::state)))
{
- if ((stateValue == "enabled") || (stateValue == "disabled"))
- {
- systemdUnitFileStateChange(conn, sysDUnitName, stateValue);
- }
+ std::vector<std::string> unitFiles = {socketUnitName, srvUnitName};
+ systemdUnitFilesStateChange(conn, unitFiles, stateValue);
+ }
+
+ // Perform daemon reload to read new settings
+ systemdDaemonReload(conn);
+
+ if (stateValue == "enabled")
+ {
+ // Restart the socket
+ systemdUnitAction(conn, socketUnitName, "StartUnit");
}
// Reset the flag
@@ -290,8 +323,8 @@
// All done. Lets reload the properties which are applied on systemd1.
// TODO: We need to capture the service restart signal and reload data
- // inside the signal handler. So that we can update the service properties
- // modified, outside of this service as well.
+ // inside the signal handler. So that we can update the service
+ // properties modified, outside of this service as well.
syncWithSystemD1Properties();
return;
@@ -330,8 +363,6 @@
iface->register_property(
"Port", portNum, [this](const uint16_t &req, uint16_t &res) {
- phosphor::logging::log<phosphor::logging::level::ERR>(
- " Inside register_property");
if (req == res)
{
return 1;
@@ -366,7 +397,7 @@
{
return 1;
}
- if ((req != "enabled") && (req != "disabled") && (req != "static"))
+ if ((req != "enabled") && (req != "disabled"))
{
phosphor::logging::log<phosphor::logging::level::ERR>(
"Invalid value specified");
diff --git a/src/utils.cpp b/src/utils.cpp
index 07239e2..32ac69d 100644
--- a/src/utils.cpp
+++ b/src/utils.cpp
@@ -28,6 +28,7 @@
"async error: Failed to do systemd reload.");
return;
}
+ return;
},
"org.freedesktop.systemd1", "/org/freedesktop/systemd1",
"org.freedesktop.systemd1.Manager", "Reload");
@@ -51,13 +52,17 @@
{
conn->async_method_call(
[](boost::system::error_code ec,
- const sdbusplus::message::object_path &res) {
+ const sdbusplus::message::object_path &objPath) {
if (ec)
{
phosphor::logging::log<phosphor::logging::level::ERR>(
"async error: Failed to do systemd action");
return;
}
+ phosphor::logging::log<phosphor::logging::level::ERR>(
+ "Created unit action job.",
+ phosphor::logging::entry("JobID=%s", objPath.str.c_str()));
+ return;
},
"org.freedesktop.systemd1", "/org/freedesktop/systemd1",
"org.freedesktop.systemd1.Manager", actionMethod, unitName,
@@ -75,15 +80,12 @@
return;
}
-void systemdUnitFileStateChange(
+void systemdUnitFilesStateChange(
const std::shared_ptr<sdbusplus::asio::connection> &conn,
- const std::string &unitName, const std::string &unitState)
+ const std::vector<std::string> &unitFiles, const std::string &unitState)
{
try
{
- // For now, we support two states(enabled & disabled). we can extend
- // to other states (enable-runtime, mask, link etc..) if needed.
- std::vector<std::string> unitFiles = {unitName};
if (unitState == "enabled")
{
conn->async_method_call(
@@ -91,13 +93,14 @@
if (ec)
{
phosphor::logging::log<phosphor::logging::level::ERR>(
- "async error: Failed to do systemd reload.");
+ "async error: Failed to perform UnmaskUnitFiles.");
return;
}
+ return;
},
"org.freedesktop.systemd1", "/org/freedesktop/systemd1",
- "org.freedesktop.systemd1.Manager", "EnableUnitFiles",
- unitFiles, false, false);
+ "org.freedesktop.systemd1.Manager", "UnmaskUnitFiles",
+ unitFiles, false);
}
else if (unitState == "disabled")
{
@@ -106,13 +109,14 @@
if (ec)
{
phosphor::logging::log<phosphor::logging::level::ERR>(
- "async error: Failed to do systemd reload.");
+ "async error: Failed to perform MaskUnitFiles.");
return;
}
+ return;
},
"org.freedesktop.systemd1", "/org/freedesktop/systemd1",
- "org.freedesktop.systemd1.Manager", "DisableUnitFiles",
- unitFiles, false);
+ "org.freedesktop.systemd1.Manager", "MaskUnitFiles", unitFiles,
+ false, false);
}
else
{