From 15309efc2c1cebadbabdb0ef5f74a38fb8b78cfd Mon Sep 17 00:00:00 2001
From: Matt Spinler <spinler@us.ibm.com>
Date: Wed, 27 Jun 2018 13:01:25 -0500
Subject: Clear host command queue on a power on

When the RequestedHostTransition property changes to On, clear
any pending commands in the command queue.  This is done to avoid
race conditions around state transitions as well as other
scenarios like the following:

1) Host is already off
2) RequestedHostTransition is set to Off
3) RequestedHostTransition is set to On
4) Host powers on
5) Host immediately powers off because of the pending command
   sent in 2).

Resolves openbmc/openbmc#3207

Tested:  Verified the scenario above no longer occurs.

Change-Id: I26c8195c305c75b01333d1b10ff4bf16d76b91a6
Signed-off-by: Matt Spinler <spinler@us.ibm.com>
---
 host-cmd-manager.cpp | 42 +++++++++++++++++++++++++++++++++++++++++-
 host-cmd-manager.hpp | 29 +++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/host-cmd-manager.cpp b/host-cmd-manager.cpp
index 799f9bf..4316451 100644
--- a/host-cmd-manager.cpp
+++ b/host-cmd-manager.cpp
@@ -2,6 +2,7 @@
 #include <phosphor-logging/log.hpp>
 #include <phosphor-logging/elog-errors.hpp>
 #include <xyz/openbmc_project/Common/error.hpp>
+#include <xyz/openbmc_project/State/Host/server.hpp>
 #include <systemintfcmds.h>
 #include <utils.hpp>
 #include <config.h>
@@ -18,15 +19,26 @@ namespace command
 constexpr auto MAPPER_BUSNAME = "xyz.openbmc_project.ObjectMapper";
 constexpr auto MAPPER_PATH = "/xyz/openbmc_project/object_mapper";
 constexpr auto MAPPER_INTERFACE = "xyz.openbmc_project.ObjectMapper";
+constexpr auto HOST_STATE_PATH = "/xyz/openbmc_project/state/host0";
+constexpr auto HOST_STATE_INTERFACE = "xyz.openbmc_project.State.Host";
+constexpr auto HOST_TRANS_PROP = "RequestedHostTransition";
 
 // For throwing exceptions
 using namespace phosphor::logging;
 using InternalFailure = sdbusplus::xyz::openbmc_project::Common::
                             Error::InternalFailure;
 
+namespace sdbusRule = sdbusplus::bus::match::rules;
+
 Manager::Manager(sdbusplus::bus::bus& bus, sd_event* event) :
             bus(bus),
-            timer(event, std::bind(&Manager::hostTimeout, this))
+            timer(event, std::bind(&Manager::hostTimeout, this)),
+            hostTransitionMatch(bus,
+                    sdbusRule::propertiesChanged(
+                        HOST_STATE_PATH,
+                        HOST_STATE_INTERFACE),
+                    std::bind(&Manager::clearQueueOnPowerOn, this,
+                        std::placeholders::_1))
 {
     // Nothing to do here.
 }
@@ -75,6 +87,11 @@ void Manager::hostTimeout()
 {
     log<level::ERR>("Host control timeout hit!");
 
+    clearQueue();
+}
+
+void Manager::clearQueue()
+{
     // Dequeue all entries and send fail signal
     while(!this->workQueue.empty())
     {
@@ -150,6 +167,29 @@ void Manager::execute(CommandHandler command)
     return;
 }
 
+void Manager::clearQueueOnPowerOn(sdbusplus::message::message& msg)
+{
+    namespace server = sdbusplus::xyz::openbmc_project::State::server;
+
+    ::ipmi::DbusInterface interface;
+    ::ipmi::PropertyMap properties;
+
+    msg.read(interface, properties);
+
+    if (properties.find(HOST_TRANS_PROP) == properties.end())
+    {
+        return;
+    }
+
+    auto& requestedState = properties.at(HOST_TRANS_PROP).get<std::string>();
+
+    if (server::Host::convertTransitionFromString(requestedState) ==
+            server::Host::Transition::On)
+    {
+        clearQueue();
+    }
+}
+
 } // namespace command
 } // namespace host
 } // namepsace phosphor
diff --git a/host-cmd-manager.hpp b/host-cmd-manager.hpp
index 06ac5fa..c25706c 100644
--- a/host-cmd-manager.hpp
+++ b/host-cmd-manager.hpp
@@ -3,6 +3,7 @@
 #include <tuple>
 #include <queue>
 #include <sdbusplus/bus.hpp>
+#include <sdbusplus/bus/match.hpp>
 #include <timer.hpp>
 #include <host-ipmid/ipmid-host-cmd-utils.hpp>
 
@@ -68,6 +69,31 @@ class Manager
          */
         void hostTimeout();
 
+        /** @brief Clears the command queue
+         *
+         *  @detail Clears the command queue and calls all callbacks
+         *          specifying the command wasn't successful.
+         */
+        void clearQueue();
+
+        /** @brief Clears the command queue on a power on
+         *
+         *  @detail The properties changed handler for the
+         *          RequestedHostTransition property.  When this property
+         *          changes to 'On', this function will purge the command
+         *          queue.
+         *
+         *          This is done to avoid having commands that were issued
+         *          before the host powers on from getting sent to the host,
+         *          either due to race conditions around state transitions
+         *          or from a user doing something like requesting an already
+         *          powered off system to power off again and then immediately
+         *          requesting a power on.
+         *
+         *  @param[in] msg - the sdbusplus message containing the property
+         */
+        void clearQueueOnPowerOn(sdbusplus::message::message& msg);
+
         /** @brief Reference to the dbus handler */
         sdbusplus::bus::bus& bus;
 
@@ -76,6 +102,9 @@ class Manager
 
         /** @brief Timer for commands to host */
         phosphor::ipmi::Timer timer;
+
+        /** @brief Match handler for the requested host state */
+        sdbusplus::bus::match_t hostTransitionMatch;
 };
 
 } // namespace command
-- 
cgit v1.2.1