Bug 13872 - gluster vfs modules break the build on some platforms
Summary: gluster vfs modules break the build on some platforms
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: VFS Modules (show other bugs)
Version: 4.9.5
Hardware: All All
: P5 regression (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-03-30 15:59 UTC by Björn Jacke
Modified: 2019-06-26 07:03 UTC (History)
7 users (show)

See Also:


Attachments
patch from master for v4.9 and v4.10 (1.22 KB, patch)
2019-04-16 15:20 UTC, Guenther Deschner
asn: review+
bjacke: review-
abartlet: review-
Details
patch from master for v4.9 and v4.10 (4.44 KB, patch)
2019-04-26 15:08 UTC, Guenther Deschner
slow: review+
Details
Followup patch from master for 4.10 and 4.9 (7.76 KB, patch)
2019-06-17 08:51 UTC, Guenther Deschner
vl: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Björn Jacke 2019-03-30 15:59:58 UTC
with 4.9.5 there came a new vfs module glusterfs_fuse, which breaks the build on some platforms, just experienced this on AIX. The gluster module use the NAME_MAX macro, which may NOT nbe expected to be defined by POSIX, see https://pubs.opengroup.org/onlinepubs/7908799/xsh/limits.h.html, you should use pathconf() to get the value of NAME_MAX using that. I'm also wondering if a the glusterfs modules shouldn't be only built on platforms there glusterfs is actually availble on.
Comment 1 Karolin Seeger 2019-04-02 07:03:10 UTC
ping
Comment 2 Guenther Deschner 2019-04-16 15:20:30 UTC
Created attachment 15074 [details]
patch from master for v4.9 and v4.10
Comment 3 Andreas Schneider 2019-04-16 17:06:14 UTC
Karolin, please add the patches to the relevant branches. Thanks!
Comment 4 Karolin Seeger 2019-04-17 07:17:54 UTC
Pushed to autobuild-v4-{10,9}-test.
Comment 5 Björn Jacke 2019-04-17 10:44:06 UTC
this patch defies NAME_MAX statucally to 255 in replace.h. This is the wrind approach. My initial comment already mentioned the corrwct approach, please undo the static setting to 255
Comment 6 Björn Jacke 2019-04-17 10:48:28 UTC
in addition to that please do not add this module to the default modules to build for all (also glusterfs unsupported) platforms
Comment 7 Andrew Bartlett 2019-04-23 08:04:47 UTC
(In reply to Björn Jacke from comment #5)
I agree, replace.h is entirely the wrong way to solve a missing define like this.  

If this were in something glusterfs only, that might make more sense, but then one would look at the actual code and wonder why NAME_MAX is in use at all.

Samba got rid of fstring deliberately, this is just brining it back, badly.

Guenther,

The quick way out of this is to just move that to the top of the modules, defining GLUSTER_NAME_MAX to NAME_MAX or 255, but for master, please rework the code to use a dynamically allocated buffer, eg talloc_asprintf() on the input and either pathconf() or an ERANGE detecting loop on the output side.
Comment 8 Karolin Seeger 2019-04-25 09:17:04 UTC
ping

This currently blocks 4.9.7...
Comment 9 Guenther Deschner 2019-04-26 15:08:48 UTC
Created attachment 15099 [details]
patch from master for v4.9 and v4.10
Comment 10 Karolin Seeger 2019-04-29 08:58:30 UTC
(In reply to Guenther Deschner from comment #9)
Pushed to autobuild-v4-{10,9}-test.
Thanks!
Comment 11 Karolin Seeger 2019-05-06 11:16:28 UTC
(In reply to Karolin Seeger from comment #10)
Pushed to both branches, thanks!
Re-assigning to Günther due to Björn's request not to build on systems that do not support gluster.
Comment 12 Andrew Bartlett 2019-06-11 10:54:37 UTC
Assigning to Karolin for the next release.
Comment 13 Guenther Deschner 2019-06-17 08:51:18 UTC
Created attachment 15250 [details]
Followup patch from master for 4.10 and 4.9

Following Volker's advice off-list, follow-up patch (reverting the old one) using a simple re-define has been pushed upstream. The old patch was also not working as pathconf() did not deal well with "." directory causing both vfs modules not to operate correctly at all.
Comment 14 Karolin Seeger 2019-06-20 09:41:46 UTC
(In reply to Guenther Deschner from comment #13)
Pushed to autobuild-v4-{10,9}-test.
Comment 15 Karolin Seeger 2019-06-26 07:03:18 UTC
(In reply to Karolin Seeger from comment #14)
Pushed to both branches.
Closing out bug report.

Thanks!