Re: Sdbusplus-based Shared Library

[Brad Bishop <bradleyb@fuzziesquirrel.com>]

> On Mar 28, 2018, at 11:25 PM, Patrick Venture <venture@google.com> wrote:
> 
> On Wed, Mar 28, 2018 at 6:39 PM, Brad Bishop
> <bradleyb@fuzziesquirrel.com> wrote:
>> 
>>> On Mar 27, 2018, at 11:43 AM, Patrick Venture <venture@google.com> wrote:
>>> 
>>> Brad,
>>> 
>>> If you create a repository - phosphor-sdbus-utils or
>>> phosphor-sdbusplus-utils or something like that, I'll put together a
>>> starter pack on this and submit for review.
>> 
>> May I suggest
>> 
>> https://github.com/openbmc/phosphor-dbus-monitor/tree/master/src/sdevent
>> 
>> as a possibility for drawing ideas?  :-)
>> 
>>> 
>>> Patrick
>> 
>> Thanks Patrick! will do but I’d like to get a bit more clarity/discussion
>> on what the content of this new repo will be.
>> 
>> So today we have sdbusplus built around the sd-bus folder in libsystemd.
>> 
>> Do we envision something like separate repos for:
>> 
>> sdeventplus - wrappers for the sd-event folder in libsystemd
>> sddeviceplus - wrappers for the sd-device folder in libsystemd
>> …etc
>> 
>> Or should we just try and have a single c++ wrapper repository for all of
>> libsystemd?  In that case should we simply rename sdbusplus to libsystemdpp
>> and start putting more code in it?
> 
> I had envisioned all the sd-bus wrappers in one library, however, I'm
> quite flexible about any of that.  

I think sdbusplus has been forked for non-openbmc use so it probably
doesn’t make sense to add OpenBMC-isms to it, like the mapper for instance.
Do you agree?

I thought you were talking about extending the concept that was applied
to sdbus (thin raii wrappers around sdbus) to sdevent.  I definitely think
we need that.

I think C++ bindings around sdevent could also have general utility to someone
outside of OpenBMC so I’d again vote for not including OpenBMC-isms in a
library for something like that.

> My primary goal with pushing this
> line of thought it to reduce code duplication in openbmc.  So many
> daemons have the same methods that just talk to sdbusplus, etc.  So it
> makes sense to me to have a utility library for interfacing with
> sdbusplus for common tasks.  phosphor::util::getService(),
> phosphor::util::get... etc, so they can be maintained in one place.

I feel like a number of these are mapper related.  Could we add some
client bindings for the mapper that help here and put them in the
mapper repository?  Here is a possible example of how that could
look:

https://github.com/openbmc/phosphor-dbus-monitor/blob/master/src/sdbusplus.hpp

I wrote this class so I could mock sdbusplus calls.

> Even phosphor::util::network::... because those are becoming
> duplicated.  It might make sense to have multiple libraries for the
> utilities, which is also fine.

I guess I’m advocating for distributed utilities, aligned functionally.  I worry
about a monolithic library…it seems like it could easily become a free-for-all.

Thoughts?

> 
>> 
>> -brad