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>