One more. Can changes in SUNWcnetr/postinstall be avoided? That is the mechanism should not rely on postinstall script since that wouldn't work with OpenSolaris.
-tony Tony Nguyen wrote: > Michael and team, > > The SMF related changes are good in general. I have just a few comments. > > General: > > The integration of 6855845, Allow Property Modification in SMF profiles, > into b119 should be sufficient for NWAM phase next to quickly update > location specific values and simplify logic in various services and > their inter-relationships. > > Is there a reason why /etc/svc/volatile is used instead of /var/run or a > non-persistent pg? > > File specific comments: > usr/src/cmd/svc/shell/ipf_include.sh - ok > usr/src/cmd/ipf/svc/ipfilter.xml - ok > usr/src/cmd/svc/milestone/net-svc - ok > usr/src/cmd/svc/milestone/network-netmask.xml - ok > usr/src/cmd/svc/milestone/network-netcfg.xml - ok > usr/src/cmd/svc/milestone/network-ipqos.xml - ok > usr/src/cmd/svc/milestone/net-netmask - ok > usr/src/cmd/svc/milestone/net-ipqos - ok > > usr/src/cmd/cmd-inet/lib/nwamd/ipf6.conf.dfl > usr/src/cmd/cmd-inet/lib/nwamd/ipf.conf.dfl > > Comments on line 3 and 16 suggest these files aren't the final version. > > usr/src/cmd/svc/milestone/network-service.xml - No update? However, are > dependencies (e.g. NIS dependencies) and comments still accurate if the > service is now only responsible for updating DNS information? > > usr/src/cmd/svc/milestone/network-physical.xml - > > 41 & 87 - It's not clear from the design spec that there will be > additional network/physical instances. Clarification please? > > line 74 - unnecessary diff? > > usr/src/cmd/svc/milestone/network-location.xml > > 100 -113 manifest-import is defined as a dependent but the comment > states it should be a dependency. Has testing been done to verify > correct behavior? > > usr/src/cmd/svc/milestone/net-loc > usr/src/cmd/svc/milestone/net-svc > usr/src/cmd/svc/milestone/net-physical > usr/src/cmd/svc/milestone/net-nwam > > These files have similar implementation to figure out whether a service > is enabled. It's probably better to have define a single > service_is_enabled() in net_include.sh that these three files can consume. > > create/revert_legacy_loc doesn't correctly save/restore custom file. > User may have a policy other than custom but has a valid configuration > file. The current code doesn't correctly save the configuration file if > policy is something other than 'custom' thus can't properly restore it. > > usr/src/cmd/svc/milestone/net-loc > > 501 - clearing custom policy_file is probably unnecessary since policy > is set to 'none'. > 502 - refresh_ipf also needs to be set here to make the changes effective. > > stop_svc() - in all uses of this function, no configuration change was > made to the target service so a refresh isn't necessary. > > refresh_svc() - The name seems incomplete as the function really is > refreshing/starting the service with the updated configuration. Perhaps > run_svc? > > -tony > >>> >>> Date: Mon, 06 Jul 2009 15:36:11 -0700 >>> From: Michael Hunter <Michael.Hunter at Sun.COM> >>> To: nwam-dev at opensolaris.org >>> Subject: [nwam-dev] NWAM Phase 1 Code Review request >>> >>> >>> This is a request for people to review the NWAM Phase 1 code. >>> >>> The code can be found at http://cr.opensolaris.org/~mph/nwam1_cr/ >>> >>> For those inside sun there is a cscope database >>> in /net/coupe.eng/builds/mph/nwam1_cr/usr/src (sorry to those external >>> to SWAN). >>> >>> Documentation can both be found in the source tree and on the project >>> web page at http://opensolaris.org/os/project/nwam/ >>> >>> We request code review be finished by Monday July 20th. >>> >>> Michael Hunter >>> _______________________________________________ >>> nwam-dev mailing list >>> nwam-dev at opensolaris.org >>> http://mail.opensolaris.org/mailman/listinfo/nwam-dev >
