Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ rbmc_data_sync_sources = [
'external_data_ifaces.cpp',
'external_data_ifaces_impl.cpp',
'manager.cpp',
'notify_service.cpp',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit message:

  • The notification requests are copied from the sibling BMC to
    /var/lib/phosphor-data-sync/services/.

path is correct? I think /var/lib/phosphor-data-sync/notify-services/?

'notify_sibling.cpp',
'persistent.cpp',
'sync_bmc_data_ifaces.cpp',
Expand Down
201 changes: 201 additions & 0 deletions src/notify_service.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,201 @@
// SPDX-License-Identifier: Apache-2.0

#include "config.h"

#include "notify_service.hpp"

#include <nlohmann/json.hpp>
#include <phosphor-logging/lg2.hpp>

#include <fstream>
#include <iostream>
#include <map>

namespace data_sync::notify
{
namespace file_operations
{
nlohmann::json readFromFile(const fs::path& notifyFilePath)
{
std::ifstream notifyFile;
notifyFile.open(fs::path(NOTIFY_SERVICES_DIR) / notifyFilePath);
nlohmann::json notifyInfoJSON(nlohmann::json::parse(notifyFile));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

22 and 24 could be combined.

return nlohmann::json(nlohmann::json::parse(notifyFile) - If parse return the JSON then return nlohmann::json::parse(notifyFile);


return notifyInfoJSON;
}
} // namespace file_operations

NotifyService::NotifyService(sdbusplus::async::context& ctx,
const fs::path& notifyFilePath)
{
ctx.spawn(init(ctx, notifyFilePath));
}

sdbusplus::async::task<std::string>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets hold this commit?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be we can, Can push it when DBus interface things finalized.

// NOLINTNEXTLINE
NotifyService::getDbusObjectPath(sdbusplus::async::context& ctx,
const std::string& service,
const std::string& interface)
{
std::string objectPath{};

constexpr auto objMapper =
sdbusplus::async::proxy()
.service("xyz.openbmc_project.ObjectMapper")
.path("/xyz/openbmc_project/object_mapper")
.interface("xyz.openbmc_project.ObjectMapper");

/**
* Output of the DBus call will be a nested map, where
* outer map : Map of DBus Object Path and the inner map
* inner map : Map of the service name and list of interfaces it has
* implemented.
*/
using Services = std::map<std::string, std::vector<std::string>>;
using Paths = std::map<std::string, Services>;
Paths subTreesInfo = co_await objMapper.call<Paths>(
ctx, "GetSubTree", "/", 0, std::vector<std::string>{interface});

auto outerMapItr = std::ranges::find_if(subTreesInfo,
[&service](const auto& outerMap) {
return std::ranges::any_of(outerMap.second,
[&service](const auto& serviceIfacesList) {
return serviceIfacesList.first == service;
});
});
if (outerMapItr != subTreesInfo.end())
{
objectPath = outerMapItr->first;
}
else
{
lg2::error(
"Unable to find the object path which hosts the interface[{INTERFACE}],"
" of the service {SERVICE}",
"INTERFACE", interface, "SERVICE", service);
throw std::runtime_error("Unable to find the DBus object path");
}
co_return objectPath;
}

// NOLINTNEXTLINE
sdbusplus::async::task<> NotifyService::invokeNotifyDBusMethod(
[[maybe_unused]] sdbusplus::async::context& ctx,
[[maybe_unused]] const std::string& service,
[[maybe_unused]] const fs::path& modifiedDataPath)
{
// TODO : Complete DBus notification method once INTERFACE and method name
// finalized
// Step 1 : Invoke getDbusObjectPath()
// Step 2 : Using the object path,interface and service, frame the DBus
// :method call
lg2::warning("DBus notification support is not available currently");

co_return;
}

sdbusplus::async::task<>
// NOLINTNEXTLINE
NotifyService::sendDBusNotification(sdbusplus::async::context& ctx,
std::vector<std::string> services,
fs::path modifiedDataPath)
{
for (const auto& service : services)
{
// NOLINTNEXTLINE
co_await invokeNotifyDBusMethod(ctx, service, modifiedDataPath);
}

co_return;
}

sdbusplus::async::task<>
// NOLINTNEXTLINE
NotifyService::sendSystemDNotification(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to move this into the external interface class? I’d prefer to keep all DBus calls consolidated

sdbusplus::async::context& ctx,
const std::vector<std::string>& services, const std::string& method)
{
for (const auto& service : services)
{
auto systemdReload = sdbusplus::async::proxy()
.service("org.freedesktop.systemd1")
.path("/org/freedesktop/systemd1")
.interface("org.freedesktop.systemd1.Manager");
try
{
using objectPath = sdbusplus::message::object_path;
if (method == "Restart")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could merge this if and else if block.

dbusMethodName = (method == "Reload" ? "ReloadUnit" : "RestartUnit");

// Added info trace so that we will able to know why some service restarted/reloaded otherwise we wont able to know why it is restarted/reloaded.
lg2::info("{METHOD} {SERVICE} to notify synced data", "METHOD", dbusMethodName, "SERVICE", service);

co_await systemdReload.call<objectPath>(ctx, dbusMethodName, service, "replace");

{
lg2::debug("Restart request send for {SERVICE}", "SERVICE",
service);
co_await systemdReload.call<objectPath>(ctx, "RestartUnit",
service, "replace");
}
else if (method == "Reload")
{
lg2::debug("Reload request send for {SERVICE}", "SERVICE",
service);
co_await systemdReload.call<objectPath>(ctx, "ReloadUnit",
service, "replace");
}
}
catch (const sdbusplus::exception::SdBusError& e)
{
lg2::error(
"Failed to send systemd notification request to {SERVICE}: {ERROR}",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to have filepath details , I feel we could able to get from the NofifyInfo data.

"SERVICE", service, "ERROR", e.what());
}
}

co_return;
}

// NOLINTNEXTLINE
sdbusplus::async::task<> NotifyService::init(sdbusplus::async::context& ctx,
fs::path notifyFilePath)
{
nlohmann::json notifyfileData{};
try
{
notifyfileData = file_operations::readFromFile(notifyFilePath);
}
catch (const std::exception& exc)
{
lg2::error(
"Failed to read the notify request file[{FILEPATH}], Error : {ERR}",
"FILEPATH", notifyFilePath, "ERR", exc);
throw std::runtime_error("Failed to read the notify request file");
}
if (notifyfileData["NotifyInfo"]["Mode"] == "DBus")
{
// Send DBUS notification
// NOLINTNEXTLINE
co_await sendDBusNotification(
ctx,
notifyfileData["NotifyInfo"]["NotifyServices"]
.get<std::vector<std::string>>(),
notifyfileData["ModifiedDataPath"]);
}
else if ((notifyfileData["NotifyInfo"]["Mode"] == "Systemd"))
{
// Do systemd reload/restart
// NOLINTNEXTLINE
co_await sendSystemDNotification(
ctx,
notifyfileData["NotifyInfo"]["NotifyServices"]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to pass the JSON object as a const reference and use the fields as needed. For example, we can include the file-path details in the trace if a service fails to reload or restart.

.get<std::vector<std::string>>(),
notifyfileData["NotifyInfo"]["Method"].get<std::string>());
}
else
{
lg2::error("Failed to process the notify request[{PATH}], Error : "
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please include the file path we are notifying and include mode value as well. It will help during debugging to identify which file failed to notify.

"Unknown Mode/Method in the request",
"PATH", notifyFilePath);
co_return;
}
fs::remove(notifyFilePath);

co_return;
}

} // namespace data_sync::notify
113 changes: 113 additions & 0 deletions src/notify_service.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
// SPDX-License-Identifier: Apache-2.0
#pragma once

#include "data_sync_config.hpp"

#include <sdbusplus/async.hpp>

#include <filesystem>

namespace data_sync::notify
{

namespace fs = std::filesystem;

/**
* @class NotifyService
*
* @brief The class which contains the APIs to process the sibling notification
* requests received from the sibling BMC on the local BMC and issue the
* necessary notifications to the configured services.
*/
class NotifyService
{
public:
/**
* @brief Constructor
*
* @param[in] ctx - The async context object
* @param[in] notifyFilePath - The root path of the received notify request
* file.
*/
NotifyService(sdbusplus::async::context& ctx,
const fs::path& notifyFilePath);

private:
/**
* @brief Get the Dbus Object Path object associated with the service
* which implements the given interface.
*
* @param[in] ctx - The async context object
* @param[in] service - The DBus service name
* @param[in] interface - The DBus Interface name
*
* @return DBus Object path as sdbusplus::async::task<std::string>
*/
static sdbusplus::async::task<std::string>
getDbusObjectPath(sdbusplus::async::context& ctx,
const std::string& service,
const std::string& interface);

/**
* @brief API to notify the requested services by triggering the DBus method
* implemented at the service to be notified.
*
* @param[in] ctx - The async context object.
* @param[in] service - The DBus service name which need to be notified
*
* @return void
*/
sdbusplus::async::task<>
invokeNotifyDBusMethod(sdbusplus::async::context& ctx,
const std::string& service,
const fs::path& modifiedDataPath);

/**
* @brief API to the initiate the DBus notification to all the
*. configured services if the mode of notification is DBus.
*
* @param[in] ctx - The async context object.
* @param[in] services - The vector of DBus service name which need to be
* notified
* @param[in] modifiedDataPath - The root path of the modified path given in
*the request
*
* @return void
*/
sdbusplus::async::task<>
sendDBusNotification(sdbusplus::async::context& ctx,
std::vector<std::string> services,
fs::path modifiedDataPath);

/**
* @brief API to initiate the systemd notification to all the configured
* services if the mode of notification is systemd. It will trigger
* either systemd reload or restart depends on the configured Method
* in the received request.
*
* @param[in] ctx - The async context object.
* @param[in] services - The vector of DBus service name which need to be
* notified
* @param[in] method - The method to be invoked as part of systemd
* notification. Can have either 'Restart' or 'Reload' as values.
*/
static sdbusplus::async::task<>
sendSystemDNotification(sdbusplus::async::context& ctx,
const std::vector<std::string>& services,
const std::string& method);

/**
* @brief The API to trigger the notification to the configured service upon
* receiving the request from the sibling BMC
*
* @param ctx[in] - The async context object.
* @param notifyFilePath[in] - The root path of the received notify request
* file.
*
* @return void
*/
sdbusplus::async::task<> init(sdbusplus::async::context& ctx,
fs::path notifyFilePath);
};

} // namespace data_sync::notify