Hi Ethan. Thanks for your review.
On 04/14/09 01:06, Ethan Quach wrote: > Jack, > > Have you thought about doing 237-253 for the ipaddr of the interface > that's actually going to be used with the given starting_client_dhcp > ipaddr? > As it stands, it could be verifying a netmask for network X on the > server, > but the starting_client_dhcp ipaddr could be for network Y. Since the > dhcp configuration is what actually wants the network table to be correct > for the network it's configuring, we could still see a failure later on > during the dhcp configuration. OK. You are correct. I now to the same test for the client network as I do for the system network. Aside from some function shuffling, I have added a new function in installadm-common.sh called find_netmask which is almost the same as find_network but returns a netmask instead. Delta webrev: http://cr.opensolaris.org/~schwartz/090405.1/webrev.incr_3_4/ Vs slim_source: http://cr.opensolaris.org/~schwartz/090405.1/webrev/ Thanks, Jack > > > thanks, > -ethan > > > Jack Schwartz wrote: >> Hi Dave. >> >> Thanks for round 2... We're almost there... >> >> On 04/13/09 10:28, Dave Miner wrote: >>>>> 233: error message would be better as "The netmask for network >>>>> $ds_ipaddr is not configured in the netmasks(4) table." >>>> How about this: >>>> The netmask for network $ds_ipaddr cannot be found or is >>>> improperly configured. >>>> >>>> The reason the original message referred to getent(1M) instead of >>>> netmasks(4) was because I assumed getent could potentially find the >>>> netmask from somewhere other than /etc/netmasks(4), since there is >>>> a netmasks entry in /etc/nsswitch.conf. Am I correct? >>>> >>>> I don't want to refer to netmasks(4) when some other source for >>>> netmasks exists and may be used by getent. This would only confuse >>>> the user. If netmasks(4) is really the only place the netmask can >>>> come from and you don't like my suggestion above, I'll change the >>>> message to refer to netmasks(4). >>> >>> The bug there is that the netmasks man page fails to acknowledge >>> other information sources, though they are rarely used. Referring >>> to getent doesn't give the user a clue where to fix it (getent is an >>> implementation detail of the script), whereas referring to the >>> netmasks table manpage does. So no, I don't agree with using getent >>> in these messages. >> This message (at line 240) didn't refer to either getent or >> netmasks. Sounds like you want me to reference netmasks(4) there to >> better clue the user in on what to change. So we're back to your >> original suggestion of: >> >> "The netmask for network $ds_ipaddr is not configured in the >> netmasks(4) table." >> >> There was one message on line 248 which did refer to getent, which I >> will change from: >> "The netmask obtained from getent(1M) for network $ds_ipaddr does >> not equal the netmask configured for the interface for that network." >> >> to: >> "The netmask obtained from netmasks(4) for network $ds_ipaddr does >> not equal the netmask configured for the interface for that network." >> >> Does this work for you? >> >> One other change which Ethan pointed out to me in person will be to >> call the get_host_ip() from installadm-common rather than call geteng >> myself on line 165. >> >> I webrevs updated with these changes. Please bless. >> >> Delta: http://cr.opensolaris.org/~schwartz/090405.1/webrev.incr_2_3/ >> >> Vs slim_source: http://cr.opensolaris.org/~schwartz/090405.1/webrev/ >> >> Thanks, >> Jack >>> >>> Other changes are fine. >>> >>> Dave >>> >>
