On Mon, Jun 15, 2009 at 01:00:18PM -0500, Nicolas Williams wrote: > On Fri, Jun 12, 2009 at 11:37:01AM +0200, Darren Reed wrote: > > http://cr.opensolaris.org/~darrenr/6271923-20090611/ >
More comments, these all w.r.t. the fix for 6378850: - IIRC it's incorrect to refer only to $BASEDIR in the pkg scripts -- you need to also use $PKG_INSTALL_ROOT, like so: +rm -f $PKG_INSTALL_ROOT/$BASEDIR/var/tmp/dhcpsvc.tmp +if [ ! -f $PKG_INSTALL_ROOT/$BASEDIR/etc/inet/dhcpsvc.conf ] ; then + touch $PKG_INSTALL_ROOT/$BASEDIR/var/tmp/dhcpsvc.tmp +fi - Also, it's not clear to me how your fix for 6378850 works. The state of the service isn't checked (as the evaluation says it should be). Instead you're enabling the service only if /etc/inet/dhcpsvc.conf didn't exist to begin with, but, why would one want to enable the service just because one installed the package? Does the service disable itself if /etc/inet/dhcpsvc.conf doesn't exist? A quick glance at the source (collect_options()) seems to indicate that it continues along. As per the description in the CR: " The intent of this change was to make sure that pre-S10 systems with the dhcp server enabled had their SMF dhcp-server service enabled when they upgraded to 10. " But how can the pkg install scripts do that? One might have checked whether /etc/rc2.d/SXXdhcpd (or whatever) existed, but that will be gone by this point. Can in.dhcpd run without any dhcptab(4)/dhcp_network(4)/ whatever configuration? If not, why not just always enable the service on pkg install and then disable it in the start method of the service when there's nothing for it to do? Making the start method work that way should make it more aligned with the IPS way of doing things anyways. - The evaluation of 6378850 and the suggested fix don't match. - Don't forget, per my earlier comments, the pkg isn't making use of the new project CAS. Nico --
