Discussion:
Válasz: [zfs] review needed
Richard Kojedzinszky
2014-02-13 02:56:07 UTC
Permalink
Dear Matt,

I am not an Illumos developer, even I haven't compiled it at all. I was fixing FreeBSD's ZFS port when noticed this leak. And under FreeBSD the leak could be reproduced. I've also sent a mail to advocates@ a month ago and was told the same you wrote to me, get two reviewers, and a test suite. Unfortunately I even cannot compile an Illumos kernel now.

Regards,

KÃŒldve az én HTC-mről

----- Reply message -----
Feladó: "Matthew Ahrens" <***@delphix.com>
Címzett: "illumos-zfs" <***@lists.illumos.org>
Tárgy: [zfs] review needed
Dátum: Cs, febr. 13, 2014 00:30
Anyone else care to review this one-liner?The fix seems obviously correct by inspection, but what testing have you done? Ideally you would reproduce the problem (the vnode hold leak) and then test again with the fix in place and see that the problem does not reproduce.Assuming the testing is good, your next step is to submit an RTI by emailing the illumos advocates. See http://wiki.illumos.org/display/illumos/How+To+Contribute#HowToContribute-5SubmittingAPatch--mattOn Wed, Feb 12, 2014 at 5:29 AM, Richard Kojedzinszky <***@cflinux.hu> wrote:Is there someone also to review/approve?
2014-02-10 19:31 időpontban Matthew Ahrens ezt &iacute;rta:
Looks good to me.
--matt
On Mon, Feb 10, 2014 at 6:53 AM, <***@cflinux.hu> wrote:
Dear ZFS team,
I found that in zfs/zfs_ctldir.c the snapshot_inactive() function
obtains a hold on its parent directory with gfs_dir_lookup(), but
does not drop that hold when in the meantime other thread uses the
actual snapshot vnode.
Please review the attached patch to eliminate this leak.
Thanks in advance,
Richard Kojedzinszky
-------------------------------------------
illumos-zfs
Archives: https://www.listbox.com/member/archive/182191/=now [1]
RSS Feed:https://www.listbox.com/member/archive/rss/182191/21635000-ebd1d460
[2]
Modify Your Subscription:https://www.listbox.com/member/?& [3]
Powered by Listbox: http://www.listbox.com [4]
ILLUMOS-ZFS | Archives [1] [5] | Modify [6] Your Subscription [4]
Links:
------
[1] https://www.listbox.com/member/archive/182191/=now
[2] https://www.listbox.com/member/archive/rss/182191/21635000-ebd1d460
[3] https://www.listbox.com/member/?&amp;
[4] http://www.listbox.com
[5] https://www.listbox.com/member/archive/rss/182191/25402478-0858cafa
[6]https://www.listbox.com/member/?&
--
Richard Kojedzinszky
-------------------------------------------
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

| Modify
Your Subscription


-------------------------------------------
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
Jim Klimov
2014-03-05 10:53:00 UTC
Permalink
Post by Richard Kojedzinszky
Dear Matt,
I am not an Illumos developer, even I haven't compiled it at all. I was
fixing FreeBSD's ZFS port when noticed this leak. And under FreeBSD the
ago and was told the same you wrote to me, get two reviewers, and a test
suite. Unfortunately I even cannot compile an Illumos kernel now.
I believe there was a port of VirtualBox to FreeBSD in progress which
was reported by users to be "quite stable" a few years ago. Can you
use this or some other VM solution to install an illumos-based distro
for compilation of the illumos-gate (perhaps OpenIndiana as the
simplest one to get started with, thanks in part to details on its
Wiki; possibly one of its "hipster" ISO releases which are based on
recent kernels). While a VM is is not a fast way to build (may take
a day to compile), it is a functional one indeed.

I know this is not an answer you may have wanted to hear, and may be
a de-motivator for integrations work, but if any errors during illumos
compilation and/or testing come up, you as the coder might better be
the first one to know and fix.

Alternately, I hope someone on the lists can help with the compilation
and testing and review, which would also "add points" to submission of
the RTI. Of course, it is wrong to *expect* of the advocates to do this
for you. They are busy enough with day-jobs to review and accept just
the completed jobs (or return them for re-working until completed).

HTH,
//Jim
Paul Kraus
2014-03-06 13:49:33 UTC
Permalink
Post by Jim Klimov
Post by Richard Kojedzinszky
Dear Matt,
I am not an Illumos developer, even I haven't compiled it at all. I was
fixing FreeBSD's ZFS port when noticed this leak. And under FreeBSD the
ago and was told the same you wrote to me, get two reviewers, and a test
suite. Unfortunately I even cannot compile an Illumos kernel now.
I believe there was a port of VirtualBox to FreeBSD in progress which
was reported by users to be "quite stable" a few years ago.
VirtualBox is available for FreeBSD in the ports collection in the emulators directory. I have been using it for over a year under FreeBSD 9.0, 9.1, 9.2, and real soon now 10.x and it has been very stable. If you need help installing it please see http://www.freebsd.org/doc/en_US.ISO8859-1/books/handbook/virtualization-host.html

--
Paul Kraus
Deputy Technical Director, LoneStarCon 3
Sound Coordinator, Schenectady Light Opera Company

Loading...