Ethan Quach wrote:
> 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?
>
Fine.
Thanks
Tim
>
> 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
>>
