Request to create repository google-ipmi-bmc-health

[William Kennington]

My 2c... We have plenty of blob handlers that have their own repos to keep
maintainership and purposes separate. The phosphor-ipmi-blobs
repository intends to provide a framework, not specific implementations.

On Tue, Nov 10, 2020 at 10:35 PM Vijay Khemka <vijaykhemka at fb.com> wrote:

>
>
> On 11/5/20, 3:55 PM, "Sui Chen" <suichen at google.com> wrote:
>
>     On Tue, Oct 6, 2020 at 6:43 PM Patrick Williams <patrick at stwcx.xyz>
> wrote:
>     >
>     > On Tue, Oct 06, 2020 at 03:57:30PM -0700, Sui Chen wrote:
>     > > On Fri, Oct 2, 2020 at 1:54 PM Vijay Khemka <vijaykhemka at fb.com>
> wrote:
>     > > > If I understand correctly, protocol buffer will be used by
> daemon who
>     > > > Is responding to the IPMI request and connecting to this daemon
> via
>     > > > library call, then it is completely restricted for the use of
> protocol buffer.
>     > > > If you are passing protocol buffer to this daemon then we have
> to define
>     > > > some policy here.
>     > >
>     > > The Protocol buffer is only for serializing the data to be sent
>     > > outside of the BMC. It is not used for communication inside
>     > > phosphor-health-monitor and will not be passed to the daemon.
>     >
>     > Why isn't this part done from within an existing IPMI provider
> (ideally
>     > to me a google-ipmi-* repository at this time)?  I'm not especially
> keen
>     > on these details leaking out into other non-IPMI repositories.
>     >
>     > > >
>     > > >     Other than these two things I think adding new metrics to
>     > > >     phosphor-health-monitor should be manageable. I can start by
> trying to
>     > > >     add the IPMI blob handler to phosphor-health-monitor; my
> first attempt
>     > > >     might not look very elegant, but if we find answers to the
> two
>     > > >     questions above, the merged result will look a lot better.
> Hopefully
>     > > >     we can find a solution that works well for everyone.
>     > > >
>     > > > I am looking forward to your patches
>     > >
>     > > Please check out this WIP:
>     > >
> https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-health-monitor/+/37092
>     > >
>     > > This WIP currently just adds the IPMI blob-based code to
>     > > phosphor-health-monitor almost as-is.
>     > > It also shows what we already have now.
>     > >
>     > > There will be some work to merge the daemon and the blob handler
> in an
>     > > organic way, and I am open to discussion with you how to do that.
> The
>     > > first step I think I can do is to put the code for extracting the
>     > > metrics (metrics.cpp, blob/metric.cpp) into a single file and share
>     > > that between the daemon and the IPMI blob handler.
>     > >
>     > > Another issue I found is I am not using the latest sdbusplus so I
> have
>     > > to comment out the usage of ValueIface::Unit::Percent for now.
>     > >
>     > > To build this requires 1) adding a pkgconfig file to
>     > > phosphor-ipmi-blobs (before
>     > >
> https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-ipmi-blobs/+/37133
>     > > gets merged) and 2) adding phosphor-ipmi-blobs and protobuf to
> DEPENDS
>     > > in phosphor-health-monitor's Bitbake recipe.
>     > >
>     > > Hope this WIP change illustrates our intention clearly.
>     > >
>     > > Thanks!
>     >
>     > --
>     > Patrick Williams
>
>
>     Hello Patrick and Vijay,
>
>     As far as I know, the only two "google-ipmi-*" repositories are 1)
>     google-ipmi-sys and 2) google-ipmi-i2c, and neither seem to be related
>     to the health monitoring task we're doing right now.
>     In my understanding one similar library is phosphor-pid-control; its
>     IPMI handler is also in the repository rather than in a separate
>     repository.
>
>     The "health monitoring IPMI Blob Handler" (that the request in the
>     first email in this thread was indended for) was a monolithic IPMI
>     blob handler; it used to both generate metrics and handle IPMI
>     requests.
>     In the last month, I had de-coupled these two functions so the IPMI
>     blob handler does not generate metrics but reads metrics from the
>     daemon in phosphor-health-monitor via DBus. In other words, the
> "monolithic"
>     handler has now become a thin layer. On the other hand,
>     phosphor-health-monitor will have to be significantly modified to
>     generate the metrics that are in a different format from what it's
>     generating right now, and Vijay and I are working on that. I had
> create a chain
>     of changes
> https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-health-monitor/+/37659
>     to illustrate what I intend to do.
>     As a result, there comes the question of where the IPMI blob handler
>     should live, and it appears I have the following choices:
>     1. in phosphor-health-monitor, or
>     2. some centralized location, along with many other IPMI blob
> handlers, or
>     3. as a separate, new repository, or
>     4. something else?
>
>     I am facing a confusing situation as to where I should put the IPMI
>     blob handler, due to conflicting opinions:
>     1. The maintainers of phosphor-ipmi-blobs told me it's not desirable
>     to put all IPMI blob handlers into the same place.
>     2. By reading this email thread, I had the impression that it's not a
>     good idea to create too many repositories either.
>     3. Because of #1 and #2, I felt we should put the IPMI blob handler
>     into phosphor-health-monitor itself, just like phosphor-pid-control
>     does.
>     4. In the last reply from Patrick it sounds it's a bad idea to put the
>     IPMI blob handler into phosphor-health-monitor because of IPMI details
>     leaking out into non-IPMI repositories.
>     5. Vijay seemed to prefer to have all IPMI blob handlers in one place
>     based on our discussion on IRC. However, according to #1 this is going
>     to face pushback. As such, I created all my changes in
>     phosphor-health-monitor for review and for showing my intent on how
>     the IPMI implementation is done.
>     6. Because of #4 and #5, it sounds like I can't put the IPMI blob
> handler into
>     phosphor-health-monitor either.
>     So now, there is no place I can place this handler, and I am now at a
> dead end.
>
> I still feel that this should go to phosphor-ipmi-blobs, you can create a
> separate
> directory (handler) under the same repo and it can become home for all the
> futures blob handler as these are going to interact with ipmi blobs anyway.
>
>     I need to find a way out and would greatly appreciate it if we can
>     reach a consensus here so that BMC health monitoring can move forward.
>
>     Thanks!
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20201110/9b9db3d9/attachment-0001.htm>