Set powercap shouldn't except the values out of range #43

mkumatag · 2016-02-03T09:18:55Z

We have seen that set powercap call is excepting any values outside range like less than min or more than supported max powercap value. This error handling should be done in the REST API level to return with error instead of accepting as 200.

Note - though values are not written into the /org/openbmc/sensors/host/PowerCap which is expected but REST should also through Error message.

…ap object Currently master OCC hwmon sysfs path is hardcoded to '/sys/class/hwmon/hwmon3/'. This make the '/org/openbmc/sensors/host/PowerCap' interface cannot work when master OCC is initilized as hwmon device other than 'hwmon3'. 'usr_powercap' attribute should be searched when OCC is active. When skeleton creates 'PowerCap' objects, OCC may not be active. So do the search each time setting power cap. Raise an exception when there is no 'user_powercap' attribute. This would fix the issue: openbmc#42 Also raise an exception when setting power cap fails. The exception will be returned to REST API caller. This would fix the issue: openbmc#43 Signed-off-by: Yi Li <adamliyi@msn.com>

adamliyi · 2016-02-26T02:46:49Z

Please see PR: #50
An exception will be returned when setting the power cap value out of range:

curl -c cjar -b cjar -k -H "Content-Type: application/json" -X POST -d "{"data": [ 1900 ]}" https://9.3.23.24/org/openbmc/sensors/hot/PowerCap/action/setValue
{
"data": {
"description": "Internal Server Error",
"exception": "DBusException('Set PowerCap error: check value range',)",
"traceback": [
"Traceback (most recent call last):",
" File "/usr/lib/python2.7/site-packages/bottle.py", line 862, in _handle",
" return route.call(*_args)",
" File "/usr/lib/python2.7/site-packages/bottle.py", line 1734, in wrapper",
" rv = callback(_a, **ka)",
" File "/usr/sbin/obmc-rest", line 569, in call",
" return self.callback(_a, *_kw)",
" File "/usr/sbin/obmc-rest", line 657, in wrap",
" resp = { 'data': callback(_a, *_kw) }",
" File "/usr/sbin/obmc-rest", line 622, in wrap",
" return callback(_a, *_kw)",
" File "/usr/sbin/obmc-rest", line 646, in wrap",
" return callback(_a, kw)",
" File "/usr/sbin/obmc-rest", line 79, in call",
" return getattr(self, 'do' + request.method.lower())(_kw)",
" File "/usr/sbin/obmc-rest", line 240, in do_post",
" return request.route_data'method'",
" File "/usr/lib/python2.7/site-packages/dbus/proxies.py", line 145, in call",
" *_keywords)",
" File "/usr/lib/python2.7/site-packages/dbus/connection.py", line 651, in call_blocking",
" message, timeout)",
"DBusException: org.freedesktop.DBus.Python.dbus.exceptions.DBusException: Set PowerCap error: check value range"
]
},
"message": "500 Internal Server Error",
"status": "error"
}

mkumatag · 2016-02-26T05:55:50Z

500 internal error is not really accepted, we should through 4xx series error. You can refer https://en.wikipedia.org/wiki/List_of_HTTP_status_codes#4xx_Client_Error to understand more about errors.

adamliyi · 2016-02-26T10:07:17Z

Thanks Manjunath for reminding. I need to check with Brad.

…ap object Currently master OCC hwmon sysfs path is hardcoded to '/sys/class/hwmon/hwmon3/'. This make the '/org/openbmc/sensors/host/PowerCap' interface cannot work when master OCC is initilized as hwmon device other than 'hwmon3'. 'usr_powercap' attribute should be searched when OCC is active. When skeleton creates 'PowerCap' objects, OCC may not be active. So do the search each time setting power cap. Raise an exception when there is no 'user_powercap' attribute. This would fix the issue: openbmc#42 Also raise an exception when setting power cap fails. The exception will be returned to REST API caller. Need to specify the exception name to trigger a '400' return code. This would fix the issue: openbmc#43 Signed-off-by: Yi Li <adamliyi@msn.com>

adamliyi · 2016-02-26T12:03:32Z

Updated the PR.
Here is the new return code when setting invalid power cap value:

curl -c cjar -b cjar -k -H "Content-Type: application/json" -X POST -d "{"data": [ 2000 ]}" https://9.3.23.24/org/openbmc/sensors/host/PowerCap/action/setValue
{
"data": {
"description": "org.freedesktop.DBus.Error.InvalidArgs: Set PowerCap error: check value range"
},
"message": "400 Bad Request",
"status": "error"
}

…ap object Currently master OCC hwmon sysfs path is hardcoded to '/sys/class/hwmon/hwmon3/'. This make the '/org/openbmc/sensors/host/PowerCap' interface cannot work when master OCC is initilized as hwmon device other than 'hwmon3'. 'usr_powercap' attribute should be searched when OCC is active. When skeleton creates 'PowerCap' objects, OCC may not be active. So do the search each time setting power cap. Raise an exception when there is no 'user_powercap' attribute. This would fix the issue: openbmc#42 Also raise an exception when setting power cap fails. The exception will be returned to REST API caller. Need to specify the exception name to trigger a '400' return code. This would fix the issue: openbmc#43 Signed-off-by: Yi Li <adamliyi@msn.com>

