Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

samba: don't restart smbd in samba-autoshare #1919

Merged
merged 1 commit into from Sep 19, 2017

Conversation

@escalade
Copy link
Contributor

commented Aug 24, 2017

The samba-autoshare script that takes care of sharing mounted drives restarts smbd every time udevil mounts a disk. This can lead to a Samba restart frenzy on boot with several connected disks. This is unnecessary as Samba does not need to be restarted to pick up changes to shares in smb.conf.

This PR modifies samba-autoshare to simply regenerate the configuration (samba-config) and then extend the configuration according to LE settings (smbd-config).

@MilhouseVH

This comment has been minimized.

Copy link
Contributor

commented Aug 24, 2017

While it's mostly true that if you simply update /run/samba/smb.conf with a new share without restarting smbd the new share will - most of the time - be picked up automatically by smbd (although it may require a refresh on the client, and thanks to caching may not always appear immediately - sometimes several minutes).

However, that's not always the case, and sometimes a newly added share will refuse to be listed on the client - sometimes for several minutes - until smbd is restarted.

In addition, when a drive is unmounted although the share will be removed from /run/samba/smb.conf any connected clients will continue to see the disconnected share as listed and available (again, server-side/smbd caching), until smbd is restarted.

A "lingering" disconnected share could have catastrophic consequences - if a drive is unmounted without restarting smbd, the share will remain visible to the client but now appear entirely empty (as it's just the contents of the /var/media/<mount> mount-point directory, and not the mounted device). If that share formerly contained movies that are in the media library, and the user runs a video library clean after the drive is disconnected (and while the share is still connected) then all the content for the share would be deleted from the library (whereas if the share is properly disconnected and not available, Kodi will skip the database clean for that share).

Simply updating the config without restarting smbd will - thanks to server-side caching - result in shares not always appearing immediately, (sometimes they will, sometimes they won't), and disconnected shares lingering for several minutes.

The only way to reliably add or remove a share is to restart smbd, unfortunately.

So while we will see a storm of service stop/starts at startup (hopefully lessened to some extent by recent changes), I think it's a necessary evil to avoid listing non-existent shares (which could be dangerous, leading to potential data loss), or not immediately listing new shares (which can be very frustrating for the user who wants to access their new share etc.).

@MilhouseVH

This comment has been minimized.

Copy link
Contributor

commented Aug 24, 2017

Note: I've been testing with a Windows 7 client, which behaves exactly as described above.

@escalade

This comment has been minimized.

Copy link
Contributor Author

commented Aug 24, 2017

According to the manual:

"The configuration file, and any files that it includes, are automatically reloaded every minute, if they change. You can force a reload by sending a SIGHUP to the server. Reloading the configuration file will not affect connections to any service that is already established. Either the user will have to disconnect from the service, or smbd killed and restarted."

So to speed up the configuration refresh one could use SIGHUP, but yes any already established connection will probably have the same configuration as when the client connected. Doing a complete restart doesn't seem like a good reason to me either way, for the above reason and also that any ongoing file operations to a share will be interrupted.

I don't see how you can avoid these "lingering" shares that you mention, as normally samba-autoshare is triggered by udev add/remove -> udevil-mount service. A user could manually unmount a drive and the share would still linger with or without this PR.

@escalade escalade force-pushed the escalade:sambafix branch from 5eec0f2 to c734502 Aug 24, 2017
@escalade

This comment has been minimized.

Copy link
Contributor Author

commented Aug 24, 2017

Added a HUP.

@@ -21,6 +21,8 @@ if [ -f /storage/.cache/services/samba.conf ]; then
. /storage/.cache/services/samba.conf

if [ "$SAMBA_AUTOSHARE" == "true" ] ; then
systemctl restart smbd.service

This comment has been minimized.

Copy link
@stefansaraev

stefansaraev Aug 24, 2017

Contributor

if possible, systemctl reload smbd.service should be used and re-runing samba-config and smbd-config should be moved to additional ExecReload=s in smbd.service

[Service]
Type=forking
...
ExecReload=/usr/lib/samba/smbd-config
ExecReload=/usr/bin/systemctl restart samba-config.service
ExecReload=/bin/kill -HUP $MAINPID
...

sorta

This comment has been minimized.

Copy link
@escalade

escalade Aug 24, 2017

Author Contributor

Good suggestion, that also gives the user a clean way of reloading after manual changes to the configuration. I'll test it and revise.

This comment has been minimized.

Copy link
@escalade

escalade Aug 25, 2017

Author Contributor

This doesn't work as intended. Udevil mounts drives before samba is started, and a service can't be reloaded until it's active:

Aug 25 14:06:03 NUC samba-autoshare[412]: smbd.service is not active, cannot reload.
Aug 25 14:06:03 NUC systemd[1]: smbd.service: Unit cannot be reloaded because it is inactive.
Aug 25 14:06:03 NUC samba-autoshare[410]: smbd.service is not active, cannot reload.
Aug 25 14:06:03 NUC systemd[1]: smbd.service: Unit cannot be reloaded because it is inactive.
Aug 25 14:06:03 NUC samba-autoshare[662]: smbd.service is not active, cannot reload.

It's trying to reload three times, once for System and Flash, then once for my external drive. As the reload doesn't trigger, the external drive isn't shared once Samba starts. If I trigger a reload after, it works as intended.

Not sure if it's possible/feasible to get udev/udevil to start after Samba. The PR as is works satisfactory.

This comment has been minimized.

Copy link
@MilhouseVH

MilhouseVH Aug 25, 2017

Contributor

It's trying to reload three times, once for System and Flash

Is your tree up to date? udevil-mount should already ignore /system and /flash (and any other already mounted partition), see after #1800

The PR as is works satisfactory.

Yes, the addition of pkill -HUP smbd has addressed my original concerns - thanks (and @stefansaraev).

This comment has been minimized.

Copy link
@escalade

escalade Aug 27, 2017

Author Contributor

Ah, hadn't seen that change. I was testing this on 8.1 actually.

@MilhouseVH

This comment has been minimized.

Copy link
Contributor

commented Aug 25, 2017

Just occasionally udevil will manage to mount a drive before smbd has started: http://sprunge.us/GIJT

This means smbd isn't in a state to handle the HUP signal, which causes smbd to fail.

This is with one external drive (/dev/sda1, "Images") - installed at the time of booting - where the partition is mounted while smbd is still starting:

Aug 25 16:59:55 rpi22 systemd[1]: Starting Samba NMB Daemon...
Aug 25 16:59:55 rpi22 systemd[1]: Starting Samba SMB Daemon...
Aug 25 16:59:55 rpi22 systemd[1]: Created slice system-udevil\x2dmount.slice.
Aug 25 16:59:55 rpi22 systemd[1]: Starting Udevil mount service...
Aug 25 16:59:55 rpi22 udevil[503]: Mounted /dev/sda1 at /media/Images
Aug 25 16:59:55 rpi22 systemd[1]: smbd.service: PID file /run/samba/smbd.pid not readable (yet?) after start: No such file or directory
Aug 25 16:59:55 rpi22 systemd[1]: Failed to start Samba SMB Daemon.
Aug 25 16:59:55 rpi22 systemd[1]: smbd.service: Unit entered failed state.
Aug 25 16:59:55 rpi22 systemd[1]: smbd.service: Failed with result 'protocol'.
Aug 25 16:59:55 rpi22 systemd[1]: Started Udevil mount service.
Aug 25 16:59:56 rpi22 nmbd[523]: [2017/08/25 16:59:56.054985,  0] ../lib/util/become_daemon.c:135(daemon_status)
Aug 25 16:59:56 rpi22 systemd[1]: Started Samba NMB Daemon.
Aug 25 16:59:56 rpi22 nmbd[523]:   STATUS=daemon 'nmbd' : No local IPv4 non-loopback interfaces available, waiting for interface ...NOTE: NetBIOS name resolution is not supported for Internet Protocol Version 6 (IPv6).
Aug 25 16:59:58 rpi22 systemd[1]: smbd.service: Service hold-off time over, scheduling restart.
Aug 25 16:59:58 rpi22 systemd[1]: Stopped Samba SMB Daemon.
Aug 25 16:59:58 rpi22 systemd[1]: Starting Samba Configuration...
Aug 25 16:59:58 rpi22 systemd[1]: Started Samba Configuration.
Aug 25 16:59:58 rpi22 systemd[1]: Starting Samba SMB Daemon...
Aug 25 16:59:58 rpi22 systemd[1]: Started Samba SMB Daemon.

Reboot often enough and you should hit this condition.

The following works:

  [ -f /run/samba/smbd.pid ] && pkill -HUP smbd

which results in:

Aug 25 16:59:53 LibreELEC systemd[1]: Starting Samba Configuration...
Aug 25 16:59:55 rpi22 systemd[1]: Starting Samba SMB Daemon...
Aug 25 16:59:55 rpi22 systemd[1]: Starting Samba NMB Daemon...
Aug 25 16:59:55 rpi22 systemd[1]: Created slice system-udevil\x2dmount.slice.
Aug 25 16:59:55 rpi22 systemd[1]: Starting Udevil mount service...
Aug 25 16:59:55 rpi22 udevil[518]: Mounted /dev/sda1 at /media/Images
Aug 25 16:59:55 rpi22 systemd[1]: Started Udevil mount service.
Aug 25 16:59:56 rpi22 nmbd[538]: [2017/08/25 16:59:56.079530,  0] ../lib/util/become_daemon.c:135(daemon_status)
Aug 25 16:59:56 rpi22 nmbd[538]:   STATUS=daemon 'nmbd' : No local IPv4 non-loopback interfaces available, waiting for interface ...NOTE: NetBIOS name resolution is not supported for Internet Protocol Version 6 (IPv6).
Aug 25 16:59:56 rpi22 systemd[1]: Started Samba NMB Daemon.
Aug 25 16:59:56 rpi22 systemd[1]: Started Samba SMB Daemon.
Aug 25 16:59:56 rpi22 smbd[539]: [2017/08/25 16:59:56.299571,  0] ../source3/passdb/pdb_smbpasswd.c:249(startsmbfilepwent)
Aug 25 16:59:56 rpi22 smbd[539]:   startsmbfilepwent_internal: file /run/samba/smbpasswd did not exist. File successfully created.
Aug 25 16:59:56 rpi22 smbd[539]: [2017/08/25 16:59:56.341096,  0] ../lib/util/become_daemon.c:124(daemon_ready)
Aug 25 16:59:56 rpi22 smbd[539]:   STATUS=daemon 'smbd' finished starting up and ready to serve connections

Alternatively, maybe there's some additional systemd magic to ensure that udevil-mount starts after smbd, but this needs to take into account that Samba is an optional feature (SAMBA_SUPPORT="yes" etc.).

However as the pid file check seems to work, I'd go with that...

@escalade escalade force-pushed the escalade:sambafix branch from c734502 to a1a60e9 Aug 26, 2017
@escalade

This comment has been minimized.

Copy link
Contributor Author

commented Aug 27, 2017

The pid file check has been added.

@MilhouseVH

This comment has been minimized.

Copy link
Contributor

commented Sep 18, 2017

@escalade anything more to add on this? It's been in my builds for 2-3 weeks without any issue.

@escalade

This comment has been minimized.

Copy link
Contributor Author

commented Sep 19, 2017

Nothing to add, works fine here as well.

@MilhouseVH

This comment has been minimized.

Copy link
Contributor

commented Sep 19, 2017

OK thanks - removing WIP label.

@MilhouseVH MilhouseVH merged commit 837cbfa into LibreELEC:master Sep 19, 2017
@escalade escalade deleted the escalade:sambafix branch Apr 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.