Darren,
I assume it's okay for me to check-in these changes now. I've
updated the webrev.
See below for some additions comments
On 06/14/11 02:11 AM, Darren Kenny wrote:
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?
Forgot in my last mail fixed.
- 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.
Ok. It still looks like I should modify it to not output the
partition when I can determine
the profile is meant for a sparc system. I filed CR7054528 to
capture the problem.
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?
Not really. The case I'm referring to is for the Solaris 10
backport. I've been unable
to get the Manifest parser backported to Solaris 10 since it's
pulling in stuff that doesn't
exist. If there's other components that are part of the AI Install
Server that require
libbe then I'll end up have to backport that but for now I cann't
see anything other than
js2ai. Although installadm is the only command I've got fully
working on Solaris 10 at
this point in time.
Switching the validation used by installadm looks like it would
solve my problem. I just
need to explore it to see if it will. It will change the behavior
since it appears that
a service needs to be created that the profiles can be generated
against. This in itself
creates problems for me since we currently have no notion of
versions of the manifests
and sc_profiles to account for changes at various stages.
Just a problem that needs to get solved at some point.
- 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
--
Kristina
Tripp, Senior Software Engineer
Oracle
Revenue 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
|
