[EMAIL PROTECTED] writes:
> >  http://cr.grommit.com/~carlsonj/webrev-zones-features/

Thanks for the comments!

> usr/src/cmd/zoneadm/zoneadm.c
> 
>       Around line 286 - Shouldn't the "[-u uuid-match]" option be
>       documented under usage()?  Also, in long_help() there appears
>       to be a reference to "-z <zone>" which presumedly should be
>       augmented with the -u availability.

Agreed; I'll add both of those.

>       In general, "[-u uuid-match]" is available regardless of the
>       subcommand, right?  This wasn't clear from the original PSARC
>       material which seemed to tie it to "list" (which is why I asked
>       the question originally about whether the "-u" should be an
>       option to "list" rather than "zoneadm" itself).

Yes, it works the same with all of the subcommands, in any context
that would work with -z.  But it makes the most sense with "list," as
a way to find the name of the zone you want to deal with.

>       Around line 2485 - It appears you're skipping the "lofs"
>       verification for the "state incomplete" operation.  I'm not
>       sure why any of the verification makes sense for this
>       operation.  Or is this change actually necessary - when do we
>       pass through verify_details() for this command?

You're quite right; we don't pass through there.  I'll remove that
errant check.

>       Around line 1746 - Should there be a check around the switch
>       statement near the end of sanity_check() to verify that you can
>       only mark a zone "incomplete" from the "installed" state?

I don't think it's an error if the zone is already incomplete, but I
suppose I agree that going from "configured" state to "incomplete"
would be strange.  (The more I think about it, the less clear the
distinction between "incomplete" and "configured" becomes ...)

I'll add a check here; it'll need to be "installed" or better to
change state to "incomplete" -- just like with the existing detach,
move, ready, boot, and mount subcommands.

>       Upon further reflection, I realize now that the command "state"
>       doesn't really work as a verb in this context.  I wish that we
>       had called the command "set-state" or "setstate" or even
>       better, "mark".  Thoughts?

I can change it to "setstate".  I'll update the Zulu Install gate and
the ARC materials.

> usr/src/cmd/zonecfg/zonecfg.c
> 
>       Around line 843 - As -R is currently Contracted Consolidation
>       Private, we probably should not be including it in the
>       help/usage text.

Yep, I agree.  (At one point, I was headed towards making it public.)

>       Around lines 4639 and 4645 - There are some issues with the
>       fprintf() statements here.  In the second one, you're passing
>       "execname" but it's not accounted for by the format string.  I
>       think in both cases you should probably just be using zerr()
>       without passing in "execname".

Yes.  This is a cut-n-paste error.  (Zerr doesn't seem to print out
the excname, which is irritating when errors are printed from deep
within scripts, but I'll just go with the custom here.)

> usr/src/head/libzonecfg.h
> 
>       Around lines 114, 115, 119 and 120 - It would be nice to
>       provide comments indicating which user names these [ug]ids map
>       to.

OK.

> usr/src/lib/libzonecfg/common/libzonecfg.c
> 
>       Around line 833 - Can this strncmp() become confused if
>       "zonecfg_root" is a substring of "zonepath".  For example,
>       imagine the case of
> 
>       global# zonecfg -R /alt/roo set -F zonepath=/alt/root/zones/mypath

Well, now that's twisted.  ;-}

Yes, that would allow a bad path through.  I've changed it like this:

        len = strlen(zonecfg_root);
-->     if (strncmp(zonepath, zonecfg_root, len) != 0 || zonepath[len] != '/')
                return (Z_BAD_PROPERTY);
        zonepath += len;

This has the nice side-effect of making sure that the path is absolute
in all cases.

I've made the requested changes, done some testing, and updated the
webrev.  The new webrev is here:

  http://zhadum.east/export/build1/ws/carlsonj/zones-features/webrev3/

-- 
James Carlson, KISS Network                    <[EMAIL PROTECTED]>
Sun Microsystems / 1 Network Drive         71.232W   Vox +1 781 442 2084
MS UBUR02-212 / Burlington MA 01803-2757   42.496N   Fax +1 781 442 1677
_______________________________________________
zones-discuss mailing list
zones-discuss@opensolaris.org

Reply via email to