Request to create repository google-ipmi-bmc-health

[Sui Chen]

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 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!