On 10/19/10 06: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
Consider this a light review of the non-test code. Intend to review the
manifests as well.
Dave
distro_const/__init__.py: Much of this seems like stuff that would be
shared across apps using the DOC; is there a better way to implement so
that it can be shared?
ai_publish_pkg.py
-----------------
97: I'm surprised we need to use tmp files to pass this around; why
shouldn't this be in the DOC if not provided in the arguments?
114,125,129: Shouldn't we be capturing error messages to the log? Or am
I misunderstanding the effect of _NULL here?
boot_archive_archive.py
-----------------------
300: No Fat Albert in the logs, please :-)
341, others: Stylistically, I think the logs would be more palatable if
SHOUTING was reserved for errors (if anything), not info messages.
boot_archive_configure.py
-------------------------
446: Primary Administrator is going away; this needs to be adjusted to
pick up the changes made in slim_source changeset 861:ccd399d2c6f7
create_iso.py
-------------
97: Need comment here
171: can we either use a symlink or pass a reference to the correct file
via the DOC to create_usb so that we don't have an extra several hundred
MB of copied ISO around (and a simpler create_usb.py that doesn't have
to grope around trying to find its ISO)?
create_usb.py, 125: this really should use some tmpification to a
per-process name so that multiple create_usb's on the same machine don't
collide with each other. Probably something that usbgen should just
handle itself, really...
custom_script.py, 74: /dev/null for stdout and stderr means we don't
capture any output from the scripts in the logs. that seems bad.
grub_setup.py, 287, 340: this is probably no longer desirable, as recent
CR 6984779 has embedded this in the image
pkg_img_mod.py, 173: sure we don't want to capture output to at least
the detailed log?
pre_pkg_img_mod.py, 254: I'm a little confused here. If the vi->vim
symlink exists and vim is there, why replace it? I would assume the
reason to alter the link is when vim is not present (in which case, it
might make sense that it was run always, not just for AI). Note that
implementation of the mediated links proposal
http://mail.opensolaris.org/pipermail/pkg-discuss/2010-October/024604.html
would probably obsolete this problem, anyway.
distro_const.py
---------------
159 + others: perhaps put the log files into private per-run subdir of
/var/tmp so that collisions don't happen with multiple users on same machine
236: commensurate, perhaps?
477: since we're only accepting datasets now, seems like we can close
bug 3947?
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
