Hi. I''ve some ZFS patches, maybe you''d find them useful: http://people.freebsd.org/~pjd/opensolaris/00.patch http://people.freebsd.org/~pjd/opensolaris/01.patch http://people.freebsd.org/~pjd/opensolaris/02.patch http://people.freebsd.org/~pjd/opensolaris/03.patch http://people.freebsd.org/~pjd/opensolaris/04.patch http://people.freebsd.org/~pjd/opensolaris/05.patch http://people.freebsd.org/~pjd/opensolaris/06.patch http://people.freebsd.org/~pjd/opensolaris/07.patch -- Pawel Jakub Dawidek http://www.wheel.pl pjd at FreeBSD.org http://www.FreeBSD.org FreeBSD committer Am I Evil? Yes, I Am! -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 187 bytes Desc: not available URL: <http://mail.opensolaris.org/pipermail/zfs-code/attachments/20060821/a4b5a87e/attachment.bin>
Pawel Jakub Dawidek wrote:> Hi. > > I''ve some ZFS patches, maybe you''d find them useful: > > http://people.freebsd.org/~pjd/opensolaris/00.patch > http://people.freebsd.org/~pjd/opensolaris/01.patch > http://people.freebsd.org/~pjd/opensolaris/02.patch > http://people.freebsd.org/~pjd/opensolaris/03.patch > http://people.freebsd.org/~pjd/opensolaris/04.patch > http://people.freebsd.org/~pjd/opensolaris/05.patch > http://people.freebsd.org/~pjd/opensolaris/06.patch > http://people.freebsd.org/~pjd/opensolaris/07.patchGreat. What problems do they solve? James C. McPherson
Are you willing to talk about what you''re working on? To summarize, is
the following an accurate description of the fixes?
00, 01, 02:
Geunine bugs. Note that 01 was fixed by 6456642 in build 46.
03:
Comparison of 0 == NULL. Is this on a system where NULL is
#defined to be ((void *)0)?
04:
Our compilers are a little more forgiving.
05:
Not sure what this is about. Does the compiler not support
mathematical operations on constants in a case statement?
Although I like the if() else form much better for stylistic
reasons.
06:
Can you explain this one?
07:
Other platforms don''t have { 0 } indicate a default initialized
mutex (we''ve seen this porting other pieces of software as well).
At this point, I''m not sure whether we need to treat these patches as
contributions or just bug reports. I''ll ping someone more familiar
with
the process to find out whether you need to agree to the Sun Contributor
Agreement at:
http://www.opensolaris.org/os/about/sun_contributor_agreement
- Eric
--
Eric Schrock, Solaris Kernel Development http://blogs.sun.com/eschrock
On Tue, Aug 22, 2006 at 07:46:21AM +1000, James C. McPherson wrote:> Pawel Jakub Dawidek wrote: > >Hi. > >I''ve some ZFS patches, maybe you''d find them useful: > > http://people.freebsd.org/~pjd/opensolaris/00.patch > > http://people.freebsd.org/~pjd/opensolaris/01.patch > > http://people.freebsd.org/~pjd/opensolaris/02.patch > > http://people.freebsd.org/~pjd/opensolaris/03.patch > > http://people.freebsd.org/~pjd/opensolaris/04.patch > > http://people.freebsd.org/~pjd/opensolaris/05.patch > > http://people.freebsd.org/~pjd/opensolaris/06.patch > > http://people.freebsd.org/~pjd/opensolaris/07.patch > > > Great. What problems do they solve?I thought they are quite straightforward, but some comments will be in place, I agree: 00.patch - missing new line character 01.patch - /dev/zfs open failure is not properly detected and will not be reported properly (at least here) 02.patch - returning NULL here is wrong, as I will be read by the caller as success (0) 03.patch - some pointers-integers confusion fixes 04.patch - those values are too big to be handled as ''int'', at least this is what gcc says 05.patch - switch() can work with ''int'' and those values are too big 06.patch - signal arc_reclaim_thread, so we don''t have to wait 1 second before cv_timedwait() return 07.patch - there are plently of locks which are not initialized nor destroyed and it seems it is not detected on Solaris -- Pawel Jakub Dawidek http://www.wheel.pl pjd at FreeBSD.org http://www.FreeBSD.org FreeBSD committer Am I Evil? Yes, I Am! -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 187 bytes Desc: not available URL: <http://mail.opensolaris.org/pipermail/zfs-code/attachments/20060821/d5f751e1/attachment.bin>
On Mon, Aug 21, 2006 at 02:52:28PM -0700, Eric Schrock wrote:> 04: > > Our compilers are a little more forgiving.In the interest of encouraging porting, however, it would be nice if you''d consider taking this one anyway. Ceri PS The fact that Pawel and I share a domain name should not be interpreted as anything other than a coincidence that we''re both subscribed here! -- That must be wonderful! I don''t understand it at all. -- Moliere -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 187 bytes Desc: not available URL: <http://mail.opensolaris.org/pipermail/zfs-code/attachments/20060821/59c6656e/attachment.bin>
On Mon, Aug 21, 2006 at 02:52:28PM -0700, Eric Schrock wrote:> Are you willing to talk about what you''re working on? [...]Soon:)> [...] To summarize, is > the following an accurate description of the fixes?[...]> 03: > Comparison of 0 == NULL. Is this on a system where NULL is > #defined to be ((void *)0)?Yes, which seems to be more correct - NULL is a pointer, while 0 is not.> 05: > > Not sure what this is about. Does the compiler not support > mathematical operations on constants in a case statement? > Although I like the if() else form much better for stylistic > reasons.gcc warns, that those values are too big to be used in switch/case, it can only handle ints.> 06: > > Can you explain this one?Explained in the other mail.> 07: > > Other platforms don''t have { 0 } indicate a default initialized > mutex (we''ve seen this porting other pieces of software as well).Actually, on FreeBSD we initialize with some magic value to detect missing initializations, multiple initializations with destruction, etc.> At this point, I''m not sure whether we need to treat these patches as > contributions or just bug reports. I''ll ping someone more familiar with > the process to find out whether you need to agree to the Sun Contributor > Agreement at: > > http://www.opensolaris.org/os/about/sun_contributor_agreementI think those changes are too minor bother anyone:) -- Pawel Jakub Dawidek http://www.wheel.pl pjd at FreeBSD.org http://www.FreeBSD.org FreeBSD committer Am I Evil? Yes, I Am! -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 187 bytes Desc: not available URL: <http://mail.opensolaris.org/pipermail/zfs-code/attachments/20060822/d0c85318/attachment.bin>
Absolutely. I wasn''t suggesting otherwise, just trying to clarify what each patch was about. It''s interesting because we compile ON with gcc, but must be using a different set of warning flags. - Eric On Mon, Aug 21, 2006 at 11:00:41PM +0100, Ceri Davies wrote:> On Mon, Aug 21, 2006 at 02:52:28PM -0700, Eric Schrock wrote: > > > 04: > > > > Our compilers are a little more forgiving. > > In the interest of encouraging porting, however, it would be nice if > you''d consider taking this one anyway. > > Ceri > > PS The fact that Pawel and I share a domain name should not be > interpreted as anything other than a coincidence that we''re both > subscribed here! > -- > That must be wonderful! I don''t understand it at all. > -- Moliere-- Eric Schrock, Solaris Kernel Development http://blogs.sun.com/eschrock
On Mon, Aug 21, 2006 at 03:11:27PM -0700, Eric Schrock wrote:> Absolutely. I wasn''t suggesting otherwise, just trying to clarify what > each patch was about. It''s interesting because we compile ON with gcc, > but must be using a different set of warning flags.OK, thank you. Ceri -- That must be wonderful! I don''t understand it at all. -- Moliere -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 187 bytes Desc: not available URL: <http://mail.opensolaris.org/pipermail/zfs-code/attachments/20060821/75743a20/attachment.bin>
Pawel - I''ve filed the following bugs, which should cover all but the cv_signal() patch (which I need to look at again): 6463348 ZFS code could be more portable 6463349 error message from zpool(1M) is missing a newline 6463350 getcomponent() has spurious and wrong check Thansk for the patches! Let us know about any other snags or bugs you hit during your work. - Eric -- Eric Schrock, Solaris Kernel Development http://blogs.sun.com/eschrock