Comments, mostly on libbe ...
General comment - Since we don't yet support boot environments
within a zone, beadm should fail gracefully when run in a zone, like:
# beadm <subcommand>
beadm is not supported in a zone.
rather then trying to do something and failing miserably. Most of
the if(GLOBAL_ZONEID) checks we have in libbe now are really
just primers for when we build in that support.
usr/src/man/beadm.1m.txt
----------------------------------
333 - SUNWinstall -> SUNWbeadm
messages.py
----------------
126 - BE_ERR_NO_ACTIVE_ROOT should be in here.
be_activate.c
-----------------
136 - initialize to { 0 }
169-199 - This whole chunk needs to be encapsulated inside a
if (GLOBAL) {
}
215-221, 241-245 - These two chunks seem like they could be
moved up into the if (GLOBAL) {} from 169.
With some refactoring, the zfs_promote() calls in set_canmount()
could probably be ripped out and replaced by calls to be_promote_zone_ds()
(renamed to something more generic like be_promote_ds() of course).
I never understood why the zfs_promote() calls were hidden in the
set_canmount() function anyway. If we don't get to this per the code
review changes, please file a bug to track this.
972-1023 - This whole chunk can be removed if you replace 1025-1026
with a direct call to the callback giving it the root dataset of the
zone BE.
i.e. replace 1025-1026 with:
if (be_promote_sub_ds_callback(z_zhp, NULL) != 0) {
1028 - remove "subordinate"
Rename the following functions to be more generic:
be_promote_zone_ds() --> be_promote_ds()
be_promote_sub_ds_callback() --> be_promote_ds_callback()
be_create.c:
---------------
1880 - future
1878-1883 - this comment needs to be removed. It doesn't apply to
this section of code.
1884 - within the context of this function - that is, code running in the
global zone that's processing non-global zones - this if() can be removed,
along with 1895.
1944 - A comment stating this is getting the uuid of the newly cloned
parent BE would be nice.
2772-2774 - Missing function block comment.
libbe.h:
---------
117 - "not" -> "no"
177 - Remove this line; its not used anywhere.
thanks,
-ethan
Evan Layton wrote:
> Hello All,
>
> We're down to the wire on the zone support changes to SNAP upgrade and are
> looking for code review comments. We'll be taking comments up until COB
> Tuesday
> October 7th. Your comments are as always welcome and appreciated.
>
> Defect 3686 is the blocker bug that was submitted to cover this work and the
> webrev is available at:
>
> http://cr.opensolaris.org/~equach/webrev.snap_zones/
>
> Thank you in advance for your comments and help!
> -evan
> _______________________________________________
> zones-discuss mailing list
> zones-discuss@opensolaris.org
>
_______________________________________________
zones-discuss mailing list
zones-discuss@opensolaris.org
