Re: Request to create repository google-ipmi-bmc-health

[Sui Chen <suichen@google.com>]

On Fri, Oct 2, 2020 at 1:54 PM Vijay Khemka <vijaykhemka@fb.com> wrote:
>
> Hi Sui,
>
> On 10/1/20, 6:52 PM, "Sui Chen" <suichen@google.com> wrote:
>
>     Hi Vijay,
>
>     We can use whatever means that gets health monitoring done.
>     I have the following questions on how to merge the proposed IPMI
>     Blob-based implementation, google-ipmi-bmc-health (referred to as
>     "IPMI health blob") with phosphor-health-monitor. The intent of having
>     a separate "google-ipmi-bmc-health" was to avoid these questions:
>
>     1) The IPMI health blob is a library, not a daemon, so after the IPMI
>     health blob is added, phosphor-health-monitor will have both a library
>     and a daemon. The user needs to have a way to configure it. What is
>     the recommended way of doing this configuration?
>
> Yes the same repo can generate library as well as daemon. Currently it is
> configuring 2 metrics cpu and memory, we can add another entry like
> IPMI blob and if it is there then only it will build ipmi blobs.
>
>     2) We are sending a protocol buffer through the IPMI interface to the
>     BMC, and the protocol buffer may be only used for the IPMI path and
>     not anywhere else. Would there be any concerns on the usage of a
>     protocol buffer here?
>
> 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.

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