[fm-discuss] Proposal: libtopo eumeration of power supplies and fans via IPMI

[Rob Johnston]

cindi wrote:
> 
> 
> Rob Johnston wrote:
>> Cynthia McGuire wrote:
> 
> 
> 
>>>>
>>>> 6. LIBTOPO ENHANCEMENTS
>>>>
>>>> To make the implementation of this project possible, a handful of
>>>> extensions to both the libtopo enumerator module API and XML schema are
>>>> necessary.
>>>>
>>>> Currently it is not possible to register module methods on nodes that
>>>> are statically enumerated via XML map files.  Typically, node methods
>>>> are registered onto a node by the enumerator module after the node is
>>>> bound to the topology.  However, since statically enumerated modules
>>>> aren't created by the enumerator module this registration doesn't 
>>>> occur.
>>>>
>>>> While there will be cases where we will be forced to statically define
>>>> psu and fan topologies via XML, these nodes still need to support the
>>>> node methods that are implemented by the ipmi enumerator module.  In
>>>> order to allow these methods to be registered on statically defined
>>>> nodes, the topo_modops_t struct will be extended with a new operation
>>>> (tmo_meth_reg) as shown below:
>>>>
>>>> typedef int topo_meth_reg_f(topo_mod_t *, tnode_t *);
>>>>
>>>> typedef struct topo_modops {
>>>>     topo_enum_f *tmo_enum;        /* enumeration op */
>>>>     topo_release_f *tmo_release;    /* resource release op */
>>>>     topo_meth_reg_f *tmo_meth_reg;    /* method registration op */
>>>> } topo_modops_t;
>>>>
>>>> The tmo_meth_reg operation will be optional.  Enumerator modules
>>>> which implement this operation will register the appropriate set of
>>>> methods on the topo node that is passed in.
>>>>
>>>> To provide a connection between this new operation and nodes that are
>>>> statically defined in XML, the syntax of the <node> element will be
>>>> extended to include a new optional "mod" attribute.  The value of this
>>>> attribute should be set to the name of an enumerator module, whose 
>>>> methods
>>>> should be registered on that node.  Below is an example usage of this
>>>> new attribute:
>>>>
>>>>    <range name='fan' min='0' max='2'>
>>>>          <node instance='0' mod='ipmi'>
>>>>              . . .
>>>>          </node>
>>>>    </range>
>>>>
>>>
>>> I don't see a need for the extra registration op nor the new 
>>> attribute name, mod.  I prefer to see the XML extended to permit 
>>> enumerators to be called after the XML parser has created the static 
>>> node. The enumerators can then register their methods and perform any 
>>> post-processing of the node (i.e. add other properties).  The 
>>> attribute name should be 'enum' as for dynamically created nodes.
>>
>> Ok - so along those lines an alternative approach could do something 
>> like:
>>
>> <range name='fan' min='0' max='2'>
>>    <node instance='0'>
>>        . . .
>>    </node>
>>    ...
>>   <enum-process name='ipmi' version='1'>
>> </range>
>>
>> This could invoke a new entry point in the module which would walk the 
>> tree and post-process any nodes that it's responsible for (like fan 
>> and psu nodes).  Is this closer to what you're thinking of?
> 
> Not exactly.  The XML should look like:
> 
>  <range name='fan' min='0' max='2'>
>     <node instance='0'>
>     <enum-method name='ipmi' version='1'>
>         . . .
>     </node>
>     ...
>  </range>
> 
> The topo XML parser should:
> 
> 1) create the range 'fan' and reserve 0 - 2 nodes (code already in place)
> 2) create a node for instance 0: (code already in place)
> 3) topo_mod_load() the IPMI plugin (need to add this as part of 
> pad_process())
> 4) call topo_mod_enumerate() on the node just created (need to add this 
> as part of pad_process())
> 
> The IPMI module should:
> 1) register any methods for the node
> 2) decorate properties
> 3) it could also decide that the node should not exist and 
> topo_node_unbind() it from the topo tree
> 
> This change should require 0 changes to the module API and a minor 
> change to the DTD to permit an enum-method element for node.

Ok - that'll work.  I'll make this change.


>>
>>>> Additionally, the syntax of the <range> element will also be 
>>>> extended to
>>>> allow a new "set" attribute.  The intention is to allow for conditional
>>>> enumeration of a range of nodes based on the platform type.  This is
>>>> analagous to the conditional specification of properties which is
>>>> currently supported via the <propset> element.  Below is an example
>>>> usage of this new attribute:
>>>>
>>>>    <range name='fanmodule' min='0' max='4' 
>>>> set='Sun-Fire-X4500|Sun-Fire-X4540'>
>>>>        . . .
>>>>    </range>
>>>>
>>>
>>> This looks backwards.  Shouldn't the set contain the range?
>>
>> Effectively it does.  What I was trying to avoid was creating a brand 
>> new element (ala <rangeset>).  By having the set that the range 
>> applies to be just an attribute of the <range> element greatly 
>> simplifies the actual code changes that I have to make to libtopo's 
>> parser.
>>
>> I suppose an alternative would be to do something like this:
>>
>> <rangeset type='product' set="Sun-Fire-X4500|Sun-Fire-X4540'>
>>    <range name='fanmodule' min='0' max='4'>
>>        . . .
>>    </range>
>> </range>
>>
>> However this ends up being quite a bit more difficult to implement in 
>> the parser wothout any functional benefit.  I also think the xml code 
>> looks a bit cleaner the other way.
>>
>>> You will also need to specify the set type (someplace) just as we do 
>>> in propset.
> 
> The reason for the propset element is to permit groupings of properties 
> by a some criteria (i.e. product name).  I can see that being useful for 
> ranges as well.  For example, we may want to do this:
> 
> <rangeset type='product' set='Sun-Fire-X4500|Sun-Fire-X4540'>
>     <range name='fanmodule' min='0' max='4'>
>         ...
>     </range>
>     range name='psu' min='0' max='4'>
>     </range>
> </rangeset>
> 
> We may also want to define sets for nodes:
> 
> <range name='fanmodule' min='0' max='4'>
>     <nodeset type='product' set='Sun-Fire-X4500|Sun-Fire-X4540'>
>         <node instance='0' />
>         <node instance='1' />
>     </nodeset>
> </range>
> 
> I'd like to see a common element defined for all sets rather than 
> propset, rangeset and nodeset.  The set element should be defined in the 
> DTD to include 0 or more propgroup, node or range elements.


Yeah, upon further reflection, it would be good to have a single element that we 
can use for this sort of conditional parsing, whether it be a node, range or 
propgroup.  I'll get to work on that.


I wanted to
> use the word, grouping, rather than propset, but it was already in use. 
>  Perhaps you can come up with something more brilliant. :)

Reaching for the thesaurus :)

thanks,

rob