From f148c5c4a1ffa353836bfd18a94d457acf83bd36 Mon Sep 17 00:00:00 2001
From: aszlig <aszlig@nix.build>
Date: Mon, 30 Apr 2018 01:43:17 +0200
Subject: [PATCH] nixos/tests: Fix QEMU flags for SCSI disks
The ability to specify "-drive if=scsi" has been removed in QEMU version
2.12 (introduced in 3e3b39f173f9abc99da84084a1f4657c9de885bd).
Quote from https://wiki.qemu.org/ChangeLog/2.12#Incompatible_changes:
> The deprecated way of configuring SCSI devices with "-drive if=scsi"
> on x86 has been removed. Use an appropriate SCSI controller together
> "-device scsi-hd" or "-device scsi-cd" and a corresponding "-blockdev"
> parameter instead.
So whenever the diskInterface is "scsi" we use the new way to specify
the drive and fall back to the deprecated way for the time being. The
reason why I'm not using the new way for "virtio" and "ide" as well is
because there is no simple generic way anymore to specify these.
This also turns the type of the virtualisation.qemu.diskInterface option
to be an enum, so the user knows which values are allowed but we can
also make sure the right value is provided to prevent typos.
I've tested this against a few non-disk-related NixOS VM tests but also
the installer.grub1 test (because it uses "ide" as its drive interface),
the installer.simple test (just to be sure it still works with
"virtio") and all the tests in nixos/tests/boot.nix.
In order to be able to run the grub1 test I had to go back to
8b1cf100cd8badad6e1b6d4650b904b88aa870db (which is a known commit where
that test still works) and apply the QEMU update and this very commit,
because right now the test is broken.
Apart from the tests here in nixpkgs, I also ran another[1] test in
another repository which uses the "scsi" disk interface as well (in
comparison to most of the installer tests, this one actually failed
prior to this commit).
All of them now succeed.
[1]: https://github.com/openlab-aux/vuizvui/blob/9b5a119972e9c2d327500638d89063f4fce243ec/tests/system/kernel/bfq.nix
Signed-off-by: aszlig <aszlig@nix.build>
Cc: @edostra, @grahamc, @dezgeg, @abbradar, @ts468
---
nixos/lib/test-driver/Machine.pm | 17 +++++++++++---
nixos/modules/virtualisation/qemu-vm.nix | 30 +++++++++++++++++-------
2 files changed, 35 insertions(+), 12 deletions(-)
diff --git a/nixos/lib/test-driver/Machine.pm b/nixos/lib/test-driver/Machine.pm
index 7e269b43e70..b18f48464ce 100644
--- a/nixos/lib/test-driver/Machine.pm
+++ b/nixos/lib/test-driver/Machine.pm
@@ -33,9 +33,20 @@ sub new {
$startCommand =
"qemu-kvm -m 384 " .
"-net nic,model=virtio \$QEMU_OPTS ";
- my $iface = $args->{hdaInterface} || "virtio";
- $startCommand .= "-drive file=" . Cwd::abs_path($args->{hda}) . ",if=$iface,werror=report "
- if defined $args->{hda};
+
+ if (defined $args->{hda}) {
+ if ($args->{hdaInterface} eq "scsi") {
+ $startCommand .= "-drive id=hda,file="
+ . Cwd::abs_path($args->{hda})
+ . ",werror=report,if=none "
+ . "-device scsi-hd,drive=hda ";
+ } else {
+ $startCommand .= "-drive file=" . Cwd::abs_path($args->{hda})
+ . ",if=" . $args->{hdaInterface}
+ . ",werror=report ";
+ }
+ }
+
$startCommand .= "-cdrom $args->{cdrom} "
if defined $args->{cdrom};
$startCommand .= "-device piix3-usb-uhci -drive id=usbdisk,file=$args->{usb},if=none,readonly -device usb-storage,drive=usbdisk "
diff --git a/nixos/modules/virtualisation/qemu-vm.nix b/nixos/modules/virtualisation/qemu-vm.nix
index 3d4bd315f81..66ff43c8547 100644
--- a/nixos/modules/virtualisation/qemu-vm.nix
+++ b/nixos/modules/virtualisation/qemu-vm.nix
@@ -27,6 +27,21 @@ let
kernelConsole = if cfg.graphics then "" else "console=${qemuSerialDevice}";
ttys = [ "tty1" "tty2" "tty3" "tty4" "tty5" "tty6" ];
+ # XXX: This is very ugly and in the future we really should use attribute
+ # sets to build ALL of the QEMU flags instead of this mixed mess of Nix
+ # expressions and shell script stuff.
+ mkDiskIfaceDriveFlag = idx: driveArgs: let
+ inherit (cfg.qemu) diskInterface;
+ # The drive identifier created by incrementing the index by one using the
+ # shell.
+ drvId = "drive$((${idx} + 1))";
+ # NOTE: DO NOT shell escape, because this may contain shell variables.
+ commonArgs = "index=${idx},id=${drvId},${driveArgs}";
+ isSCSI = diskInterface == "scsi";
+ devArgs = "${diskInterface}-hd,drive=${drvId}";
+ args = "-drive ${commonArgs},if=none -device lsi53c895a -device ${devArgs}";
+ in if isSCSI then args else "-drive ${commonArgs},if=${diskInterface}";
+
# Shell script to start the VM.
startVM =
''
@@ -68,7 +83,7 @@ let
if ! test -e "empty$idx.qcow2"; then
${qemu}/bin/qemu-img create -f qcow2 "empty$idx.qcow2" "${toString size}M"
fi
- extraDisks="$extraDisks -drive index=$idx,file=$(pwd)/empty$idx.qcow2,if=${cfg.qemu.diskInterface},werror=report"
+ extraDisks="$extraDisks ${mkDiskIfaceDriveFlag "$idx" "file=$(pwd)/empty$idx.qcow2,werror=report"}"
idx=$((idx + 1))
'')}
@@ -83,14 +98,14 @@ let
-virtfs local,path=$TMPDIR/xchg,security_model=none,mount_tag=xchg \
-virtfs local,path=''${SHARED_DIR:-$TMPDIR/xchg},security_model=none,mount_tag=shared \
${if cfg.useBootLoader then ''
- -drive index=0,id=drive1,file=$NIX_DISK_IMAGE,if=${cfg.qemu.diskInterface},cache=writeback,werror=report \
- -drive index=1,id=drive2,file=$TMPDIR/disk.img,media=disk \
+ ${mkDiskIfaceDriveFlag "0" "file=$NIX_DISK_IMAGE,cache=writeback,werror=report"} \
+ ${mkDiskIfaceDriveFlag "1" "file=$TMPDIR/disk.img,media=disk"} \
${if cfg.useEFIBoot then ''
-pflash $TMPDIR/bios.bin \
'' else ''
''}
'' else ''
- -drive index=0,id=drive1,file=$NIX_DISK_IMAGE,if=${cfg.qemu.diskInterface},cache=writeback,werror=report \
+ ${mkDiskIfaceDriveFlag "0" "file=$NIX_DISK_IMAGE,cache=writeback,werror=report"} \
-kernel ${config.system.build.toplevel}/kernel \
-initrd ${config.system.build.toplevel}/initrd \
-append "$(cat ${config.system.build.toplevel}/kernel-params) init=${config.system.build.toplevel}/init regInfo=${regInfo}/registration ${kernelConsole} $QEMU_KERNEL_PARAMS" \
@@ -338,11 +353,8 @@ in
mkOption {
default = "virtio";
example = "scsi";
- type = types.str;
- description = ''
- The interface used for the virtual hard disks
- (<literal>virtio</literal> or <literal>scsi</literal>).
- '';
+ type = types.enum [ "virtio" "scsi" "ide" ];
+ description = "The interface used for the virtual hard disks.";
};
};