IPMI Sensor Read-only role updates.
emilyshaffer at google.com
Wed Feb 28 05:54:22 AEDT 2018
Okay, I guess I didn't understand that the proposal was for the MRW config
yaml, which is different from the config yaml which is sent to the mako
template :) Carry on.
On Tue, Feb 27, 2018 at 10:48 AM Ratan Gupta <ratagupt at linux.vnet.ibm.com>
> Hi Emily,
> Minor correction inline.
> On Tuesday 27 February 2018 09:06 PM, Emily Shaffer wrote:
> Hi Jayanath,
> Find my comments inline.
> It seems you may be thinking of only changes for a system specific yaml,
> in which case I don't think you need to ask for permission :) But the
> architecture you're discussing is already there today.
> On Tue, Feb 27, 2018, 6:22 AM Ratan Gupta <ratagupt at linux.vnet.ibm.com>
>> Please find my comments inline
>> Ratan Gupta
>> On Tuesday 27 February 2018 04:51 PM, Jayanth Othayoth wrote:
>> > I am working on an issue openbmc/openbmc#2500 (Add support for Turbo
>> > allowed sensor). As part of this issue implementation , there is a
>> > requirement to change this IPMI sensor to read only for host.
>> > Currently all sensors are read/write by design and doesn’t have way to
>> > change this behavior. Planing to change this behavior by adding
>> > “Mutability” field in the config.yaml.
> ES: The Mutability field already exists in sensor config yaml.
> RG>> Here in mako we always set the mutability property to READ as
> config YAML doesn't have the Mutability property.
> RG>>We have the Mutability check in the getSensorReading but we don't
> have that check in the setSensorReading so we can introduce that check
> there with the required changes in the config.YAML.
>> > As part of this issue am planing to Initialize mutability with
>> > Mutability::Write|Mutability::Read for all sensors and set the
>> > required sensor mutability Mutability::Read.
> ES: You're saying you'd like to ensure all sensors are readable? This
> doesn't make sense in some cases, notably fan PWM in some cases where the
> only read response would be DBus parroting back what we wrote (not an
> actual value reflecting reality).
>> > - Minimal impact on testing and not breaking the existing
>> > functionality.
>> RG>>How we would be doing it? If we are planning to do introduce a check
>> for a specific sensor then we should not do like this.
>> In the current design we have not introduced any such checks and we
>> should avoid that.
>> Please correct me if I misunderstood it.
> RG>>if we add the Mutability property in the config.yaml with configured
> values as READ and WRITE for all other sensor
> Except this one(turboAllowedSensor) which is required to be
> In this sense we are not breaking the current code as the
> interpretation in the code for all the sensor is READ and WRITE
>> > Follow-on story is needed to fix this in generic way like
>> > -Add “mutable” field in the config.yaml and initialize the the field
> ES: Today it's defaulted to read-only for all uninitialized sensors.
>> > based on System types also update
>> > sensorhandler.cpp->ipmi_sen_set_sensor() to do set only for write
>> > enabled sensor.
>> > - This required system specific IPMI sensor behavior review and
>> > updates in corresponding config.yaml.
>> RG>> I agree with this approach.What do you mean by "System types"
>> here,We have already system specfic YAML in the system specfic meta
>> > Please add your view on this.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the openbmc