Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
williamspatrick opened this issue Sep 8, 2016 · 6 comments
Closed
Labels

Comments

@williamspatrick
Copy link
Member

@gkeishin commented on Tue May 31 2016

Please refer to mkumatag/openbmc-automation#61
for detail description...

Our concern was if this is some sort of REST server code bug returning Error even when that PowerCap property existing on the system..

If by design ( openbmc/skeleton#43 ) if itis not going to return HTTP OK ... then, why is this return HTTP OK here on Palmetto and throwing error on Barrelleye when PowerCap Property exist on both the systems..

Example:

On Palmetto :
openbmc-automation$ curl -c cjar -b cjar -k -H "Content-Type: application/json" -X POST -d "{"data": [ 1900 ]}" http://9.3.164.177/org/openbmc/sensors/host/PowerCap/action/setValue
{
"data": null,
"message": "200 OK",
"status": "ok"
}

On Barrelleye:
openbmc-automation$ curl -c cjar -b cjar -k -H "Content-Type: application/json" -X POST-d "{"data": [ 1900 ]}" https://9.3.23.24/org/openbmc/sensors/host/PowerCap/action/setValue
{
"data": {
"description": "org.openbmc.objectmapper.Error.NotFound: path or object not found: /org/openbmc/sensors/host/PowerCap"
},
"message": "404 Not Found",
"status": "error"
}


@gkeishin commented on Thu Sep 08 2016

We are seeing the powercap setting errors now frequently.
Tests . Test Generic Conf
7 Test 4 pass 3 3 failed

out of 7 , 3 are failing

/org/openbmc/settings/host0, power_cap

  1. Set the power with string of characters (abcdefg)
  2. Set the power with greater then MAX_POWER_VALUE (1010)

/org/openbmc/settings/host0, boot_flags

  1. Set the boot flags with string ( 3ab56f )

We would like to know what are the valid policies and what are not from those attributes..

@williamspatrick
Copy link
Member Author

I don't really understand what the issue is asking. Is this about PowerCap sensors not being present? Or is this about settings not being set properly? The latest comment makes it sound like you are expecting some sort of verification of invalid settings, which is not currently implemented. #552 will give us the ability to do that.

@causten
Copy link
Member

causten commented Sep 8, 2016

the test case was written to check if invalid settings could be set. The max for pcap is 1000 so teh test was to set it above that level. It allowed a value of 1010. Hence the failure. Same with setting the max pcap to abcdefg. So there is no validation code in the pcap code. If you have input validation handled via issue 552 then issue 557 should wait until 552 is ready

@gkeishin
Copy link
Member

gkeishin commented Sep 8, 2016

Thanks will keep an eye on #552

@anoo1
Copy link
Contributor

anoo1 commented Sep 12, 2016

The power cap value was eventually implemented in the sensors path after the placeholder in the settings file was created (with its arbitrary default value of 100).
We should determine if the power cap should be in 2 places and synced (sensors and settings), or just one place and remove the redundant one.

@gkeishin
Copy link
Member

gkeishin commented Sep 15, 2016

another for powercap

Not able to set here /org/openbmc/sensors/host/powercap/action/setValue
The interfaces under sensors/powercap are
{
"data": [
"/org/openbmc/sensors/powercap/user_cap",
"/org/openbmc/sensors/powercap/system_power",
"/org/openbmc/sensors/powercap/curr_cap",
"/org/openbmc/sensors/powercap/n_cap",
"/org/openbmc/sensors/powercap/max_cap",
"/org/openbmc/sensors/powercap/min_cap"
],
as per the ccode https://github.com/openbmc/pyphosphor/blob/a88b3239b0a9592defaeee59c0ff17253a86aa39/obmc/sensors.py#L196
We should be as well set using the Sensor powercap setValue..

The other which we see in this issue ,we are able to Set here /org/openbmc/settings/host0,power_cap

So there seems not to be clear until this is streamlined..

@adamliyi any thoughts ?

@gkeishin
Copy link
Member

Closing this as the implementation are working

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants