diff options
author | Adriana Kobylak <anoo@us.ibm.com> | 2018-07-10 10:20:56 -0500 |
---|---|---|
committer | Adriana Kobylak <anoo@us.ibm.com> | 2018-07-10 10:39:22 -0500 |
commit | aea48f27e46be9454e79a77e3594e021464ec903 (patch) | |
tree | 0efb3b02987b12a198bd9ad08a3af5ed2b87b284 | |
parent | 910ce68d2f2aa62dfe18561a7b6e63eed195af35 (diff) | |
download | phosphor-bmc-code-mgmt-aea48f27e46be9454e79a77e3594e021464ec903.tar.gz phosphor-bmc-code-mgmt-aea48f27e46be9454e79a77e3594e021464ec903.zip |
activation: Improve error handling in Subscribe
Handle exceptions from the sdbusplus method call_noreply API.
The Activation constructor was subscribing to systemd signals, which
would cause an "Already Subscribed" error when creating multiple
Activation instances because the bus is common.
There is no reason to subscribe to systemd signals in the constructor,
since the signals are only triggered during the activation process.
Move the Subscribe call to the activation process which calls
Unsubscribe at the end.
There's a scenario where there could still be an "Already Subscribed"
error if the activation fails, so add error handling there.
Tested: Verified code update worked with this change, and the app
would not core dump with the latest sdbusplus changes.
Change-Id: I065baca2586fa64c138d847187303464123cc01a
Signed-off-by: Adriana Kobylak <anoo@us.ibm.com>
-rw-r--r-- | activation.cpp | 25 | ||||
-rw-r--r-- | activation.hpp | 2 |
2 files changed, 24 insertions, 3 deletions
diff --git a/activation.cpp b/activation.cpp index b64f1be..92de6a6 100644 --- a/activation.cpp +++ b/activation.cpp @@ -3,6 +3,7 @@ #include "config.h" #include "serialize.hpp" #include <phosphor-logging/log.hpp> +#include <sdbusplus/exception.hpp> #ifdef WANT_SIGNATURE_VERIFY #include <phosphor-logging/elog.hpp> @@ -21,6 +22,7 @@ namespace updater namespace softwareServer = sdbusplus::xyz::openbmc_project::Software::server; using namespace phosphor::logging; +using sdbusplus::exception::SdBusError; #ifdef WANT_SIGNATURE_VERIFY using InternalFailure = @@ -32,7 +34,25 @@ void Activation::subscribeToSystemdSignals() { auto method = this->bus.new_method_call(SYSTEMD_BUSNAME, SYSTEMD_PATH, SYSTEMD_INTERFACE, "Subscribe"); - this->bus.call_noreply(method); + try + { + this->bus.call_noreply(method); + } + catch (const SdBusError& e) + { + if (e.name() != nullptr && + strcmp("org.freedesktop.systemd1.AlreadySubscribed", e.name()) == 0) + { + // If an Activation attempt fails, the Unsubscribe method is not + // called. This may lead to an AlreadySubscribed error if the + // Activation is re-attempted. + } + else + { + log<level::ERR>("Error subscribing to systemd", + entry("ERROR=%s", e.what())); + } + } return; } @@ -59,6 +79,9 @@ auto Activation::activation(Activations value) -> Activations { if (rwVolumeCreated == false && roVolumeCreated == false) { + // Enable systemd signals + Activation::subscribeToSystemdSignals(); + parent.freeSpace(); if (!activationProgress) diff --git a/activation.hpp b/activation.hpp index 9315fc8..ba7c618 100644 --- a/activation.hpp +++ b/activation.hpp @@ -216,8 +216,6 @@ class Activation : public ActivationInherit, Flash std::bind(std::mem_fn(&Activation::unitStateChange), this, std::placeholders::_1)) { - // Enable systemd signals - subscribeToSystemdSignals(); // Set Properties. activation(activationStatus); associations(assocs); |