Bug 13129 - The samba daemons should not double fork when started by systemd
Summary: The samba daemons should not double fork when started by systemd
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: AD: LDB/DSDB/SAMDB (show other bugs)
Version: 4.7.1
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
: 10801 (view as bug list)
Depends on:
Blocks:
 
Reported: 2017-11-10 08:27 UTC by Andreas Schneider
Modified: 2018-02-07 08:37 UTC (History)
6 users (show)

See Also:

Attachments
patch for 4.7 (124 bytes, patch)
2017-11-28 15:31 UTC, Andreas Schneider 		no flags Details
patch for 4.7 (6.28 KB, patch)
2017-11-28 15:32 UTC, Andreas Schneider 		ab: review+
Details
patch for 4.7 (10.75 KB, patch)
2017-11-29 10:56 UTC, Andreas Schneider 		ab: review+
Details
possible patch for master (988 bytes, patch)
2017-12-19 03:34 UTC, Andrew Bartlett 		no flags Details
Backport regression fix for 4.7 (1.20 KB, patch)
2017-12-19 22:53 UTC, Garming Sam 		abartlet: review+
Details
Small systemd fix (1.40 KB, patch)
2018-02-04 18:19 UTC, Marcos Mello 		no flags Details
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Schneider 2017-11-10 08:27:05 UTC 
The samba daemons should not double fork when started by systemd. The problem in notify mode if we double fork, systemd is sending SIGTERM to the process so the startup fails.

They need to be started with -foreground and --no-process-group.

The 'samba' binary doesn't support that yet. Patches will follow.
Comment 1 Andrew Bartlett 2017-11-14 10:51:08 UTC 
Take care (and example) of how selftest.pl starts Samba.  It would be great if this was more consistent, without breaking existing configurations.
Comment 2 Andreas Schneider 2017-11-28 15:31:32 UTC 
Created attachment 13816 [details]
patch for 4.7
Comment 3 Andreas Schneider 2017-11-28 15:32:22 UTC 
Created attachment 13817 [details]
patch for 4.7
Comment 4 Alexander Bokovoy 2017-11-28 16:30:57 UTC 
Comment on attachment 13817 [details]
patch for 4.7

LGTM
Comment 5 Alexander Bokovoy 2017-11-28 16:31:40 UTC 
Comment on attachment 13817 [details]
patch for 4.7

LGTM
Comment 6 Andreas Schneider 2017-11-28 16:36:46 UTC 
Karolin, please add the patch to 4.7. Thanks.
Comment 7 Karolin Seeger 2017-11-29 07:50:45 UTC 
(In reply to Andreas Schneider from comment #6)
Pushed to autobuild-v4-7-test.
Comment 8 Karolin Seeger 2017-11-29 08:21:54 UTC 
(In reply to Karolin Seeger from comment #7)
Patch breaks the build:

[2010/4210] Compiling source4/smbd/server.c
../source4/smbd/server.c: In function ‘binary_smbd_main’:
../source4/smbd/server.c:454:27: error: ‘opt_no_process_group’ undeclared (first use in this function)
   become_daemon(opt_fork, opt_no_process_group, false);
                           ^
../source4/smbd/server.c:454:27: note: each undeclared identifier is reported only once for each function it appears in
Waf: Leaving directory `/memdisk/kseeger/a47/b1867796/samba/bin'

Re-assigning to Andreas.
Comment 9 Andreas Schneider 2017-11-29 10:56:01 UTC 
Created attachment 13821 [details]
patch for 4.7

Ups, the patchset is two small. Two commits missing. Correct version is attached now. Sorry.
Comment 10 Alexander Bokovoy 2017-11-29 11:29:01 UTC 
Comment on attachment 13821 [details]
patch for 4.7

Ok, looks good now.
Comment 11 Andreas Schneider 2017-11-29 15:05:25 UTC 
Karolin, this time it should work :-)
Comment 12 Karolin Seeger 2017-11-30 08:50:37 UTC 
(In reply to Andreas Schneider from comment #11)
Patchset does not apply on current v4-7-test branch.

The last patch "systemd: Start processes in forground and without a
 process group" has already been pushed (I only removed the patch that broke the build obviously).

Now I pushed your patch after removing the last patch to autobuild-v4-7-test.

To be on the safe side, please double-check.

Thanks!
Comment 13 Marcos Mello 2017-12-10 12:25:48 UTC 
*** Bug 10801 has been marked as a duplicate of this bug. ***
Comment 14 Andrew Bartlett 2017-12-19 03:34:29 UTC 
Created attachment 13875 [details]
possible patch for master
Comment 15 Garming Sam 2017-12-19 22:53:51 UTC 
Created attachment 13878 [details]
Backport regression fix for 4.7
Comment 16 Andrew Bartlett 2017-12-19 22:55:48 UTC 
Please take the additional patch for 4.7
Comment 17 Karolin Seeger 2017-12-20 19:45:24 UTC 
(In reply to Andrew Bartlett from comment #16)
Pushed additional patch to autobuild-v4-7-test.
Comment 18 Karolin Seeger 2017-12-22 17:03:58 UTC 
Pushed to v4-7-test.
Closing out bug report.

Thanks!
Comment 19 Marcos Mello 2018-02-04 18:19:16 UTC 
Created attachment 13940 [details]
Small systemd fix

Now that --foreground is default in service files, fix status message passing to systemd.
Comment 20 Andreas Schneider 2018-02-07 08:37:48 UTC 
Marcos,

please open a new bug for this and you need to sign-off the patch, see README.Contributing.


  Andreas