Ethan Quach wrote:
> 
> 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 }

fixed

> 
> 169-199 - This whole chunk needs to be encapsulated inside a
>    if (GLOBAL) {
> 
>    }

moved

> 
> 215-221, 241-245 - These two chunks seem like they could be
> moved up into the if (GLOBAL) {} from 169.

moved

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

refactored this so that the promote's are now done with calls to 
be_promote_ds_callback and are no longer in set_callback (where they never 
really should have been in the first place...).

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

done

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

I renamed be_promote_sub_ds_callback() to be_promote_ds_callback() However I 
left be_promote_zone_ds() as is since it's doing a bunch of zones lookup to 
find 
the zones and the correct zones datasets to promote before calling
be_promote_ds_callback().

> 
> 
> 
> be_create.c:
> ---------------
> 1880 - future

fixed

> 
> 1878-1883 - this comment needs to be removed.  It doesn't apply to
> this section of code.

fixed

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

I removed them.

> 
> 1944 - A comment stating this is getting the uuid of the newly cloned
> parent BE would be nice.

comment added

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

Reply via email to