Bug 14552 - samba.service launches as a foreground process but doesn't tell systemd about its startup
Summary: samba.service launches as a foreground process but doesn't tell systemd about...
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: AD: LDB/DSDB/SAMDB (show other bugs)
Version: 4.13.0
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
Depends on:
Reported: 2020-10-24 13:51 UTC by Alexander Bokovoy
Modified: 2020-11-03 12:36 UTC (History)
1 user (show)

See Also:

patch for v4.13 branch (3.98 KB, patch)
2020-10-27 07:50 UTC, Alexander Bokovoy
asn: review+

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Bokovoy 2020-10-24 13:51:30 UTC
When systemd launches samba service, the configuration we have in samba.service expects that the main process (/usr/sbin/samba) would use sd_notify() to report back its status. However, we only use sd_notify() when running become_daemon().

As a result, samba process never reports back its status and the status updates from other daemons (smbd, winbindd, etc) are not accepted as we now have implied NotifyAccess=main since 

commit d1740fb3d5a72cb49e30b330bb0b01e7ef3e09cc
Author: Marcos Mello <marcosfrm@gmail.com>
Date:   Fri May 8 07:44:51 2020 -0300

    Drop NotifyAccess=all from systemd units
    The implicit NotifyAccess=main is enough since Samba daemons do not
    fork()/exit() anymore under systemd.
    Signed-off-by: Marcos Mello <marcosfrm@gmail.com>
    Reviewed-by: Andreas Schneider <asn@samba.org>
    Reviewed-by: Alexander Bokovoy <ab@samba.org>

This leads to a timeout and killing samba process by systemd. Situation is reproducible in Fedora 33, for example.

I think we should change source4/smbd/server.c:binary_smbd_main() from

        if (opt_daemon) {
                DBG_NOTICE("Becoming a daemon.\n");
                become_daemon(opt_fork, opt_no_process_group, false);
        if (opt_daemon) {

to the code that still reports its status to the systemd:

        if (opt_daemon) {
                DBG_NOTICE("Becoming a daemon.\n");
                become_daemon(opt_fork, opt_no_process_group, false);
        } else {
                daemon_status("samba", "Starting process...");

This would give a behavior expected by systemd from such services.
Comment 1 Alexander Bokovoy 2020-10-24 14:01:45 UTC
Fix is included into https://gitlab.com/samba-team/samba/-/merge_requests/1643
Comment 2 Alexander Bokovoy 2020-10-24 14:04:47 UTC
I think we also need to fix this for smbd, nmbd, and winbindd because they also do not report their status when running in foreground under systemd.
Comment 3 Jeremy Allison 2020-10-24 16:37:55 UTC
Sounds good to me. Can you update the MR with those extra fixes and I'll review !
Comment 4 Alexander Bokovoy 2020-10-24 17:24:04 UTC
I did already. Please see the MR
Comment 5 Jeremy Allison 2020-10-25 00:24:04 UTC
Oh sorry. I'll take a look Monday ! Feel free to assign to me for review.
Comment 6 Jeremy Allison 2020-10-26 18:06:21 UTC
Hmmm. I think we only need patch #2 of that set for systemd fixes. The first patch looks part of a separate bug to me.

However, Andreas has already reviewed patch #1 so I'll push with his review for patch #1, and both of our reviews for patch #2.
Comment 7 Alexander Bokovoy 2020-10-26 18:24:58 UTC
Yes, both are required for working Samba on newer Linux distributions.
Comment 8 Samba QA Contact 2020-10-26 19:59:07 UTC
This bug was referenced in samba master:

Comment 9 Alexander Bokovoy 2020-10-27 07:50:46 UTC
Created attachment 16311 [details]
patch for v4.13 branch
Comment 10 Andreas Schneider 2020-10-27 08:15:49 UTC
Karolin, please apply the patch to 4.13. Thanks!
Comment 11 Karolin Seeger 2020-10-27 08:21:43 UTC
Pushed to autobuild-v4-13-test.
Comment 12 Samba QA Contact 2020-10-27 10:51:36 UTC
This bug was referenced in samba v4-13-test:

Comment 13 Karolin Seeger 2020-10-30 12:15:21 UTC
Closing out bug report.

Comment 14 Samba QA Contact 2020-11-03 12:36:49 UTC
This bug was referenced in samba v4-13-stable (Release samba-4.13.2):