DBus ObjectManager Interface usages within the project

[Lei Yu]

On Wed, Jul 13, 2022 at 2:49 AM Ed Tanous <edtanous at google.com> wrote:
>
> We've had a couple cases in the project where patches have needed to be slowed down because of inconsistencies in our use of object manager, so IMO, the time has come to make our usage of ObjectManager consistent, documented, and drop support for the (not upstream) daemons that don't follow the rules.  As part of this, we will port forward all the upstream daemons to be consistent, and do our best to avoid bugs, but this email is intended to inform those that have non-upstream daemons about the change so they can resolve their implementations.
>
> The basics:
> ObjectManager DBus interface will now be required for any daemon implementing a Sensor.Value interface or Inventory.Item interface.
>
> Daemons producing sensors will be required to place their ObjectManager at /xyz/openbmc_project/sensors
> Daemons producing inventory items will be required to place their ObjectManager at /xyz/openbmc_project/inventory.
>
> Both of these interfaces will be required to be published before claiming a well known name on dbus, to allow for the possibility of caching implementations at some point in future.
>
> Functionally, this gets a little complicated because the sdbusplus::asio::object_manager bindings in their default invocation create an ObjectManager at the root object /, so we need to execute this in a few steps.
>
> 0. Send this email out, outlining the problem, and warn the community that if this is an assumption you make in your downstream daemons, those assumptions will need to change.  Get consensus from maintainers.
> 1. Update the phosphor-dbus-interfaces documentation for both Sensor.Value and Inventory.Item to call out the required-ness of ObjectManager, and the explicit dbus path where it's required to be placed.
> 2. Identify all the asio daemons that need changes, and publish changes that move the object_manager to the appropriate path.  This is mostly going to be a tree-wide grep for sdbusplus::asio::object_server, and look for daemonst that don't make use of the add_manager() API.  Anyone directly calling the Sensor or Invertory ObjectManager interfaces will need to port to the new paths, but luckily, this isn't a very common operation, and I beleive bmcweb and phosphor-ipmid-host might be the only direct users.  In bmcweb, there is actually a convoluted piece of code that uses the mapper to sort out the location that the ObjectManager exists at so Redfish sensors should remain consistent, and we don't yet have code that relies on ObjectManager for Inventory items.  I believe phosphor-ipmi-host has a similar piece of code that should protect us in this case, but I defer to those maintainers.

Is it better for the asio API to provide an extra constructor to take
a object_path?

E.g.
 object_server(const std::shared_ptr<sdbusplus::asio::connection>&
conn, const std::string& path)

Then the users could initialize its object_server at expected path directly:

 sdbusplus::asio::object_server objectServer(bus, "/my/path");

instead of two lines of code like:

 sdbusplus::asio::object_server objectServer(bus, true);
 objectServer.add_manager("/my/path");

-- 
BRs,
Lei YU