Evan Layton wrote:
> Hi Moriah,
> 
> The changes looks pretty good. I do have one comment though. It appears 
> that we have redundant includes of instzones_api.h. Since we include 
> instzones_api.h in libbe_priv.h we shouldn't need to have this included 
> in be_activate.c, be_create.c, be_mount.c and be_zones.c. I know this 
> was already in issue in this code but it would probably be good to clean 
> this up while you're working in this area.
> 

I cleaned up those redundant includes. Thanks!

-Moriah


> Other than that I think the changes are fine.
> 
> Thanks!
> 
> -evan
> 
> Moriah Waterland wrote:
>> Evan,
>>
>> I'd appreciate a code review for the fix:
>>    Bug 10687 - Remove libinstzones from slim_source
>>       http://defect.opensolaris.org/bz/show_bug.cgi?id=10687
>>
>>
>> My webrev is at:
>>    http://cr.opensolaris.org/~mwaterl/f10687
>>
>>
>> There is a little extra noise in the webrev because I also cleaned up
>> the permissions for the files under usr/src/pkgdefs/SUNWinstall-libs. I
>> also pulled the libspmizones links from SUNWinstall-libs/prototype_com.
>> These links had been retained because the pkgcmds in the Legacy Install
>> gate needed them, but this is no longer necessary. The SVr4 packaging
>> source in ON links to the same libinstzones as this fix is used by the
>> slim_source libraries.
>>
>> FYI: This fix will result in a flag day for anyone maintaining a build
>> machine for slim_source. A new package, SUNWinstallint must be installed
>> on build servers. This package has already been installed on
>> indiana-build.central and indiana-build-sp.central.
>>
>> I have uploaded the package to:
>>    http://dlc.sun.com/osol/install/downloads/current/
>>
>> If your interested, this is what SUNWinstallint delivers:
>>   f none usr/include/instzones_api.h 644 root bin
>>   s none usr/lib/libinstzones.so=./libinstzones.so.1
>>
>> Testing
>> --------
>> I completed unit tests.  I built an image and ran basic sanity testing
>> (verified installation was functional).  I also verified that libbe,
>> libtd, and libti all link to the correct version of libinstzones (which
>> lives in: /usr/lib).
>>
>>
>>
>> thanks,
>>
>> ----
>> Moriah Waterland
> 

Reply via email to