Missing fan present object in fansensor (and expected usage of object manager)

[Patrick Williams]

On Thu, Dec 01, 2022 at 09:38:17PM +0800, Lei Yu wrote:
> This email is to discuss an issue found in fansensor service, and
> about the expected usage of object manager.
> 
> # The issue
> With upstream dbus-sensors (7627c86), the fan-present objects are
> missing on DBus.
> The fansensor service should create objects like
> `/xyz/openbmc_project/inventory/fan0` to represent the fan presence
> status [1].
> However, with the changes of "fixing the ObjectManager in the right
> place"[2], it creates an object manager with
> `/xyz/openbmc_project/sensors` instead of `/`, and the objects created
> on different object paths are actually not created anymore.
> 
> I see there is a fix for the "control" objects in the fanensor[3], but
> the fan-present objects are still missing.
> 
> The fix is simple that we could add an extra object manager on
> `/xyz/openbmc_project/inventory/`.
> 
> # Expected usage of object manager
> But that involves extra object managers.

There isn't really any harm in extra object managers.  They are "cheap"
as they're effectively just a string held in the sd-bus library.

> For a service that creates objects on different paths (e.g. sensors,
> control, inventory), should it really create different object
> managers?

We've had some ambiguity in where the object manager should be hosted.
From a dbus perspective there is no requirement, but it makes it
difficult to call GetManagedObjects.  The direction that we've been
going is that:

    1. Some hierarchies are now explicitly documenting where an ObjectManager
       is required (like sensors does now).  This should generally go at
       the level 1 deeper than "openbmc_project".

    2. Daemons can still optionally host an ObjectManager at the root if
       this is somehow helpful.

> The caller of such service (e.g. bmcweb) usually calls
> `GetManagedObjects` to the object manager interface to get the
> objects. In the above case, it has to call `GetManagedObjects`
> multiple times on different paths.

The reason for having the ObjectManager lower in the hierarchy is so
that the GetManagedObjects call is cheaper and doesn't give you every
object hosted by that service.  In something like the PLDM daemon this
might be lots of objects that the particular call has no interest in.

I don't suspect there is really any case, with the current
implementation, where bmcweb is going to attempt to prune the calls by
grabbing objects from two different hierarchies in a single call.  We've
decided to simplify the "where do I find the ObjectManager" instead of
optimizing the performance of "give me exactly what I want with this specific
query".

> So the question is, what is the *expected* and *proper* way to use
> object manager for such case?
> 
> [1]: https://github.com/openbmc/dbus-sensors/blob/master/src/TachSensor.cpp#L77
> [2]: https://github.com/openbmc/dbus-sensors/commit/14ed5e99
> [3]: https://github.com/openbmc/dbus-sensors/commit/d9067251

FWIW, Nan fixed[1] the same issue you're seeing in dbus-sensors in the
phosphor-health-monitor by adding the ObjectManager at the root.  We
don't currently require one at `/xyz/openbmc_project/inventory` but I
wouldn't be surprised if we do in the not-too-distant future.

1. https://github.com/openbmc/phosphor-health-monitor/commit/af109947dad9c6dbf496c4111c625e5ae407dd81

-- 
Patrick Williams
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20221201/0c9c0b14/attachment-0001.sig>