On Tue, May 31, 2011 at 10:55:03PM -0600, Susan Kamm-Worrell wrote:
>
>
> 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

can you provide some examples?  i thought that brand options can be
intermixed with zoneadm options, and zoneadm will just pass unknown
options onto the brand callback.

> 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]
>
>

well, cli processing in general looks like:

        <command> <options> <arguments>

i really really really don't like the idea of supporting:

        <command> <options> <arguments> <brand options>

aside from being a bad idea in general, doing this will prevent us from
ever being able to add another argument to the clone subcommand.

wrt arc, i'm sorry i missed this.  but we can fix this with a quick
email to the case alias.  also, if you think that fixing this will put
you at risk of missing this build then i'd say we just drop the clone
changes from this wad, update the arc case, and deliver the clone
changes next build.  (i doubt AI depends on these clone changes.)

> >- 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
>

please add a comment saying we're not creating the ROOT/export datasets.

> >- 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.
>

cool.  so this is more transitory stuff.  that makes me feel better.

>
> >- 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.
>

but i thought the -c option allows the user to specify a new sysconfig
profile.  i didn't think we were going to support installing old style
sysidconfig profiles with this same option.  do we really need to do
that?  why not just generate an error if -c or -m are specified and the
new sysconfig mechanism aren't active?  (it seems like we're delivering
new features for the old framework that we're about to rip out anyway.)

ed
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to