[responding to Tom Whitten's comments, i've added them here with his permission. thanks Tom!]
updated webrevs: internal: http://whitestar1-4.east/quagga_internal_fix/webrev/ external: http://cr.grommit.com/~amaguire/quagga_fix/ > usr/src/cmd/cmd-inet/usr.lib/in.ndpd/ndp.xml: > nit - why is line 112 not intented? > > i thought i'd noticed a few other manifests didn't indent the common name, so i figured that might be the convention here. taking another look, i can't find any examples of unindented localized text, so it was obviously my imagination ;-). anyhow i've fixed the indentation for common names and descriptions now. > usr/src/cmd/cmd-inet/usr.lib/in.ndpd/svc-ndp: OK > usr/src/cmd/svc/shell/routing_include.sh: OK > usr/src/cmd/cmd-inet/usr.lib/in.ripngd/ripng.xml: > line 115: "Equivalent to -s option is true," > if-------^ > > fixed. > line 137: indentation > > fixed. > usr/src/cmd/cmd-inet/usr.lib/in.ripngd/svc-ripng: > line 76: Do you care if svcadm fails? > > > yep, i've updated to catch the return code and bail if it's not SMF_EXIT_OK. > Is there any need to use -s, so that the ndp > service is online before starting in.ripngd? > I don't know enough about networking to know > whether or not this is necessary. > > yes, there is actually, good point! i've fixed this. > usr/src/cmd/cmd-inet/usr.sbin/in.rdisc/rdisc.xml: > line 123: indentation > > fixed. > usr/src/cmd/cmd-inet/usr.sbin/in.rdisc/svc-rdisc: OK > usr/src/cmd/cmd-inet/usr.sbin/in.routed/route.xml: > line 141: indentation > > fixed. > usr/src/cmd/cmd-inet/usr.sbin/in.routed/svc-route: > line 39: Should "in.ndpd" be "in.routed"? > > yep, fixed. > usr/src/cmd/cmd-inet/usr.sbin/routeadm/forwarding.xml: > lines 126,131,230,235: indentation > usr/src/cmd/cmd-inet/usr.sbin/routeadm/legacy-routing.xml: > lines 72, 82: Why not use %m to be consistent with > forwarding.xml? > > fixed. > lines 146, 151: indentation > > fixed. > usr/src/cmd/cmd-inet/usr.sbin/routeadm/svc-forwarding: > line 49: Usage message has method and proto reversed. > > good catch, thanks! > usr/src/cmd/cmd-inet/usr.sbin/routeadm/svc-legacy-routing: > lines 33,35,38: I'm curious as to why you use svccfg to get > property values rather than svcprop. > > these properties can contain spaces, which svcprop escapes, so i used svccfg instead. > line 59: Is there a race condition in capturing the newest > process? What happens if some other unrelated > service is starting at the same time? > > true, i've updated this to use the daemon program name in the pgrep command also, so we avoid this race condition. > usr/src/cmd/svc/milestone/fs-root: OK > usr/src/cmd/svc/milestone/net-init: OK > usr/src/cmd/svc/milestone/net-loopback: > line 64: Again curious about the use of svccfg. > > replaced it with svcprop (no escaped spaces here for a boolean value). > usr/src/cmd/svc/milestone/net-routing-setup: > I don't pretend to understand all the networking logic, but the SMF > part looks OK. > > excellent! (btw, the networking logic hasn't changed, it's just been moved into this service and updated to deal with the appropriate SMF propeties as opposed to configuration stored in routing.conf). > usr/src/cmd/svc/milestone/network-initial.xml: OK > usr/src/cmd/svc/milestone/network-loopback.xml: OK > usr/src/cmd/svc/milestone/network-routing-setup.xml: OK > > thanks very much for the review! alan This message posted from opensolaris.org
