On 10/19/10 03:46 PM, Alok Aggarwal wrote:
This is a code review request for the rewrite of DC
to make use of CUD. The implementation here closely
follows the design document that can be found here:
http://cvs.opensolaris.org/source/xref/caiman/caiman-docs/distro_constructor/dc_cud_design.pdf
Note that this wad does not include the support for
VMC. The support for VMC will be a separate, follow on
project.
Since the changes are fairly substantial we are breaking
up the review into the following sections:
DC app, RBAC, Makefiles and packaging [1]
Manifests and XML serialization/de-serialization [2]
Pre pkg image mod, boot archive configuration and archive archive checkpoints
[3]
All the other remaining checkpoints [4]
Please sign up to review one or more of the sections
and let us know exactly which ones you will be reviewing.
Webrev location is:
http://cr.opensolaris.org/~aalok/cud_dc
Please have your comments in by COB November 5th.
Thanks,
Drew and Alok
[1] usr/src/Makefile.master
usr/src/Targetdirs
usr/src/cmd/Makefile.targ
usr/src/cmd/distro_const/Makefile
usr/src/cmd/distro_const/distro_const.py
usr/src/cmd/distro_const/cli.py
usr/src/cmd/rbac/Makefile
usr/src/cmd/rbac/exec_attr.distribution-constructor
usr/src/cmd/rbac/prof_attr.distribution-constructor
usr/src/cmd/distro_const/checkpoints/Makefile
usr/src/cmd/distro_const/manifest/Makefile
usr/src/cmd/distro_const/profile/Makefile
usr/src/pkg/manifests/install-distribution-constructor.mf
usr/src/cmd/distro_const/checkpoints/defaultfiles/Makefile
usr/src/cmd/distro_const/checkpoints/defaultfiles/coreadm.default
usr/src/cmd/distro_const/checkpoints/defaultfiles/rtc_config.default
[2] usr/src/cmd/distro_const/__init__.py
usr/src/cmd/distro_const/configuration.py
usr/src/cmd/distro_const/distro_spec.py
usr/src/cmd/distro_const/execution_checkpoint.py
usr/src/cmd/distro_const/manifest/boot_archive_contents_sparc.xml
usr/src/cmd/distro_const/manifest/boot_archive_contents_x86.xml
usr/src/cmd/distro_const/manifest/dc_ai_sparc.xml
usr/src/cmd/distro_const/manifest/dc_ai_x86.xml
usr/src/cmd/distro_const/manifest/dc_livecd.xml
usr/src/cmd/distro_const/manifest/dc_text_sparc.xml
usr/src/cmd/distro_const/manifest/dc_text_x86.xml
usr/src/cmd/distro_const/profile/ai.xml
usr/src/cmd/distro_const/profile/generic.xml
usr/src/cmd/distro_const/profile/livecd.xml
usr/src/cmd/distro_const/profile/text.xml
[3] usr/src/cmd/distro_const/checkpoints/pre_pkg_img_mod.py
usr/src/cmd/distro_const/checkpoints/boot_archive_configure.py
usr/src/cmd/distro_const/checkpoints/boot_archive_archive.py
usr/src/cmd/distro_const/checkpoints/test/test_pre_pkg_img_mod.py
usr/src/cmd/distro_const/checkpoints/test/test_boot_archive_configure.py
usr/src/cmd/distro_const/checkpoints/test/test_boot_archive_archive.py
[4] usr/src/cmd/distro_const/checkpoints/ai_publish_pkg.py
usr/src/cmd/distro_const/checkpoints/create_iso.py
usr/src/cmd/distro_const/checkpoints/create_usb.py
usr/src/cmd/distro_const/checkpoints/custom_script.py
usr/src/cmd/distro_const/checkpoints/grub_setup.py
usr/src/cmd/distro_const/checkpoints/pkg_img_mod.py
usr/src/cmd/distro_const/checkpoints/test/test_ai_publish_package.py
usr/src/cmd/distro_const/checkpoints/test/test_create_iso.py
usr/src/cmd/distro_const/checkpoints/test/test_create_usb.py
usr/src/cmd/distro_const/checkpoints/test/test_custom_script.py
usr/src/cmd/distro_const/checkpoints/test/test_grub_setup.py
usr/src/cmd/distro_const/checkpoints/test/test_pkg_img_mod.py
usr/src/cmd/distro_const/checkpoints/test/testlib.py
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
Hi Alok and Drew,
__init__.py
23 Copyright should be range of 2008-2010
execution_checkpoint.py
28 ExecutionChkpt -> Execution ?
131 indent Checkpoint under kwargs
169 Need docstring
204 indent under "invalid
209 ditto
211 remove line
pkg_img_mod.py
50 after to boot -> after the boot
87, 106 don't need "\"
112 don't need "+ \", implied continuation within parens
88 line up str under "Error
105-106 Move up before 101, if the directory doesn't exist, the file under
it won't either
141 line up under "Compressing
150, 185 This seems fragile, making a decision based on the output to stderr.
118-191 create_usr_archive and create_misc_archive seem to have a fair amount
of code in common. Would it make send for them to share a common routine to
(maybe to do the lofiadm?)
176 indentation
175, 186, 190, 244, 469 no need for "+ \", implied continuation within parens
217 PkgImgMod -> LiveCDPkgImgMod
217 after to boot -> after the boot
370-371 Should this be moved to the top of execute? That way, if something else
is added later that changes the dir in a call from 355-370, you'll already have
it saved.
384 - wasn't this different in the code walk-through?
cli.py
o A couple of commands are not alphabetized properly
pre_pkg_img_mod.py
93,96 don't need "\", indent str under "Error
167 I was under the impression that it wasn't necessary to set
LD_LIBRARY_PATH anymore. Do you need this?
188 You check if the path exists, but then raise an error saying it isn't
executable?
216, 286, 354, 356, 412, 469, 509 don't need "+ \"
217 line up left " with previous line
install-distribution-constructor.mf
o Shouldn't rtc_config.default and coreadm.default be in here?
o Files should be alphabetized
test/test_pre_pkg_img_mod.py.html
82 You need to resolve the 'XXX'
test_ai_publish_package.py
o add docstrings to all tests
o I prefer the self.assertTrue and self.AssertEquals constructs to the
self.assert_ construct. Seems clearer. Same for other test files.
test_boot_archive_archive.py
o add docstrings to all tests
120 should be end -> should end
248 -249 Looks like this isn't quite finished yet?
dc_livecd.xml (and other xml files)
99 Would be good to have a comment explaining the notation.
321 Don't forget to take care of this (and other files too)
Thanks,
Sue
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
