From 5b16d4c9ce16acffc944c01ff95ea97a807d8f4a Mon Sep 17 00:00:00 2001
From: Niklas Hambüchen <mail@nh2.me>
Date: Fri, 3 Jul 2020 01:35:36 +0200
Subject: qemu-vm.nix: Fix device name hardcodes on `useBootLoader`.

boot.loader.grub.device` was hardcoded to `bootDevice`, which is
wrong, because that's the device for `/`, and with `useBootLoader`
the boot loader is not on that device.

This bug probably came into existence because of bad naming;
`virtualisation.bootDevice` has description
"The disk to be used for the root filesystem", which is very confusing;
it should be `.rootDevice` then!
Unfortunately, the description is right and the attribute name is wrong,
so it is not easy to change this without deprecation.

This commit ensures that even if you use `useBootLoader` and
`diskInterface == "scsi"`, the created VM can boot through, and can run
`nixos-rebuild afterwards.

It also adds extra commentary to explain what's going on in this module
in general in relation to `useBootLoader`.
---
 nixos/modules/virtualisation/qemu-vm.nix | 42 ++++++++++++++++++++++++++++++--
 1 file changed, 40 insertions(+), 2 deletions(-)

(limited to 'nixos')

diff --git a/nixos/modules/virtualisation/qemu-vm.nix b/nixos/modules/virtualisation/qemu-vm.nix
index 975b131d265df..c61dc059b8118 100644
--- a/nixos/modules/virtualisation/qemu-vm.nix
+++ b/nixos/modules/virtualisation/qemu-vm.nix
@@ -164,6 +164,8 @@ let
 
   # Generate a hard disk image containing a /boot partition and GRUB
   # in the MBR.  Used when the `useBootLoader' option is set.
+  # Uses `runInLinuxVM` to create the image in a throwaway VM.
+  # See note [Disk layout with `useBootLoader`].
   # FIXME: use nixos/lib/make-disk-image.nix.
   bootDisk =
     pkgs.vmTools.runInLinuxVM (
@@ -197,6 +199,19 @@ let
             --partition-guid=2:970C694F-AFD0-4B99-B750-CDB7A329AB6F \
             --hybrid 2 \
             --recompute-chs /dev/vda
+
+          ${optionalString (config.boot.loader.grub.device != "/dev/vda")
+            # In this throwaway VM, we only have the /dev/vda disk, but the
+            # actual VM described by `config` (used by `switch-to-configuration`
+            # below) may set `boot.loader.grub.device` to a different device
+            # that's nonexistent in the throwaway VM.
+            # Create a symlink for that device, so that the `grub-install`
+            # by `switch-to-configuration` will hit /dev/vda anyway.
+            ''
+              ln -s /dev/vda ${config.boot.loader.grub.device}
+            ''
+          }
+
           ${pkgs.dosfstools}/bin/mkfs.fat -F16 /dev/vda2
           export MTOOLS_SKIP_CHECK=1
           ${pkgs.mtools}/bin/mlabel -i /dev/vda2 ::boot
@@ -483,7 +498,27 @@ in
 
   config = {
 
-    boot.loader.grub.device = mkVMOverride cfg.bootDevice;
+    # Note [Disk layout with `useBootLoader`]
+    #
+    # If `useBootLoader = true`, we configure 2 drives:
+    # `/dev/?da` for the root disk, and `/dev/?db` for the boot disk
+    # which has the `/boot` partition and the boot loader.
+    # Concretely:
+    #
+    # * The second drive's image `disk.img` is created in `bootDisk = ...`
+    #   using a throwaway VM. Note that there the disk is always `/dev/vda`,
+    #   even though in the final VM it will be at `/dev/*b`.
+    # * The disks are attached in `virtualisation.qemu.drives`.
+    #   Their order makes them appear as devices `a`, `b`, etc.
+    # * `fileSystems."/boot"` is adjusted to be on device `b`.
+
+    # If `useBootLoader`, GRUB goes to the second disk, see
+    # note [Disk layout with `useBootLoader`].
+    boot.loader.grub.device = mkVMOverride (
+      if cfg.useBootLoader
+        then driveDeviceName 2 # second disk
+        else cfg.bootDevice
+    );
 
     boot.initrd.extraUtilsCommands =
       ''
@@ -574,6 +609,8 @@ in
         driveExtraOpts.werror = "report";
       }]
       (mkIf cfg.useBootLoader [
+        # The order of this list determines the device names, see
+        # note [Disk layout with `useBootLoader`].
         {
           name = "boot";
           file = "$TMPDIR/disk.img";
@@ -628,7 +665,8 @@ in
           };
       } // optionalAttrs cfg.useBootLoader
       { "/boot" =
-          { device = "${lookupDriveDeviceName "boot" cfg.qemu.drives}2";
+          # see note [Disk layout with `useBootLoader`]
+          { device = "${lookupDriveDeviceName "boot" cfg.qemu.drives}2"; # 2 for e.g. `vdb2`, as created in `bootDisk`
             fsType = "vfat";
             noCheck = true; # fsck fails on a r/o filesystem
           };
-- 
cgit 1.4.1