-
Notifications
You must be signed in to change notification settings - Fork 138
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
Enable Time Machine support via Samba. Fixes #1910. #2144
Enable Time Machine support via Samba. Fixes #1910. #2144
Conversation
@FroggyFlox This is an excellent pull request. I'll review shortly and if all is well we can get this in soon. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great stuff and quite an exiting feature.
Thanks for all the string.format and black work here. Much appreciated. Also thanks for splitting these out. We will be done with this in time and things should then get a little easier pull request prep / review wise. But it’s good that we get it done as we go along also.
I’m looking forward to making broader use of your established avahi configuration routines here. Nice that you made them with flexibility in mind.
I think this pull request is almost there bar the 'fresh install' issue I have detailed against the 'avahi --reload' command use when avahi service is dead. Note the vendor preset: enabled. But on my config here it still showed as dead, possibly due to an initially empty config or the like. I'm guessing here though and imagine you have an idea of what's not being accounted for.
Let me know if you can't reproduce this issue and I'll take a further look.
I also see the same initial state of avahi on it's fresh install in tumbleweed:
tumbleweed:~ # systemctl status avahi-daemon
● avahi-daemon.service - Avahi mDNS/DNS-SD Stack
Loaded: loaded (/usr/lib/systemd/system/avahi-daemon.service; enabled; vendor preset: enabled)
Active: inactive (dead)
TriggeredBy: ● avahi-daemon.socket
with the same Command exception:
[10/Mar/2020 18:01:29] ERROR [storageadmin.middleware:33] Error running a command. cmd = /usr/sbin/avahi-daemon --reload. rc = 255. stdout = ['']. stderr = ['Failed to kill daemon: No such file or directory', '']
upon adding a new TM share.
Let me know if I've missed something here as I'm not familiar with avahi at this level. But if a fresh install of this package on both Leap15.1 and Tumbleweed end up in Dead state from install I think we are going to have to account for it as those upgrading from a prior test release will have the avahi installed afresh via the now added rpm dependency.
Thanks for you excellent work on this by they way, and apologies if I've just missed something obvious here.
@FroggyFlox I think it's just this --reload issue outstanding here as the config generated in both the avahi service and smb.conf was exactly as stated. |
…chine service file instead of simply reloading its configuration. Remove one unnecessary logger statement as well.
Thanks again for spotting this. I have now switched to restarting the systemd As a result, I also removed the Of note, as soon as the Samba service is started (even for the very first time), it automatically starts the avahi service as well (which, I believe, is due to the fact that Samba now comes with avahi support by default) so that should give us another safety to ensure it's on when needed. For instance, I believe a samba service restart should turn the avahi-daemon service on as well. It seems to not have happened in your case, however, so that might have been a problem with the order with which we do things... Regardless, after the last commit (6a739eb), this shouldn't be a problem. Thanks again for your help, and let me know if all is well on your side now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@FroggyFlox Thanks for sorting that last little issue out, much appreciated.
And good of you to remove the now unused references to AVAHID_BIN
Thanks a lot for your review and help! |
Fixes #1910.
@phillxnet: ready for review.
This pull request (PR) proposes to support Time Machine via Samba by surfacing a simple check-box while exporting a share via Samba. By checking this checkbox, Time Machine specific options are automatically applied to this share's section in
smb.conf
. Finally, the new Time Machine-enabled export is added to a dedicated Avahi static service file to be advertised.Notably, a new field has been added to the
SambaShare
model to store this new Time Machine option. The existing Samba unit tests were thus slightly modified to ensure compatibility.Aims & Logic
While exporting a share, a new checkbox is displayed to enable compatibility with Time Machine. Upon form submission, the value of this checkbox is stored in the
SambaShare
model under the newtime_machine
boolean field. Thanks to this new field, we can now detect if a Samba export needs to be compatible with Time Machine when writingsmb.conf
and write specific options in the share's section.Then, we check for the presence of Time Machine-enabled shares and advertise them through Avahi by writing a dedicated
/etc/avahi/services/timemachine.service
file.Database migrations
A new migration file was created as follows:
... and first applied as:
When building the current branch on a fresh Leap 15.1 install, it does install/migrate as needed:
Demonstration
On a freshly built Leap 15.1, the Samba service is configured and turned ON.
Then, start the process to create a new share and enable compatibility with Time Machine. Note how checking the Time Machine checkbox disables the Shadow copy option as I believe enabling these two options on the same export would be prone to conflicts.
The resulting
smb.conf
is then written with the time machine-specific options.smb.conf
as well as the avahi service file.Summary of commits
I have left three main commits:
The commits related to the main changes are:
from Django settings when needed.
Testing
Existing unit tests were altered for compatibility with code changes.
CentOS
Leap15.1
Tumbleweed
Caveats & shortcomings
@phillxnet, due to my limited options for testing, I was not able to conduct a long term test and exhaustive test for these Samba options, but they represent a consensus between the Apple documentation, the samba vfs fruit module, as well as user reports.
Full Testing Outputs
CentOS
Leap15.1
Tumbleweed