DBus ObjectManager Interface usages within the project

[Nan Zhou]

>
> I don't think "OM is optional" is really a true statement.  It is
> optional from a dbus perspective, true, but practically speaking all of
> our daemons have it (just at an inconsistent location).  One of the
> primary features of the object manager is to emit the signals for
> managed objects, such as PropertiesChanged.  If you don't have an object
> manager you don't get those signals.  We have to have that on Sensors,
> at least, and I'm pretty sure everyone adds them for Inventory as well.


As you're seeing, there are typically two behaviors of implementation:


1. Anything using ASIO places OM at the root by default.
> 2. Anything using PDI-bindings places the OM at the lowest common part
>    in their hierarchy.  (Something like /xyz/openbmc_project/sensors or
>    /xyz/openbmc_project typically).


Is this documented somewhere formally so that when people ask or question
about it, we can have a pointer.

If not, is PDI a good place to place this requirement?

On Wed, Jul 13, 2022 at 7:36 AM Ed Tanous <edtanous at google.com> wrote:

> On Wed, Jul 13, 2022 at 4:24 AM Patrick Williams <patrick at stwcx.xyz>
> wrote:
> >
> > On Tue, Jul 12, 2022 at 11:48:31AM -0700, Ed Tanous 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.
> >
> > There isn't much in the dbus spec that puts requirements on where the
> > object manager is, but that certainly doesn't mean we can't add our own
> > design requirements on top of it.  Thanks for starting this.
> >
> > > 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.
> >
> > This means we can end up having N object managers in a single daemon if
> > it is hosting multiple namespaces?  Why not just host it at
> > /xyz/openbmc_project?
>
> Because rarely is the question being asked "What are all the openbmc
> interfaces and values for those interfaces that this application
> supports".  Usually the question is more specific, like "What sensors
> does this application support".  If we had to get all the inventory
> items, nics, and other stuff at the same time that we got all the
> sensors, it would make the responses larger.
>
> >
> > > This was discussed pretty heavily on discord in the #development
> topic, and
> > > some of the nitty gritty details on why this is needed is available
> there,
> > > as well as I'm happy to discuss here.  This is one of those nasty
> > > spaghetti-code things that we've lived with for a while;  Hopefully if
> we
> > > can get this behind us, we can get some good real world performance
> > > improvements.
> >
> > I see the background being discussed when I read through the history on
> > #development, but I don't see the rationale on why this was chosen.  I
> see one
> > comment that placing the OM at / is "wrong" but I don't see any
> justification.
> > Why is ".../sensors" right but "/" or "/xyz/openbmc_project" is not?
>
> GetManagedObjects doesn't have a way to filter down further, and for
> daemons that might return both, it's advantageous to be able to filter
> down more and not return every interface in one response.
>
> >
> > We had a good chunk of this discussion about 6 months back in
> > phosphor-virtual-sensors where some Redfish code was broken against that
> > daemon because it _was_ using "/xyz/openbmc_project/sensors" and there
> > was a patch to move it to "/" which ended up getting merged.
>
> Yep, this is essentially the continuation of that discussion.
>
> > Fundamentally, I think it boiled down to neither being in opposition to
> > the standard and there was a bunch of code that already implied "/" so
> > it was the simplest way forward to achieve compatibility.
> >
> > >
> > > Thoughts?
> >
> > I do think that moving the OM lower in the hierarchy is probably better
> > because it allows us to have parts of the hierarchy which do not emit
> > signals, where having it on "/" does not.  I'm just trying to understand
> > (and hopefully document more) the rationale on why this choice was made.
>
> Sounds good.
>
> >
> > --
> > Patrick Williams
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20220713/b01c9b4b/attachment.htm>