Tim,
Thanks for looking at this...
Tim Knitter wrote:
> Ethan,
>
> Ignore the previously sent email. Case of the early morning fat finger
> syndrome. :-(
>
> be_snapshot.c:
>
> 452 /* Get the SPA version of the pool where this dataset resides */
> 455 "get SPA version for %s: %s\n"), zfs_get_name(zhp),
>
>
> From just reading the code it isn't clear what SPA is. Could you spell
> it out once in the comment and/or the error message printed?
>
I'll just change this to "ZFS pool version" to be clear.
> 464 if (version >= 12) {
>
> "version" could stand to be a little more descriptive.
>
How's pool_version instead?
thanks,
-ethan
> Other than that it looks fine.
>
> Thanks
> Tim
>
>
>
>> 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
>>
> _______________________________________________
> caiman-discuss mailing list
> caiman-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
>
