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

virtualbox: 6.1.10 -> 6.1.14 #97411

Merged
merged 2 commits into from Sep 25, 2020
Merged

virtualbox: 6.1.10 -> 6.1.14 #97411

merged 2 commits into from Sep 25, 2020

Conversation

Magicloud
Copy link
Contributor

@Magicloud Magicloud commented Sep 8, 2020 •
edited

Motivation for this change

6.1.14 is the latest release of VirtualBox. And it contains some necessary changes to be built on Linux 5.8 kernel.

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.

turboMaCk
turboMaCk requested changes Sep 8, 2020
View reviewed changes
Copy link
Member

@turboMaCk turboMaCk left a comment •
edited

Choose a reason for hiding this comment

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

Thanks for contribution @Magicloud.

However, the commit message doesn't follow the guideline. please check example section in https://github.com/NixOS/nixpkgs/blob/master/.github/CONTRIBUTING.md#submitting-changes . Can you please git commit --amend it?

@evenbrenden
Copy link
Contributor

Looks like VirtualBox 6.1.10 build is broken on current nixos-unstable (6152513) when using kernelPackages = pkgs.linuxPackages_latest:

In file included from /nix/store/lm27zw14mmif9prrslgawdi0i5wpx25v-linux-5.8.6-dev/lib/modules/5.8.6/source/include/asm-generic/percpu.h:7,
                 from /nix/store/lm27zw14mmif9prrslgawdi0i5wpx25v-linux-5.8.6-dev/lib/modules/5.8.6/source/arch/x86/include/asm/percpu.h:556,
                 from /nix/store/lm27zw14mmif9prrslgawdi0i5wpx25v-linux-5.8.6-dev/lib/modules/5.8.6/source/arch/x86/include/asm/preempt.h:6,
                 from /nix/store/lm27zw14mmif9prrslgawdi0i5wpx25v-linux-5.8.6-dev/lib/modules/5.8.6/source/include/linux/preempt.h:78,
                 from /nix/store/lm27zw14mmif9prrslgawdi0i5wpx25v-linux-5.8.6-dev/lib/modules/5.8.6/source/include/linux/spinlock.h:51,
                 from /build/virtualbox-6.1.10-modsrc/vboxdrv/linux/../SUPDrvInternal.h:79,
                 from /build/virtualbox-6.1.10-modsrc/vboxdrv/linux/SUPDrv-linux.c:32:
/build/virtualbox-6.1.10-modsrc/vboxdrv/linux/SUPDrv-linux.c: In function 'supdrvOSChangeCR4':
/build/virtualbox-6.1.10-modsrc/vboxdrv/linux/SUPDrv-linux.c:760:38: error: 'cpu_tlbstate' undeclared (first use in this function); did you mean 'cpuhp_state'?
  760 |     RTCCUINTREG uOld = this_cpu_read(cpu_tlbstate.cr4);
      |                                      ^~~~~~~~~~~~
/nix/store/lm27zw14mmif9prrslgawdi0i5wpx25v-linux-5.8.6-dev/lib/modules/5.8.6/source/include/linux/percpu-defs.h:318:9: note: in definition of macro '__pcpu_size_call_return'
  318 |  typeof(variable) pscr_ret__;     \
      |         ^~~~~~~~
/build/virtualbox-6.1.10-modsrc/vboxdrv/linux/SUPDrv-linux.c:760:24: note: in expansion of macro 'this_cpu_read'
  760 |     RTCCUINTREG uOld = this_cpu_read(cpu_tlbstate.cr4);
      |                        ^~~~~~~~~~~~~
/build/virtualbox-6.1.10-modsrc/vboxdrv/linux/SUPDrv-linux.c:760:38: note: each undeclared identifier is reported only once for each function it appears in
  760 |     RTCCUINTREG uOld = this_cpu_read(cpu_tlbstate.cr4);
      |                                      ^~~~~~~~~~~~
/nix/store/lm27zw14mmif9prrslgawdi0i5wpx25v-linux-5.8.6-dev/lib/modules/5.8.6/source/include/linux/percpu-defs.h:318:9: note: in definition of macro '__pcpu_size_call_return'
  318 |  typeof(variable) pscr_ret__;     \
      |         ^~~~~~~~
/build/virtualbox-6.1.10-modsrc/vboxdrv/linux/SUPDrv-linux.c:760:24: note: in expansion of macro 'this_cpu_read'
  760 |     RTCCUINTREG uOld = this_cpu_read(cpu_tlbstate.cr4);
      |                        ^~~~~~~~~~~~~
make[4]: *** [/nix/store/lm27zw14mmif9prrslgawdi0i5wpx25v-linux-5.8.6-dev/lib/modules/5.8.6/source/scripts/Makefile.build:280: /build/virtualbox-6.1.10-modsrc/vboxdrv/linux/SUPDrv-linux.o] Error 1
make[3]: *** [/nix/store/lm27zw14mmif9prrslgawdi0i5wpx25v-linux-5.8.6-dev/lib/modules/5.8.6/source/Makefile:1756: /build/virtualbox-6.1.10-modsrc/vboxdrv] Error 2
make[2]: *** [/nix/store/lm27zw14mmif9prrslgawdi0i5wpx25v-linux-5.8.6-dev/lib/modules/5.8.6/source/Makefile:185: __sub-make] Error 2
make[2]: Leaving directory '/nix/store/lm27zw14mmif9prrslgawdi0i5wpx25v-linux-5.8.6-dev/lib/modules/5.8.6/build'
make[1]: *** [/build/virtualbox-6.1.10-modsrc/vboxdrv/Makefile-footer.gmk:114: vboxdrv] Error 2
make[1]: Leaving directory '/build/virtualbox-6.1.10-modsrc/vboxdrv'
make: *** [Makefile:58: vboxdrv] Error 2
builder for '/nix/store/1009iybgsnfr9p5vvkc19aajcxcqnklj-virtualbox-modules-6.1.10-5.8.6.drv' failed with exit code 2
cannot build derivation '/nix/store/cv7lscscdxnbnixn1dh7272az51qg7mf-kernel-modules.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/imcfygi6v5yvbd1pzav77nrj50k0z3w0-linux-5.8.6-modules.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/rjxwdazq3zff1iwb5pssgy5iql31wgkx-linux-5.8.6-modules-shrunk.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/yqq8z4y33g46yggpbah68crqd7xzj465-stage-1-init.sh.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/fh0jm66daaf4qnxpd1yx1qci05v36glm-initrd-linux-5.8.6.drv': 1 dependencies couldn't be built

