Ethan,

Thanks for the review.


On 05/22/12 21:17, Ethan Quach wrote:
Hi Matt,

172 - Question: Is it indeed required that /var/share live in the root pool?

As var share is specified to be "in_be", and a BE can only reside on the root pool then yes it has to be on the root pool.

266 - With this line being moved here, are we missing the setting for canmount for /var now?


Setting of "canmount" on /var was never required and is not being done anywhere. It's only set on VARSHARE.

For cases where VARHSARE is in the manifest, a user must also explicitly specify compression=<something>, for our code to not glean that property from /var, is that correct?


If VARSHARE is specified in the manifest and it does NOT have a specifiic compression value set, then our code
will only attempt to set compression on VARSHARE :
  - if Specified on /var
  - if Specifiied on BE

Specification on /var Filesystem element takes precedence over BE element.


Nit -- The code paths starting at 211 and 266 are now getting a bit unwieldy, and they seem to try to handle the same thing -- setting the Options for the filesystem at hand. Is there any way we can consolidate these paths?


We are attempting to set the Options in two slightly different scenarios e.g. VARSHARE exists or VARSHARE does not exist. I've tried adding utility functions as best I can, and I feel what's there at the moment is sufficient. As pointed out to be before it's actually more efficient in Python to not have too many utility methods.

cheers

Matt


thanks,
-ethan


On 05/22/12 06:46, 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
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss

_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss

Reply via email to