Ethan,
Comments interspersed.
>
> 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.
>
I'll file a bug for this.
> 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
>
>
fixed. Nice catch BTW.
> messages.py
> ----------------
> 126 - BE_ERR_NO_ACTIVE_ROOT should be in here.
>
I don't see it anywhere else in libbe. Can you point out where it is used?
>
> 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.
>
fixed.
Thanks
Tim
>
>
> 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 at opensolaris.org
> >
> _______________________________________________
> caiman-discuss mailing list
> caiman-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/caiman-di
> scuss
--
This message posted from opensolaris.org
