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: fix browsing and clean up smb.conf #849

Merged
merged 1 commit into from
Oct 21, 2016

Conversation

escalade
Copy link
Contributor

@escalade escalade commented Oct 18, 2016

As pointed out in this thread, browsing in LibreELEC is currently (and seems to have been for some time) broken. The default value for "smb ports" is "445 139", so setting only 445 effectively disables NetBIOS. As Samba by default (as it should) participates in local master browser elections, browsing will stop working if it wins.

Other changes:

os level <- 20 is the default
name resolve order <- we do not ship /etc/samba/lmhosts and we do not use wins by default so it should be last. hosts should always take preference
preferred master, domain master, local master <- these are all at default values so should be omitted

@@ -27,18 +27,12 @@
security = share
guest account = root
socket options = TCP_NODELAY IPTOS_LOWDELAY
smb ports = 445
max protocol = SMB2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would leave max protocol = SMB2 otherwise samba will use NT1

From samba 3.6.0 release notes:

SMB2 support in 3.6.0 is fully functional (with one omission)
...
As this is the first release containing what we consider
to be a fully featured SMB2 protocol, we are not enabling
this by default, but encourage users to enable SMB2 and
test it.

Copy link
Contributor Author

@escalade escalade Oct 18, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, I was looking at a more recent manual page where SMB3 was the default. Corrected.

@lrusak
Copy link
Member

lrusak commented Oct 18, 2016

Please don't change the licence header.

@escalade
Copy link
Contributor Author

Changed it back, but why wouldn't you want to change it? It's not like Samba configuration directives is a product of OpenELEC.

mangled names = no
syslog only = yes
syslog = 2
name resolve order = lmhosts wins bcast host
preferred master = auto
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd leave the three master settings and os level in the config even if they're at default values. If people need to create a custom config it's one of the more frequent things to edit.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say you really shouldn't be changing the preferred master and os level unless you know what you're doing, and if you do then you probably don't need them listed. Anyways, I'll put them back if you like.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or include them, but comment them out?

# local master = yes
# preferred master = auto
# domain master = auto
# os level = 20
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we go with the commented solution I suppose an extra comment "# The following are the default values, but included for informational purposes" (or words to that effect) might be useful so that someone in 6 months time doesn't submit a PR removing them because they're the default values and therefore unnecessary etc. etc.

# local master = yes
# preferred master = auto
# domain master = auto
# os level = 20

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

@chewitt
Copy link
Member

chewitt commented Oct 21, 2016

It's no issue to change the license header as long as we carry forward the original copyright owner information and dates, but at some point we can do a bulk fixup on that. It's not a big deal for now. Thanks @escalade :)

@chewitt chewitt merged commit 7fe628e into LibreELEC:master Oct 21, 2016
@MilhouseVH
Copy link
Contributor

Now getting reports of broken smb:// (see earlier in thread for another report with log snippet) - installing the original samba.conf restores working smb://. Not entirely sure why that should be, anyone want to help diagnose?

@escalade
Copy link
Contributor Author

Looks like the report you mention is regarding Kodi SMB to a NAS, so nothing to do with the Samba service.

@MilhouseVH
Copy link
Contributor

Yes, it's rather confusing. There were eventually two problem reports, one now has a working system after build #1023 (clean build, no other relevant changes). The other person reporting a problem now seems to have a name resolution issue. Neither is running the local Samba server, but were adamant that restoring the original smb.conf restored working smb://, a solution that doesn't make any sense. I think it's most likely a random and unrelated local network failure that coincided with this smb.conf change, but can't be entirely certain.

@escalade escalade deleted the sambafix branch November 18, 2016 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants