Ed,
Thanks for the review.
Responses below.
Susan
On 05/31/11 07:08 PM, Edward Pilatowicz wrote:
[ resend after dropping lots of quoted command output ]
- in clone, it seems weird that we're adding arguments after the
zonename, it seems like all arguments should come before the zonename.
For the rest of the commands the brand-specific-options come after all the
common command arguments. I arc'd the extension of clone with that
thought in mind:
(ipkg)
clone [-m copy] [-s zfs_snapshot] source_zone [-c sc_profile.xml | dir]
(s10)
clone [-m copy] [-s zfs_snapshot] source_zone [-c sysidcfg]
- it's really weird that unconfigure_zone() can reconfigure a zone.
can we rename it to reconfigure_zone() (and then if no argument is
supplied it doesn an unconfigure.
yes. This makes sense and adds to the readability of the code.
- in unconfigure_zone(), for the old sys-unconfig case, you are no
longer checking the error code being returned. (instead your check
the error code from the safe_copy())
Good catch - will fix.
- in pkgcreatezone, shouldn't we be checking the return codes from mktemp and
aimanifest?
yes -- will fix
- in pkgcreatezone, you do:
create_active_ds -r zone
what's the -r option mean. looking at the implementation of
create_active_ds in /usr/lib/brand/shared/common.ksh i don't see
support for a -r option. what am i missing?
Change was pushed into ON build 167 last week.
See file:///net/coupe/builds/sw83825/on-AIinstall-cr2/webrev.ON.3
- in pkgcreatezone, could you add some comments about how aimanifest is
used? it seems weird that you load a manifest, add entries, but never
save it. but i guess saving is implicit (because otherwise none of
this would be persistent)? so "load" is really copy? and the target
of the copy is AIM_MANIFEST?
Yes - will add comments
yes - saving is implicit
yes - load is a copy
yes - target is env variable AIM_MANIFEST
The environment variable AIM_MANIFEST contains the file where all the
changes will be made. The load operation loads that manifest
into the working file. The add operation adds the entries to the
working file per their dtd.
Let's hope for a more user-friendly interface to AI manifests
in future AI work.
- in pkgcreatezone, previously you updated this script to be aware of
both the old (sysidconfig) and new (sysconfig) mechanisms. (i don't
know if those names are right.) is this putback supposed to remove
support for the old mechanisms? or are we still supporting both? if
we're still supporting both when do we anticipate removing support for
the old one?
I don't know if the names are right either. I just use old sysconfig framework
vs. new sysconfig framework :)
No, this putback isn't to remove the old mechanisms. They need to stay in place
until the new SC framework is pushed (build 169) and is working. Then I can
file
a bug and remove the old interfaces.
Having both mechanisms in place has been a HUGE help to the SC group since they
are
able to test the new SC framework in a zone. I've converted several of them
to using zones since they are so much easier to clone/boot/destroy than setting
up a real system and then having it hang up due to some SC bug.
- why do we care if the svc:/milestone/unconfig:default service is
running in the gz? is that service modifying zone images?
No. That service will be enabled when the new SC framework goes live.
Parts of the SC framework have already been pushed to the install gate, but
the final push to enable the framework is enabling that unconfig milestone.
All the checks for this unconfig milestone will be removed once the new SC
framework is integrated and is working.
it seems poor to me that we're supporting two different ways of
handling sysconfig for zones. i'd much rather just have one way of
doing this. either just require the unconfig service always be online
and always use it to apply a config, or always ignore that service and
copy over the sysconfig so the zone can configure itself.
Going away as soon as the new SC works :)
Having both ways has given the zones code a lot of soak time with the
new SC framework.
- in pkgcreatezone, if the specified sysconfig file is
/usr/share/auto_install/sc_profiles/enable_sci.xml you don't copy it
in. so we silently ignore an option that the user passed us? this
seems bad. why don't we detect this situation and generate an error?
that'd be better than accepting the users option, ignoring them, and
then returning success.
This is an odd situation that will be fixed when the new SC framework goes live.
I need the code in place to use enable_sci.xml for the new SC framework so that
the SC team can test everything in a zone.
For the rest of us still running with the old SC framework, copying that
enable_sci.xml
file into /etc/sysidcfg really confuses the old sysidcfg parser and it prints a
lot of
alarming messages out. There's no failure, but the messages caused concern.
So, I'm checking for the common case of the zones-install service passing in the
enable_sci.xml file and to ignore that file if still using the old SC framework.
All of this mess will be removed soon.
I'll add more comments about this.
ed
On Tue, May 31, 2011 at 01:14:28PM -0600, Susan Kamm-Worrell wrote:
Quick update - testing is going well. Dave is running ZTS against
these latest changes.
I made some minor changes to the code and have updated the webrev at:
http://coupe/sw83825/ips-ipkgAIinstall/webrev.ipkg.1
Here's the output from the install. I've also attached the detailed
Install Log listed
in the install output.
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
