[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
[email protected]
