Thanks Sundar! -evan
Sundar Yamunachari wrote: > Evan, > > The changes addressed my comments. It looks good now. > > - Sundar > > Evan Layton wrote: >> I've created a new webrev with these changes as well as some comment >> clean-up that was missed for get_service_props() and set_service_props() >> in installadm_util.c. We also changed check_for_enabled_install_services >> in installadm.c so that if the service is already in maintenance mode we >> print out a mesage and return skipping the code that would attempt to >> put the smf service into maintenance. >> >> The new webrev is available at: >> http://cr.opensolaris.org/~evanl/7218.02/ >> >> Thanks! >> >> -evan >> >> >> jan damborsky wrote: >>> Hi Evan, >>> >>> I have only couple of nits - please see below. >>> >>> Thank you, >>> Jan >>> >>> >>> libaiscf/Makefile >>> ----------------- >>> 50,51 - please change ${} -> $() >>> >>> ai_utils.c >>> ---------- >>> 454 name = malloc(namelen); >>> 455 valuestr = malloc(vallen); >>> -> >>> 454 name = malloc(namelen + 1); >>> 455 valuestr = malloc(vallen + 1); >>> >>> 578 buff = malloc(namelen); >>> -> >>> 578 buff = malloc(namelen + 1); >>> >>> >>> >>> >>> On 04/01/09 01:01, Evan Layton wrote: >>>> This webrev didn't have the fix for calling create-service more than >>>> once on the save AI service name. The fix was to simply return >>>> success if a property group already exists for the AI service name. >>>> >>>> I've updated the webrev to include the fix for this. >>>> >>>> Thanks, >>>> -evan >>>> >>>> Evan Layton wrote: >>>>> The source has been updated with all the changes made from the code >>>>> review comments up to this point. I've created a new webrev which >>>>> contains these changes. >>>>> http://cr.opensolaris.org/~evanl/7218.01 >>>>> >>>>> Thanks for all the comments! >>>>> >>>>> -evan >>>>> >>>>> Evan Layton wrote: >>>>>> My apologies for the delay. I have fixed the problems with the >>>>>> mis-merge, sanity tested the changes and updated the webrev. I've >>>>>> listed out the files and split things up between the library and >>>>>> installadm. Just pick a subset of files and let me know what >>>>>> you're looking at som we make sure everyting gets covered. >>>>>> >>>>>> And thanks again to Sue for catching the mis-merge!!! >>>>>> >>>>>> Thanks! >>>>>> -evan >>>>>> >>>>>> installadm files: >>>>>> usr/src/cmd/installadm/Makefile >>>>>> usr/src/cmd/installadm/installadm-common.sh >>>>>> usr/src/cmd/installadm/installadm.c >>>>>> usr/src/cmd/installadm/installadm.h >>>>>> usr/src/cmd/installadm/installadm_util.c >>>>>> usr/src/cmd/installadm/server.xml >>>>>> usr/src/cmd/installadm/setup-service.sh >>>>>> usr/src/cmd/installadm/svc-install-server >>>>>> usr/src/pkgdefs/SUNWinstall-libs/prototype_com >>>>>> >>>>>> libaiscf Library files: >>>>>> usr/src/lib/Makefile >>>>>> usr/src/lib/libaiscf/Makefile >>>>>> usr/src/lib/libaiscf/ai_create.c >>>>>> usr/src/lib/libaiscf/ai_trans.c >>>>>> usr/src/lib/libaiscf/ai_utils.c >>>>>> usr/src/lib/libaiscf/libaiscf.h >>>>>> >>>>>> >>>>>> And if anybody is REALLY board here's the test files... >>>>>> >>>>>> optional files: >>>>>> usr/src/cmd/installadm/test/installadm_test.c >>>>>> usr/src/cmd/installadm/test/manual_tests >>>>>> >>>>>> >>>>>> Evan Layton wrote: >>>>>>> Sue pointed out that the webrev shows that I have a mis-merge so >>>>>>> if you're looking at the changes please stop. I'll get the >>>>>>> mis-merge fixed and send out a new webrev as soon as possible. >>>>>>> >>>>>>> Thanks, >>>>>>> -evan >>>>>>> >>>>>>> >>>>>>> Evan Layton wrote: >>>>>>>> >>>>>>>> We need to get the fix for 7218 code reviewed. There are around >>>>>>>> 2000 lines of changes and new code so we'll need volunteers that >>>>>>>> can take sections of the code to look at. I'm sending this out >>>>>>>> now so the link is out there but I'll want to get things divided >>>>>>>> up tomorrow morning and get folks looking at this tomorrow. >>>>>>>> >>>>>>>> The bug is 7218 >>>>>>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=7218 >>>>>>>> >>>>>>>> Webrev: >>>>>>>> http://cr.opensolaris.org/~evanl/7218/ >>>>>>>> >>>>>>>> Thanks! >>>>>>>> -evan >>>>>>>> _______________________________________________ >>>>>>>> caiman-discuss mailing list >>>>>>>> caiman-discuss at opensolaris.org >>>>>>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >>>>>>> >>>>>>> _______________________________________________ >>>>>>> caiman-discuss mailing list >>>>>>> caiman-discuss at opensolaris.org >>>>>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >>>>>> >>>>> >>>> >>>> _______________________________________________ >>>> caiman-discuss mailing list >>>> caiman-discuss at opensolaris.org >>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >>> >> >> _______________________________________________ >> caiman-discuss mailing list >> caiman-discuss at opensolaris.org >> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >
