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: 4.2.0 -> 5.0.0 #86291

Merged
merged 1 commit into from May 19, 2020
Merged

qemu: 4.2.0 -> 5.0.0 #86291

merged 1 commit into from May 19, 2020

Conversation

@chkno
Copy link
Member

chkno commented Apr 29, 2020

Motivation for this change

A new version of qemu is available.

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.
@petabyteboy
Copy link
Contributor

petabyteboy commented Apr 29, 2020

Built and tested basic functionality 👍

@chkno chkno force-pushed the chkno:qemu5 branch from 1d6245f to 85aafb5 May 1, 2020
@Robertof
Copy link

Robertof commented May 9, 2020

Hey,

thanks for this!
I have had some issues when using a local nixpkgs checkout with this PR applied and trying to switch to a system configuration with boot.binfmt.emulatedSystems = [ "aarch64-linux" ]; enabled.

I have investigated it a bit and here are my findings:

When enabling the emulatedSystems option, a special overridden version of the QEMU package is built which has the following hardcoded flags:

emulator = pkgs: let
qemu-user = pkgs.qemu.override {
smartcardSupport = false;
spiceSupport = false;
openGLSupport = false;
virglSupport = false;
vncSupport = false;
gtkSupport = false;
sdlSupport = false;
pulseSupport = false;
smbdSupport = false;
seccompSupport = false;
hostCpuTargets = ["${final.qemuArch}-linux-user"];
};

This sets, in my case, --target-list=aarch64-linux-user.

Since at least 2019, QEMU disables the guest agent when no system emulation is required, and I am fairly certain this is what happens when passing a target which ends with -user. However, this package always exports the guest agent as an output:

This causes my build to fail during the fixup phase, as it's expecting a binary in the ga output directory. I fixed it by temporarily removing the ga output, but I'm unsure if my issues derive from something I am doing wrong or if this is an actual problem.

Moreover, I am not sure what would be the best (and most idiomatic) way to solve this. What do you think?

@Robertof
Copy link

Robertof commented May 9, 2020

I can confirm that using the existing derivation (4.2.0) the guest agent is built, even if not necessary/used at all:

configuring
configure flags: --disable-dependency-tracking --prefix=/nix/store/s8cv8wa06d16w77ws1fnjf3f1amfq92l-qemu-4.2.0 --bindir=/nix/store/s8cv8wa06d16w77ws1fnjf3f1amfq92l-qemu-4.2.0/bin --sbindir=/nix/store/s8cv8wa06d16w77ws1fnjf3f1amfq92l-qemu-4.2.0/sbin --includedir=/nix/store/s8cv8wa06d16w77ws1fnjf3f1amfq92l-qemu-4.2.0/include --oldincludedir=/nix/store/s8cv8wa06d16w77ws1fnjf3f1amfq92l-qemu-4.2.0/include --mandir=/nix/store/s8cv8wa06d16w77ws1fnjf3f1amfq92l-qemu-4.2.0/share/man --infodir=/nix/store/s8cv8wa06d16w77ws1fnjf3f1amfq92l-qemu-4.2.0/share/info --docdir=/nix/store/s8cv8wa06d16w77ws1fnjf3f1amfq92l-qemu-4.2.0/share/doc/ --libdir=/nix/store/s8cv8wa06d16w77ws1fnjf3f1amfq92l-qemu-4.2.0/lib --libexecdir=/nix/store/s8cv8wa06d16w77ws1fnjf3f1amfq92l-qemu-4.2.0/libexec --localedir=/nix/store/s8cv8wa06d16w77ws1fnjf3f1amfq92l-qemu-4.2.0/share/locale --audio-drv-list=alsa\, --sysconfdir=/etc --localstatedir=/var --enable-docs --enable-numa --target-list=aarch64-linux-user --enable-linux-aio
[...]
build guest agent yes

So either we always build the guest agent even if unneeded or add some logic which detects when it's built and when not.

@Renha
Copy link

Renha commented May 10, 2020

