Thanks Sarah!
Sarah Jelinek wrote:
> Looks fine.
>
> sarah
> ****
>> 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
>
