Hi Ethan, Overall this looks pretty good. One thing I did wonder about is the use of the number 12 for checking the supported ZFS version. I was wondering if it might be better to define that in libbe_priv.h so that if in the future we end up with the need to limit things to another ZFS version we can just change it there.
Thanks! -evan Ethan Quach wrote: > Defect: > --------- > http://defect.opensolaris.org/bz/show_bug.cgi?id=1197 > > Webrev: > ---------- > http://cr.opensolaris.org/~equach/webrev.1197 > > > This fix changes the auto snapshot name such that the cleanup policy > type is no longer stored in the snapshot name. The cleanup policy is > now stored as a user property in the snapshot dataset itself. > > I've successfully tested the cases for providing a non-default cleanup > policy names, and not providing one at all (which causes fallback to setting > the default cleanup policy.) > > A more interesting scenario is backwards compatibility with existing > boot environment snapshots that don't have the cleanup policy set in > a user property. For this case, the code no longer parses the cleanup > policy > type out of the snapshot name, and will inherit it from its parent dataset. > Since auto-cleanup is not implemented yet, the only policy ever set was the > default, so this shouldn't result in any problems. > > An even more interesting scenario is when the pool has not been upgraded > to a zfs version that supports snapshot user properties. To handle > this, the > pool version is always checked to determine if it supports snapshot user > properties. If it does not, then attempting to set a non-default cleanup > policy will result in a failure - the user must upgrade their zfs version to > to be able to do this. Currently, there is no way to set a non-default user > property (auto-cleanup has not been implemented yet), so this does not > result in any issues. When auto-cleanup is implemented, users will be > required to upgrade their pools to a version that supports snapshot user > properties to use the new feature. > > > thanks, > -ethan > > _______________________________________________ > caiman-discuss mailing list > caiman-discuss at opensolaris.org > http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
