host-check: discover host state within state manager
Currently the phosphor-host-state-manager is started before the logic
has run to detect if the host is already running. This results in
phosphor-host-state-manager temporarily reporting that the host is not
running, even if it actually is. This can cause confusion for clients
monitoring this property.
The solution is to move the logic which discovers if the host is running
into phosphor-host-state-manager. Having the logic to do this in a
separate application was a nice separation of concerns but when the
requirement is a co-req, best to just combine them.
This change results in the phosphor-host-state-manager service starting
later in the boot to BMC Ready but testing has shown no impacts to
overall time to reach BMC Ready or impacts to other services.
As a part of this change, the phosphor-reset-sensor-states service was
moved out of the obmc-host-reset target to ensure it and the
phosphor-host-state-manager service have the correct dependency between
them. The phosphor-host-state-manager service now ensures it runs after
the pldm and ipmi services if they are being started (they are
utilized to check if the host is running).
Testing:
- Chassis On, Host On
Jul 30 14:40:37 rainxxx phosphor-host-state-manager[696]: Check if host is running
Jul 30 14:40:39 rainxxx phosphor-host-state-manager[696]: Host is running!
Jul 30 14:40:39 rainxxx phosphor-host-state-manager[696]: Initial Host State will be Running
- Chassis Off, Host Off
Jul 30 14:55:17 rainxxx phosphor-host-state-manager[710]: Check if host is running
Jul 30 14:55:17 rainxxx phosphor-host-state-manager[710]: Chassis power not on, exit
Jul 30 14:55:17 rainxxx phosphor-host-state-manager[710]: Initial Host State will be Off
- Chassis On, Host Off
Jul 30 14:57:11 rainxxx phosphor-host-state-manager[1193]: Check if host is running
Jul 30 14:57:18 rainxxx phosphor-host-state-manager[1193]: Host is not running!
Jul 30 14:57:18 rainxxx phosphor-host-state-manager[1193]: Initial Host State will be Off
Signed-off-by: Andrew Geissler <geissonator@yahoo.com>
Change-Id: I938902f526fba2f857be4b21b01149f8250b972d
diff --git a/host_check_main.cpp b/host_check.cpp
similarity index 78%
rename from host_check_main.cpp
rename to host_check.cpp
index 601edbd..23715a1 100644
--- a/host_check_main.cpp
+++ b/host_check.cpp
@@ -1,5 +1,7 @@
#include "config.h"
+#include "host_check.hpp"
+
#include <unistd.h>
#include <boost/range/adaptor/reversed.hpp>
@@ -29,6 +31,11 @@
constexpr auto CONDITION_HOST_PROPERTY = "CurrentFirmwareCondition";
constexpr auto PROPERTY_INTERFACE = "org.freedesktop.DBus.Properties";
+constexpr auto CHASSIS_STATE_SVC = "xyz.openbmc_project.State.Chassis";
+constexpr auto CHASSIS_STATE_PATH = "/xyz/openbmc_project/state/chassis0";
+constexpr auto CHASSIS_STATE_INTF = "xyz.openbmc_project.State.Chassis";
+constexpr auto CHASSIS_STATE_POWER_PROP = "CurrentPowerState";
+
// Find all implementations of Condition interface and check if host is
// running over it
bool checkFirmwareConditionRunning(sdbusplus::bus::bus& bus)
@@ -111,12 +118,51 @@
return false;
}
-int main()
+// Helper function to check if chassis power is on
+bool isChassiPowerOn(sdbusplus::bus::bus& bus)
+{
+ try
+ {
+ auto method = bus.new_method_call(CHASSIS_STATE_SVC, CHASSIS_STATE_PATH,
+ PROPERTY_INTERFACE, "Get");
+ method.append(CHASSIS_STATE_INTF, CHASSIS_STATE_POWER_PROP);
+
+ auto response = bus.call(method);
+
+ std::variant<std::string> currentPowerState;
+ response.read(currentPowerState);
+
+ if (std::get<std::string>(currentPowerState) ==
+ "xyz.openbmc_project.State.Chassis.PowerState.On")
+ {
+ return true;
+ }
+ }
+ catch (const SdBusError& e)
+ {
+ log<level::ERR>("Error reading Chassis Power State",
+ entry("ERROR=%s", e.what()),
+ entry("SERVICE=%s", CHASSIS_STATE_SVC),
+ entry("PATH=%s", CHASSIS_STATE_PATH));
+
+ throw;
+ }
+ return false;
+}
+
+bool isHostRunning()
{
log<level::INFO>("Check if host is running");
auto bus = sdbusplus::bus::new_default();
+ // No need to check if chassis power is not on
+ if (!isChassiPowerOn(bus))
+ {
+ log<level::INFO>("Chassis power not on, exit");
+ return false;
+ }
+
// This applications systemd service is setup to only run after all other
// application that could possibly implement the needed interface have
// been started. However, the use of mapper to find those interfaces means
@@ -142,9 +188,9 @@
std::snprintf(buf.get(), size, HOST_RUNNING_FILE, 0);
std::ofstream outfile(buf.get());
outfile.close();
- return 0;
+ return true;
}
}
log<level::INFO>("Host is not running!");
- return 0;
+ return false;
}
diff --git a/host_check.hpp b/host_check.hpp
new file mode 100644
index 0000000..0634f61
--- /dev/null
+++ b/host_check.hpp
@@ -0,0 +1,7 @@
+#pragma once
+
+/** @brief Determine if host is running
+ *
+ * @return True if host running, False otherwise
+ */
+bool isHostRunning();
diff --git a/host_state_manager.cpp b/host_state_manager.cpp
index ffb37b1..2b435a2 100644
--- a/host_state_manager.cpp
+++ b/host_state_manager.cpp
@@ -2,6 +2,8 @@
#include "host_state_manager.hpp"
+#include "host_check.hpp"
+
#include <fmt/format.h>
#include <stdio.h>
#include <systemd/sd-bus.h>
@@ -106,7 +108,7 @@
void Host::determineInitialState()
{
- if (stateActive(HOST_STATE_POWERON_MIN_TGT))
+ if (stateActive(HOST_STATE_POWERON_MIN_TGT) || isHostRunning())
{
log<level::INFO>("Initial Host State will be Running",
entry("CURRENT_HOST_STATE=%s",
diff --git a/meson.build b/meson.build
index 931136f..0d88e4f 100644
--- a/meson.build
+++ b/meson.build
@@ -63,6 +63,7 @@
'host_state_manager.cpp',
'host_state_manager_main.cpp',
'settings.cpp',
+ 'host_check.cpp',
dependencies: [
sdbusplus, sdeventplus, phosphorlogging,
phosphordbusinterfaces, cppfs
@@ -115,15 +116,6 @@
install: true
)
-executable('phosphor-host-check',
- 'host_check_main.cpp',
- dependencies: [
- sdbusplus, phosphorlogging
- ],
- implicit_include_directories: true,
- install: true
-)
-
executable('phosphor-systemd-target-monitor',
'systemd_target_monitor.cpp',
'systemd_target_parser.cpp',
diff --git a/service_files/meson.build b/service_files/meson.build
index 31b1974..c7a380a 100644
--- a/service_files/meson.build
+++ b/service_files/meson.build
@@ -2,7 +2,6 @@
'phosphor-systemd-target-monitor.service',
'phosphor-discover-system-state@.service',
'phosphor-reboot-host@.service',
- 'phosphor-reset-host-check@.service',
'phosphor-reset-host-reboot-attempts@.service',
'phosphor-reset-host-running@.service',
'phosphor-reset-sensor-states@.service',
diff --git a/service_files/phosphor-reset-host-check@.service b/service_files/phosphor-reset-host-check@.service
deleted file mode 100644
index 19b217e..0000000
--- a/service_files/phosphor-reset-host-check@.service
+++ /dev/null
@@ -1,18 +0,0 @@
-[Unit]
-Description=Check Host%i status on BMC reset
-After=phosphor-ipmi-host.service
-After=pldmd.service
-Wants=obmc-host-reset-running@%i.target
-Before=obmc-host-reset-running@%i.target
-Wants=op-reset-chassis-on@%i.service
-After=op-reset-chassis-on@%i.service
-Conflicts=obmc-host-stop@%i.target
-ConditionPathExists=/run/openbmc/chassis@%i-on
-
-[Service]
-RemainAfterExit=yes
-Type=oneshot
-ExecStart=/usr/bin/phosphor-host-check
-
-[Install]
-WantedBy=obmc-host-reset@%i.target
diff --git a/service_files/phosphor-reset-sensor-states@.service b/service_files/phosphor-reset-sensor-states@.service
index 689372f..102e969 100644
--- a/service_files/phosphor-reset-sensor-states@.service
+++ b/service_files/phosphor-reset-sensor-states@.service
@@ -1,8 +1,8 @@
[Unit]
Description=Reset host sensors
-After=phosphor-reset-host-check@%i.service
Wants=mapper-wait@-xyz-openbmc_project-state-host%i.service
After=mapper-wait@-xyz-openbmc_project-state-host%i.service
+After=obmc-host-reset@%i.target
ConditionPathExists=!/run/openbmc/host@%i-on
[Service]
@@ -12,4 +12,4 @@
ExecStart=/bin/sh -c "busctl set-property `mapper get-service /xyz/openbmc_project/state/host0` /xyz/openbmc_project/state/host0 xyz.openbmc_project.State.OperatingSystem.Status OperatingSystemState s xyz.openbmc_project.State.OperatingSystem.Status.OSStatus.Inactive"
[Install]
-WantedBy=obmc-host-reset@%i.target
+WantedBy=multi-user.target
diff --git a/service_files/xyz.openbmc_project.State.Host.service b/service_files/xyz.openbmc_project.State.Host.service
index 123d339..50b3483 100644
--- a/service_files/xyz.openbmc_project.State.Host.service
+++ b/service_files/xyz.openbmc_project.State.Host.service
@@ -3,9 +3,13 @@
Wants=mapper-wait@-xyz-openbmc_project-control-host0-auto_reboot.service
After=mapper-wait@-xyz-openbmc_project-control-host0-auto_reboot.service
Before=mapper-wait@-xyz-openbmc_project-state-host.service
-After=phosphor-reset-host-running@0.service
+Wants=mapper-wait@-xyz-openbmc_project-state-chassis0.service
+After=mapper-wait@-xyz-openbmc_project-state-chassis0.service
Wants=obmc-mapper.target
After=obmc-mapper.target
+After=phosphor-ipmi-host.service
+After=pldmd.service
+Before=obmc-host-reset@0.target
[Service]
ExecStart=/usr/bin/phosphor-host-state-manager