Hi Drew,
Everything look good to me now.
Thanks,
--Karen
On 06/15/11 08:38 PM, Drew Fisher wrote:
Karen,
I've respun the webrev with your comments taking into account.
http://cr.opensolaris.org/~drewfish/cr_7040831/
I also successfully tested installation of a zone with these bits.
The installation, reboot and usage of zone shows no errors. Also, the
install_log shows the new ICT logging output.
See my other reply for the comment on the uniquely named file in /tmp
for the repository. I didn't think it was necessary to explicitly
check for the existence of the tempfile at the end of the
configure_smf() method, but if you'd like me to, I can add that bit of
code.
I also went back and updated the test_pre_pkg_img_mod.py to handle
these changes. We don't need any more unittest bugs! :)
Thanks again for looking!
-Drew
On 6/15/11 4:28 PM, Karen Tung wrote:
Hi Drew,
Please see my comments inline:
On 06/15/11 02:05 PM, Drew Fisher wrote:
Good afternoon!
Could I please get a code review for the following bug:
7040831 <http://monaco.us.oracle.com/detail.jsf?cr=7040831> DC needs
to be a bit smarter about importing manifests and using the seed repos
http://cr.opensolaris.org/~drewfish/cr_7040831/
-------------------------------------------------------------------------------
usr/src/cmd/distro_const/checkpoints/pre_pkg_img_mod.py
-------------------------------------------------------------------------------
- line 166. We can not call the repository "/tmp/install-repo.db".
This will cause problem more than 1 DC build happens
simultaneously on the same machine. This will happen
on public build machines
like indiana-build. I understand you that having the repository in /tmp
will improve the build speed. However, I personally prefer to have
all the build related data in the build area I specified. Yes, I won't
have a speed gain in this case, but at least, I know I won't overwrite
other people's data. If you think the speed gain is important and
you must have it in /tmp, you need to come up with a way to create unique
file names.
- lines 180-186: Would this be a good time to change to using the
solaris_install.Popen()?
That way, you can just pass in the logger, and the code can be
simplified.
- lines 227-231: instead of hard coding the values here, why not get
all the
keys from "smf_env_vars", and unset them so you don't need to worry about
missing anything or keeping the list up-to-date in the future.
Other files in the webrev look OK to me.
This fix updates how DC constructs the SMF repository for the ISOs.
Before, we used manifest-import to create the repository. This
change moves us to using svccfg instead which will allow DC to stay
more in-line with changes made to SMF in the future.
As part of the SMF Decorations project[1], the initialize_smf ICT
has changed to no longer copy the global.db or nonglobal.db seed
repository to the installed image. The imminent push of the SMF
project (likely later today) will change both files into empty files
with the intention to do away with them eventually. The changes to
the ICT are there to simply remove the file if it exists.
I've tested full installs (from DC -> install -> reboot) with both
stock 167 bits from ipkg.us.oracle.com and SMF project nightly
bits. Everything checks out with no issues.
You only mention DC->install->reboot, and didn't mention about
observing that all the SMF services are registered
without errors, and the specified user and role are created as
expected..etc.. I assume that's all done as well,
and there's no problem.
I see that you also changed the InitializeSMFZone class in
initialize_smf.py. Perhaps you need to do a zone install to
make sure that the change doesn't cause any problem?
Thanks,
--Karen
One extra benefit of the change is DC now builds the ISO's
repository.db file in /tmp rather than on the dataset. I'm seeing a
speed-up of almost 5 minutes to run the pre_pkg_img_mod checkpoint.
Yay!
I've CC'd the SMF project team in case anybody has any specific SMF
related questions.
Thanks!
-Drew
[1] http://sac.sfbay/PSARC/2011/108/
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
