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

[cindi]

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.

> 
> 
>>> 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.  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. :)

Cindi