Miscellaneous improvements to activation/item updater
This commit slightly enhances and improves the item updater and the
activation process, addressing a few issues that were raised in code
review of openbmc/openbmc#1756
1.) Service file booleans used in activation are reset to false, to
avoid a BMC-flavored issue in the vein of openbmc/openbmc#1984
2.) systemd constants used in multiple places are moved to configure.ac
3.) Activation objects are unsubscribed from systemd signals at the end
of that process.
Resolves openbmc/openbmc#2031
Change-Id: I573645b16bca28ac6bf3b173d5b4845205224e07
Signed-off-by: Michael Tritz <mtritz@us.ibm.com>
diff --git a/activation.cpp b/activation.cpp
index 1e04230..2a8c122 100644
--- a/activation.cpp
+++ b/activation.cpp
@@ -1,5 +1,6 @@
#include "activation.hpp"
#include "item_updater.hpp"
+#include "config.h"
namespace phosphor
{
@@ -10,22 +11,28 @@
namespace softwareServer = sdbusplus::xyz::openbmc_project::Software::server;
-constexpr auto SYSTEMD_SERVICE = "org.freedesktop.systemd1";
-constexpr auto SYSTEMD_PATH = "/org/freedesktop/systemd1";
-constexpr auto SIGNAL_INTERFACE = "/org/freedesktop/systemd1";
-constexpr auto MANAGER_INTERFACE = "org.freedesktop.systemd1.Manager";
-
void Activation::subscribeToSystemdSignals()
{
- auto method = this->bus.new_method_call(SYSTEMD_SERVICE,
+ auto method = this->bus.new_method_call(SYSTEMD_BUSNAME,
SYSTEMD_PATH,
- SIGNAL_INTERFACE,
+ SYSTEMD_INTERFACE,
"Subscribe");
this->bus.call_noreply(method);
return;
}
+void Activation::unsubscribeFromSystemdSignals()
+{
+ auto method = this->bus.new_method_call(SYSTEMD_BUSNAME,
+ SYSTEMD_PATH,
+ SYSTEMD_INTERFACE,
+ "Unsubscribe");
+ this->bus.call_noreply(method);
+
+ return;
+}
+
auto Activation::activation(Activations value) ->
Activations
{
@@ -48,9 +55,9 @@
}
auto method = bus.new_method_call(
- SYSTEMD_SERVICE,
+ SYSTEMD_BUSNAME,
SYSTEMD_PATH,
- MANAGER_INTERFACE,
+ SYSTEMD_INTERFACE,
"StartUnit");
method.append("obmc-flash-bmc-ubirw.service", "replace");
bus.call_noreply(method);
@@ -58,9 +65,9 @@
auto roServiceFile = "obmc-flash-bmc-ubiro@" + versionId +
".service";
method = bus.new_method_call(
- SYSTEMD_SERVICE,
+ SYSTEMD_BUSNAME,
SYSTEMD_PATH,
- MANAGER_INTERFACE,
+ SYSTEMD_INTERFACE,
"StartUnit");
method.append(roServiceFile, "replace");
bus.call_noreply(method);
@@ -78,6 +85,11 @@
}
activationBlocksTransition.reset(nullptr);
+
+ rwVolumeCreated = false;
+ roVolumeCreated = false;
+ Activation::unsubscribeFromSystemdSignals();
+
return softwareServer::Activation::activation(
softwareServer::Activation::Activations::Active);
}
diff --git a/activation.hpp b/activation.hpp
index 41d055e..1e91759 100644
--- a/activation.hpp
+++ b/activation.hpp
@@ -173,6 +173,16 @@
*/
void subscribeToSystemdSignals();
+ /**
+ * @brief unsubscribe from the systemd signals
+ *
+ * systemd signals are only of interest during the activation process.
+ * Once complete, we want to unsubscribe to avoid unnecessary calls of
+ * unitStateChange().
+ *
+ */
+ void unsubscribeFromSystemdSignals();
+
/** @brief Persistent sdbusplus DBus bus connection */
sdbusplus::bus::bus& bus;
diff --git a/configure.ac b/configure.ac
index 96a51cd..71ede44 100755
--- a/configure.ac
+++ b/configure.ac
@@ -69,6 +69,13 @@
AC_DEFINE(FILEPATH_IFACE, "xyz.openbmc_project.Common.FilePath",
[The common file path interface])
+AC_DEFINE(SYSTEMD_BUSNAME, "org.freedesktop.systemd1",
+ [The systemd busname])
+AC_DEFINE(SYSTEMD_PATH, "/org/freedesktop/systemd1",
+ [The systemd path])
+AC_DEFINE(SYSTEMD_INTERFACE, "org.freedesktop.systemd1.Manager",
+ [The systemd interface])
+
AC_ARG_VAR(IMG_UPLOAD_DIR, [Directory where downloaded software images are placed])
AS_IF([test "x$IMG_UPLOAD_DIR" == "x"], [IMG_UPLOAD_DIR="/tmp/images"])
AC_DEFINE_UNQUOTED([IMG_UPLOAD_DIR], ["$IMG_UPLOAD_DIR"], [Directory where downloaded software images are placed])
diff --git a/item_updater.cpp b/item_updater.cpp
index 04a9eb4..2ec60c0 100644
--- a/item_updater.cpp
+++ b/item_updater.cpp
@@ -22,10 +22,6 @@
constexpr auto bmcImage = "image-rofs";
-constexpr auto SYSTEMD_BUSNAME = "org.freedesktop.systemd1";
-constexpr auto SYSTEMD_PATH = "/org/freedesktop/systemd1";
-constexpr auto SYSTEMD_INTERFACE = "org.freedesktop.systemd1.Manager";
-
void ItemUpdater::createActivation(sdbusplus::message::message& msg)
{