The same configuration builds and runs fine with 6.1.14, so this is a very welcome change 🙂

@Magicloud
virtualbox: 6.1.10 -> 6.1.14
e1538e4 
6.1.14 is the latest release of Virtualbox.

Also disabled VirtIO since the source tarball does not include necessary source code.
@Magicloud Magicloud changed the title Upgrade VirtualBox to 6.1.14 virtualbox: 6.1.10 -> 6.1.14 Sep 9, 2020
@Magicloud
Copy link
Contributor Author

Magicloud commented Sep 9, 2020 •
edited

@turboMaCk Done.

@Magicloud
Copy link
Contributor Author

@evenbrenden One change of 6.1.14 is to support some new features of 5.8 kernel, as I recalled. But not sure if that was exactly what you got.

@pbogdan pbogdan mentioned this pull request Sep 15, 2020
Closed
@bryanasdev000
Copy link
Member

@evenbrenden One change of 6.1.14 is to support some new features of 5.8 kernel, as I recalled. But not sure if that was exactly what you got.

Yep, thats what currently happens when you use kernel 5.8 with Virtualbox 6.1.10, more information available at:
https://www.virtualbox.org/ticket/19644?cversion=0&cnum_hist=8
But basically, we need this version bump.

Maybe @svanderburg can help with PR review?

@Ma27
Copy link
Member

Ma27 commented Sep 24, 2020

Currently testing this on my 20.09 as I'd like to upgrade my kernel to 5.8, but I need VirtualBox for work atm. If everything is fine, I'd merge & backport, but just to be sure - @evenbrenden @bryanasdev000 @Magicloud @turboMaCk are there any issues with this PR I'm currently missing?

@evenbrenden
Copy link
Contributor

evenbrenden commented Sep 24, 2020 •
edited

No problems here. Been using it with a couple of VMs (Windows and Linux). Running NixOS at a5931fa6e3 with this PR on top and linux-5.8.9.

EDIT: Added kernel version.

@bryanasdev000
Copy link
Member

Currently testing this on my 20.09 as I'd like to upgrade my kernel to 5.8, but I need VirtualBox for work atm. If everything is fine, I'd merge & backport, but just to be sure - @evenbrenden @bryanasdev000 @Magicloud @turboMaCk are there any issues with this PR I'm currently missing?

I haven't had time to test it yet, but from what I read I believe that this PR would solve our problem.

@Ma27
Copy link
Member

Ma27 commented Sep 24, 2020

I can confirm this as well and will merge tonight. First I'd like to take a look at the failing virtualbox VM test though.

@Ma27 Ma27 added this to the 20.09 milestone Sep 24, 2020
@Ma27 Ma27 added the 9.needs: port to stable A PR needs a backport to the stable release. label Sep 24, 2020
@Ma27
virtualbox: re-add support for virtio
579404c 
For some reason the original source tarball for version 6.1.14 didn't
bundle the sources for `virtio`-support causing a build-failure.

After this was reported, a new tarball named
`VirtualBox-6.1.14a.tar.bz2` was published which fixes the issue[1].

[1] https://www.virtualbox.org/ticket/19862
@Ma27
Copy link
Member

Ma27 commented Sep 25, 2020

Just fixed the virtio support that was originally disabled by @Magicloud due to a misconfigured tarball from upstream. Currently running the VM test locally, after that I'll merge!

@Ma27 Ma27 self-assigned this Sep 25, 2020
Ma27
Ma27 approved these changes Sep 25, 2020
View reviewed changes
Copy link
Member

@Ma27 Ma27 left a comment

Choose a reason for hiding this comment

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

Works for me fine since yesterday on my machine and I'm not the only one.
The test is unfortunately failing as it times out to boot a VM but this is most likely due to the test being rather slow (because of VBox in QEMU) though it got very far so the functionality of the package update is sufficiently tested IMHO.

@Ma27 Ma27 merged commit 76dcad0 into NixOS:master Sep 25, 2020
@Ma27
Copy link
Member

Ma27 commented Sep 25, 2020

Ported to 20.09 as 07504c9, f3d099f

@Ma27 Ma27 added 8.has: port to stable A PR already has a backport to the stable release. and removed 9.needs: port to stable A PR needs a backport to the stable release. labels Sep 25, 2020
This was referenced Sep 27, 2020
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@turboMaCk turboMaCk turboMaCk requested changes

@Ma27 Ma27 Ma27 approved these changes

@svanderburg svanderburg Awaiting requested review from svanderburg

@cdepillabout cdepillabout Awaiting requested review from cdepillabout

Assignees

@Ma27 Ma27

Labels
8.has: port to stable A PR already has a backport to the stable release. 10.rebuild-darwin: 0 10.rebuild-linux: 11-100 12. first-time contribution
Projects
None yet
Milestone
20.09
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@Magicloud @evenbrenden @bryanasdev000 @Ma27 @turboMaCk @Janik-Haag