Hi Kristina, On 13/06/2011 20:15, Kristina Tripp wrote: > Thanks for the review. Comments inline > > On 06/ 9/11 02:01 AM, Darren Kenny wrote: >> Hi Kristina, >> >> Apologies for not getting to this sooner, it's code I'm not familiar with in >> any way. >> >> Overall things are looking good, but I do have some questions/comments: >> >> - In conv.py: >> >> - The method __create_any_boot_disk() doesn't appear to be of any use, >> should it be still there? Lines 1710-1716 suggest that something should >> be >> done. > Removed >> - In __add_device, line 705-708, you appear to be creating a diskname node >> in-line rather than using the method __create_diskname_node(), why not >> use >> the method? >> >> - In __add_device, line 709, you seem to add a partition, but this should >> only be done for an x86 based profile, if it's SPARC based the generated >> manifest will fail, on SPARC slices are direct children of the <disk> >> node. While the DTD will validate, AI itself will fail. > Interesting. This isn't something I was aware of and a problem since we > typically have no notion of what platform the system is intended for. A few of > the Jumpstart keywords are targeted for are for sparc or x86 and I can > certainly > address it when I have a notion of the architure of the system. But it's > possible to generate a Jumpstart profile that requires me to generate a disk > entries for the root ZFS pool that I'll have no notion of the architecture. > > Are there any options available to me?
I've sent an e-mail to caiman-discuss about this issue, to see what other people think is a reasonable approach for handling this in AI. I could add a specific exception on SPARC to not fail, but just ignore things for this specific case, or I could ad a more general "ignore all partitions". > > If your agreeable I'll write a CR to address this since this is going to > require > some large changes if there is no common way to make this work for both > platforms. Instead of > trying to fold this into this current set of changes. I think a separate CR would be fine - since it's looking like the best way to resolve this would be to change AI's behaviour. > >> If you've not done it already, it would really be worth running the >> generated XML manifests, on snv_167+, through a call to: >> >> auto-install -i -m <manifest> >> >> The -i will stop before doing anything destructive, of course this would >> also assume that the machine has the right devices. >> >> Or alternatively, in the tests, you could take a look at the >> $SRC/cmd/auto-install/test/test_target_selection_*.py and mimic what it >> does in calling select_targets, passing your XML to it to ensure that it >> will get through that validation at least. > We currently validate the manifests and sc profiles as part of the conversion > process. > All the unit tests all perform validation of the generated xml code. > > The current validation occurs via the ManifestParser. Ethan did question > whether > we wanted to do the validation against the image associated with a service. > This may be desirable since then I could use the validation code used by > installadm. The ManifestParser code base is causing me problems on the > backport > since it wants to pull in all the target/libbe > code. > Maybe you need to also set the LD_LIBRARY_PATH to the image's libraries directory to pick up the image's libbe? Would that work? >> - Similarly in __valid_to_add_slice(), line 729, you assume that a >> partition >> should always be present if specifying a slice, that is not true on >> SPARC. > Ok. Discussed above >> - Lines 1249-1257, are these commented lines needed or can they be removed? > Removed Thanks, Darren. >> Thanks, >> >> Darren. >> >> >> On 06/06/2011 15:01, Kristina Tripp wrote: >>> I still need at least two volunteers to do a review for the following. I'm >>> out >>> of the office next week Jun 9th and 10th so I'd appreciate getting this >>> into the >>> tree before then. The webrev has been updated to reflect a change from the >>> tech >>> writer that js2ai(1) be moved to js2ai(1m). >>> >>> Webrev >>> http://cr.opensolaris.org/~enpointe/cr_7021883/ >>> >>> 7021883 <http://bugs.opensolaris.org/bugdatabase/view_bug.do?bug_id=7021883> >> js2ai needs to be updated to work with the new target DTD >>> 7040753 <http://bugs.opensolaris.org/bugdatabase/view_bug.do?bug_id=7040753> >> js2ai specifying -p with long path name failure >>> 7044397 <http://bugs.opensolaris.org/bugdatabase/view_bug.do?bug_id=7044397> >> js2ai update to support SC Profile changes made in svn_164 >>> 7050992 <http://bugs.opensolaris.org/bugdatabase/view_bug.do?bug_id=7050992> >> js2ai move man page from js2ai(1) to js2ai(1m) >>> >>> >>> Testing: >>> Most of the testing for this product still relies on the unit tests and >>> running >>> against some known jumpstart configuration profiles and ensuring the >>> converted >>> profiles have no validation errors. The unit test code coverage is 80%+ >>> for all >>> files. >>> >>> There is still a need to test js2ai by running it against some Jumpstart >>> profiles and doing some installs based on the files created by js2ai. >>> However, >>> I haven't been able to get a AI Server running with all the proper bits so >>> this >>> work will need to occur at a latter time period. >>> -- >>> >>> <http://www.oracle.com/><http://www.oracle.com/> >>> *Kristina Tripp, Senior Software Engineer* >>> OracleRevenue Product Engineering >>> 500 Eldorado Blvd, MS UBRM05-171 >>> Broomfield, CO, 80021 >>> Office: 303-272-8655 >>> Email: [email protected] >>> >>> Oracle is committed to developing practices and products that help protect >>> the >>> environment >>> >>> >>> >>> _______________________________________________ >>> caiman-discuss mailing list >>> [email protected] >>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss > > > -- > > <http://www.oracle.com/><http://www.oracle.com/> > *Kristina Tripp, Senior Software Engineer* > OracleRevenue Product Engineering > 500 Eldorado Blvd, MS UBRM05-171 > Broomfield, CO, 80021 > Office: 303-272-8655 > Email: [email protected] > > Oracle is committed to developing practices and products that help protect the > environment > _______________________________________________ caiman-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
