Bug 8777 - Add sys_statvfs() wrapper support for OpenBSD/FreeBSD/DragonFly
Add sys_statvfs() wrapper support for OpenBSD/FreeBSD/DragonFly
Status: RESOLVED FIXED
Product: Samba 3.6
Classification: Unclassified
Component: Build environment
3.6.3
All OpenBSD
: P5 normal
: ---
Assigned To: Volker Lendecke
Samba QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-02-24 20:33 UTC by Brad Smith
Modified: 2012-08-27 12:56 UTC (History)
1 user (show)

See Also:


Attachments
sys_statvfs() wrapper support for OpenBSD/FreeBSD/DragonFly (2.66 KB, patch)
2012-02-24 20:33 UTC, Brad Smith
no flags Details
sys_statvfs() wrapper support for OpenBSD/FreeBSD/DragonFly (3.15 KB, patch)
2012-02-24 22:00 UTC, Brad Smith
vl: review+
jra: review+
Details
sys_statvfs() wrapper support for OpenBSD/FreeBSD/DragonFly try #2 (2.92 KB, patch)
2012-03-01 06:56 UTC, Brad Smith
no flags Details
Improvements to statvfs() wrapper function. (3.97 KB, patch)
2012-03-11 05:02 UTC, Brad Smith
no flags Details
Improvements to statvfs() wrapper function. (1.73 KB, patch)
2012-03-11 05:05 UTC, Brad Smith
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Brad Smith 2012-02-24 20:33:18 UTC
Created attachment 7343 [details]
sys_statvfs() wrapper support for OpenBSD/FreeBSD/DragonFly

The attached patch adds sys_statvfs() wrapper support for OpenBSD/FreeBSD/DraonFly.
Comment 1 Brad Smith 2012-02-24 22:00:34 UTC
Created attachment 7344 [details]
sys_statvfs() wrapper support for OpenBSD/FreeBSD/DragonFly
Comment 2 Volker Lendecke 2012-02-25 17:59:53 UTC
Comment on attachment 7344 [details]
sys_statvfs() wrapper support for OpenBSD/FreeBSD/DragonFly

Looks good to me. Jeremy, can you ack it for 3.6.next?
Comment 3 Jeremy Allison 2012-02-27 20:19:11 UTC
Comment on attachment 7344 [details]
sys_statvfs() wrapper support for OpenBSD/FreeBSD/DragonFly

LGTM.
Comment 4 Jeremy Allison 2012-02-27 20:19:52 UTC
Reassigning to Karolin for inclusion in 3.6.next.
Jeremy.
Comment 5 Karolin Seeger 2012-02-28 19:46:56 UTC
Pushed to v3-6-test. Will be included in the next release.
Closing out bug report.

Thanks!
Comment 6 Guenther Deschner 2012-02-29 17:07:19 UTC
Are waf configure checks dealt with as well ? I think that would be important not to forget.
Comment 7 Brad Smith 2012-03-01 06:56:59 UTC
Created attachment 7353 [details]
sys_statvfs() wrapper support for OpenBSD/FreeBSD/DragonFly try #2
Comment 8 Brad Smith 2012-03-01 07:00:40 UTC
The initial patch although built Ok it would have also attempted to build on NetBSD which we don't want (no statfs() for recent versions and different struct members for older NetBSD). The newly attached patch reverts the changes I made to the existing autoconf statfs() check and adds another statfs() check that'll do the right thing and this ensures only building on OpenBSD/FreeBSD/DragonFly.
Comment 9 Volker Lendecke 2012-03-02 00:32:41 UTC
Karolin, would it be possible that you revert a0d51949abde68134eb35150d797387a1fb57ab7 from v3-6-test? I did test this on FreeBSD, but it makes the build fail on NetBSD. This patch needs to grow a bit in master and is not ready for 3.6.

Sorry for the confusion, I should have tested on many more platforms before I give my blessing for this patch. I now ha ve installed NetBSD and can verify further patches.

Volker
Comment 10 Volker Lendecke 2012-03-02 15:48:10 UTC
(In reply to comment #8)
> The initial patch although built Ok it would have also attempted to build on
> NetBSD which we don't want (no statfs() for recent versions and different
> struct members for older NetBSD). The newly attached patch reverts the changes
> I made to the existing autoconf statfs() check and adds another statfs() check
> that'll do the right thing and this ensures only building on
> OpenBSD/FreeBSD/DragonFly.