…ap object Currently master OCC hwmon sysfs path is hardcoded to '/sys/class/hwmon/hwmon3/'. This make the '/org/openbmc/sensors/host/PowerCap' interface cannot work when master OCC is initilized as hwmon device other than 'hwmon3'. 'usr_powercap' attribute should be searched when OCC is active. When skeleton creates 'PowerCap' objects, OCC may not be active. So do the search each time setting power cap. Raise an exception when there is no 'user_powercap' attribute. Fixes: openbmc#42 Also raise an exception when setting power cap fails. The exception will be returned to REST API caller. Need to specify the exception name to trigger a '400' return code. Fixes: openbmc#43 Add code to validate input user powercap value. Changed the logic of "reading powercap" using PowerCap.getValue(). Previously PowerCap.getValue() returns current user set powercap value. Now PowerCap.getValue() returns actual system powercap reading from OCC. Signed-off-by: Yi Li <adamliyi@msn.com>

adamliyi · 2016-03-09T10:17:21Z

@mkumatag , I've setup the barreley 9.3.23.24. Could you please verify issue #42, and issue #43.

I have replaced the two files on that server with my fix.
/usr/sbin/Sensors.py and /usr/sbin/sensor_manager2.py

Thanks,

mkumatag · 2016-03-10T15:10:10Z

I just verified the tests on that machine for issue #43 and working fine.

Test Occ Powercap :: This suite is for testing OCC: Power capping ... | PASS |
6 critical tests, 6 passed, 0 failed

6 tests total, 6 passed, 0 failed

Output: /home/manjunath/git/openbmc-automation/output.xml
Log: /home/manjunath/git/openbmc-automation/log.html
Report: /home/manjunath/git/openbmc-automation/report.html
______________________________________________________________________________________________ summary ______________________________________________________________________________________________
custom: commands succeeded
congratulations :)
[manjunath@oc1275007250 openbmc-automation]$

mkumatag · 2016-03-17T06:07:18Z

Why this issue has been closed? is code already merged?

…ap object Currently master OCC hwmon sysfs path is hardcoded to '/sys/class/hwmon/hwmon3/'. This make the '/org/openbmc/sensors/host/PowerCap' interface cannot work when master OCC is initilized as hwmon device other than 'hwmon3'. 'usr_powercap' attribute should be searched when OCC is active. When skeleton creates 'PowerCap' objects, OCC may not be active. So do the search each time setting power cap. Raise an exception when there is no 'user_powercap' attribute. Fixes: openbmc#42 Also raise an exception when setting power cap fails. The exception will be returned to REST API caller. Need to specify the exception name to trigger a '400' return code. Fixes: openbmc#43 Add code to validate input user powercap value. Changed the logic of "reading powercap" using PowerCap.getValue(). Previously PowerCap.getValue() returns current user set powercap value. Now PowerCap.getValue() returns actual system powercap reading from OCC. Signed-off-by: Yi Li <adamliyi@msn.com>

adamliyi mentioned this issue Feb 26, 2016

skeleton: fixed hard-coding of master OCC hwmon sysfs path for PowerCap object #50

Closed

nkskjames closed this as completed Mar 17, 2016

mkumatag mentioned this issue May 31, 2016

PreTag Set Less Than Minimum PowerCAP - Wrong Implemtation mkumatag/openbmc-automation#61

Closed

gkeishin mentioned this issue May 31, 2016

PreTag openbmc/skeleton#43 - Set Less Than Minimum PowerCAP #97

Closed

williamspatrick mentioned this issue Sep 8, 2016

PreTag openbmc/skeleton#43 - Set Less Than Minimum PowerCAP openbmc/openbmc#557

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set powercap shouldn't except the values out of range #43

Set powercap shouldn't except the values out of range #43

mkumatag commented Feb 3, 2016

adamliyi commented Feb 26, 2016

mkumatag commented Feb 26, 2016

adamliyi commented Feb 26, 2016

adamliyi commented Feb 26, 2016

adamliyi commented Mar 9, 2016

mkumatag commented Mar 10, 2016

mkumatag commented Mar 17, 2016

Set powercap shouldn't except the values out of range #43

Set powercap shouldn't except the values out of range #43

Comments

mkumatag commented Feb 3, 2016

adamliyi commented Feb 26, 2016

mkumatag commented Feb 26, 2016

adamliyi commented Feb 26, 2016

adamliyi commented Feb 26, 2016

adamliyi commented Mar 9, 2016

mkumatag commented Mar 10, 2016

6 tests total, 6 passed, 0 failed

mkumatag commented Mar 17, 2016