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

qemu: Add patch to fix incorrect permissions on samba >= 2.0.5 #145573

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nh2
Copy link
Contributor

@nh2 nh2 commented Nov 12, 2021

Motivation for this change

QEMU's builtin Samba config does not work for current Samba versions (including the one used in nixpkgs), see:

https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg07156.html

Discussion so far (note lists -> mail in the URL, the old mailing list URL doesn't seem to update any longer):

https://mail.gnu.org/archive/html/qemu-devel/2021-04/msg06741.html

Unfortunately nobody has replied to my patch yet.

I think it makes sense to have this in nixpkgs, because otherwise mounting QEMU-provided Samba shares for Windows VM guests does not work.

We've been using this patch successfully on our company infrastructure for the last 9 months.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 21.11 Release Notes (or backporting 21.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

CC from #114064: @bb2020

CC from #94370: @andir

@bb2020
Copy link
Member

bb2020 commented Nov 12, 2021

My samba shared folder is working for Windows 7 guest. Is this a thing for newer Windows versions?

# Remove when https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg07156.html is merged and available
(fetchpatch {
name = "net-slirp-Fix-incorrect-permissions-on-samba-2.0.5.patch";
url = "https://github.com/nh2/qemu/commit/de30898a738bd073593d20930496ef63e54d62a3.patch";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
url = "https://github.com/nh2/qemu/commit/de30898a738bd073593d20930496ef63e54d62a3.patch";
url = "https://github.com/qemu/qemu/commit/de30898a738bd073593d20930496ef63e54d62a3.patch";

makes it easier to track when it is merged.

@nh2
Copy link
Contributor Author

nh2 commented Nov 18, 2021

My samba shared folder is working for Windows 7 guest. Is this a thing for newer Windows versions?

@bb2020 Are you using the -net user,smb=/path/to/folder QEMU option for your Samba share? That's what the patch fixes.

@alyssais
Copy link
Member

alyssais commented Nov 22, 2021

@nh2 I notice the patch hasn't been discussed on the mailing list since April. Now would be a good time to resend it, because you might be able to make it into the soon-to-be-released 6.2.0, and because IME they're especially receptive to bug fixes during release candidate season.

@nh2
Copy link
Contributor Author

nh2 commented Dec 6, 2021

Now would be a good time to resend it

I have sent another comment/request in there. Unfortunately https://mail.gnu.org/archive/html/qemu-devel/2021-04/msg06741.html doesn't seem to be anything close to realtime, so I cannot link it.

@alyssais
Copy link
Member

alyssais commented Dec 7, 2021 via email

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 12, 2022
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 20, 2024
@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 4, 2024
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Sep 18, 2024
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.

5 participants