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."


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