Re: Enable/Disable some sensors when Host On/Off

[Thu Ba Nguyen <tbnguyen1985@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 13776 bytes --]

Please see my comments below.

Thanks.
Thu Nguyen.

On Wed, Nov 4, 2020 at 11:31 PM Matt Spinler <mspinler@linux.ibm.com> wrote:

>
>
> On 11/4/2020 3:15 AM, Thu Ba Nguyen wrote:
> > Hi Matt, I implemented "Inactive the host sensors" in phosphor-hwmon
> > use below...
> > This Message Is From an External Sender
> > This message came from outside your organization.
> >
> > Hi Matt,
> >
> > I implemented "Inactive the host sensors" in phosphor-hwmon use below
> > approaching:
> > 1. Add one option in Sensors configuration, phosphor-hwmon will parse
> > this option and add host sensors to _hostSensors list.
> > 2. In mainloop::read() before going to loop to read the sensors in the
> > reading list. Query dbus to get CurrentHostState property.
> > This property belongs to service "xyz.openbmc_project.State.Host".
> > Based on the status of this property, identify host status.
> > If the host is off, remove the host sensors from _state list and dbus.
> > I expected the users wouldn't see the host sensors on the BMC Web when
> > the host is off.
> > If the host is on, add the host sensors back to _state list and also
> dbus.
> >
> > The code is working. But I have two issues with this approaching:
> >
> > 1. Too many transactions to get dbus property CurrentHostState.
> > In my case, I have 6 services to monitor the sensors which concern the
> > host.
> > With the current interval 1 second of phosphor-hwmon, I have 6
> > transactions to get CurrentHostState per seconds.
>
> The better way to implement this would be to read CurrentHostState once
> on startup, and then register for
> propertiesChanged signals for it and provide a callback to update an
> internal host state variable.
>
[Thu] Yes, This is what I'm planning to do. But don't know how to add a
propertiesChanged signals
to the current phosphor-hwmon framework.

I usually use below code to add a signal to dbus.
    boost::asio::io_service io;
    ampere::host::conn =
      std::make_shared<sdbusplus::asio::connection>(ampere::host::io);

    std::vector<std::unique_ptr<sdbusplus::bus::match::match>> matches;
    //Start tracking power state
    std::unique_ptr<sdbusplus::bus::match::match> hostMonitor =
        ampere::host::startHostStateMonitor();
    matches.emplace_back(std::move(hostMonitor));

    /* wait for the signal */
    ampere::host::io.run();
But when start io.run(), phosphor-hwmon will stop reading sensors.

> 2. When I call "ipmitool power off" the host, there is a gap between
> > the time I trigger GPIO to power off the chassis and the time Dbus
> > property CurrentHostState is updated.
> > In this gap, the phosphor-hwmon is still reading sensors. And this
> > causes the threshold warnings or errors. I want to avoid this.
> >
> > Do you have any suggestions to avoid these issues?
> >
>
> I can't think of anything at the moment.  Maybe someone else has an idea
> here.
>
> > Others question:
> > I saw that phosphor-hwmon is registering an event to smbus and trigger
> > the event after each 1 second to read sensors.
> > Can I change the phosphor-hwmon code to integrate one dbus signal event?
> > Which will be triggered when there is changing in dbus property.
>
> I'm not sure I understand what you're asking for here.  Right now it
> will do 1 second reads (the period is
> configurable) and send a properties changed signal for each sensor on
> D-Bus only when the value changes.
>
[Thu] I'm asking for how to add a signal to current phosphor-hwmon
framework as comment 1.

