comments for the p2v script are below. ed ---------- src/brand/p2v
- in general, you only need to specify full paths to binaries if you haven't explicitly set PATH. since you've set PATH, you don't really need full paths to any binaries. (except of course ones run via zlogin.) - the usage() messages has a "patchid" parameter? - fix_net() checks STACK_TYPE, but it seems that STACK_TYPE doesn't get set until after fix_net() is called. - you check for the presence of /etc/hostname.* files, but you don't check for /etc/dhcp.* files? - why are you creating tmp files in /var/tmp instead of /tmp? - in p2v you do "umount $ZONEROOT", but if the zone had any "fs" resourced defined it's zonecfg, then those filesystems will be mounted under $ZONEROOT, and so this operation will fail. - fix_smf_pre_uoa() is a huge function that doesn't seem to be called from anywhere? - in multiple places you zlogin and run svcs. you might want to set LC_ALL=C before running svcs. - when running svcs you always pipe the output to nawk. why not just use the -o option? - the comment when you run sys-unconfig says you wait for 30 seconds, but the code loops up to 10 times and does a "sleep 10" each time. On Wed, Jun 10, 2009 at 01:18:01PM -0600, Jerry Jelinek wrote: > I need another code review for the p2v support for > zones using IPS. This is bug: > > 6793 p2v support for ipkg-branded zones > > This was reviewed once before, a couple of months ago, > but I didn't finish addressing the comments in time for > 2009.06. The main differences that should be reviewed > now are the new code in src/brand/p2v to fix the > pkg variant. These are lines 201-228. This is > a temporary solution until IPS supports changing > the variant. > > The other changes that should be looked at are > in src/brand/pkgcreatezone since I had to merge > this file and update it. In particular, the > -a option behavior will be changing with this > integration. Also, the src/brand/image_install > code has been updated to correspond with the native > brand code in ON. This code depends on ON code in b114 > so I won't be putting back until I know we're syncing > up with that build or later. > > The updated webrev is at: > > http://cr.opensolaris.org/~gjelinek/webrev.6793/ > > Thanks, > Jerry > _______________________________________________ > pkg-discuss mailing list > [email protected] > http://mail.opensolaris.org/mailman/listinfo/pkg-discuss _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
