Nicolas Williams wrote: > > - $SRC/lib/libproject/common/setproject.c:93-97,111-116 > > The comment is hard to understand. I think you're trying to say that > setproject(3PROJECT) is not allowed to fail because of failure to > clear an rctl. > > I agree that that's the old behavior and that the manpage's > description of error conditions makes doing anything other than > looping in this case... seem wrong. But looping forever seems wrong > to me too, particularly given that setproject() is not atomic (i.e., > it can fail after having put the process in a new task and will leave > the process in the new task). Any chance that we can get rid of this > potentially infinite loop? At least file a CR, if there isn't one > already. >
I've filed 6851264. Nicolas Williams wrote: > 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 > Be that as it may, the rest of the package files are only using $PKG_INSTALL_ROOT. On top of that, use of $PKG_INSTALL_ROOT by itself seems quite common. I'm not going to make this change. > - 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). > The state of the service is only important in one scenario - pre-SMF DHCP server. This is inferred by the presence of the configuration file. In every other situation we want to do one thing: nothing. > 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? > Err, no, but I suspect that I updated the Evaluation ahead of the suggested fix. And ahead of updating the webrev. Please recheck the CR. > 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. > It doesn't need to. > 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. > It would seem to me that the existing behaviour has not caused anyone any problems, otherwise we'd have a bug filed saying that either DHCP became disabled over upgrade or failed into a maintenance state when it should not have. > - The evaluation of 6378850 and the suggested fix don't match. > See above. See my other email for a pointer to an updated webrev. (it may arrive in 5 or 10 minutes, please be patient.) Darren
