Ethan Quach wrote:
> 
> 
> Evan Layton wrote:
>> Ethan Quach wrote:
>>>
>>>
>>> Evan Layton wrote:
>>>> jeanm wrote:
>>>>> Looks OK to me. Maybe some comments explaining the B_FALSE and what 
>>>>> it does?
>>>>> I looked in the zfs code but that's kind of awkward.
>>>>
>>>> Probably a good idea to add that since it's a bit convoluted...
>>>>
>>>> I've added:
>>>> /*
>>>>  * The boolean set to B_FALSE and passed to zfs_destroy_snaps() tells
>>>>  * zfs to process and destroy the snapshots now. Otherwise ZFS will
>>>>  * wait to destroy these snapshots until a zfs release is called and
>>>>  * the references for all the snapshots have been released.
>>>>  */
>>>
>>> nit - for the second sentence, "Otherwise the call will potentially
>>> return where the snapshot isn't actually destroyed yet, and ZFS is
>>> waiting until all the references to the snapshot have been released
>>> before actually destroying the snapshot."
>>>
>>
>> Much better! I'll make that change.
> 
> Hmm, I may have made an incorrect assumption on the behavior
> though.  Will the call actually return and ZFS waits, or wait before
> returning?

If I understand the comments and the code correctly, ZFS returns without
actually destroying the snapshots. It waits to do that until all the
references are released and the zfs_release has been called.

-evan

> 
> 
> -ethan
> 
> 
>>
>> Thanks!
>> -evan
>>
>>>
>>> The changes look fine.
>>>
>>>
>>> thanks,
>>> -ethan
>>>
>>>
>>>>
>>>> Does that look OK?
>>>>
>>>> Thanks!
>>>> -evan
>>>>
>>>>>
>>>>> Jean
>>>>>
>>>>> Evan Layton wrote:
>>>>>>
>>>>>> I need to get a code review for this high priority (but simple)
>>>>>> bug fix:
>>>>>>
>>>>>> http://cr.opensolaris.org/~evanl/10807/
>>>>>>
>>>>>> This problem was caused by the fix for ZFS RFE 6803121.
>>>>>> (http://bugs.opensolaris.org/view_bug.do?bug_id=6803121)
>>>>>>
>>>>>> Since this fix went into build 121 you won't see this issue until
>>>>>> after you've updated. What this causes is the inability to delete
>>>>>> snapshots through libbe. This means that beadm destroy will fail.
>>>>>> As far as the impact on pkg(5), we don't cause pkg to actually fail
>>>>>> however the temporary snapshots created when doing a pkg install or
>>>>>> uninstall are not cleaned up.
>>>>>>
>>>>>> Unit testing of the bug is complete but the automated test suite
>>>>>> is still running. I will _not_ be pushing this until review
>>>>>> comments are resolved and the automated tests have completed
>>>>>> successfully...
>>>>>>
>>>>>> The other thing to note is that once this is pushed builds of
>>>>>> slim_source will fail on anything before snv 121. I will send
>>>>>> out a heads up message when I push these changes.
>>>>>>
>>>>>> Thanks,
>>>>>> -evan
>>>>>> _______________________________________________
>>>>>> 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
>>


Reply via email to