DBus ObjectManager Interface usages within the project

[Nan Zhou]

I searched all the DIMM daemons in the org.
https://github.com/openbmc/openpower-vpd-parser/search?q=DIMM

Seems to me smbios-sdr is the only public repo. Are there any other repos
that implement the DIMM interface?

On Wed, Jul 13, 2022 at 5:49 PM Nan Zhou <nanzhou at google.com> wrote:

> 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/20220912/9d43ba86/attachment-0001.htm>