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