Discussion:
[developer] REVIEW REQUEST: 4574 get_clones_stat does not call zap_count in non-debug kernel
Alexander Stetsenko
2014-02-06 05:50:15 UTC
Permalink
Hi Folks,

I've found bug in get_clones_stat function.
ASSERT0 is used to call zap_count(...), as result zap_count(...) is
never called in non-DEBUG kernel.
As result "count" variable is always 0, and "goto fail" is always reached.
This means get_clones_stat function never makes up list of clones for
"clones" properties.
The fix is simple - just to replace ASSERT0 on VERIFY0.

Please, review this simple fix.

The patch was already reviewed by Matt.
I need a second reviewer.

issue: https://illumos.org/issues/4574
webrev: http://cr.illumos.org/~webrev/ams/4574/

Thanks.


-------------------------------------------
illumos-developer
Archives: https://www.listbox.com/member/archive/182179/=now
RSS Feed: https://www.listbox.com/member/archive/rss/182179/21175072-86d49504
Modify Your Subscription: https://www.listbox.com/member/?member_id=21175072&id_secret=21175072-abdf7b7e
Powered by Listbox: http://www.listbox.com
Marcel Telka
2014-02-06 11:27:41 UTC
Permalink
Post by Alexander Stetsenko
webrev: http://cr.illumos.org/~webrev/ams/4574/
LGTM.
--
+-------------------------------------------+
| Marcel Telka e-mail: ***@telka.sk |
| homepage: http://telka.sk/ |
| jabber: ***@jabber.sk |
+-------------------------------------------+


-------------------------------------------
illumos-developer
Archives: https://www.listbox.com/member/archive/182179/=now
RSS Feed: https://www.listbox.com/member/archive/rss/182179/21175072-86d49504
Modify Your Subscription: https://www.listbox.com/member/?member_id=21175072&id_secret=21175072-abdf7b7e
Powered by Listbox: http://www.listbox.com
Garrett D'Amore
2014-02-06 15:36:59 UTC
Permalink
Ship it.

-- 
Garrett D'Amore
Sent with Airmail
Post by Alexander Stetsenko
webrev: http://cr.illumos.org/~webrev/ams/4574/
LGTM.

--
+-------------------------------------------+
| Marcel Telka e-mail: ***@telka.sk |
| homepage: http://telka.sk/ |
| jabber: ***@jabber.sk |
+-------------------------------------------+


-------------------------------------------
illumos-developer
Archives: https://www.listbox.com/member/archive/182179/=now
RSS Feed: https://www.listbox.com/member/archive/rss/182179/21239177-3604570e
Modify Your Subscription: https://www.listbox.com/member/?&
Powered by Listbox: http://www.listbox.com



-------------------------------------------
illumos-developer
Archives: https://www.listbox.com/member/archive/182179/=now
RSS Feed: https://www.listbox.com/member/archive/rss/182179/21175072-86d49504
Modify Your Subscription: https://www.listbox.com/member/?member_id=21175072&id_secret=21175072-abdf7b7e
Powered by Listbox: http://www.listbox.com
George Wilson
2014-02-06 15:43:31 UTC
Permalink
LGTM.
Post by Garrett D'Amore
Ship it.
--
Garrett D'Amore
Sent with Airmail
Post by Marcel Telka
Post by Alexander Stetsenko
webrev: http://cr.illumos.org/~webrev/ams/4574/
LGTM.
--
+-------------------------------------------+
| homepage: http://telka.sk/ |
+-------------------------------------------+
-------------------------------------------
illumos-developer
Archives: https://www.listbox.com/member/archive/182179/=now
https://www.listbox.com/member/archive/rss/182179/21239177-3604570e
Modify Your Subscription: https://www.listbox.com/member/?&
Powered by Listbox: http://www.listbox.com
*illumos-zfs* | Archives
<https://www.listbox.com/member/archive/182191/=now>
<https://www.listbox.com/member/archive/rss/182191/22008002-303f2ff4>
| Modify
<https://www.listbox.com/member/?&>
Your Subscription [Powered by Listbox] <http://www.listbox.com>
-------------------------------------------
illumos-zfs
Archives: https://www.listbox.com/member/archive/182191/=now
RSS Feed: https://www.listbox.com/member/archive/rss/182191/23047029-187a0c8d
Modify Your Subscription: https://www.listbox.com/member/?member_id=23047029&id_secret=23047029-2e85923f
Powered by Listbox: http://www.listbox.com
Matthew Ahrens
2014-02-06 20:59:08 UTC
Permalink
Looks good. Thanks for finding this, Alex.

--matt


On Wed, Feb 5, 2014 at 9:50 PM, Alexander Stetsenko <
Post by Alexander Stetsenko
Hi Folks,
I've found bug in get_clones_stat function.
ASSERT0 is used to call zap_count(...), as result zap_count(...) is never
called in non-DEBUG kernel.
As result "count" variable is always 0, and "goto fail" is always reached.
This means get_clones_stat function never makes up list of clones for
"clones" properties.
The fix is simple - just to replace ASSERT0 on VERIFY0.
Please, review this simple fix.
The patch was already reviewed by Matt.
I need a second reviewer.
issue: https://illumos.org/issues/4574
webrev: http://cr.illumos.org/~webrev/ams/4574/
Thanks.
-------------------------------------------
illumos-zfs
Archives: https://www.listbox.com/member/archive/182191/=now
RSS Feed: https://www.listbox.com/member/archive/rss/182191/
21635000-ebd1d460
Modify Your Subscription: https://www.listbox.com/
member/?&
Powered by Listbox: http://www.listbox.com
-------------------------------------------
illumos-zfs
Archives: https://www.listbox.com/member/archive/182191/=now
RSS Feed: https://www.listbox.com/member/archive/rss/182191/23047029-187a0c8d
Modify Your Subscription: https://www.listbox.com/member/?member_id=23047029&id_secret=23047029-2e85923f
Powered by Listbox: http://www.listbox.com

Loading...