Enable/Disable some sensors when Host On/Off
Matt Spinler
mspinler at linux.ibm.com
Fri Nov 6 10:24:37 AEDT 2020
On 11/5/2020 5:16 PM, Thu Ba Nguyen wrote:
> There is already an sdbusplus loop running, it's in Mainloop::run().
> [Thu] I think...
> This Message Is From an External Sender
> This message came from outside your organization.
>
> There is already an sdbusplus loop running, it's in Mainloop::run().
> [Thu] I think phosphor-hwmon is using the timer for Mainloop::run()
> which triggers each 1 seconds.
> Is this right?
> To support signal the changing in dbus property, we need a service
> which listens the dbus signal, right?
> I'm still fresh in C++ and dbus code. So please correct me if I'm wrong.
>
The current loop handles both the timer events and the D-Bus callbacks.
So all you'd need to do is make a new function on Mainloop to act as the
callback, and then
make a new sdbusplus::bus::match::match object owned by Mainloop and pass it
that callback along with the other necessary parameters.
> Thanks.
> Thu Nguyen.
>
> On Fri, Nov 6, 2020 at 3:52 AM Matt Spinler <mspinler at linux.ibm.com
> <mailto:mspinler at linux.ibm.com>> wrote:
>
>
>
> On 11/4/2020 4:18 PM, Thu Ba Nguyen wrote:
> > Please see my comments below. Thanks. Thu Nguyen. On Wed, Nov 4,
> 2020
> > at 11:31 PM...
> > This Message Is From an External Sender
> > This message came from outside your organization.
> >
> > Please see my comments below.
> >
> > Thanks.
> > Thu Nguyen.
> >
> > On Wed, Nov 4, 2020 at 11:31 PM Matt Spinler
> <mspinler at linux.ibm.com <mailto:mspinler at linux.ibm.com>
> > <mailto:mspinler at linux.ibm.com <mailto:mspinler at 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.
>
> There is already an sdbusplus loop running, it's in Mainloop::run().
>
> >
> > > 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 at gmail.com <mailto:tbnguyen1985 at gmail.com>
> <mailto:tbnguyen1985 at gmail.com <mailto:tbnguyen1985 at gmail.com>>
> > > <mailto:tbnguyen1985 at gmail.com
> <mailto:tbnguyen1985 at gmail.com> <mailto:tbnguyen1985 at gmail.com
> <mailto:tbnguyen1985 at 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 at linux.ibm.com
> <mailto:mspinler at linux.ibm.com> <mailto:mspinler at linux.ibm.com
> <mailto:mspinler at linux.ibm.com>>
> > <mailto:mspinler at linux.ibm.com
> <mailto:mspinler at linux.ibm.com> <mailto:mspinler at linux.ibm.com
> <mailto:mspinler at 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 at gmail.com
> <mailto:tbnguyen1985 at gmail.com> <mailto:tbnguyen1985 at gmail.com
> <mailto:tbnguyen1985 at gmail.com>>
> > <mailto:tbnguyen1985 at gmail.com
> <mailto:tbnguyen1985 at gmail.com> <mailto:tbnguyen1985 at gmail.com
> <mailto:tbnguyen1985 at gmail.com>>>
> > > > <mailto:tbnguyen1985 at gmail.com
> <mailto:tbnguyen1985 at gmail.com>
> > <mailto:tbnguyen1985 at gmail.com <mailto:tbnguyen1985 at gmail.com>>
> > > <mailto:tbnguyen1985 at gmail.com
> <mailto:tbnguyen1985 at gmail.com>
> > <mailto:tbnguyen1985 at gmail.com
> <mailto:tbnguyen1985 at 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 at fb.com <mailto:vijaykhemka at fb.com>
> <mailto:vijaykhemka at fb.com <mailto:vijaykhemka at fb.com>>
> > <mailto:vijaykhemka at fb.com <mailto:vijaykhemka at fb.com>
> <mailto:vijaykhemka at fb.com <mailto:vijaykhemka at fb.com>>>
> > > > <mailto:vijaykhemka at fb.com
> <mailto:vijaykhemka at fb.com>
> > <mailto:vijaykhemka at fb.com <mailto:vijaykhemka at fb.com>>
> <mailto:vijaykhemka at fb.com <mailto:vijaykhemka at fb.com>
> > <mailto:vijaykhemka at fb.com <mailto:vijaykhemka at fb.com>>>>>
> > > wrote:
> > > >
> > > > *From: *openbmc
> > > >
> > <openbmc-bounces+vijaykhemka=fb.com at lists.ozlabs.org
> <mailto:fb.com at lists.ozlabs.org>
> > <mailto:fb.com at lists.ozlabs.org
> <mailto:fb.com at lists.ozlabs.org>>
> > > <mailto:fb.com at lists.ozlabs.org
> <mailto:fb.com at lists.ozlabs.org>
> > <mailto:fb.com at lists.ozlabs.org
> <mailto:fb.com at lists.ozlabs.org>>>
> > > > <mailto:fb.com at lists.ozlabs.org
> <mailto:fb.com at lists.ozlabs.org>
> > <mailto:fb.com at lists.ozlabs.org
> <mailto:fb.com at lists.ozlabs.org>>
> > > <mailto:fb.com at lists.ozlabs.org
> <mailto:fb.com at lists.ozlabs.org>
> > <mailto:fb.com at lists.ozlabs.org
> <mailto:fb.com at lists.ozlabs.org>>>>> on behalf of Thu Ba Nguyen
> > > > <tbnguyen1985 at gmail.com
> <mailto:tbnguyen1985 at gmail.com>
> > <mailto:tbnguyen1985 at gmail.com <mailto:tbnguyen1985 at gmail.com>>
> > > <mailto:tbnguyen1985 at gmail.com
> <mailto:tbnguyen1985 at gmail.com>
> > <mailto:tbnguyen1985 at gmail.com
> <mailto:tbnguyen1985 at gmail.com>>> <mailto:tbnguyen1985 at gmail.com
> <mailto:tbnguyen1985 at gmail.com>
> > <mailto:tbnguyen1985 at gmail.com <mailto:tbnguyen1985 at gmail.com>>
> > > <mailto:tbnguyen1985 at gmail.com
> <mailto:tbnguyen1985 at gmail.com>
> > <mailto:tbnguyen1985 at gmail.com
> <mailto:tbnguyen1985 at gmail.com>>>>>
> > > > *Date: *Monday, October 19, 2020 at 11:23 AM
> > > > *To: *Ed Tanous <ed at tanous.net
> <mailto:ed at tanous.net>
> > <mailto:ed at tanous.net <mailto:ed at tanous.net>>
> > > <mailto:ed at tanous.net <mailto:ed at tanous.net>
> <mailto:ed at tanous.net <mailto:ed at tanous.net>>>
> > <mailto:ed at tanous.net <mailto:ed at tanous.net>
> <mailto:ed at tanous.net <mailto:ed at tanous.net>>
> > > <mailto:ed at tanous.net <mailto:ed at tanous.net>
> <mailto:ed at tanous.net <mailto:ed at tanous.net>>>>>
> > > > *Cc: *OpenBMC Maillist
> > <openbmc at lists.ozlabs.org <mailto:openbmc at lists.ozlabs.org>
> <mailto:openbmc at lists.ozlabs.org <mailto:openbmc at lists.ozlabs.org>>
> > > <mailto:openbmc at lists.ozlabs.org
> <mailto:openbmc at lists.ozlabs.org>
> > <mailto:openbmc at lists.ozlabs.org
> <mailto:openbmc at lists.ozlabs.org>>>
> > > > <mailto:openbmc at lists.ozlabs.org
> <mailto:openbmc at lists.ozlabs.org>
> > <mailto:openbmc at lists.ozlabs.org
> <mailto:openbmc at lists.ozlabs.org>>
> > > <mailto:openbmc at lists.ozlabs.org
> <mailto:openbmc at lists.ozlabs.org>
> > <mailto:openbmc at lists.ozlabs.org
> <mailto:openbmc at 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.
> > > >
> > >
> >
>
More information about the openbmc
mailing list