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 >
