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.

Sign up for GitHub

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

Jump to bottom

pkgs/qemu: add qemu_full package #94370

Merged
merged 1 commit into from Jan 26, 2021
Merged

pkgs/qemu: add qemu_full package #94370

merged 1 commit into from Jan 26, 2021

Conversation

bb2020
Copy link
Member

@bb2020 bb2020 commented Jul 31, 2020 •
edited

I think samba support for Qemu should be enabled by default.
It is widely used, so this will prevent re-compilation of Qemu if the flag is overridden.

A quick workaround is to install samba package and imperatively create a symlink to smbd like this:
ln -s /etc/profiles/per-user/nixuser/bin/smbd /usr/sbin/smbd

Any ideas?

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@bb2020 bb2020 changed the title pkgs/qemu: always enable smbd pkgs/qemu: always enable smbdSupport Jul 31, 2020
@ofborg ofborg bot requested a review from edolstra July 31, 2020 19:45
@SuperSandro2000
Copy link
Member

Please remove the master merge commit.

andir
andir requested changes Nov 25, 2020
View reviewed changes
Copy link
Member

@andir andir left a comment

Choose a reason for hiding this comment

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

We shouldn't set it to true unconditionally. I left a comment in the code review.

Also please add the motivation you detailed in your PR description to the commit changing the knob. It will make it much easier for someone (likely neither me nor you) to figure out the why when debugging/refactoring something.

pkgs/applications/virtualization/qemu/default.nix Outdated Show resolved Hide resolved
@bb2020 bb2020 closed this Nov 25, 2020
@bb2020 bb2020 reopened this Nov 25, 2020
@bb2020
pkgs/qemu: add qemu_full package
f2e8426 
qemu_full enables samba and ceph support because otherwise enabling
them triggers recompilation of qemu package that takes a long time.
Similar options can be enabled later on. qemu_full is based on complete
qemu package, so hostCpuOnly is not enabled.
@bb2020 bb2020 changed the title pkgs/qemu: always enable smbdSupport pkgs/qemu: add qemu_full package Nov 26, 2020
@bb2020 bb2020 requested a review from andir January 25, 2021 22:13
andir
andir approved these changes Jan 26, 2021
View reviewed changes
Copy link
Member

@andir andir left a comment

Choose a reason for hiding this comment

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

I am still not super happy about the amounts of QEMU packages that we have in nixpkgs but that isn't your fault nor something that I can ask you to change.

@andir andir merged commit 59e73f2 into NixOS:master Jan 26, 2021
@bb2020
Copy link
Member Author

bb2020 commented Jan 26, 2021

Thanks for merging despite the confusion!

@bb2020 bb2020 mentioned this pull request Feb 23, 2021
Closed
10 tasks
@nh2 nh2 mentioned this pull request Nov 12, 2021
Open
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SuperSandro2000 SuperSandro2000 SuperSandro2000 left review comments

@andir andir andir approved these changes

@edolstra edolstra Awaiting requested review from edolstra

Assignees
No one assigned
Labels
8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 10.rebuild-linux: 1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@bb2020 @SuperSandro2000 @andir @ryantm