Sundar Yamunachari wrote: > Hi Jan and Sue: > > Please review the code for bug fixes. > > The webrev is at: http://cr.opensolaris.org/~ysundar/3976/ > > 3976 AI DHCP lease is short and not being extended > http://defect.opensolaris.org/bz/show_bug.cgi?id=3976 > > 4070 Client booted with AI image cannot access outside the subnet > http://defect.opensolaris.org/bz/show_bug.cgi?id=4070 > > 3968 installadm does not create directory > http://defect.opensolaris.org/bz/show_bug.cgi?id=3968 > > 3945 installadm delete-service doesn't remove the target directories > http://defect.opensolaris.org/bz/show_bug.cgi?id=3945 > > Thanks, > Sundar
Hi Sundar, installadm.c ------------ General: bug 3945 says the target directory isn't being deleted, but the code changes at 566 look like it's the service that wasn't being deleted? 1259 - not in your changes, but should there be an error message here? 1269 - if you intended for this file to be in /var/tmp rather than /var, you're missing the final "/" in "/var/tmp/". And maybe call it /var/tmp/ai.<pid> or /var/tmp/installadm.<pid> ? 1281 - before you return, you need to cleanup tmp_fp and associated file 1299 - Once you get service_name and directory, why not do the check on 1312? Why do you look for file and data? In other words, it seems like 1299-1306 could be deleted? 1309 This comment needs to be rewritten to reflect what code is actually doing (i.e., only writing line if it isn't a match to existng service/image) setup-dhcp.sh ------------- 165 - should the error message be expanded here so that if you can't get the default route, you tell the user that they won't be able to access outside the subnet or whatever the implications would be? And/or say "continuing anyway", since you don't abort? Sue
