From c6ed122a09b1e41b9eab0032ff428b8b1a999534 Mon Sep 17 00:00:00 2001 From: Jae Hyun Yoo Date: Wed, 4 Aug 2021 15:50:34 -0700 Subject: [PATCH] Revert "Support new boot override setting design" This reverts commit c21865c469cfc9dffdc15d87710293115cf6d9e4. Change-Id: Icfd03551dd9ea2fb216519d8ab05b92521838542 Signed-off-by: Jae Hyun Yoo --- redfish-core/lib/systems.hpp | 493 +++++++++++++++++------------------ 1 file changed, 245 insertions(+), 248 deletions(-) diff --git a/redfish-core/lib/systems.hpp b/redfish-core/lib/systems.hpp index fc6e2c78fd1f..5ad065b3518a 100644 --- a/redfish-core/lib/systems.hpp +++ b/redfish-core/lib/systems.hpp @@ -769,8 +769,11 @@ inline int assignBootParameters(const std::shared_ptr& aResp, const std::string& rfSource, std::string& bootSource, std::string& bootMode) { - bootSource = "xyz.openbmc_project.Control.Boot.Source.Sources.Default"; - bootMode = "xyz.openbmc_project.Control.Boot.Mode.Modes.Regular"; + // The caller has initialized the bootSource and bootMode to: + // bootMode = "xyz.openbmc_project.Control.Boot.Mode.Modes.Regular"; + // bootSource = "xyz.openbmc_project.Control.Boot.Source.Sources.Default"; + // Only modify the bootSource/bootMode variable needed to achieve the + // desired boot action. if (rfSource == "None") { @@ -917,14 +920,45 @@ inline void getBootProgress(const std::shared_ptr& aResp) } /** - * @brief Retrieves boot override type over DBUS and fills out the response + * @brief Checks if the current boot override state can be considered as + * Disabled * * @param[in] aResp Shared pointer for generating response message. * * @return None. */ +inline void + checkIfOverrideIsDisabled(const std::shared_ptr& aResp) +{ + // If the BootSourceOverrideTarget is still "None" at the end, + // reset the BootSourceOverrideEnabled to indicate that + // overrides are disabled + if (aResp->res.jsonValue["Boot"]["BootSourceOverrideTarget"] == "None") + { + // If the BootSourceOverrideMode is supported we should + // check if it is still "UEFI" too + if (aResp->res.jsonValue["Boot"].contains("BootSourceOverrideMode")) + { + if (aResp->res.jsonValue["Boot"]["BootSourceOverrideMode"] != + "UEFI") + { + return; + } + } + aResp->res.jsonValue["Boot"]["BootSourceOverrideEnabled"] = "Disabled"; + } +} -inline void getBootOverrideType(const std::shared_ptr& aResp) +/** + * @brief Retrieves boot type over DBUS and fills out the response + * + * @param[in] aResp Shared pointer for generating response message. + * @param[in] bootDbusObj The dbus object to query for boot properties. + * + * @return None. + */ +inline void getBootType(const std::shared_ptr& aResp, + const std::string& bootDbusObj) { crow::connections::systemBus->async_method_call( [aResp](const boost::system::error_code ec, @@ -932,6 +966,12 @@ inline void getBootOverrideType(const std::shared_ptr& aResp) if (ec) { // not an error, don't have to have the interface + + // Support Disabled override state in a way: + // "BootSourceOverrideEnabled=Disabled" = + // "BootSourceOverrideMode=UEFI" + + // "BootSourceOverrideTarget=None" + checkIfOverrideIsDisabled(aResp); return; } @@ -958,26 +998,31 @@ inline void getBootOverrideType(const std::shared_ptr& aResp) } aResp->res.jsonValue["Boot"]["BootSourceOverrideMode"] = rfType; + + // Support Disabled override state in a way: + // "BootSourceOverrideEnabled=Disabled" = + // "BootSourceOverrideMode=UEFI" + "BootSourceOverrideTarget=None" + checkIfOverrideIsDisabled(aResp); }, - "xyz.openbmc_project.Settings", - "/xyz/openbmc_project/control/host0/boot", + "xyz.openbmc_project.Settings", bootDbusObj, "org.freedesktop.DBus.Properties", "Get", "xyz.openbmc_project.Control.Boot.Type", "BootType"); } /** - * @brief Retrieves boot override mode over DBUS and fills out the response + * @brief Retrieves boot mode over DBUS and fills out the response * * @param[in] aResp Shared pointer for generating response message. + * @param[in] bootDbusObj The dbus object to query for boot properties. * * @return None. */ - -inline void getBootOverrideMode(const std::shared_ptr& aResp) +inline void getBootMode(const std::shared_ptr& aResp, + const std::string& bootDbusObj) { crow::connections::systemBus->async_method_call( - [aResp](const boost::system::error_code ec, - const std::variant& bootMode) { + [aResp, bootDbusObj](const boost::system::error_code ec, + const std::variant& bootMode) { if (ec) { BMCWEB_LOG_DEBUG << "DBUS response error " << ec; @@ -1010,27 +1055,39 @@ inline void getBootOverrideMode(const std::shared_ptr& aResp) rfMode; } } + + // Get BootType inside this async call as we need all of the + // BootSource/BootMode/BootType to support + // "BootSourceOverrideEnabled"="Disabled" state. + getBootType(aResp, bootDbusObj); }, - "xyz.openbmc_project.Settings", - "/xyz/openbmc_project/control/host0/boot", + "xyz.openbmc_project.Settings", bootDbusObj, "org.freedesktop.DBus.Properties", "Get", "xyz.openbmc_project.Control.Boot.Mode", "BootMode"); } /** - * @brief Retrieves boot override source over DBUS + * @brief Retrieves boot source over DBUS * * @param[in] aResp Shared pointer for generating response message. + * @param[in] oneTimeEnable Boolean to indicate boot properties are one-time. * * @return None. */ - -inline void - getBootOverrideSource(const std::shared_ptr& aResp) +inline void getBootSource(const std::shared_ptr& aResp, + bool oneTimeEnabled) { + std::string bootDbusObj = + oneTimeEnabled ? "/xyz/openbmc_project/control/host0/boot/one_time" + : "/xyz/openbmc_project/control/host0/boot"; + + BMCWEB_LOG_DEBUG << "Is one time: " << oneTimeEnabled; + aResp->res.jsonValue["Boot"]["BootSourceOverrideEnabled"] = + (oneTimeEnabled) ? "Once" : "Continuous"; + crow::connections::systemBus->async_method_call( - [aResp](const boost::system::error_code ec, - const std::variant& bootSource) { + [aResp, bootDbusObj](const boost::system::error_code ec, + const std::variant& bootSource) { if (ec) { BMCWEB_LOG_DEBUG << "DBUS response error " << ec; @@ -1057,43 +1114,32 @@ inline void // Get BootMode as BootSourceOverrideTarget is constructed // from both BootSource and BootMode - getBootOverrideMode(aResp); + getBootMode(aResp, bootDbusObj); }, - "xyz.openbmc_project.Settings", - "/xyz/openbmc_project/control/host0/boot", + "xyz.openbmc_project.Settings", bootDbusObj, "org.freedesktop.DBus.Properties", "Get", "xyz.openbmc_project.Control.Boot.Source", "BootSource"); } /** - * @brief This functions abstracts all the logic behind getting a - * "BootSourceOverrideEnabled" property from an overall boot override enable - * state + * @brief Retrieves "One time" enabled setting over DBUS and calls function to + * get boot source and boot mode. * * @param[in] aResp Shared pointer for generating response message. * * @return None. */ - -inline void - processBootOverrideEnable(const std::shared_ptr& aResp, - const bool bootOverrideEnableSetting) +inline void getBootProperties(const std::shared_ptr& aResp) { - if (!bootOverrideEnableSetting) - { - aResp->res.jsonValue["Boot"]["BootSourceOverrideEnabled"] = "Disabled"; - return; - } + BMCWEB_LOG_DEBUG << "Get boot information."; - // If boot source override is enabled, we need to check 'one_time' - // property to set a correct value for the "BootSourceOverrideEnabled" crow::connections::systemBus->async_method_call( [aResp](const boost::system::error_code ec, const std::variant& oneTime) { if (ec) { BMCWEB_LOG_DEBUG << "DBUS response error " << ec; - messages::internalError(aResp->res); + // not an error, don't have to have the interface return; } @@ -1104,19 +1150,7 @@ inline void messages::internalError(aResp->res); return; } - - bool oneTimeSetting = *oneTimePtr; - - if (oneTimeSetting) - { - aResp->res.jsonValue["Boot"]["BootSourceOverrideEnabled"] = - "Once"; - } - else - { - aResp->res.jsonValue["Boot"]["BootSourceOverrideEnabled"] = - "Continuous"; - } + getBootSource(aResp, *oneTimePtr); }, "xyz.openbmc_project.Settings", "/xyz/openbmc_project/control/host0/boot/one_time", @@ -1124,60 +1158,6 @@ inline void "xyz.openbmc_project.Object.Enable", "Enabled"); } -/** - * @brief Retrieves boot override enable over DBUS - * - * @param[in] aResp Shared pointer for generating response message. - * - * @return None. - */ - -inline void - getBootOverrideEnable(const std::shared_ptr& aResp) -{ - crow::connections::systemBus->async_method_call( - [aResp](const boost::system::error_code ec, - const std::variant& bootOverrideEnable) { - if (ec) - { - BMCWEB_LOG_DEBUG << "DBUS response error " << ec; - messages::internalError(aResp->res); - return; - } - - const bool* bootOverrideEnablePtr = - std::get_if(&bootOverrideEnable); - - if (!bootOverrideEnablePtr) - { - messages::internalError(aResp->res); - return; - } - - processBootOverrideEnable(aResp, *bootOverrideEnablePtr); - }, - "xyz.openbmc_project.Settings", - "/xyz/openbmc_project/control/host0/boot", - "org.freedesktop.DBus.Properties", "Get", - "xyz.openbmc_project.Object.Enable", "Enabled"); -} - -/** - * @brief Retrieves boot source override properties - * - * @param[in] aResp Shared pointer for generating response message. - * - * @return None. - */ -inline void getBootProperties(const std::shared_ptr& aResp) -{ - BMCWEB_LOG_DEBUG << "Get boot information."; - - getBootOverrideSource(aResp); - getBootOverrideType(aResp); - getBootOverrideEnable(aResp); -} - /** * @brief Retrieves the Last Reset Time * @@ -1479,47 +1459,59 @@ inline void getTrustedModuleRequiredToBoot( * @brief Sets boot properties into DBUS object(s). * * @param[in] aResp Shared pointer for generating response message. + * @param[in] overrideEnabled The source override "enable". + * @param[in] bootObj Path to the DBUS object. * @param[in] bootType The boot type to set. * @return Integer error code. */ inline void setBootType(const std::shared_ptr& aResp, + const bool overrideEnabled, const std::string& bootObj, const std::optional& bootType) { - std::string bootTypeStr; - - if (!bootType) - { - return; - } + std::string bootTypeStr = "xyz.openbmc_project.Control.Boot.Type.Types.EFI"; - // Source target specified - BMCWEB_LOG_DEBUG << "Boot type: " << *bootType; - // Figure out which DBUS interface and property to use - if (*bootType == "Legacy") - { - bootTypeStr = "xyz.openbmc_project.Control.Boot.Type.Types.Legacy"; - } - else if (*bootType == "UEFI") + if (bootType && overrideEnabled) { - bootTypeStr = "xyz.openbmc_project.Control.Boot.Type.Types.EFI"; - } - else - { - BMCWEB_LOG_DEBUG << "Invalid property value for " - "BootSourceOverrideMode: " - << *bootType; - messages::propertyValueNotInList(aResp->res, *bootType, - "BootSourceOverrideMode"); - return; + // Source target specified + BMCWEB_LOG_DEBUG << "Boot type: " << *bootType; + // Figure out which DBUS interface and property to use + if (*bootType == "Legacy") + { + bootTypeStr = "xyz.openbmc_project.Control.Boot.Type.Types.Legacy"; + } + else if (*bootType == "UEFI") + { + bootTypeStr = "xyz.openbmc_project.Control.Boot.Type.Types.EFI"; + } + else + { + BMCWEB_LOG_DEBUG << "Invalid property value for " + "BootSourceOverrideMode: " + << *bootType; + messages::propertyValueNotInList(aResp->res, *bootType, + "BootSourceOverrideMode"); + return; + } } // Act on validated parameters BMCWEB_LOG_DEBUG << "DBUS boot type: " << bootTypeStr; crow::connections::systemBus->async_method_call( - [aResp](const boost::system::error_code ec) { + [aResp, bootType](const boost::system::error_code ec) { if (ec) { + if (!bootType) + { + // If bootType wasn't explicitly present in the incoming + // message don't output error. The error could come from a + // fact that the BootType interface may be not present in + // the settings object. It could happen because this + // interface is not relevant for some Host architectures + // (for example POWER). + return; + } + BMCWEB_LOG_DEBUG << "DBUS response error " << ec; if (ec.value() == boost::asio::error::host_unreachable) { @@ -1531,8 +1523,7 @@ inline void setBootType(const std::shared_ptr& aResp, } BMCWEB_LOG_DEBUG << "Boot type update done."; }, - "xyz.openbmc_project.Settings", - "/xyz/openbmc_project/control/host0/boot", + "xyz.openbmc_project.Settings", bootObj, "org.freedesktop.DBus.Properties", "Set", "xyz.openbmc_project.Control.Boot.Type", "BootType", std::variant(bootTypeStr)); @@ -1542,48 +1533,42 @@ inline void setBootType(const std::shared_ptr& aResp, * @brief Sets boot properties into DBUS object(s). * * @param[in] aResp Shared pointer for generating response message. - * @param[in] bootType The boot type to set. + * @param[in] overrideEnabled The source override "enable". + * @param[in] bootObj Path to the DBUS object. + * @param[in] bootSource The boot source to set. + * * @return Integer error code. */ -inline void setBootEnable(const std::shared_ptr& aResp, - const std::optional& bootEnable) +inline void setBootModeOrSource(const std::shared_ptr& aResp, + const bool overrideEnabled, + const std::string& bootObj, + const std::optional& bootSource) { - if (!bootEnable) - { - return; - } - // Source target specified - BMCWEB_LOG_DEBUG << "Boot enable: " << *bootEnable; + std::string bootSourceStr = + "xyz.openbmc_project.Control.Boot.Source.Sources.Default"; + std::string bootModeStr = + "xyz.openbmc_project.Control.Boot.Mode.Modes.Regular"; - bool bootOverrideEnable = false; - bool bootOverridePersistent = false; - // Figure out which DBUS interface and property to use - if (*bootEnable == "Disabled") - { - bootOverrideEnable = false; - } - else if (*bootEnable == "Once") - { - bootOverrideEnable = true; - bootOverridePersistent = false; - } - else if (*bootEnable == "Continuous") + if (bootSource && overrideEnabled) { - bootOverrideEnable = true; - bootOverridePersistent = true; - } - else - { - BMCWEB_LOG_DEBUG << "Invalid property value for " - "BootSourceOverrideEnabled: " - << *bootEnable; - messages::propertyValueNotInList(aResp->res, *bootEnable, - "BootSourceOverrideEnabled"); - return; + // Source target specified + BMCWEB_LOG_DEBUG << "Boot source: " << *bootSource; + // Figure out which DBUS interface and property to use + if (assignBootParameters(aResp, *bootSource, bootSourceStr, + bootModeStr)) + { + BMCWEB_LOG_DEBUG + << "Invalid property value for BootSourceOverrideTarget: " + << *bootSource; + messages::propertyValueNotInList(aResp->res, *bootSource, + "BootSourceTargetOverride"); + return; + } } // Act on validated parameters - BMCWEB_LOG_DEBUG << "DBUS boot override enable: " << bootOverrideEnable; + BMCWEB_LOG_DEBUG << "DBUS boot source: " << bootSourceStr; + BMCWEB_LOG_DEBUG << "DBUS boot mode: " << bootModeStr; crow::connections::systemBus->async_method_call( [aResp](const boost::system::error_code ec) { @@ -1593,23 +1578,12 @@ inline void setBootEnable(const std::shared_ptr& aResp, messages::internalError(aResp->res); return; } - BMCWEB_LOG_DEBUG << "Boot override enable update done."; + BMCWEB_LOG_DEBUG << "Boot source update done."; }, - "xyz.openbmc_project.Settings", - "/xyz/openbmc_project/control/host0/boot", + "xyz.openbmc_project.Settings", bootObj, "org.freedesktop.DBus.Properties", "Set", - "xyz.openbmc_project.Object.Enable", "Enabled", - std::variant(bootOverrideEnable)); - - if (!bootOverrideEnable) - { - return; - } - - // In case boot override is enabled we need to set correct value for the - // 'one_time' enable DBus interface - BMCWEB_LOG_DEBUG << "DBUS boot override persistent: " - << bootOverridePersistent; + "xyz.openbmc_project.Control.Boot.Source", "BootSource", + std::variant(bootSourceStr)); crow::connections::systemBus->async_method_call( [aResp](const boost::system::error_code ec) { @@ -1619,86 +1593,45 @@ inline void setBootEnable(const std::shared_ptr& aResp, messages::internalError(aResp->res); return; } - BMCWEB_LOG_DEBUG << "Boot one_time update done."; + BMCWEB_LOG_DEBUG << "Boot mode update done."; }, - "xyz.openbmc_project.Settings", - "/xyz/openbmc_project/control/host0/boot/one_time", + "xyz.openbmc_project.Settings", bootObj, "org.freedesktop.DBus.Properties", "Set", - "xyz.openbmc_project.Object.Enable", "Enabled", - std::variant(!bootOverridePersistent)); + "xyz.openbmc_project.Control.Boot.Mode", "BootMode", + std::variant(bootModeStr)); } /** - * @brief Sets boot properties into DBUS object(s). + * @brief Sets "One time" enabled setting into DBUS object * - * @param[in] aResp Shared pointer for generating response message. - * @param[in] bootSource The boot source to set. + * @param[in] aResp Shared pointer for generating response message. + * @param[in] oneTime Enable property for one-time object * * @return Integer error code. */ -inline void setBootModeOrSource(const std::shared_ptr& aResp, - const std::optional& bootSource) +inline void setOneTime(const std::shared_ptr& aResp, + bool oneTime) { - std::string bootSourceStr; - std::string bootModeStr; - - if (!bootSource) - { - return; - } - - // Source target specified - BMCWEB_LOG_DEBUG << "Boot source: " << *bootSource; - // Figure out which DBUS interface and property to use - if (assignBootParameters(aResp, *bootSource, bootSourceStr, bootModeStr)) - { - BMCWEB_LOG_DEBUG - << "Invalid property value for BootSourceOverrideTarget: " - << *bootSource; - messages::propertyValueNotInList(aResp->res, *bootSource, - "BootSourceTargetOverride"); - return; - } - - // Act on validated parameters - BMCWEB_LOG_DEBUG << "DBUS boot source: " << bootSourceStr; - BMCWEB_LOG_DEBUG << "DBUS boot mode: " << bootModeStr; - crow::connections::systemBus->async_method_call( - [aResp](const boost::system::error_code ec) { + [aResp{aResp}](const boost::system::error_code ec) { if (ec) { BMCWEB_LOG_DEBUG << "DBUS response error " << ec; messages::internalError(aResp->res); return; } - BMCWEB_LOG_DEBUG << "Boot source update done."; + BMCWEB_LOG_DEBUG << "Boot enable update done."; }, "xyz.openbmc_project.Settings", - "/xyz/openbmc_project/control/host0/boot", - "org.freedesktop.DBus.Properties", "Set", - "xyz.openbmc_project.Control.Boot.Source", "BootSource", - std::variant(bootSourceStr)); - - crow::connections::systemBus->async_method_call( - [aResp](const boost::system::error_code ec) { - if (ec) - { - BMCWEB_LOG_DEBUG << "DBUS response error " << ec; - messages::internalError(aResp->res); - return; - } - BMCWEB_LOG_DEBUG << "Boot mode update done."; - }, - "xyz.openbmc_project.Settings", - "/xyz/openbmc_project/control/host0/boot", + "/xyz/openbmc_project/control/host0/boot/one_time", "org.freedesktop.DBus.Properties", "Set", - "xyz.openbmc_project.Control.Boot.Mode", "BootMode", - std::variant(bootModeStr)); + "xyz.openbmc_project.Object.Enable", "Enabled", + std::variant(oneTime)); } /** - * @brief Sets Boot source override properties. + * @brief Retrieves "One time" enabled setting over DBUS and calls function to + * set boot source/boot mode properties. * * @param[in] aResp Shared pointer for generating response message. * @param[in] bootSource The boot source from incoming RF request. @@ -1707,17 +1640,81 @@ inline void setBootModeOrSource(const std::shared_ptr& aResp, * * @return Integer error code. */ - -inline void setBootProperties(const std::shared_ptr& aResp, - const std::optional& bootSource, - const std::optional& bootType, - const std::optional& bootEnable) +inline void + setBootSourceProperties(const std::shared_ptr& aResp, + std::optional bootSource, + std::optional bootType, + std::optional bootEnable) { BMCWEB_LOG_DEBUG << "Set boot information."; - setBootModeOrSource(aResp, bootSource); - setBootType(aResp, bootType); - setBootEnable(aResp, bootEnable); + crow::connections::systemBus->async_method_call( + [aResp, bootSource{std::move(bootSource)}, + bootType{std::move(bootType)}, + bootEnable{std::move(bootEnable)}](const boost::system::error_code ec, + const std::variant& oneTime) { + if (ec) + { + BMCWEB_LOG_DEBUG << "DBUS response error " << ec; + messages::internalError(aResp->res); + return; + } + + const bool* oneTimePtr = std::get_if(&oneTime); + + if (!oneTimePtr) + { + messages::internalError(aResp->res); + return; + } + + BMCWEB_LOG_DEBUG << "Got one time: " << *oneTimePtr; + + bool oneTimeSetting = *oneTimePtr; + bool overrideEnabled = true; + + // Validate incoming parameters + if (bootEnable) + { + if (*bootEnable == "Once") + { + oneTimeSetting = true; + } + else if (*bootEnable == "Continuous") + { + oneTimeSetting = false; + } + else if (*bootEnable == "Disabled") + { + BMCWEB_LOG_DEBUG << "Boot source override will be disabled"; + oneTimeSetting = false; + overrideEnabled = false; + } + else + { + BMCWEB_LOG_DEBUG << "Unsupported value for " + "BootSourceOverrideEnabled: " + << *bootEnable; + messages::propertyValueNotInList( + aResp->res, *bootEnable, "BootSourceOverrideEnabled"); + return; + } + } + + std::string bootObj = "/xyz/openbmc_project/control/host0/boot"; + if (oneTimeSetting) + { + bootObj += "/one_time"; + } + + setBootModeOrSource(aResp, overrideEnabled, bootObj, bootSource); + setBootType(aResp, overrideEnabled, bootObj, bootType); + setOneTime(aResp, oneTimeSetting); + }, + "xyz.openbmc_project.Settings", + "/xyz/openbmc_project/control/host0/boot/one_time", + "org.freedesktop.DBus.Properties", "Get", + "xyz.openbmc_project.Object.Enable", "Enabled"); } /** @@ -2806,11 +2803,11 @@ inline void requestRoutesSystems(App& app) { return; } - if (bootSource || bootType || bootEnable) { - setBootProperties(asyncResp, bootSource, bootType, - bootEnable); + setBootSourceProperties( + asyncResp, std::move(bootSource), + std::move(bootType), std::move(bootEnable)); } if (automaticRetryConfig) { -- 2.17.1