Bug 13129 - The samba daemons should not double fork when started by systemd
The samba daemons should not double fork when started by systemd
Status: RESOLVED FIXED
Product: Samba 4.1 and newer
Classification: Unclassified
Component: AD: LDB/DSDB/SAMDB
4.7.1
All All
: P5 normal
: ---
Assigned To: Karolin Seeger
Samba QA Contact
:
: 10801 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2017-11-10 08:27 UTC by Andreas Schneider
Modified: 2017-12-22 17:03 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

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!