On Fri, 2009-09-04 at 10:05 +0300, Jeff Squyres wrote: > On Sep 3, 2009, at 12:23 PM, Nadia Derbey wrote: > > > What: Define a way for the system administrator to prevent users from > > overwriting the default system-wide MCA parameters settings. > > > > In general, I think this is great stuff. I have a few nit picks.
Thanks for having a look at it! > > (BTW: you might want to run contrib/hg/build-hgignore.pl in your svn > +hg tree to generate a proper .hgignore file...?) > > I'm currently unable to build the hg tree -- it gets a version with a > newline in it which causes badness in the Makefile. For example: > > ------ > *** Checking versions > checking for SVN version... done > checking Open MPI version... 1.4a1hgf11244ed72b5 > up to changeset c4b117c5439b > checking Open MPI release date... Unreleased developer copy > checking Open MPI Subversion repository version... hgf11244ed72b5 > up to changeset c4b117c5439b > checking for SVN version... done > ...etc. > ------ ??? May be something went wrong with my mqueues for the last changeset (I recognize the qnew message in the version...). Until I fix that, the solution would be to revert back to changeset db3595643dd2 (the very last changeset is not important at all). > > Do you see this, or do you get a single-line version number? > > > When openmpi-priv-mca-params.conf is parsed, any parameter listed in > > that file is moved from the mca_base_param_file_values list to a new > > parallel list (mca_base_priv_param_file_values). The parameter remains > > "hidden" in that new list. > > > > I haven't looked at the code deeply, so forgive me if I'm parsing this > wrong: is the code actually reading the file into one list and then > moving the values to another list? If so, that seems a little > hackish. Can't it just read directly to the target list? We could read directly to target list if we had 2 files giving settings (as described in ticket 75): one for the "classical" system-wide parameters one for for the "privileged", system-wide parameters. But the solution I'm proposing is different: we have one file for the settings, and one file that lists the parameters names that should not be changed once they have been set: 1) openmpi-mca-params.conf is the one we all know. It contains the system-wide mca parameters settings. ex: notifier_threshold_severity = notice 2) openmpi-priv-mca-params.conf. This file contains the list of mca parameters that cannot be changed once they have been set in openmpi-mca-params.conf (only the parameter name is in that file, not its setting). ex: notifier_threshold_severity openmpi-mca-params.conf is parsed first. The list mca_base_param_file_values is populated with all the parameters set in that file. Then openmpi-priv-mca-params.conf is parsed. This file, as described earlier, only contains the list of those parameters that if set in openmpi-mca-params.conf, shouldn't be changed. So, any parameter listed in openmpi-priv-mca-params.conf is moved from mca_base_param_file_values to a newly defined list (mca_base_priv_param_file_values). Then the remaining files are parsed as usual. As explained below, the lookup order is changed to ensure that mca_base_priv_param_file_values is the very first list to be scanned: if a parameter is found there, we are done. > > > The lookup order has been changed: the new > > mca_base_priv_param_file_values list is the very 1st one to be > > scanned: > > this is how we ensure that the "privileged" parameters are never > > overwritten once defined on a system-wide basis. > > > > Other external changes: > > 1. The man page for mpirun(1) has been changed. > > 2. The ompi_info(1) output has been changed to reflect the status of > > these "privileged" parameters. The status field can now be one of > > "read-only", "writable" or "privileged". > > > > This seems a little funky, too. Isn't a privileged param actually > read only (in an abstract sense)? Meaning: you can't change priv > param values, so they're "read only". "Priv" feels more like a > "source" attribute, doesn't it...? I.e., it's a read-only param, and > the source of the attribute is the special priv file. It's true that I first thought of that read-only status, but I haven't kept the idea: from an open mpi pov, read-only is an "immutable" status: it is hard coded, and a variable that is read-only cannot easily become rightable. So an mpi user who does an ompi_info and sees that a parameter is read-only knows that he will never have a chance to change it. While these "privileged" params have a different status: they are read-only while listed in the openmpi-priv-mca-params.conf, but they can become writable if removed from that file. So, instead of read-only, why not "system-wide-only"? > > > 3. A new option has been added to ompi_info(1): --privileged > > it outputs only the list of parameters that have been defined as > > system-wide-only. > > > > This also feels a bit weird -- I took a quick look through the --priv > handling in ompi_info and it seems there's a bunch of corner cases. > Can you describe what happens and what corner cases occur with --priv? Don't if I'm answering your question: Let's take the example above (notifier_threshold_severity decalred as privileged, with a value = notice): $ ompi_info --param btl sm --privileged should not output anything, since we don't have any priv parameter in btl/sm $ ompi_info --param notifier twitter --privileged --parsable mca:notifier:base:param:notifier_threshold_severity:value:notice mca:notifier:base:param:notifier_threshold_severity:data_source:file: /home_nfs/derbeyn/DISTS/openmpi-default/etc/openmpi-mca-params.conf mca:notifier:base:param:notifier_threshold_severity:status:privileged mca:notifier:base:param:notifier_threshold_severity:help:Report all events at or above this severity [default: critical] mca:notifier:base:param:notifier_threshold_severity:deprecated:no Here notifier_threshold_severity is output since it belongs to the notifier framework, but is not related to any component. $ ompi_info --param notifier all --privileged --parsable should have the same output as the previous command for the same reason $ ompi_info -a --privileged --parsable Still same as above since we are asking for all the parameters Well, after writing all this, I had a look at the code and discovered what might look as an unhandled corner cases: --privileged not combined with one of --all or --param: in that case, the "--privileged" option has the same effect as ompi_info without any option. ex: "ompi_info --path bindir --privileged" acts as "ompi_info" May be a syntax error should be output in such a case? I'll look at this more carefully. Regards, Nadia > > > TODO list: > > . Make this feature configurable. > > > > Per my prior post, I think that if we make this configurable, it has > to be a configure script option (not a run-time mca parameter). > > > . Output a warning message as described in the ticket. > > > > That would be nice. > > > Possible extension to this functionality: > > > > This new functionality can be extended in the future in the following > > way: allow the administrator to specify boundaries within which an MCA > > parameter is allowed to be changed by a higher priority setting. This > > means that the administrator should declare min and max values (or > > even > > a set of discrete values) for any such parameter. Then, any higher > > priority setting will be done only if the new value belongs to the > > declared set. > > In other word, each MCA parameter can be > > 1. set to a given value in openmpi-mca-params.conf > > 2. declared in openmpi-priv-mca-params.conf with its allowed > > boundaries > > Then, if the user calls mpirun with that MCA parameter set in his > > private mca-params.conf file, or in an AMCA file, or in an environment > > variable, or on the command line, the new value will override the > > system-wide value only if it is within the declared boundaries. > > > > This could be thought of as a future development. > > > > > Agreed. > > -- > Jeff Squyres > jsquy...@cisco.com > > _______________________________________________ > devel mailing list > de...@open-mpi.org > http://www.open-mpi.org/mailman/listinfo.cgi/devel > -- Nadia Derbey <nadia.der...@bull.net>