-
Notifications
You must be signed in to change notification settings - Fork 19
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
redfishpower: support plug parents #164
Conversation
896d531
to
fddf7f2
Compare
rebased & re-pushed, now that #159 is now merged |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice. OK, to be honest redfishpower is getting hard for me to follow and I didn't look at this in too much detail, but it sounds like this does what we need and there are tests.
It might be good in a future PR to add support for running redfsihpower under valgrind with all the new features you have added active.
src/redfishpower/plugs.c
Outdated
int plugs_is_descendant(plugs_t *p, | ||
const char *plugname, | ||
const char *ancestor) | ||
{ | ||
struct plug_data *pd = zhashx_lookup(p->plug_map, plugname); | ||
if (!pd) | ||
return 0; | ||
|
||
while (pd->parent) { | ||
if (strcmp(pd->parent, ancestor) == 0) | ||
return 1; | ||
if (!(pd = zhashx_lookup(p->plug_map, pd->parent))) | ||
return 0; | ||
} | ||
|
||
return 0; | ||
} | ||
|
||
char *plugs_child_of_ancestor(plugs_t *p, | ||
const char *plugname, | ||
const char *ancestor) | ||
{ | ||
struct plug_data *pd = zhashx_lookup(p->plug_map, plugname); | ||
if (!pd) | ||
return NULL; | ||
|
||
while (pd->parent) { | ||
if (strcmp(pd->parent, ancestor) == 0) | ||
return pd->plugname; | ||
if (!(pd = zhashx_lookup(p->plug_map, pd->parent))) | ||
return NULL; | ||
} | ||
|
||
return NULL; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These two functions are a lot alike. Couldn't plug_is_descendant()
be eliminated and callers just call plugs_child_of_ancestor()
and check for NULL?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ohh good idea, I didn't catch that mini-optimization.
Problem: Some chassis environments have a power control hierarchy, where some components must be powered on before the "child" components can be powered on. There is no infrastructure for this in the plug module. Add support of a "parent" within each plug and a collection of helper functions to navigate the resulting hierarchy that can be configured within plugs. Update callers to plugs_add() accordingly.
Problem: There is no coverage for plug parents and the new plug hierarchy functions. Add coverage in redfishpower/test/plug.c.
Problem: In some hardware, there is a hierarchy for power control. For example, In bladed systems the chassis must be powered on before blades can be powered on. Without knowledge of this dependency, power control can become difficult to use. Solution: Support configuration of hierarchies through an optional "parent" configuration in the "setplugs" option. With knowledge of hierarchy, support following behavior semantics in redfishpower. When doing a power operation, check parents/ancestors power status first. If ancestors are all on, then redfishpower can perform power on/off/status on the target. If ancestor is off, power status is defined as off for all descendants. Power on cannot be done. Power off is assumed to succeed. As special case, if powering on both ancestors and children, power on will not be allowed. Testing has shown that delays are necessary between levels due to hardware "initialization" going on in the background. This delay is highly variable and behavior was not consistent. Powering off both ancestors and children is allowed, descendants will wait until ancestor power offs are complete. Then by definition, descendants are then assumed to be off. If any ancestors have status unknown or get an error, that status carries to descendants and results in errors.
Problem: redfishpower is out of date with recent support of configuring parents. Document configuration of parents via the setplugs config. Document parent information.
Problem: There is no coverage for the new redfishpower parent/hierarchy support. Add coverage in t0034-redfishpower.t and add test device files t/etc/redfishpower-parents-2-levels.dev and t/etc/redfishpower-parents-3-levels.dev.
fddf7f2
to
fc8c66e
Compare
Good idea. I guess I just don't have a mechanism to deal with input/output with redfishpower at its prompt which is why all the tests are run under powerman ... i guess would have to be done with expect scripts or something (personally haven't used before, although I'm sure isn't that bad). Edit: Thinking about this for 10 more seconds, some type of "take prompt commands from stdin" might actually be easier. Then I can compare stdout in tests and easily run redfishpower under valgrind with a set of commands I care about. |
Problem: In some hardware, there is a hierarchy for power control. For example, In bladed systems the chassis must be powered on before blades can be powered on. Without knowledge of this dependency, power control can become difficult to use.
Solution: Support configuration of hierarchies through an optional "parent" configuration in the "setplugs" option.
With knowledge of hierarchy, support following behavior semantics in redfishpower.
When doing a power operation, check parents/ancestors power status first.
If ancestors are all on, then redfishpower can perform power on/off/status on the target.
If ancestor is off, power status is defined as off for all descendants. Power on cannot be done. Power off is assumed to succeed.
As special case, if powering on both ancestors and children, power on will not be allowed. Testing has shown that delays are necessary between levels due to hardware "initialization" going on in the background. This delay is highly variable and behavior was not consistent.
Powering off both ancestors and children is allowed, descendants will wait until ancestor power offs are complete. Then by definition, descendants are then assumed to be off.
If any ancestors have status unknown or get an error, that status carries to descendants and results in errors.
built on top of #159