Matt,
Looks good now.
Thanks,
Sue
On 05/23/12 01:55 AM, Matt Keenan wrote:
Sue,
Thanks for the review, basically I agree with all your comments and have made
the relevant changes.
New webrev uploaded :
https://cr.opensolaris.org/action/browse/caiman/mattman/7165978
thanks
Matt
On 05/22/12 18:36, Sue Sohn wrote:
On 05/22/12 06:46 AM, Matt Keenan wrote:
Hi,
Can I get two pairs of eyes to look over this fix please:
Bug
http://monaco.us.oracle.com/detail.jsf?cr=7165978
7165978 AI should create VARSHARE with compression as per /var
Webrev:
https://cr.opensolaris.org/action/browse/caiman/mattman/7165978
This bug was uncovered via zones testing. The default manifest for
zones installation specifies a BE
option to turn on compression for all datasets created within that
zone. However as VARSHARE is not
within the BE it needs to inherit the compression property from var
dataset that's created within
the BE.
The changes I've made allow for a user to still specify compression
for var and VARSHARE to be
different via manually specification in an AI manifest. A warning
message is shown if they manually
set it, but installation will complete.
This CR also fixes a scenario for canmount not being set if VARSHARE
was specified in a manifest.
Testing:
- Build ISOS for AI, TI and GUI and tested full installs and verified
on first boot compression property set to the same value for var and
VARSHARE datasets.
- For AI and zones installs, I tried out a number of different
scenarios where compression is set with within BE or manually via
Filesystem elements for var and VARSHARE. Always ensuring that var
and VARSHARE are the same unless VARSHARE manually set.
- Ran full set of slim_source unit tests and no regressions found.
cheers
Matt
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
Hi Matt,
214 Suggest changing this comment as follows and removing the comment
from 221-222
# If compression is set on /var, ensure it's inherited by VARSHARE
# unless compression for VARSHARE was specified manually
223-233 Can the elif logic in this clause be simplified:
elif (var_comp is not None and varshare_comp is not None \
and var_comp != varshare_comp) or \
(var_comp is None and varshare_comp is not None):
->
elif var_comp != varshare_comp:
270 indentation looks off
285 fix comment
287 for key in opt_dict.keys(): -> for key in opt_dict:
294 This looks like it gets any option, not just compression. Also, add
that it returns None if option not set
297 fix comment
Sue
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss