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
_______________________________________________
networking-discuss mailing list
[email protected]
