#1695 bad return from saHpiHotSwapStateGet in ipmidirect plugin

Future
open
nobody
OpenHPI Daemon
5
2015-10-06
2012-02-15
No

When a resource that is a FRU does not have a hotswap sensor, so that SAHPI_CAPABILITY_MANAGED_HOTSWAP is not set, then saHpiHotSwapStateGet should return SAHPI_HS_STATE_ACTIVE per HPI B.03.01+. The ipmidirect plugin returns SA_ERR_HPI_INVALID_PARAMS in this case.

See bug #2948127; it suggests that plugins may have this issue. Indeed, the ipmidirect plugin does.

Discussion

  • Anton Pak

    Anton Pak - 2012-04-20
    • milestone: 2040045 --> 3.1.x
     
  • Anton Pak

    Anton Pak - 2015-06-08

    The bug was always shadowed by the fact, that a hotswap sensor is mandatory for an ATCA FRU.

    If ipmidirect claims HPI-to-ATCA compliance, then it should never happen.

    Though it will happen :)

     
  • Anton Pak

    Anton Pak - 2015-06-08

    Also there is a case when sensor repository reading is in progress and we haven't read the hotswap sensor yet. Usually in such case the FRU state is INACTIVE or INS_PENDING.

     
  • dr_mohan

    dr_mohan - 2015-06-10

    Thanks for David for the following comments

    The bug is (or at least was, as of release 3.0, which is the latest source I have easy access to) in the hotswap.cpp module, in function cIpmi::IfGetHotswapState. There, around line 50, it says:

    SaErrorT
    cIpmi::IfGetHotswapState( cIpmiResource res, SaHpiHsStateT &state )
    {
    // get hotswap sensor
    cIpmiSensorHotswap
    hs = res->GetHotswapSensor();

    if ( !hs )
    return SA_ERR_HPI_INVALID_PARAMS;

    // get hotswap state
    return hs->GetHpiState( state );
    }

    In the case of a FRU with unmanaged Hotswap (SAHPI_CAPABILITY_FRU set, but SAHPI_CAPABILITY_MANAGED_HOTSWAP not set), the spec says that saHpiHotSwapStateGet() should be supported, and by implication, should return SAHPI_HS_STATE_ACTIVE when the resource is present.
    The absence of a "hot swap sensor" for the resource indicates just this condition, so I believe that the code should be more like this:

    SaErrorT
    cIpmi::IfGetHotswapState( cIpmiResource res, SaHpiHsStateT &state )
    {
    // get hotswap sensor
    cIpmiSensorHotswap
    hs = res->GetHotswapSensor();

    if ( !hs )
    {
    state = SAHPI_HS_STATE_ACTIVE;
    return SA_OK;
    }

    // get hotswap state
    return hs->GetHpiState( state );
    }

     
  • dr_mohan

    dr_mohan - 2015-10-06
    • Subsystem: --> OpenHPI Daemon
    • 3.7.0: 3.1.x --> Future
     

Log in to post a comment.

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:

JavaScript is required for this form.





No, thanks