Hey there! I've tried to run nixpkgs-review on macOS to help with the unset check, but either I've used the tool incorrectly (first time I've tried that) or there is some issue with building:

Result of nixpkgs-review pr 86291 1

11 package marked as broken and skipped:
  • cloud-init
  • cloud-utils
  • python27Packages.guestfs
  • python37Packages.guestfs
  • python38Packages.guestfs
  • qemu_xen
  • qemu_xen-light
  • qemu_xen_4_10
  • qemu_xen_4_10-light
  • qemu_xen_4_8
  • qemu_xen_4_8-light
4 package failed to build:
  • out-of-tree
  • qemu
  • qemu_kvm
  • qemu_test
@petabyteboy
Copy link
Contributor

petabyteboy commented May 11, 2020

Hey there! I've tried to run nixpkgs-review on macOS to help with the unset check, but either I've used the tool incorrectly (first time I've tried that) or there is some issue with building:

Result of nixpkgs-review pr 86291 1

11 package marked as broken and skipped:
4 package failed to build:

Hey, thanks for testing! Can you upload the build log for the qemu package on some pastebin service?

@clefru
Copy link
Contributor

clefru commented May 11, 2020

Can we add "--enable-guest-agent" to the configure flags to always build the guest agent? That would allow us to not change too many things about the current assumptions of the derivation. @Robertof can you test whether your build problems go away when you add that flag to the array around line 105?

@Robertof
Copy link

Robertof commented May 11, 2020

@clefru I can confirm that this diff:

diff --git a/pkgs/applications/virtualization/qemu/default.nix b/pkgs/applications/virtualization/qemu/default.nix
index 9ff2f06d..62a9e533 100644
--- a/pkgs/applications/virtualization/qemu/default.nix
+++ b/pkgs/applications/virtualization/qemu/default.nix
@@ -107,6 +107,8 @@ stdenv.mkDerivation rec {
       "--sysconfdir=/etc"
       "--localstatedir=/var"
       "--enable-docs"
+      "--enable-tools"
+      "--enable-guest-agent"
     ]
     # disable sysctl check on darwin.
     ++ optional stdenv.isDarwin "--cpu=x86_64"

fixes the issue with boot.binfmt.emulatedSystems = [ "aarch64-linux" ]; 👍

There is this weird (non-fatal) "no such file or directory":

patching script interpreter paths in /nix/store/b2k55yi7gz9ymq7bfgwx8zl64pww9zpz-qemu-5.0.0
checking for references to /build/ in /nix/store/b2k55yi7gz9ymq7bfgwx8zl64pww9zpz-qemu-5.0.0...
cannot find section .dynamic
cannot find section .dynamic
cannot find section .dynamic
cannot find section .dynamic
cannot find section .dynamic
cannot find section .dynamic
find: '/nix/store/zlqhgxwz0klwh7xpb6waqy0p3xdwiqyn-qemu-5.0.0-ga': No such file or directory
strip is /nix/store/1zf4cnaaidjajwb4gx4mnkqc5dypkcdy-binutils-2.31.1/bin/strip
building '/nix/store/nkn0nlk68p8a0nyf9kzafv0cn2zch3ik-dbus-1.drv'..

but it was there even with qemu-4.2.0 so I'd say it's ok.

NOTE: --enable-tools is required as it was made non-default a while ago, and guest-agent is automatically disabled if tools are disabled and QEMU is compiling user-only

@chkno chkno force-pushed the chkno:qemu5 branch from 85aafb5 to 27d84ae May 11, 2020
Copy link

Robertof left a comment

LGTM, thanks! @Renha feel free to try the reviewer tool again with the rebased diff, hopefully it works OK in all cases now.

@Renha
Copy link

Renha commented May 12, 2020

Successfully built on macOS!

reviewer tool is trying to build linux-only packages for some reason (maybe it is designed this way and it's my bad I was trying to use it on macOS), so I've built it manually.

Copy link
Contributor

petabyteboy left a comment

Still works for me with the latest changes! 👍

@Renha
Renha approved these changes May 19, 2020
@petabyteboy petabyteboy merged commit f2406c6 into NixOS:staging May 19, 2020
16 checks passed
16 checks passed
qemu, qemu.passthru.tests on aarch64-linux Failure
Details
qemu, qemu.passthru.tests on x86_64-linux Failure
Details
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ec005d4"; rev="ec005d4a834044fa992d7e406a7aa798fd319411"; } ./pkgs/t
Details
grahamcofborg-eval-lib-tests nix-build --arg pkgs import ./. {} ./lib/tests/release.nix
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ec005d4"; rev="ec005d4a834044fa992d7e406a7aa798fd319411"; } ./nixos/
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ec005d4"; rev="ec005d4a834044fa992d7e406a7aa798fd319411"; } ./nixos/
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ec005d4"; rev="ec005d4a834044fa992d7e406a7aa798fd319411"; } ./nixos/
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ec005d4"; rev="ec005d4a834044fa992d7e406a7aa798fd319411"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ec005d4"; rev="ec005d4a834044fa992d7e406a7aa798fd319411"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ec005d4"; rev="ec005d4a834044fa992d7e406a7aa798fd319411"; } ./pkgs/t
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
@ajs124 ajs124 mentioned this pull request May 20, 2020
1 of 6 tasks complete
@mebubo mebubo mentioned this pull request May 21, 2020
2 of 10 tasks complete
@clefru
Copy link
Contributor

clefru commented May 30, 2020

When will this hit master? I'd like to add virtiofsd support and I don't want to work on top of staging.

@Renha
Copy link

Renha commented Jun 4, 2020

It's already in staging-next if you don't want to work on top of staging
But yeah, I wait for it to hit master too.

@ajs124 ajs124 mentioned this pull request Jun 7, 2020
0 of 10 tasks complete
@JJJollyjim
Copy link
Contributor

JJJollyjim commented Jun 10, 2020

@clefru I've been using virtiofsd on this commit -- what do you mean by that? The only issue is that the daemon is in libexec instead of bin.

@clefru
Copy link
Contributor

clefru commented Jun 10, 2020

Thanks @JJJollyjim. When I commented, I haven't seen that yet. Sorry for the noise.

For the record, virtiofsd is working fine for me. I manually spawn it via this systemd unit:

  systemd.services.virtiofsd-siona = {
    wantedBy = [ "multi-user.target" ];
    before = [ "libvirtd.service" ];
    path = [ pkgs.qemu ];
    serviceConfig =
      { ExecStart = pkgs.writeScript "virtiofsd-loop" ''#!/bin/sh
            while true; do ${pkgs.qemu}/libexec/virtiofsd --socket-path=/var/lib/libvirt/siona-vhost-socket -o no_flock -o no_posix_lock -o source=/z/siona -o cache=always; done
'';         
        User = "root";
        Restart = "always";
      };
  };
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants
You can’t perform that action at this time.