[pve-devel] [PATCH v2 qemu-server 4/7] fix #3010: add 'bootorder' parameter for better control of boot devices

[Stefan Reiter <s.reiter@proxmox.com>]

(also fixes #3011)

Deprecates the old-style 'boot' and 'bootdisk' options by adding a new
'order=' subproperty to 'boot'.

This allows a user to specify more than one disk in the boot order,
helping with newer versions of SeaBIOS/OVMF where disks without a
bootindex won't be initialized at all (breaks soft-raid and some LVM
setups).

This also allows specifying a bootindex for USB and hostpci devices,
which was not possible before. Floppy boot support is not supported in
the new model, but I doubt that will be a problem (AFAICT we can't even
attach floppy disks to a VM?).

Default behaviour is intended to stay the same, i.e. while new VMs will
receive the new 'order' property, it will be set so the VM starts the
same as before (using get_default_bootorder).

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
 PVE/API2/Qemu.pm        |  6 ++--
 PVE/CLI/qm.pm           |  4 +--
 PVE/QemuServer.pm       | 67 ++++++++++++++++++-----------------------
 PVE/QemuServer/Drive.pm | 21 +++++++++++--
 PVE/QemuServer/PCI.pm   |  3 +-
 PVE/QemuServer/USB.pm   | 14 ++++++---
 6 files changed, 64 insertions(+), 51 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 8da616a..0d82d3e 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -656,9 +656,9 @@ __PACKAGE__->register_method({
 		eval {
 		    $vollist = &$create_disks($rpcenv, $authuser, $conf, $arch, $storecfg, $vmid, $pool, $param, $storage);
 
-		    if (!$conf->{bootdisk}) {
-			my $firstdisk = PVE::QemuServer::Drive::resolve_first_disk($conf);
-			$conf->{bootdisk} = $firstdisk if $firstdisk;
+		    if (!$conf->{boot}) {
+			my $devs = PVE::QemuServer::get_default_bootdevices($conf);
+			$conf->{boot} = PVE::QemuServer::print_bootorder($devs);
 		    }
 
 		    # auto generate uuid if user did not specify smbios1 option
diff --git a/PVE/CLI/qm.pm b/PVE/CLI/qm.pm
index 282fa86..6243b06 100755
--- a/PVE/CLI/qm.pm
+++ b/PVE/CLI/qm.pm
@@ -656,8 +656,8 @@ __PACKAGE__->register_method ({
 
 	    # reload after disks entries have been created
 	    $conf = PVE::QemuConfig->load_config($vmid);
-	    my $firstdisk = PVE::QemuServer::Drive::resolve_first_disk($conf);
-	    $conf->{bootdisk} = $firstdisk if $firstdisk;
+	    my $devs = PVE::QemuServer::get_default_bootdevices($conf);
+	    $conf->{boot} = PVE::QemuServer::print_bootorder($devs);
 	    PVE::QemuConfig->write_config($vmid, $conf);
 	};
 
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index cfac03a..8279571 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -397,15 +397,14 @@ EODESC
     },
     boot => {
 	optional => 1,
-	type => 'string',
-	description => "Boot on floppy (a), hard disk (c), CD-ROM (d), or network (n).",
-	pattern => '[acdn]{1,4}',
-	default => 'cdn',
+	type => 'string', format => 'pve-qm-boot',
+	description => "Specify guest boot order. Use with 'order=', usage with"
+		     . " no key or 'legacy=' is deprecated.",
     },
     bootdisk => {
 	optional => 1,
 	type => 'string', format => 'pve-qm-bootdisk',
-	description => "Enable booting from specified disk.",
+	description => "Enable booting from specified disk. Deprecated: Use 'boot: order=foo;bar' instead.",
 	pattern => '(ide|sata|scsi|virtio)\d+',
     },
     smp => {
@@ -1614,8 +1613,6 @@ sub print_drive_commandline_full {
 sub print_netdevice_full {
     my ($vmid, $conf, $net, $netid, $bridges, $use_old_bios_files, $arch, $machine_type) = @_;
 
-    my $bootorder = $conf->{boot} || $confdesc->{boot}->{default};
-
     my $device = $net->{model};
     if ($net->{model} eq 'virtio') {
          $device = 'virtio-net-pci';
@@ -3213,17 +3210,30 @@ sub config_to_command {
 	push @$devices, '-device', $kbd if defined($kbd);
     }
 
+    my $bootorder = {};
+    my $boot = parse_property_string($boot_fmt, $conf->{boot}) if $conf->{boot};
+    if (!defined($boot) || $boot->{legacy}) {
+	$bootorder = bootorder_from_legacy($conf, $boot);
+    } elsif ($boot->{order}) {
+	# start at 100 to allow user to insert devices before us with -args
+	my $i = 100;
+	for my $dev (PVE::Tools::split_list($boot->{order})) {
+	    $bootorder->{$dev} = $i++;
+	}
+    }
+
     # host pci device passthrough
     my ($kvm_off, $gpu_passthrough, $legacy_igd) = PVE::QemuServer::PCI::print_hostpci_devices(
-	$vmid, $conf, $devices, $winversion, $q35, $bridges, $arch, $machine_type);
+	$vmid, $conf, $devices, $winversion, $q35, $bridges, $arch, $machine_type, $bootorder);
 
     # usb devices
     my $usb_dev_features = {};
     $usb_dev_features->{spice_usb3} = 1 if min_version($machine_version, 4, 0);
 
     my @usbdevices = PVE::QemuServer::USB::get_usb_devices(
-        $conf, $usbdesc->{format}, $MAX_USB_DEVICES, $usb_dev_features);
+        $conf, $usbdesc->{format}, $MAX_USB_DEVICES, $usb_dev_features, $bootorder);
     push @$devices, @usbdevices if @usbdevices;
+
     # serial devices
     for (my $i = 0; $i < $MAX_SERIAL_PORTS; $i++)  {
 	if (my $path = $conf->{"serial$i"}) {
@@ -3291,15 +3301,6 @@ sub config_to_command {
     }
     push @$cmd, '-nodefaults';
 
-    my $bootorder = $conf->{boot} || $confdesc->{boot}->{default};
-
-    my $bootindex_hash = {};
-    my $i = 1;
-    foreach my $o (split(//, $bootorder)) {
-	$bootindex_hash->{$o} = $i*100;
-	$i++;
-    }
-
     push @$cmd, '-boot', "menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg";
 
     push @$cmd, '-no-acpi' if defined($conf->{acpi}) && $conf->{acpi} == 0;
@@ -3469,17 +3470,7 @@ sub config_to_command {
 
 	$use_virtio = 1 if $ds =~ m/^virtio/;
 
-	if (drive_is_cdrom ($drive)) {
-	    if ($bootindex_hash->{d}) {
-		$drive->{bootindex} = $bootindex_hash->{d};
-		$bootindex_hash->{d} += 1;
-	    }
-	} else {
-	    if ($bootindex_hash->{c}) {
-		$drive->{bootindex} = $bootindex_hash->{c} if $conf->{bootdisk} && ($conf->{bootdisk} eq $ds);
-		$bootindex_hash->{c} += 1;
-	    }
-	}
+	$drive->{bootindex} = $bootorder->{$ds} if $bootorder->{$ds};
 
 	if ($drive->{interface} eq 'virtio'){
            push @$cmd, '-object', "iothread,id=iothread-$ds" if $drive->{iothread};
@@ -3530,22 +3521,21 @@ sub config_to_command {
     });
 
     for (my $i = 0; $i < $MAX_NETS; $i++) {
-	next if !$conf->{"net$i"};
-	my $d = parse_net($conf->{"net$i"});
+	my $netname = "net$i";
+
+	next if !$conf->{$netname};
+	my $d = parse_net($conf->{$netname});
 	next if !$d;
 
 	$use_virtio = 1 if $d->{model} eq 'virtio';
 
-	if ($bootindex_hash->{n}) {
-	    $d->{bootindex} = $bootindex_hash->{n};
-	    $bootindex_hash->{n} += 1;
-	}
+	$d->{bootindex} = $bootorder->{$netname} if $bootorder->{$netname};
 
-	my $netdevfull = print_netdev_full($vmid, $conf, $arch, $d, "net$i");
+	my $netdevfull = print_netdev_full($vmid, $conf, $arch, $d, $netname);
 	push @$devices, '-netdev', $netdevfull;
 
 	my $netdevicefull = print_netdevice_full(
-	    $vmid, $conf, $d, "net$i", $bridges, $use_old_bios_files, $arch, $machine_type);
+	    $vmid, $conf, $d, $netname, $bridges, $use_old_bios_files, $arch, $machine_type);
 
 	push @$devices, '-device', $netdevicefull;
     }
@@ -3827,7 +3817,8 @@ sub vm_deviceunplug {
     my $devices_list = vm_devices_list($vmid);
     return 1 if !defined($devices_list->{$deviceid});
 
-    die "can't unplug bootdisk" if $conf->{bootdisk} && $conf->{bootdisk} eq $deviceid;
+    my $bootdisks = PVE::QemuServer::Drive::get_bootdisks($conf);
+    die "can't unplug bootdisk '$deviceid'\n" if grep {$_ eq $deviceid} @$bootdisks;
 
     if ($deviceid eq 'tablet' || $deviceid eq 'keyboard') {
 
diff --git a/PVE/QemuServer/Drive.pm b/PVE/QemuServer/Drive.pm
index b71fc93..6f69a4b 100644
--- a/PVE/QemuServer/Drive.pm
+++ b/PVE/QemuServer/Drive.pm
@@ -501,11 +501,28 @@ sub print_drive {
     return PVE::JSONSchema::print_property_string($drive, $alldrive_fmt, $skip);
 }
 
+sub get_bootdisks {
+    my ($conf) = @_;
+
+    my $bootcfg = PVE::JSONSchema::parse_property_string('pve-qm-boot', $conf->{boot})
+	if $conf->{boot};
+
+    if (!defined($bootcfg) || $bootcfg->{legacy}) {
+	return [$conf->{bootdisk}] if $conf->{bootdisk};
+	return [];
+    }
+
+    my @list = PVE::Tools::split_list($bootcfg->{order});
+    @list = grep {is_valid_drivename($_)} @list;
+    return \@list;
+}
+
 sub bootdisk_size {
     my ($storecfg, $conf) = @_;
 
-    my $bootdisk = $conf->{bootdisk};
-    return undef if !$bootdisk;
+    my $bootdisks = get_bootdisks($conf);
+    return undef if !@$bootdisks;
+    my $bootdisk = $bootdisks->[0];
     return undef if !is_valid_drivename($bootdisk);
 
     return undef if !$conf->{$bootdisk};
diff --git a/PVE/QemuServer/PCI.pm b/PVE/QemuServer/PCI.pm
index cb36845..2df2708 100644
--- a/PVE/QemuServer/PCI.pm
+++ b/PVE/QemuServer/PCI.pm
@@ -357,7 +357,7 @@ sub parse_hostpci {
 }
 
 sub print_hostpci_devices {
-    my ($vmid, $conf, $devices, $winversion, $q35, $bridges, $arch, $machine_type) = @_;
+    my ($vmid, $conf, $devices, $winversion, $q35, $bridges, $arch, $machine_type, $bootorder) = @_;
 
     my $kvm_off = 0;
     my $gpu_passthrough = 0;
@@ -446,6 +446,7 @@ sub print_hostpci_devices {
 		$devicestr .= "$xvga";
 		$devicestr .= ",multifunction=on" if $multifunction;
 		$devicestr .= ",romfile=/usr/share/kvm/$d->{romfile}" if $d->{romfile};
+		$devicestr .= ",bootindex=$bootorder->{$id}" if $bootorder->{$id};
 	    }
 
 	    push @$devices, '-device', $devicestr;
diff --git a/PVE/QemuServer/USB.pm b/PVE/QemuServer/USB.pm
index d328148..4a843cd 100644
--- a/PVE/QemuServer/USB.pm
+++ b/PVE/QemuServer/USB.pm
@@ -74,13 +74,14 @@ sub get_usb_controllers {
 }
 
 sub get_usb_devices {
-    my ($conf, $format, $max_usb_devices, $features) = @_;
+    my ($conf, $format, $max_usb_devices, $features, $bootorder) = @_;
 
     my $devices = [];
 
     for (my $i = 0; $i < $max_usb_devices; $i++)  {
-	next if !$conf->{"usb$i"};
-	my $d = eval { PVE::JSONSchema::parse_property_string($format,$conf->{"usb$i"}) };
+	my $devname = "usb$i";
+	next if !$conf->{$devname};
+	my $d = eval { PVE::JSONSchema::parse_property_string($format,$conf->{$devname}) };
 	next if !$d;
 
 	if (defined($d->{host})) {
@@ -93,8 +94,10 @@ sub get_usb_devices {
 
 		push @$devices, '-chardev', "spicevmc,id=usbredirchardev$i,name=usbredir";
 		push @$devices, '-device', "usb-redir,chardev=usbredirchardev$i,id=usbredirdev$i,bus=$bus.0";
+
+		warn "warning: spice usb port set as bootdevice, ignoring\n" if $bootorder->{$devname};
 	    } else {
-		push @$devices, '-device', print_usbdevice_full($conf, "usb$i", $hostdevice);
+		push @$devices, '-device', print_usbdevice_full($conf, $devname, $hostdevice, $bootorder);
 	    }
 	}
     }
@@ -103,7 +106,7 @@ sub get_usb_devices {
 }
 
 sub print_usbdevice_full {
-    my ($conf, $deviceid, $device) = @_;
+    my ($conf, $deviceid, $device, $bootorder) = @_;
 
     return if !$device;
     my $usbdevice = "usb-host";
@@ -120,6 +123,7 @@ sub print_usbdevice_full {
     }
 
     $usbdevice .= ",id=$deviceid";
+    $usbdevice .= ",bootindex=$bootorder->{$deviceid}" if $bootorder->{$deviceid};
     return $usbdevice;
 }
 
-- 
2.20.1