Hi Neil,

Thanks for your review comments. Please see my replies inline.

On 06/29/09 13:24, Neil Garthwaite wrote:
> Hi Hemachandran, 
>
> Many thanks for this. I have just a few comments below, however please 
> note that I have not been able to verify the x86 non-LDoms deployment, 
> i.e. I don't have xVM + OHAC installed to test. 
>
> - functions.ksh
>
> Lines 84-86, are you missing SET_DEBUG=
Fixed it.
>
> Lines 543-540, the re-work has replaced ${ADMIN} with a CCR table 
> entry, as such this message needs to be changed as ${ADMIN} doesn't 
> exist now.
>
Fixed it.
> Line 912, function stop_domain has variable rc specified as global, 
> i.e. there is no typeset -i rc=0
>
Incorporated this.
> Line 1219, /opt/SUNWscxvm/bin/ldm_migrate is being run. I can see an 
> entry within prototype_sparc however I can't see a webrev entry for 
> this file.
>
The webrev now is able to list this file in the review.
> General comments for functions.ksh. 
>
> - Some functions do not have debug_message Begin|End, as such 
> ${SET_DEBUG} may not be set.  
>
I have included the debug_message for all the functions.

-Thanks and Regards
Hemachandran
> The rest looks good.
>
> Regards
> Neil
>
> On 29 Jun 2009, at 05:53, Hemachandran Namachivayam wrote:
>
>> Hi Neil,
>>
>> On 06/28/09 23:50, Neil Garthwaite wrote:
>>> Hi Hemachandran, 
>>>
>>> I note that things have been redesigned and reworked accordingly, as 
>>> such there is a reasonable amount to review. Nevertheless, I plan to 
>>> go through that later. 
>>>
>>> I do note that the CCR is being used to store, amend and retrieve 
>>> information, as such if I refer to the design 
>>> document 
>>> http://opensolaris.org/os/project/ha-xvm/ha-ldoms-design-doc_v2.pdf, I 
>>> can see that /usr/cluster/lib/sc/ccrutil is referenced however 
>>> within the code you have /usr/cluster/lib/sc/ccradm, is this a typo 
>>> within the design document? Either way, as this is an Open HA 
>>> Cluster review, the ability to use the CCR to store, amend and 
>>> retrieve information is likely to be extremely useful to others as 
>>> simply put it provides a synchronized repository across the cluster. 
>>>
>> Thanks for pointing this out. The table file operations to store, 
>> amend and retrieve information are now contained in "ccradm" utility, 
>> to fulfill the HA-xVM needs to share the information across cluster 
>> nodes. I have changed the references of ccrutil to "ccradm" in the 
>> design document. Please find the updated document at 
>> http://opensolaris.org/os/project/ha-xvm/ha-ldoms-design-doc_v2.pdf
>>
>>> As such I feel you need to expand on how the ccradm utility can be 
>>> used in the context of storing, amending and retrieving information. 
>>> I could only find 
>>> http://src.opensolaris.org/source/xref/ohac/ohac/usr/src/cmd/dtk/man/man1m/ccradm.1m
>>>  
>>> which does not contain the arguments you are using. 
>>>
>> A revised edition of the man page is yet to be committed to the gate. 
>> For the time being, You can refer to the man page changes at 
>> http://opensolaris.org/os/project/ha-xvm/ccradm.pdf
>>
>> -Thanks and Regards
>> Hemachandran
>>> Regards
>>> Neil
>>>
>>> On 26 Jun 2009, at 07:05, Hemachandran Namachivayam wrote:
>>>
>>>> Hi Folks,
>>>>
>>>> I'd welcome the code review for the "Solaris cluster failover agent 
>>>> for LDoms Guest domains"
>>>>
>>>> The failover agent for LDoms Guest domains is an extension of the 
>>>> HA-xVM agent and introduces a new resource type SUNW.ldom. The 
>>>> agent is redesigned to use the Generic Data Service (SUNW.gds) 
>>>> binaries and hence the resource type (SUNW.xvm) have been reworked 
>>>> upon. The HA-xVM agent makes use of CCR to store the xVM/LDom guest 
>>>> domain configuration; thereby a need to configure a separate admin 
>>>> directory is eliminated.
>>>>
>>>> To refer to the requirements specification and design document, 
>>>> please visit http://opensolaris.org/os/project/ha-xvm/
>>>>
>>>> The webrev, for review, is located at 
>>>> http://cr.opensolaris.org/~hnamachi/ha-ldom/ 
>>>> <http://cr.opensolaris.org/%7Ehnamachi/ha-ldom/>
>>>>
>>>> -Thanks and Regards
>>>> Hemachandran
>>>>
>>>> _______________________________________________
>>>> ha-clusters-discuss mailing list
>>>> ha-clusters-discuss at opensolaris.org 
>>>> <mailto:ha-clusters-discuss at opensolaris.org>
>>>> http://mail.opensolaris.org/mailman/listinfo/ha-clusters-discuss
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: 
<http://mail.opensolaris.org/pipermail/ha-clusters-discuss/attachments/20090629/409e3275/attachment.html>

Reply via email to