With http://git.samba.org/?p=samba.git;a=commitdiff;h=6c1c092f079492d3 and its two parents I have pushed a patchset to master that works fine for me on OpenBSD, FreeBSD, NetBSD and Linux. I have not tried Dragonfly. Can you verify that this patchset works for you?
Comment 11 Karolin Seeger 2012-03-02 19:34:24 UTC
(In reply to comment #9)
> Karolin, would it be possible that you revert
> a0d51949abde68134eb35150d797387a1fb57ab7 from v3-6-test? 

Of course. Has been pushed a few minutes ago (14fe979a).
Comment 12 Volker Lendecke 2012-03-02 19:40:28 UTC
(In reply to comment #11)
> (In reply to comment #9)
> > Karolin, would it be possible that you revert
> > a0d51949abde68134eb35150d797387a1fb57ab7 from v3-6-test? 
> 
> Of course. Has been pushed a few minutes ago (14fe979a).

Thanks!
Comment 13 Brad Smith 2012-03-11 05:02:46 UTC
Created attachment 7375 [details]
Improvements to statvfs() wrapper function.
Comment 14 Brad Smith 2012-03-11 05:05:57 UTC
Created attachment 7376 [details]
Improvements to statvfs() wrapper function.
Comment 15 Brad Smith 2012-03-11 05:11:34 UTC
The attached patch adds support for the statvfs() wrapper to indicate read-only FS's as well as quota's support on NetBSD and HP-UX. The vfs_default code was changed to always call the statvfs wrapper function instead of doing it conditionally based on the ifdef's and only uses the FS capabilities field if
the result indicates successful completion.
Comment 16 Volker Lendecke 2012-03-11 11:38:44 UTC
(In reply to comment #15)
> The attached patch adds support for the statvfs() wrapper to indicate read-only
> FS's as well as quota's support on NetBSD and HP-UX. The vfs_default code was
> changed to always call the statvfs wrapper function instead of doing it
> conditionally based on the ifdef's and only uses the FS capabilities field if
> the result indicates successful completion.

Thanks -- pushed to master. For the future -- could you submit your patches in "git format-patch" form?

Volker
Comment 17 Brad Smith 2012-03-12 13:38:49 UTC
Ok. Will do.

I think this stuff is ready to go back to 3.6-test.

I'm not sure what the process is but here are the commits
that were made to master.

71a6d334321d42fd0f02646d1649405ccf197c00
dcb1cd293364b5269aaf3b0ac0e475aeb18e9bab
8bdc2890999c850519913be0e829e9ced979ac2f
f0bba969d87f0fd5c7d3f1ba001a5c9a754cd7ed

I'm not sure if you want to bother with the typo fix but
IMO might as well..

341bd82fbf1f9209aae94bdbc6461805f826e39e
Comment 18 Karolin Seeger 2012-03-20 20:18:30 UTC
(In reply to comment #17)
> I think this stuff is ready to go back to 3.6-test.
> 
> I'm not sure what the process is but here are the commits
> that were made to master.
> 
> 71a6d334321d42fd0f02646d1649405ccf197c00
> dcb1cd293364b5269aaf3b0ac0e475aeb18e9bab
> 8bdc2890999c850519913be0e829e9ced979ac2f
> f0bba969d87f0fd5c7d3f1ba001a5c9a754cd7ed
> 
> I'm not sure if you want to bother with the typo fix but
> IMO might as well..
> 
> 341bd82fbf1f9209aae94bdbc6461805f826e39e

Volker,

as you have pushed these patches to the master branch, can I push them to v3-6-test? (Fishing for the explicit review confirmation ;-).

Thanks!

Karolin
Comment 19 Brad Smith 2012-04-18 21:21:37 UTC
This still needs to be dealt with..
Comment 20 Brad Smith 2012-06-09 10:45:10 UTC
ping.
Comment 21 Björn Jacke 2012-06-28 18:10:03 UTC
As Volker wrote, this still needs to grow in master before it can maybe be backported later on to 3.6. I just saw that those patches break Tru64, which seems to have a statvfs struct like Linux does but the ifdef's currently enable to the new BSD code.
Comment 22 Björn Jacke 2012-06-29 14:30:40 UTC
additional fixes and cleanups are now in.

7560b1cea6d2c0b2962f5802f724525fc0ec9bf9
b526a0d6730796057c5486bf07fbb6b1b79c92b4

Shall we backport the statvfs stuff to 3.6 or just close this one as fixed and let it come into the next major release?
Comment 23 Brad Smith 2012-08-24 21:05:22 UTC
I'd prefer to see this go into v3-6-test at this point.
Comment 24 Björn Jacke 2012-08-27 12:56:43 UTC
given that even with small looking changes we saw regressions in the 3.6 tree that needed a number of rounds to get fixed again (see the md5 symbol bug #9037), I think we should be strict with our own policy not to get new features into stable release branches. If the BSD maintainers want to backport the fixes to their ports collections they can do that of course. Upstream we should not. As this is fixed in master I'll close this as fixed now.