Quoth Alan Maguire on Wed, Jan 09, 2008 at 04:34:11PM +0000:
> thanks Liane (and Tom!) for taking a look. i've refreshed
> the webrev, so let me know if there's anything else amiss.
> 
> http://cr.opensolaris.org/~amaguire/svccfg_refresh/

cmd/svc/svccfg/svccfg_help.c
  124: Since the snapshot is named "running", please don't capitalize
    its name here.

  124: The other help text uses "currently selected instance", so please
    follow suit here.

  124: "Update the Running snapshot..." is technically correct, but
    I suspect we can make the purpose more apparent to users with
    something like "Commit any changes to the directly attached
    properties to running snapshot."  Note that the "directly attached
    properties" language is from the svcprop manpage.  I'm not sure
    whether we should also use "effective properties" from there, too.
    The svcadm manpage also uses "running configuration snapshot", which
    I am not sure is worth the extra terminal space here.

  125: As in the description of refresh on svcadm(1M), we don't know
    that the service's restarter acknowledges refresh methods.  So
    I suspect we should change this to something like "If the repository
    is being monitored, the service's restarter will be informed of the
    change.  If the restarter is svc.startd(1M), the refresh method will
    be invoked."  Though that seems like an unnatural combination of
    genericity and specificity, for some reason.

cmd/svc/svccfg/svccfg_libscf.c
  4986: Please note that snap is used as scratch space.

  12369,378,399,408: I think the colon obviates the parentheses.  But
    I believe the rest of svccfg uses parentheses without the colon.

  12384,419: I don't think you should free(fmribuf) before bad_error()
    so that fmribuf will be in the corefile.


David

Reply via email to