Bug 8850 - rely on sys/inotify.h for inotify
Summary: rely on sys/inotify.h for inotify
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.0
Classification: Unclassified
Component: File services (show other bugs)
Version: unspecified
Hardware: All Linux
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on: 9428
Blocks: 8622
  Show dependency treegraph
 
Reported: 2012-04-09 18:18 UTC by Adrian Bunk
Modified: 2012-12-11 11:04 UTC (History)
2 users (show)

See Also:


Attachments
rely on sys/inotify.h for inotify (6.07 KB, patch)
2012-04-09 18:18 UTC, Adrian Bunk
no flags Details
Back-port for 3.6.x. (5.67 KB, patch)
2012-04-13 17:52 UTC, Jeremy Allison
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Adrian Bunk 2012-04-09 18:18:19 UTC
Created attachment 7441 [details]
rely on sys/inotify.h for inotify

sys/inotify.h was added to glibc 2.4 in 2006.
Comment 1 Jeremy Allison 2012-04-13 17:48:30 UTC
Pushed to autobuild. I will add a back-port for 3.6.x.
Jeremy.
Comment 2 Jeremy Allison 2012-04-13 17:52:36 UTC
Created attachment 7452 [details]
Back-port for 3.6.x.

Bjorn please review for 3.6.next.
Comment 3 Björn Jacke 2012-05-04 13:17:20 UTC
Comment on attachment 7452 [details]
Back-port for 3.6.x.

looks correct but I'm not sure if we should get this into 3.6. This makes things clearly cleaner but apart from that it doesn't buy us anything. It might even make inotify stop from being used on systems with kernels that support it but that have a glibc that does not support it yet. For that reason I would vote to not get this in 3.6. Or did I miss a point where we get adavantage from this change?
Comment 4 Björn Jacke 2012-11-26 09:10:18 UTC
after Volker found bug 9428 with this bug, I had another look at this one. We should definetely not get this in to 3.6 because that would mean that older glibc versions will lose inotify support. RHEL4 would be affected for example.

Further we should make sure that we either

- add to the 4.0 release notes that RHEL4 and distributions of the same age will have no file change notification any more

or

- that part of this patch should be re-added again.

(imho we should not simply drop support for this functionality for RHEL4 and distributions that are "just" 5 years old)
Comment 5 Björn Jacke 2012-11-27 09:55:33 UTC
Jeremy, I think we should revert this for now. Later on we can make a better patch which just uses the glibc inotify_init() when that is available.
Comment 6 Adrian Bunk 2012-11-27 10:05:13 UTC
(In reply to comment #5)
> Jeremy, I think we should revert this for now. Later on we can make a better
> patch which just uses the glibc inotify_init() when that is available.

"just uses the glibc inotify_init() when that is available" is what my patch does. When glibc ships sys/inotify.h, then inotify_init() is available in glibc.

(Except for the now fixed brown paper bag bug #9428, for which I apologize.)
Comment 7 Björn Jacke 2012-11-27 11:00:22 UTC
you have a different meaning of "just". Your patch removes our inotify_init() so that RHEL4 etc. lose inotify support.
Comment 8 Adrian Bunk 2012-11-27 11:20:21 UTC
(In reply to comment #7)
> you have a different meaning of "just". Your patch removes our inotify_init()
> so that RHEL4 etc. lose inotify support.

I am not a native English speaker, I read your sentence as "simply use glibc inotify_init() when that is available, and don't do anything else" - seems I misread it.

In any case, I agree with you that this change is not a good idea for 3.6.

(BTW: RHEL4 is not 5 but 7 years old, and since it's regular lifecycle support has ended, it is only on limited extended lifecycle support now.)
Comment 9 Andrew Bartlett 2012-12-07 09:06:15 UTC
The 4.0 release seems as reasonable a point as any to drop support for direct syscalls for this.  We shouldn't carry these workarounds forever. 

I propose to unblock on this, rather than make a late revert.
Comment 10 Björn Jacke 2012-12-10 17:55:47 UTC
we have more workarounds for much older systems but if this is what we want, then Karo should add to the release notes that RHEL4 and some older distis will lose inotify support and then just close this bug.
Comment 11 Karolin Seeger 2012-12-11 11:04:40 UTC
Added a hint to the release notes of Samba 4.0.0.
Closing out bug report.

Thanks!