runInLinuxVM: remove hwclock -s invocation

[Ma27]

Commit 97ed6b4 broke the systemd-boot tests (among others) because of the hwclock -s invocation. This was broken for a while, but not noticed because we didn't have a set -e before.

The error

hwclock: select() to /dev/rtc0 to wait for clock tick timed out

MAY be related to an open QEMU bug[1]: I can't reproduce the error on aarch64-linux and x86_64-linux with partitionTableType = "legacy";. Also, the issue disappears on x86_64-linux when adding --directisa.

However, the invocation was added in f73ff05 10 years ago which didn't give any reasoning or pointer to what KVM bug this may be. Given that this must have happened on an ancient version, we agreed on removing it altogether[2].

[1] https://gitlab.com/qemu-project/qemu/-/issues/1762
[2] #354535 (comment)

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 25.05 Release Notes (or backporting 24.11 and 25.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.

---

Add a 👍 reaction to pull requests you find important.

[wolfgangwalther]

f73ff05 also added code in two more places - any chance we can remove some more unused stuff there?

[ElvishJerricco]

@Ma27 Do you know why this only broke for some use cases of runInLinuxVM? e.g. nixosTests.systemd-boot.* (due to useBootLoader = true; using make-disk-image.nix in a way that uses runInLinuxVM).

[ElvishJerricco]

@ofborg test systemd-boot

[Ma27]

@ElvishJerricco my theory is that https://gitlab.com/qemu-project/qemu/-/issues/1762 is the culprit.
The legacy boot variant is also working fine there and make-disk-image.nix does legacy boot by default AFAIU.

[wolfgangwalther]

f73ff05 also added code in two more places - any chance we can remove some more unused stuff there?

I tried the following patch:

---
 nixos/lib/make-multi-disk-zfs-image.nix  | 3 +--
 nixos/lib/make-single-disk-zfs-image.nix | 3 +--
 nixos/modules/system/boot/kernel.nix     | 3 ---
 pkgs/build-support/vm/default.nix        | 1 -
 4 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/nixos/lib/make-multi-disk-zfs-image.nix b/nixos/lib/make-multi-disk-zfs-image.nix
index 1dac232e2b8c..b2489989092a 100644
--- a/nixos/lib/make-multi-disk-zfs-image.nix
+++ b/nixos/lib/make-multi-disk-zfs-image.nix
@@ -236,8 +236,7 @@ let
   image = (
     pkgs.vmTools.override {
       rootModules =
-        [ "zfs" "9p" "9pnet_virtio" "virtio_pci" "virtio_blk" ] ++
-          (pkgs.lib.optional pkgs.stdenv.hostPlatform.isx86 "rtc_cmos");
+        [ "zfs" "9p" "9pnet_virtio" "virtio_pci" "virtio_blk" ];
       kernel = modulesTree;
     }
   ).runInLinuxVM (
diff --git a/nixos/lib/make-single-disk-zfs-image.nix b/nixos/lib/make-single-disk-zfs-image.nix
index e37b79797dca..cd65d596e051 100644
--- a/nixos/lib/make-single-disk-zfs-image.nix
+++ b/nixos/lib/make-single-disk-zfs-image.nix
@@ -226,8 +226,7 @@ let
   image = (
     pkgs.vmTools.override {
       rootModules =
-        [ "zfs" "9p" "9pnet_virtio" "virtio_pci" "virtio_blk" ] ++
-        (pkgs.lib.optional pkgs.stdenv.hostPlatform.isx86 "rtc_cmos");
+        [ "zfs" "9p" "9pnet_virtio" "virtio_pci" "virtio_blk" ];
       kernel = modulesTree;
     }
   ).runInLinuxVM (
diff --git a/nixos/modules/system/boot/kernel.nix b/nixos/modules/system/boot/kernel.nix
index 4854119b2538..d25a6e98497e 100644
--- a/nixos/modules/system/boot/kernel.nix
+++ b/nixos/modules/system/boot/kernel.nix
@@ -306,9 +306,6 @@ in
           ] ++ optionals pkgs.stdenv.hostPlatform.isx86 [
             # Misc. x86 keyboard stuff.
             "pcips2" "atkbd" "i8042"
-
-            # x86 RTC needed by the stage 2 init script.
-            "rtc_cmos"
           ]);
 
         boot.initrd.kernelModules =
diff --git a/pkgs/build-support/vm/default.nix b/pkgs/build-support/vm/default.nix
index aa504babdac5..29ae21e50c4d 100644
--- a/pkgs/build-support/vm/default.nix
+++ b/pkgs/build-support/vm/default.nix
@@ -6,7 +6,6 @@
 , storeDir ? builtins.storeDir
 , rootModules ?
     [ "virtio_pci" "virtio_mmio" "virtio_blk" "virtio_balloon" "virtio_rng" "ext4" "unix" "9p" "9pnet_virtio" "crc32c_generic" ]
-      ++ pkgs.lib.optional pkgs.stdenv.hostPlatform.isx86 "rtc_cmos"
 }:
 
 let
-- 
2.47.0

The following two build fine:

nix-build nixos/release.nix -A amazonImageZfs.x86_64-linux
nix-build -A nixosTests.systemd-boot

The only use for make-single-disk-zfs-image is the openstack-image. But nix-build nixos/tests/openstack-image.nix already fails regardless. But those change should be good by analogy to them in make-multi-disk-zfs-image, which are tested via the amazonImageZfs.

[wolfgangwalther]

The fix itself builds fine for me. Except for the patch posted above: LGTM.

[ElvishJerricco]

@Ma27 I'm not asking "why does the hwclock command fail." I'm asking "why does it only fail for this use of runInLinuxVM and not all uses?" It's not like make-disk-image.nix is using runInLinuxVM in any novel way.

[ElvishJerricco]

Ok I see. I didn't notice that make-disk-image.nix adds stuff to run under UEFI so that we can extract the resulting EFI variables:

nixpkgs/nixos/lib/make-disk-image.nix

Lines 547 to 555 in 26bdab5

	QEMU_OPTS =
	lib.concatStringsSep " " (lib.optional useEFIBoot "-drive if=pflash,format=raw,unit=0,readonly=on,file=${efiFirmware}"
	++ lib.optionals touchEFIVars [
	"-drive if=pflash,format=raw,unit=1,file=$efiVars"
	] ++ lib.optionals (OVMF.systemManagementModeRequired or false) [
	"-machine" "q35,smm=on"
	"-global" "driver=cfi.pflash01,property=secure,value=on"
	]
	);

So the issue that @Ma27 linked is only relevant in this case because it seems to only be relevant when using qemu for UEFI.

So I think we can say that this issue is understood and merge this.

[Ma27]

@wolfgangwalther do you want to file your patch as a follow-up? While I think it was correct to merge the current state to unbreak a bunch of things, your patch also looks reasonable to have.

[wolfgangwalther]

@wolfgangwalther do you want to file your patch as a follow-up? While I think it was correct to merge the current state to unbreak a bunch of things, your patch also looks reasonable to have.

Done in #359416.