David Bustos wrote: > Quoth Liane Praza on Fri, Aug 15, 2008 at 03:41:37PM -0700: >> Tom, Antonello, and I would like to invite reviews of the code for >> extending the existing SMF templates to incorporate general service >> property metadata. >> >> We previously published and discussed ARC materials on this alias, so >> the interfaces and goals of the project shouldn't be much of a surprise. >> >> I'm re-posting our manpage diffs and interface information here for >> reference, though it is not under review: >> http://cr.opensolaris.org/~lianep/templates-0815/ > > While reading about the interfaces, I came across some text which > I think many users will find unclear. And during the review, I had some > questions. Since this documentation is not under review, however, feel > free to ignore. David,
Thanks for the feedback. I am trying to improve the documentation based on your comments. Please see my answers below. > > smf_template(5) > "Templates may be defined for a service to describe metadata about > the service in general...": I think this would be more useful to > users if it were more concrete. Specifically by describing when > users might want to produce or consume templates. Perhaps saying > that service developers use templates to describe what > configuration values are valid, administrators can use templates > to validate configuration, and tool developers can use templates > to provide more helpful configuration user interfaces. I agree with you, but I'll wait for Liane come back from vacations since she will have much better real examples than me. > > "svcs -lv...": Perhaps you should specify that the output of svcs > -lv and svccfg describe are human-readable, and for programmatic > or stable access the libscf interfaces should be used. I re-phrased to: "svcs -lv and svccfg describe can be used to access metadata about properties in a human-readable format." Documented libraries APIs are stable by definition. > > "A template is created...": This paragraph confuses me because the > first sentence says what creates a template, but the rest > describes, I think, how service developers should write templates > in their manifests. Though maybe it's trying to describe which > templates should be used for which properties. I suppose it turns > on the definition of "templated". With Tom's input, I changed the paragraph to: "Templates can be defined at several levels. Templates that apply to a specific instance or service are defined in the service manifest. Templates that apply to all services that are controlled by a restarter are defined in the restarter's manifest. Finally, SMF framework property groups and properties are templated in the manifest for svc:/system/svc/global. SMF takes a composed view of the template specifications in these levels." > > "Templates defined globally...": I think you should clarify when > a template is considered to re-define another template. Do they > have individual names? Or is it based on their matching criteria? > It seems this information is most relevent to template authors, > except for when template consumers need to understand that > configuration validation can fail due to miscoordination between > different template authors. So I'm not sure that the "Template > Composition" section is the best place for this. I'll wait for Liane's input on this one too. > > "Future work...": Is future work appropriate for a manpage? Removed. > > "Capital letters...": I suspect this should be qualified with "in > English locales". It would also qualify for any Latin based locale. I guess you're considering Asian, Arabic or locales without the concept of Capital Letters. I would leave it like this as if you're capable of reading English you should be able to understand the meaning of the phrase. > > "If name or type is omitted, it acts like a wildcard.": I think you > mean that the template acts as though it has a wildcard for that > field. Though saying that the template can act in such a way > might be considered bad diction. Changed to: "name and/or type may be omitted. If either of these attributes is omitted it is treated as a wild card. For instance if the name attribute is omitted from the pg_pattern definition, the pg_pattern will be applied to all property groups that have the specified type." > > "This property lets us declare which.": First person doesn't seem > appropriate for a manpage. Changed to "This property allow the user to declare which." > > "internal_separators tells us...": Can this be deprecated now that > SMF Property Value Ordering has been integrated? I don't think so. For example a set of range constraints is store a list of min,max values. E.g. "150,160 155,170". I don't see how Value Ordering would maintain the semantics of the ranges above. > > svccfg(1M): > "... if invoked with the -s option...": -s is missing from the > synopsis. I removed the -s option reference. The paragraph now reads: "When given a file, the file is processed similarly to import -V, but no changes are made to the repository. If any errors are detected, svccfg(1M) prints the errors, and exits with a nonzero exit status." > > scf_tmpl_pg_name(3SCF): > "this funtion will return a string containing SCF_TMPL_WILDCARD": > SCF_TMPL_WILDCARD doesn't seem to be in the interface table of the > ARC case. Is it appropriate to mention it here? It is in the man pages that have been updated since the ARC case approval. None of the newly introduced macros are explicitly in the interface table of the ARC case. Why should it be different for this one? > > "The caller is responsible for freeing the *out buffer on success.": > I presume you mean the caller should use free(3C). Though in the > past, the library was required to provide its own freeing function > in case malloc() or free() were interposed, or the function used > an allocator in a library other than libc. Is that no longer > necessary due to direct bindings? Isn't direct bindings amazing? ;-) > > "The scf_tmpl_pg_target() function will retrieve the property > group's target as currently templated...": I don't understand what > you mean by a property group's target. Do you mean the template's > target? I would say that terminology is cumbersome all over SMF. We meant the template Property Group Pattern's target. What is unexpected in this particular case is that you did not have the same doubt for scf_tmpl_pg_name() and scf_tmpl_pg_type() since they use the exact same wording. Anything in particular throw you off? I discussed the terminology a bit with Tom and we thought that it could be helpful using pg_pattern to refer to a template Property Group Pattern and prop_pattern to refer to a template Property Pattern. Both are somewhat defined in smf_templates(5). Although this terminology change has the potential to clarify the different terms is some places, I am sure it will make things more confusing in other places. Would you have any suggestions on how we could differentiate template 'Property Group Pattern' from 'Property Group' and template 'Property Pattern' besides 'pg_pattern' and 'prop_pattern'? I'll wait Liane's input before start making changes on the docs. > > scf_tmpl_prop_name(3SCF): > scf_tmpl_prop_internal_seps(): I don't understand how this function > will fill the scf_values_t. Will there always be a single element > for the entire string of separators, or will there be a separate > element for each character of the separators string? internal_separators is a list of separator characters. scf_tmpl_prop_internal_seps() will place one separator as a single character string in each element of out. Do you think "The scf_tmpl_prop_internal_seps() function will retrieve the list of internal separators as currently templated and place each of them in a different element of out." would make it easier to understand or just more wordy? > > scf_tmpl_validate_fmri(3SCF): > "The template validation functions offer a way to validate an > FMRI...": Really you're validating the configuration data associated > with an instance, eh? Re-worded to: "The template validation functions offer a way to validate the configuration data of an instance FMRI against the appropriate template data." > > "By default, the validation functions look up composed data...": Is > this referring to the data to be validated, or the data to > validate against? It is the data to be validated. I re-worded to: "By default, the validation is performed on the composed data from the running snapshot of an instance." Please let me know if it's still not clear. > > lib/libscf/inc/libscf.h > scf_values_t: These member names don't match the ARC materials. Fixed the man page (scf_tmpl_prop_name_3SCF.new) > > scf_tmpl_visibility_to_string(): The type of this argument doesn't > match the ARC materials. Fixed the man page (scf_tmpl_prop_name_3SCF.new)