> >
> > I knew how to create a service which adds the call back function when
> > there is change in dbus property.
> > But don't know how to intergrace it to hwmon.
> >
> > Regards.
> > Thu Nguyen.
> >
> >
> >
> > On Fri, Oct 23, 2020 at 5:45 AM Thu Ba Nguyen <tbnguyen1985@gmail.com
> > <mailto:tbnguyen1985@gmail.com>> wrote:
> >
> >     Just remove all of added code, rebase the phosphor-hwmon source to
> >     commit
> >     "5906173 (12 months ago) Brad Bishop: build: add support for
> >     building with meson"
> >
> >     Add the include:
> >     #include<sdbusplus/asio/connection.hpp>
> >     I can see the error
> >     |
> >
>  /openbmc/jade_build/tmp/work/arm1176jzs-openbmc-linux-gnueabi/phosphor-hwmon/1.0+gitAUTOINC+5906173aec-r1/recipe-sysroot-native/usr/bin/arm-openbmc-linux-gnueabi/../../libexec/arm-openbmc-linux-gnueabi/gcc/arm-openbmc-linux-gnueabi/10.1.0/ld:
> >     phosphor_hwmon_readd-readd.o: undefined reference to symbol
> >     'pthread_key_delete@@GLIBC_2.4'
> >     |
> >
>  /openbmc/jade_build/tmp/work/arm1176jzs-openbmc-linux-gnueabi/phosphor-hwmon/1.0+gitAUTOINC+5906173aec-r1/recipe-sysroot-native/usr/bin/arm-openbmc-linux-gnueabi/../../libexec/arm-openbmc-linux-gnueabi/gcc/arm-openbmc-linux-gnueabi/10.1.0/ld:
> >
>  /openbmc/jade_build/tmp/work/arm1176jzs-openbmc-linux-gnueabi/phosphor-hwmon/1.0+gitAUTOINC+5906173aec-r1/recipe-sysroot/lib/libpthread.so.0:
> >     error adding symbols: DSO missing from command line
> >     | collect2: error: ld returned 1 exit status
> >     | make[2]: *** [Makefile:643: phosphor-hwmon-readd] Error 1
> >     | make[2]: Leaving directory
> >
>  '/openbmc/jade_build/tmp/work/arm1176jzs-openbmc-linux-gnueabi/phosphor-hwmon/1.0+gitAUTOINC+5906173aec-r1/build'
> >     | make[1]: *** [Makefile:801: all-recursive] Error 1
> >     | make[1]: Leaving directory
> >
>  '/openbmc/jade_build/tmp/work/arm1176jzs-openbmc-linux-gnueabi/phosphor-hwmon/1.0+gitAUTOINC+5906173aec-r1/build'
> >     | make: *** [Makefile:524: all] Error 2
> >     | ERROR: oe_runmake failed
> >     | WARNING: exit code 1 from a shell command.
> >     | ERROR: Execution of
> >
>  '/openbmc/jade_build/tmp/work/arm1176jzs-openbmc-linux-gnueabi/phosphor-hwmon/1.0+gitAUTOINC+5906173aec-r1/temp/run.do_compile.2045'
> >     failed with exit code 1:
> >     | Makefile:800: target 'check-valgrind-recursive' given more than
> >     once in the same rule
> >     | Makefile:800: target 'check-valgrind-memcheck-recursive' given
> >     more than once in the same rule
> >     | Makefile:800: target 'check-valgrind-helgrind-recursive' given
> >     more than once in the same rule
> >     | Makefile:800: target 'check-valgrind-drd-recursive' given more
> >     than once in the same rule
> >     | Makefile:800: target 'check-valgrind-sgcheck-recursive' given
> >     more than once in the same rule
> >     | make  all-recursive
> >
> >     I think we should add thread lib.
> >
> >     Regards.
> >     Thu Nguyen.
> >
> >     On Thu, Oct 22, 2020 at 10:51 PM Matt Spinler
> >     <mspinler@linux.ibm.com <mailto:mspinler@linux.ibm.com>> wrote:
> >
> >
> >
> >         On 10/22/2020 9:49 AM, Thu Ba Nguyen wrote:
> >         > I started on supporting enable/disable host sensors.
> >         Everythings is
> >         > fine until I...
> >         > This Message Is From an External Sender
> >         > This message came from outside your organization.
> >         >
> >         > I started on supporting enable/disable host sensors.
> >         > Everythings is fine until I add code to monitor host status
> >         as below.
> >         > bool MainLoop::isHostOn(void)
> >         > {
> >         > char buff[128];
> >         > if (!powerMatch)
> >         > {
> >         > log<level::ERR>("Power Match Not Created");
> >         > }
> >         > sprintf(buff, "isHostOn powerStatusOn: %d\n",powerStatusOn);
> >         > log<level::INFO>(buff);
> >         > return powerStatusOn;
> >         > }
> >         > std::shared_ptr<sdbusplus::bus::match::match>
> >         > MainLoop::startHostStateMonitor(void) {
> >         > return std::make_shared<sdbusplus::bus::match::match>(
> >         > *conn,
> >         > "type='signal',interface='org.freedesktop.DBus.Properties',"
> >         >
> >
>  "member='PropertiesChanged',arg0='xyz.openbmc_project.State.Host'",
> >         > [](sdbusplus::message::message &msg) {
> >         > std::string interfaceName;
> >         > boost::container::flat_map<std::string,
> >         std::variant<std::string>>
> >         > propertiesChanged;
> >         > try {
> >         > msg.read(interfaceName, propertiesChanged);
> >         > } catch (std::exception &e) {
> >         > log<level::ERR>("Unable to read host state\n");
> >         > return;
> >         > }
> >         > // We only want to check for CurrentHostState
> >         > if (propertiesChanged.begin()->first != "CurrentHostState") {
> >         > return;
> >         > }
> >         > auto findState = propertiesChanged.find(powProperty);
> >         > if (findState != propertiesChanged.end())
> >         > {
> >         > powerStatusOn = boost::ends_with(
> >         > std::get<std::string>(findState->second), "Running");
> >         > }
> >         > powerMatch = true;
> >         > });
> >         > }
> >         > void MainLoop::read()
> >         > {
> >         > // TODO: Issue#3 - Need to make calls to the dbus sensor
> >         cache here to
> >         > // ensure the objects all exist?
> >         > /* Host changed state from On to Off */
> >         > if (!isHostOn() && (lastPowerState == HOST_ON ||
> >         > lastPowerState == HOST_NA)) {
> >         > removeHostSensors();
> >         > lastPowerState = HOST_OFF;
> >         > }
> >         > /* Host changed state from Off to On */
> >         > if (isHostOn() && lastPowerState == HOST_OFF) {
> >         > addDroppedHostSensors();
> >         > lastPowerState = HOST_ON;
> >         > }
> >         > When build openBMC I got error:
> >         >
> >
>  tmp/work/arm1176jzs-openbmc-linux-gnueabi/phosphor-hwmon/1.0+gitAUTOINC+5906173aec-r1/recipe-sysroot/lib/libpthread.so.0:
> >
> >         > error adding symbols: DSO missing from command line
> >         > | collect2: error: ld returned 1 exit status
> >         > | make[2]: *** [Makefile:643: phosphor-hwmon-readd] Error 1
> >         >
> >         > It seems I need adding the threads lib to defend lib.
> >         > Any suggestion to add threads lib to build configuration?
> >         >
> >
> >         That must be because you're using that single boost function?
> >         While you
> >         could add the dependency,
> >         the ideal thing to do since this repo already uses
> >         phosphor-dbus-interfaces is to use the function:
> >
> >                  /** @brief Convert a string to an appropriate enum
> value.
> >                   *  @param[in] s - The string to convert in the form of
> >                   * "xyz.openbmc_project.State.Host.<value name>"
> >                   *  @return - The enum value.
> >                   */
> >                  static HostState convertHostStateFromString(const
> >         std::string& s);
> >
> >         to convert it to the actual HostState enum to check against:
> >
> >                  enum class HostState
> >                  {
> >                      Off,
> >                      Running,
> >                      Quiesced,
> >                      DiagnosticMode,
> >                  };
> >
> >         This is all in xyz/openbmc_project/State/Host/server.hpp
> >         provided by
> >         phosphor-dbus-interfaces.
> >
> >         > Thanks.
> >         > Thu.
> >         >
> >         > On Wed, Oct 21, 2020 at 11:54 PM Thu Ba Nguyen
> >         <tbnguyen1985@gmail.com <mailto:tbnguyen1985@gmail.com>
> >         > <mailto:tbnguyen1985@gmail.com
> >         <mailto:tbnguyen1985@gmail.com>>> wrote:
> >         >
> >         >     Hi Vijay,
> >         >
> >         >     I took a look on entity-manager and openbmc source.
> >         >     Don't have many companies  using entity-manager model to
> >         support
> >         >     sensors.
> >         >
> >         >     Regards
> >         >     Thu Nguyen.
> >         >
> >         >
> >         >     On Wed, Oct 21, 2020 at 7:15 AM Vijay Khemka
> >         <vijaykhemka@fb.com <mailto:vijaykhemka@fb.com>
> >         >     <mailto:vijaykhemka@fb.com <mailto:vijaykhemka@fb.com>>>
> >         wrote:
> >         >
> >         >         *From: *openbmc
> >         >         <openbmc-bounces+vijaykhemka=fb.com@lists.ozlabs.org
> >         <mailto:fb.com@lists.ozlabs.org>
> >         >         <mailto:fb.com@lists.ozlabs.org
> >         <mailto:fb.com@lists.ozlabs.org>>> on behalf of Thu Ba Nguyen
> >         >         <tbnguyen1985@gmail.com
> >         <mailto:tbnguyen1985@gmail.com> <mailto:tbnguyen1985@gmail.com
> >         <mailto:tbnguyen1985@gmail.com>>>
> >         >         *Date: *Monday, October 19, 2020 at 11:23 AM
> >         >         *To: *Ed Tanous <ed@tanous.net
> >         <mailto:ed@tanous.net> <mailto:ed@tanous.net
> >         <mailto:ed@tanous.net>>>
> >         >         *Cc: *OpenBMC Maillist <openbmc@lists.ozlabs.org
> >         <mailto:openbmc@lists.ozlabs.org>
> >         >         <mailto:openbmc@lists.ozlabs.org
> >         <mailto:openbmc@lists.ozlabs.org>>>
> >         >         *Subject: *Re: Enable/Disable some sensors when Host
> >         On/Off
> >         >
> >         >         Hi Ed Tanous,
> >         >
> >         >         > Thanks for your info,
> >         >
> >         >         > But in your platform we are using phosphor-hwmon
> >         to manage
> >         >         sensors.
> >         >
> >         >         > We don't use entity-manager.
> >         >
> >         >         > As I knew we can't use both entity-manager and
> >         >         phosphor-hwmon for one project.
> >         >
> >         >         You can use both but for different sensors. You can
> >         decide
> >         >         what sensors to configure
> >         >
> >         >         via EM/dbus-sensors and which one for phosphor-hwmon.
> >         >
> >         >         Regards
> >         >
> >         >         Thu Nguyen.
> >         >
> >
>
>

[-- Attachment #2: Type: text/html, Size: 20342 bytes --]