[pve-devel] [PATCH v2 0/7] Improve boot device/order configuration

[pve-devel] [PATCH v2 0/7] Improve boot device/order configuration

[Stefan Reiter]

The goal of the series is to modernize boot order selection by allowing the user
to specify arbitrary VM disks and devices in any order. Fixes #3010 and #3011.

First patch is a standalone cleanup. Tests are seperated into extra patches.

v2:
* split qemu-server changes into seperate patches (hence changelog only here)
* use order= property in boot: instead of standalone bootorder:
** update API and GUI to match
* fix get_default_bootdevices and bootorder_from_legacy to *actually* match the
old behaviour, as is now verified with the test
=> see GUI patch for changes there


qemu-server: Stefan Reiter (6):
  fix indentation
  cfg2cmd: add test for legacy-style bootorder
  add new 'boot' property format and introduce legacy conversion helpers
  fix #3010: add 'bootorder' parameter for better control of boot
    devices
  api: add handling for new boot order format
  cfg2cmd: add tests for new boot order property

 PVE/API2/Qemu.pm                       |  37 ++++-
 PVE/CLI/qm.pm                          |   4 +-
 PVE/QemuServer.pm                      | 207 ++++++++++++++++++++-----
 PVE/QemuServer/Drive.pm                |  32 +++-
 PVE/QemuServer/PCI.pm                  |   3 +-
 PVE/QemuServer/USB.pm                  |  14 +-
 test/cfg2cmd/bootorder-empty.conf      |  16 ++
 test/cfg2cmd/bootorder-empty.conf.cmd  |  38 +++++
 test/cfg2cmd/bootorder-legacy.conf     |  17 ++
 test/cfg2cmd/bootorder-legacy.conf.cmd |  38 +++++
 test/cfg2cmd/bootorder.conf            |  16 ++
 test/cfg2cmd/bootorder.conf.cmd        |  38 +++++
 12 files changed, 398 insertions(+), 62 deletions(-)
 create mode 100644 test/cfg2cmd/bootorder-empty.conf
 create mode 100644 test/cfg2cmd/bootorder-empty.conf.cmd
 create mode 100644 test/cfg2cmd/bootorder-legacy.conf
 create mode 100644 test/cfg2cmd/bootorder-legacy.conf.cmd
 create mode 100644 test/cfg2cmd/bootorder.conf
 create mode 100644 test/cfg2cmd/bootorder.conf.cmd

manager: Stefan Reiter (1):
  ui: improve boot order editor with 'bootorder' support

 www/css/ext6-pve.css               |   4 +
 www/manager6/qemu/BootOrderEdit.js | 355 ++++++++++++++++++-----------
 www/manager6/qemu/Options.js       |  32 ++-
 3 files changed, 257 insertions(+), 134 deletions(-)

-- 
2.20.1

[pve-devel] [PATCH v2 qemu-server 1/7] fix indentation

[Stefan Reiter]

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
 PVE/QemuServer.pm | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 2747c66..bd59616 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -3468,24 +3468,24 @@ sub config_to_command {
     });
 
     for (my $i = 0; $i < $MAX_NETS; $i++) {
-	 next if !$conf->{"net$i"};
-	 my $d = parse_net($conf->{"net$i"});
-	 next if !$d;
+	next if !$conf->{"net$i"};
+	my $d = parse_net($conf->{"net$i"});
+	next if !$d;
 
-	 $use_virtio = 1 if $d->{model} eq 'virtio';
+	$use_virtio = 1 if $d->{model} eq 'virtio';
 
-	 if ($bootindex_hash->{n}) {
+	if ($bootindex_hash->{n}) {
 	    $d->{bootindex} = $bootindex_hash->{n};
 	    $bootindex_hash->{n} += 1;
-	 }
+	}
 
-	 my $netdevfull = print_netdev_full($vmid, $conf, $arch, $d, "net$i");
-	 push @$devices, '-netdev', $netdevfull;
+	my $netdevfull = print_netdev_full($vmid, $conf, $arch, $d, "net$i");
+	push @$devices, '-netdev', $netdevfull;
 
-	 my $netdevicefull = print_netdevice_full(
+	my $netdevicefull = print_netdevice_full(
 	    $vmid, $conf, $d, "net$i", $bridges, $use_old_bios_files, $arch, $machine_type);
 
-	 push @$devices, '-device', $netdevicefull;
+	push @$devices, '-device', $netdevicefull;
     }
 
     if ($conf->{ivshmem}) {
-- 
2.20.1

[pve-devel] [PATCH v2 qemu-server 2/7] cfg2cmd: add test for legacy-style bootorder

[Stefan Reiter]

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
 test/cfg2cmd/bootorder-legacy.conf     | 17 ++++++++++++
 test/cfg2cmd/bootorder-legacy.conf.cmd | 38 ++++++++++++++++++++++++++
 2 files changed, 55 insertions(+)
 create mode 100644 test/cfg2cmd/bootorder-legacy.conf
 create mode 100644 test/cfg2cmd/bootorder-legacy.conf.cmd

diff --git a/test/cfg2cmd/bootorder-legacy.conf b/test/cfg2cmd/bootorder-legacy.conf
new file mode 100644
index 0000000..f49b084
--- /dev/null
+++ b/test/cfg2cmd/bootorder-legacy.conf
@@ -0,0 +1,17 @@
+# TEST: Test for a specific bootorder given by legacy 'boot' value
+# QEMU_VERSION: 5.1
+cores: 3
+boot: ndca
+bootdisk: virtio1
+ide2: none,media=cdrom
+memory: 768
+name: simple
+net0: virtio=A2:C0:43:77:08:A0,bridge=vmbr0
+numa: 0
+ostype: l26
+scsi4: local:8006/vm-8006-disk-0.qcow2,discard=on,size=104858K
+smbios1: uuid=7b10d7af-b932-4c66-b2c3-3996152ec465
+sockets: 1
+virtio0: local:8006/vm-8006-disk-0.qcow2,discard=on,iothread=1,size=104858K
+virtio1: local:8006/vm-8006-disk-0.qcow2,discard=on,iothread=1,size=104858K
+vmgenid: c773c261-d800-4348-9f5d-167fadd53cf8
diff --git a/test/cfg2cmd/bootorder-legacy.conf.cmd b/test/cfg2cmd/bootorder-legacy.conf.cmd
new file mode 100644
index 0000000..f624ea2
--- /dev/null
+++ b/test/cfg2cmd/bootorder-legacy.conf.cmd
@@ -0,0 +1,38 @@
+/usr/bin/kvm \
+  -id 8006 \
+  -name simple \
+  -chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server,nowait' \
+  -mon 'chardev=qmp,mode=control' \
+  -chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5' \
+  -mon 'chardev=qmp-event,mode=control' \
+  -pidfile /var/run/qemu-server/8006.pid \
+  -daemonize \
+  -smbios 'type=1,uuid=7b10d7af-b932-4c66-b2c3-3996152ec465' \
+  -smp '3,sockets=1,cores=3,maxcpus=3' \
+  -nodefaults \
+  -boot 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg' \
+  -vnc unix:/var/run/qemu-server/8006.vnc,password \
+  -cpu kvm64,enforce,+kvm_pv_eoi,+kvm_pv_unhalt,+lahf_lm,+sep \
+  -m 768 \
+  -object 'iothread,id=iothread-virtio0' \
+  -object 'iothread,id=iothread-virtio1' \
+  -device 'pci-bridge,id=pci.1,chassis_nr=1,bus=pci.0,addr=0x1e' \
+  -device 'pci-bridge,id=pci.2,chassis_nr=2,bus=pci.0,addr=0x1f' \
+  -device 'vmgenid,guid=c773c261-d800-4348-9f5d-167fadd53cf8' \
+  -device 'piix3-usb-uhci,id=uhci,bus=pci.0,addr=0x1.0x2' \
+  -device 'usb-tablet,id=tablet,bus=uhci.0,port=1' \
+  -device 'VGA,id=vga,bus=pci.0,addr=0x2' \
+  -device 'virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3' \
+  -iscsi 'initiator-name=iqn.1993-08.org.debian:01:aabbccddeeff' \
+  -drive 'if=none,id=drive-ide2,media=cdrom,aio=threads' \
+  -device 'ide-cd,bus=ide.1,unit=0,drive=drive-ide2,id=ide2,bootindex=200' \
+  -device 'lsi,id=scsihw0,bus=pci.0,addr=0x5' \
+  -drive 'file=/var/lib/vz/images/8006/vm-8006-disk-0.qcow2,if=none,id=drive-scsi4,discard=on,format=qcow2,cache=none,aio=native,detect-zeroes=unmap' \
+  -device 'scsi-hd,bus=scsihw0.0,scsi-id=4,drive=drive-scsi4,id=scsi4' \
+  -drive 'file=/var/lib/vz/images/8006/vm-8006-disk-0.qcow2,if=none,id=drive-virtio0,discard=on,format=qcow2,cache=none,aio=native,detect-zeroes=unmap' \
+  -device 'virtio-blk-pci,drive=drive-virtio0,id=virtio0,bus=pci.0,addr=0xa,iothread=iothread-virtio0' \
+  -drive 'file=/var/lib/vz/images/8006/vm-8006-disk-0.qcow2,if=none,id=drive-virtio1,discard=on,format=qcow2,cache=none,aio=native,detect-zeroes=unmap' \
+  -device 'virtio-blk-pci,drive=drive-virtio1,id=virtio1,bus=pci.0,addr=0xb,iothread=iothread-virtio1,bootindex=302' \
+  -netdev 'type=tap,id=net0,ifname=tap8006i0,script=/var/lib/qemu-server/pve-bridge,downscript=/var/lib/qemu-server/pve-bridgedown,vhost=on' \
+  -device 'virtio-net-pci,mac=A2:C0:43:77:08:A0,netdev=net0,bus=pci.0,addr=0x12,id=net0,bootindex=100' \
+  -machine 'type=pc+pve0'
-- 
2.20.1

[pve-devel] [PATCH v2 qemu-server 3/7] add new 'boot' property format and introduce legacy conversion helpers

[Stefan Reiter]

The format is unused in this commit, but will replace the current
string-based format of the 'boot' property. It is included since the
parameter of bootorder_from_legacy follows it.

Two helper methods are introduced:
* bootorder_from_legacy: Parses the legacy format into a hash closer to
    what the new format represents
* get_default_bootdevices: Encapsulates the legacy default behaviour if
    nothing is specified in the boot order

resolve_first_disk is simplified and gets a new $cdrom parameter to
control the behaviour of excluding CD-ROMs or instead searching for only
them.

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
 PVE/QemuServer.pm       | 130 ++++++++++++++++++++++++++++++++++++++++
 PVE/QemuServer/Drive.pm |  11 ++--
 2 files changed, 135 insertions(+), 6 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index bd59616..cfac03a 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -1091,6 +1091,68 @@ for (my $i = 0; $i < $MAX_USB_DEVICES; $i++)  {
     $confdesc->{"usb$i"} = $usbdesc;
 }
 
+my $boot_fmt = {
+    legacy => {
+	optional => 1,
+	default_key => 1,
+	type => 'string',
+	description => "Boot on floppy (a), hard disk (c), CD-ROM (d), or network (n)."
+		     . " Deprecated, use 'order=' instead.",
+	pattern => '[acdn]{1,4}',
+	format_description => "[acdn]{1,4}",
+
+	# note: this is also the fallback if boot: is not given at all
+	default => 'cdn',
+    },
+    order => {
+	optional => 1,
+	type => 'string',
+	format => 'pve-qm-bootdev-list',
+	format_description => "device[;device...]",
+	description => <<EODESC,
+The guest will attempt to boot from devices in the order they appear here.
+
+Disks, optical drives and passed-through storage USB devices will be directly
+booted from, NICs will load PXE, and PCIe devices will either behave like disks
+(e.g. NVMe) or load an option ROM (e.g. RAID controller, hardware NIC).
+
+Note that only devices in this list will be marked as bootable and thus loaded
+by the guest firmware (BIOS/UEFI). If you require multiple disks for booting
+(e.g. software-raid), you need to specify all of them here.
+
+Overrides the deprecated 'legacy=[acdn]*' value when given.
+EODESC
+    },
+};
+PVE::JSONSchema::register_format('pve-qm-boot', $boot_fmt);
+
+PVE::JSONSchema::register_format('pve-qm-bootdev', \&verify_bootdev);
+sub verify_bootdev {
+    my ($dev, $noerr) = @_;
+
+    return $dev if PVE::QemuServer::Drive::is_valid_drivename($dev) && $dev !~ m/^efidisk/;
+
+    my $check = sub {
+	my ($base) = @_;
+	return 0 if $dev !~ m/^$base\d+$/;
+	return 0 if !$confdesc->{$dev};
+	return 1;
+    };
+
+    return $dev if $check->("net");
+    return $dev if $check->("usb");
+    return $dev if $check->("hostpci");
+
+    return undef if $noerr;
+    die "invalid boot device '$dev'\n";
+}
+
+sub print_bootorder {
+    my ($devs) = @_;
+    my $data = { order => join(';', @$devs) };
+    return PVE::JSONSchema::print_property_string($data, $boot_fmt);
+}
+
 my $kvm_api_version = 0;
 
 sub kvm_version {
@@ -7152,6 +7214,74 @@ sub clear_reboot_request {
     return $res;
 }
 
+sub bootorder_from_legacy {
+    my ($conf, $bootcfg) = @_;
+
+    my $boot = $bootcfg->{legacy} || $boot_fmt->{legacy}->{default};
+    my $bootindex_hash = {};
+    my $i = 1;
+    foreach my $o (split(//, $boot)) {
+	$bootindex_hash->{$o} = $i*100;
+	$i++;
+    }
+
+    my $bootorder = {};
+
+    PVE::QemuConfig->foreach_volume($conf, sub {
+	my ($ds, $drive) = @_;
+
+	if (drive_is_cdrom ($drive, 1)) {
+	    if ($bootindex_hash->{d}) {
+		$bootorder->{$ds} = $bootindex_hash->{d};
+		$bootindex_hash->{d} += 1;
+	    }
+	} elsif ($bootindex_hash->{c}) {
+	    $bootorder->{$ds} = $bootindex_hash->{c}
+		if $conf->{bootdisk} && $conf->{bootdisk} eq $ds;
+	    $bootindex_hash->{c} += 1;
+	}
+    });
+
+    if ($bootindex_hash->{n}) {
+	for (my $i = 0; $i < $MAX_NETS; $i++) {
+	    my $netname = "net$i";
+	    next if !$conf->{$netname};
+	    $bootorder->{$netname} = $bootindex_hash->{n};
+	    $bootindex_hash->{n} += 1;
+	}
+    }
+
+    return $bootorder;
+}
+
+# Generate default device list for 'boot: order=' property. Matches legacy
+# default boot order, but with explicit device names. This is important, since
+# the fallback for when neither 'order' nor the old format is specified relies
+# on 'bootorder_from_legacy' above, and it would be confusing if this diverges.
+sub get_default_bootdevices {
+    my ($conf) = @_;
+
+    my @ret = ();
+
+    # harddisk
+    my $first = PVE::QemuServer::Drive::resolve_first_disk($conf, 0);
+    push @ret, $first if $first;
+
+    # cdrom
+    $first = PVE::QemuServer::Drive::resolve_first_disk($conf, 1);
+    push @ret, $first if $first;
+
+    # network
+    for (my $i = 0; $i < $MAX_NETS; $i++) {
+	my $netname = "net$i";
+	next if !$conf->{$netname};
+	push @ret, $netname;
+	last;
+    }
+
+    return \@ret;
+}
+
 # bash completion helper
 
 sub complete_backup_archives {
diff --git a/PVE/QemuServer/Drive.pm b/PVE/QemuServer/Drive.pm
index 91c33f8..b71fc93 100644
--- a/PVE/QemuServer/Drive.pm
+++ b/PVE/QemuServer/Drive.pm
@@ -584,16 +584,15 @@ sub is_volume_in_use {
 }
 
 sub resolve_first_disk {
-    my $conf = shift;
+    my ($conf, $cdrom) = @_;
     my @disks = valid_drive_names();
-    my $firstdisk;
-    foreach my $ds (reverse @disks) {
+    foreach my $ds (@disks) {
 	next if !$conf->{$ds};
 	my $disk = parse_drive($ds, $conf->{$ds});
-	next if drive_is_cdrom($disk);
-	$firstdisk = $ds;
+	next if drive_is_cdrom($disk) xor $cdrom;
+	return $ds;
     }
-    return $firstdisk;
+    return undef;
 }
 
 1;
-- 
2.20.1

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

[Stefan Reiter]

(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

[pve-devel] [PATCH v2 qemu-server 5/7] api: add handling for new boot order format

[Stefan Reiter]

The API is updated to handle the deprecation correctly, i.e. when
updating the 'order' attribute, the old 'legacy' (default_key) values
are removed (would now be ignored anyway).

When removing a device that is in the bootorder list, it will be removed
from the aforementioned. Note that non-existing devices in the list will
not cause an error - they will simply be ignored - but it's still nice
to not have them in there.

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
 PVE/API2/Qemu.pm | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 0d82d3e..f1e9759 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -1191,6 +1191,12 @@ my $update_vm_api  = sub {
 
 	    my $modified = {}; # record what $option we modify
 
+	    my $bootcfg = PVE::JSONSchema::parse_property_string('pve-qm-boot', $conf->{boot})
+		if $conf->{boot};
+	    my @bootorder = PVE::Tools::split_list($bootcfg->{order})
+		if $bootcfg && $bootcfg->{order};
+	    my $bootorder_deleted = grep {$_ eq 'bootorder'} @delete;
+
 	    foreach my $opt (@delete) {
 		$modified->{$opt} = 1;
 		$conf = PVE::QemuConfig->load_config($vmid); # update/reload
@@ -1205,6 +1211,13 @@ my $update_vm_api  = sub {
 		my $is_pending_val = defined($conf->{pending}->{$opt});
 		delete $conf->{pending}->{$opt};
 
+		# remove from bootorder if necessary
+		if (!$bootorder_deleted && @bootorder && grep {$_ eq $opt} @bootorder) {
+		    @bootorder = grep {$_ ne $opt} @bootorder;
+		    $conf->{pending}->{boot} = PVE::QemuServer::print_bootorder(\@bootorder);
+		    $modified->{boot} = 1;
+		}
+
 		if ($opt =~ m/^unused/) {
 		    my $drive = PVE::QemuServer::parse_drive($opt, $val);
 		    PVE::QemuConfig->check_protection($conf, "can't remove unused disk '$drive->{file}'");
@@ -1283,6 +1296,24 @@ my $update_vm_api  = sub {
 		    $conf->{pending}->{$opt} = $param->{$opt};
 		} else {
 		    $conf->{pending}->{$opt} = $param->{$opt};
+
+		    if ($opt eq 'boot') {
+			my $new_bootcfg = PVE::JSONSchema::parse_property_string('pve-qm-boot', $param->{$opt});
+			if ($new_bootcfg->{order}) {
+			    my @devs = PVE::Tools::split_list($new_bootcfg->{order});
+			    for my $dev (@devs) {
+				my $exists = $conf->{$dev} || $conf->{pending}->{$dev};
+				my $deleted = grep {$_ eq $dev} @delete;
+				die "invalid bootorder: device '$dev' does not exist'\n"
+				    if !$exists || $deleted;
+			    }
+
+			    # remove legacy boot order settings if new one set
+			    $conf->{pending}->{$opt} = PVE::QemuServer::print_bootorder(\@devs);
+			    PVE::QemuConfig->add_to_pending_delete($conf, "bootdisk")
+				if $conf->{bootdisk};
+			}
+		    }
 		}
 		PVE::QemuConfig->remove_from_pending_delete($conf, $opt);
 		PVE::QemuConfig->write_config($vmid, $conf);
-- 
2.20.1

[pve-devel] [PATCH v2 qemu-server 6/7] cfg2cmd: add tests for new boot order property

[Stefan Reiter]

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
 test/cfg2cmd/bootorder-empty.conf     | 16 +++++++++++
 test/cfg2cmd/bootorder-empty.conf.cmd | 38 +++++++++++++++++++++++++++
 test/cfg2cmd/bootorder.conf           | 16 +++++++++++
 test/cfg2cmd/bootorder.conf.cmd       | 38 +++++++++++++++++++++++++++
 4 files changed, 108 insertions(+)
 create mode 100644 test/cfg2cmd/bootorder-empty.conf
 create mode 100644 test/cfg2cmd/bootorder-empty.conf.cmd
 create mode 100644 test/cfg2cmd/bootorder.conf
 create mode 100644 test/cfg2cmd/bootorder.conf.cmd

diff --git a/test/cfg2cmd/bootorder-empty.conf b/test/cfg2cmd/bootorder-empty.conf
new file mode 100644
index 0000000..75c6f33
--- /dev/null
+++ b/test/cfg2cmd/bootorder-empty.conf
@@ -0,0 +1,16 @@
+# TEST: Test for an empty boot parameter producing no bootindices either
+# QEMU_VERSION: 5.1
+cores: 3
+boot: 
+ide2: none,media=cdrom
+memory: 768
+name: simple
+net0: virtio=A2:C0:43:77:08:A0,bridge=vmbr0
+numa: 0
+ostype: l26
+scsi4: local:8006/vm-8006-disk-0.qcow2,discard=on,size=104858K
+smbios1: uuid=7b10d7af-b932-4c66-b2c3-3996152ec465
+sockets: 1
+virtio0: local:8006/vm-8006-disk-0.qcow2,discard=on,iothread=1,size=104858K
+virtio1: local:8006/vm-8006-disk-0.qcow2,discard=on,iothread=1,size=104858K
+vmgenid: c773c261-d800-4348-9f5d-167fadd53cf8
diff --git a/test/cfg2cmd/bootorder-empty.conf.cmd b/test/cfg2cmd/bootorder-empty.conf.cmd
new file mode 100644
index 0000000..1f2b2fb
--- /dev/null
+++ b/test/cfg2cmd/bootorder-empty.conf.cmd
@@ -0,0 +1,38 @@
+/usr/bin/kvm \
+  -id 8006 \
+  -name simple \
+  -chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server,nowait' \
+  -mon 'chardev=qmp,mode=control' \
+  -chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5' \
+  -mon 'chardev=qmp-event,mode=control' \
+  -pidfile /var/run/qemu-server/8006.pid \
+  -daemonize \
+  -smbios 'type=1,uuid=7b10d7af-b932-4c66-b2c3-3996152ec465' \
+  -smp '3,sockets=1,cores=3,maxcpus=3' \
+  -nodefaults \
+  -boot 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg' \
+  -vnc unix:/var/run/qemu-server/8006.vnc,password \
+  -cpu kvm64,enforce,+kvm_pv_eoi,+kvm_pv_unhalt,+lahf_lm,+sep \
+  -m 768 \
+  -object 'iothread,id=iothread-virtio0' \
+  -object 'iothread,id=iothread-virtio1' \
+  -device 'pci-bridge,id=pci.1,chassis_nr=1,bus=pci.0,addr=0x1e' \
+  -device 'pci-bridge,id=pci.2,chassis_nr=2,bus=pci.0,addr=0x1f' \
+  -device 'vmgenid,guid=c773c261-d800-4348-9f5d-167fadd53cf8' \
+  -device 'piix3-usb-uhci,id=uhci,bus=pci.0,addr=0x1.0x2' \
+  -device 'usb-tablet,id=tablet,bus=uhci.0,port=1' \
+  -device 'VGA,id=vga,bus=pci.0,addr=0x2' \
+  -device 'virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3' \
+  -iscsi 'initiator-name=iqn.1993-08.org.debian:01:aabbccddeeff' \
+  -drive 'if=none,id=drive-ide2,media=cdrom,aio=threads' \
+  -device 'ide-cd,bus=ide.1,unit=0,drive=drive-ide2,id=ide2' \
+  -device 'lsi,id=scsihw0,bus=pci.0,addr=0x5' \
+  -drive 'file=/var/lib/vz/images/8006/vm-8006-disk-0.qcow2,if=none,id=drive-scsi4,discard=on,format=qcow2,cache=none,aio=native,detect-zeroes=unmap' \
+  -device 'scsi-hd,bus=scsihw0.0,scsi-id=4,drive=drive-scsi4,id=scsi4' \
+  -drive 'file=/var/lib/vz/images/8006/vm-8006-disk-0.qcow2,if=none,id=drive-virtio0,discard=on,format=qcow2,cache=none,aio=native,detect-zeroes=unmap' \
+  -device 'virtio-blk-pci,drive=drive-virtio0,id=virtio0,bus=pci.0,addr=0xa,iothread=iothread-virtio0' \
+  -drive 'file=/var/lib/vz/images/8006/vm-8006-disk-0.qcow2,if=none,id=drive-virtio1,discard=on,format=qcow2,cache=none,aio=native,detect-zeroes=unmap' \
+  -device 'virtio-blk-pci,drive=drive-virtio1,id=virtio1,bus=pci.0,addr=0xb,iothread=iothread-virtio1' \
+  -netdev 'type=tap,id=net0,ifname=tap8006i0,script=/var/lib/qemu-server/pve-bridge,downscript=/var/lib/qemu-server/pve-bridgedown,vhost=on' \
+  -device 'virtio-net-pci,mac=A2:C0:43:77:08:A0,netdev=net0,bus=pci.0,addr=0x12,id=net0' \
+  -machine 'type=pc+pve0'
diff --git a/test/cfg2cmd/bootorder.conf b/test/cfg2cmd/bootorder.conf
new file mode 100644
index 0000000..8de64bf
--- /dev/null
+++ b/test/cfg2cmd/bootorder.conf
@@ -0,0 +1,16 @@
+# TEST: Test for a specific bootorder given by 'boot: order=' property
+# QEMU_VERSION: 5.1
+cores: 3
+boot: order=virtio1;net0;scsi4;ide2
+ide2: none,media=cdrom
+memory: 768
+name: simple
+net0: virtio=A2:C0:43:77:08:A0,bridge=vmbr0
+numa: 0
+ostype: l26
+scsi4: local:8006/vm-8006-disk-0.qcow2,discard=on,size=104858K
+smbios1: uuid=7b10d7af-b932-4c66-b2c3-3996152ec465
+sockets: 1
+virtio0: local:8006/vm-8006-disk-0.qcow2,discard=on,iothread=1,size=104858K
+virtio1: local:8006/vm-8006-disk-0.qcow2,discard=on,iothread=1,size=104858K
+vmgenid: c773c261-d800-4348-9f5d-167fadd53cf8
diff --git a/test/cfg2cmd/bootorder.conf.cmd b/test/cfg2cmd/bootorder.conf.cmd
new file mode 100644
index 0000000..86cae07
--- /dev/null
+++ b/test/cfg2cmd/bootorder.conf.cmd
@@ -0,0 +1,38 @@
+/usr/bin/kvm \
+  -id 8006 \
+  -name simple \
+  -chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server,nowait' \
+  -mon 'chardev=qmp,mode=control' \
+  -chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5' \
+  -mon 'chardev=qmp-event,mode=control' \
+  -pidfile /var/run/qemu-server/8006.pid \
+  -daemonize \
+  -smbios 'type=1,uuid=7b10d7af-b932-4c66-b2c3-3996152ec465' \
+  -smp '3,sockets=1,cores=3,maxcpus=3' \
+  -nodefaults \
+  -boot 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg' \
+  -vnc unix:/var/run/qemu-server/8006.vnc,password \
+  -cpu kvm64,enforce,+kvm_pv_eoi,+kvm_pv_unhalt,+lahf_lm,+sep \
+  -m 768 \
+  -object 'iothread,id=iothread-virtio0' \
+  -object 'iothread,id=iothread-virtio1' \
+  -device 'pci-bridge,id=pci.1,chassis_nr=1,bus=pci.0,addr=0x1e' \
+  -device 'pci-bridge,id=pci.2,chassis_nr=2,bus=pci.0,addr=0x1f' \
+  -device 'vmgenid,guid=c773c261-d800-4348-9f5d-167fadd53cf8' \
+  -device 'piix3-usb-uhci,id=uhci,bus=pci.0,addr=0x1.0x2' \
+  -device 'usb-tablet,id=tablet,bus=uhci.0,port=1' \
+  -device 'VGA,id=vga,bus=pci.0,addr=0x2' \
+  -device 'virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3' \
+  -iscsi 'initiator-name=iqn.1993-08.org.debian:01:aabbccddeeff' \
+  -drive 'if=none,id=drive-ide2,media=cdrom,aio=threads' \
+  -device 'ide-cd,bus=ide.1,unit=0,drive=drive-ide2,id=ide2,bootindex=103' \
+  -device 'lsi,id=scsihw0,bus=pci.0,addr=0x5' \
+  -drive 'file=/var/lib/vz/images/8006/vm-8006-disk-0.qcow2,if=none,id=drive-scsi4,discard=on,format=qcow2,cache=none,aio=native,detect-zeroes=unmap' \
+  -device 'scsi-hd,bus=scsihw0.0,scsi-id=4,drive=drive-scsi4,id=scsi4,bootindex=102' \
+  -drive 'file=/var/lib/vz/images/8006/vm-8006-disk-0.qcow2,if=none,id=drive-virtio0,discard=on,format=qcow2,cache=none,aio=native,detect-zeroes=unmap' \
+  -device 'virtio-blk-pci,drive=drive-virtio0,id=virtio0,bus=pci.0,addr=0xa,iothread=iothread-virtio0' \
+  -drive 'file=/var/lib/vz/images/8006/vm-8006-disk-0.qcow2,if=none,id=drive-virtio1,discard=on,format=qcow2,cache=none,aio=native,detect-zeroes=unmap' \
+  -device 'virtio-blk-pci,drive=drive-virtio1,id=virtio1,bus=pci.0,addr=0xb,iothread=iothread-virtio1,bootindex=100' \
+  -netdev 'type=tap,id=net0,ifname=tap8006i0,script=/var/lib/qemu-server/pve-bridge,downscript=/var/lib/qemu-server/pve-bridgedown,vhost=on' \
+  -device 'virtio-net-pci,mac=A2:C0:43:77:08:A0,netdev=net0,bus=pci.0,addr=0x12,id=net0,bootindex=101' \
+  -machine 'type=pc+pve0'
-- 
2.20.1

[pve-devel] [PATCH v2 manager 7/7] ui: improve boot order editor

[Stefan Reiter]

The new boot order property can express many more scenarios than the old
one. Update the editor so it can handle it.

Features a grid with all supported boot devices which can be reordered
using drag-and-drop, as well as toggled on and off with an inline
checkbox.

Support for configs still using the old format is given, with the first
write automatically updating the VM config to use the new one.

The renderer for the Options panel is updated with support for the new
format.

Note that it is very well possible to disable all boot devices, in which
case an empty 'boot: ' will be stored to the config file. I'm not sure
what that would be useful for, but there's no reason to forbid it
either, just warn the user that it's probably not what they want.

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---

Depends on updated qemu-server for API support.

v2:
* improve GUI with two new columns
* make more schematic, putting less in initComponent
* use existing format for renderer in Options panel, instead of changing it
  (I opted to just use the existing one instead of changing it at all now,
  just updated to support the new format)
* update for new 'empty' (no bootdevs) behaviour
* update legacy conversion code to deal with 4 characters, ignoring 'a' (floppy)
  (this appears to be a never-triggered bug in the old implementation as well?)

 www/css/ext6-pve.css               |   4 +
 www/manager6/qemu/BootOrderEdit.js | 355 ++++++++++++++++++-----------
 www/manager6/qemu/Options.js       |  32 ++-
 3 files changed, 257 insertions(+), 134 deletions(-)

diff --git a/www/css/ext6-pve.css b/www/css/ext6-pve.css
index a91f1aaf..6430ffc4 100644
--- a/www/css/ext6-pve.css
+++ b/www/css/ext6-pve.css
@@ -583,6 +583,10 @@ table.osds td:first-of-type {
     cursor: pointer;
 }
 
+.cursor-move {
+    cursor: move;
+}
+
 .x-grid-filters-filtered-column {
     font-style: italic;
     font-weight: bold;
diff --git a/www/manager6/qemu/BootOrderEdit.js b/www/manager6/qemu/BootOrderEdit.js
index 19d5d50a..c5726e96 100644
--- a/www/manager6/qemu/BootOrderEdit.js
+++ b/www/manager6/qemu/BootOrderEdit.js
@@ -1,150 +1,250 @@
+Ext.define('pve-boot-order-entry', {
+    extend: 'Ext.data.Model',
+    fields: [
+	{name: 'name', type: 'string'},
+	{name: 'enabled', type: 'bool'},
+	{name: 'desc', type: 'string'},
+    ]
+});
+
 Ext.define('PVE.qemu.BootOrderPanel', {
     extend: 'Proxmox.panel.InputPanel',
     alias: 'widget.pveQemuBootOrderPanel',
+
     vmconfig: {}, // store loaded vm config
+    store: undefined,
 
-    bootdisk: undefined,
-    selection: [],
-    list: [],
-    comboboxes: [],
+    inUpdate: false,
+    controller: {
+	xclass: 'Ext.app.ViewController',
+    },
 
-    isBootDisk: function(value) {
+    isDisk: function(value) {
 	return PVE.Utils.bus_match.test(value);
     },
 
-    setVMConfig: function(vmconfig) {
-	var me = this;
-	me.vmconfig = vmconfig;
-	var order = me.vmconfig.boot || 'cdn';
-	me.bootdisk = me.vmconfig.bootdisk || undefined;
-
-	// get the first 3 characters
-	// ignore the rest (there should never be more than 3)
-	me.selection = order.split('').slice(0,3);
-
-	// build bootdev list
-	me.list = [];
-	Ext.Object.each(me.vmconfig, function(key, value) {
-	    if (me.isBootDisk(key) &&
-		!(/media=cdrom/).test(value)) {
-		me.list.push([key, "Disk '" + key + "'"]);
-	    }
-	});
-
-	me.list.push(['d', 'CD-ROM']);
-	me.list.push(['n', gettext('Network')]);
-	me.list.push(['__none__', Proxmox.Utils.noneText]);
-
-	me.recomputeList();
-
-	me.comboboxes.forEach(function(box) {
-	    box.resetOriginalValue();
-	});
+    isBootdev: function(dev, value) {
+	return this.isDisk(dev) ||
+	    (/^net\d+/).test(dev) ||
+	    (/^hostpci\d+/).test(dev) ||
+	    ((/^usb\d+/).test(dev) && !(/spice/).test(value));
     },
 
-    onGetValues: function(values) {
-	var me = this;
-	var order = me.selection.join('');
-	var res = { boot: order };
+    setVMConfig: function(vmconfig) {
+	let me = this;
+	me.vmconfig = vmconfig;
 
-	if  (me.bootdisk && order.indexOf('c') !== -1) {
-	    res.bootdisk = me.bootdisk;
-	} else {
-	    res['delete'] = 'bootdisk';
+	me.store.removeAll();
+
+	let boot = PVE.Parser.parsePropertyString(me.vmconfig.boot, "legacy");
+
+	let bootorder = [];
+	if (boot.order) {
+	    bootorder = boot.order.split(';').map(dev => ({name: dev, enabled: true}));
+	} else if (!(/^\s*$/).test(me.vmconfig.boot)) {
+	    // legacy style, transform to new bootorder
+	    let order = boot.legacy || 'cdn';
+	    let bootdisk = me.vmconfig.bootdisk || undefined;
+
+	    // get the first 4 characters (acdn)
+	    // ignore the rest (there should never be more than 4)
+	    let orderList = order.split('').slice(0,4);
+
+	    // build bootdev list
+	    for (let i = 0; i < orderList.length; i++) {
+		let list = [];
+		if (orderList[i] === 'c') {
+		    if (bootdisk !== undefined && me.vmconfig[bootdisk]) {
+			list.push(bootdisk);
+		    }
+		} else if (orderList[i] === 'd') {
+		    Ext.Object.each(me.vmconfig, function(key, value) {
+			if (me.isDisk(key) && (/media=cdrom/).test(value)) {
+			    list.push(key);
+			}
+		    });
+		} else if (orderList[i] === 'n') {
+		    Ext.Object.each(me.vmconfig, function(key, value) {
+			if ((/^net\d+/).test(key)) {
+			    list.push(key);
+			}
+		    });
+		}
+
+		// Object.each iterates in random order, sort alphabetically
+		list.sort();
+		list.forEach(dev => bootorder.push({name: dev, enabled: true}));
+	    }
 	}
 
+	// add disabled devices as well
+	let disabled = [];
+	Ext.Object.each(me.vmconfig, function(key, value) {
+	    if (me.isBootdev(key, value) &&
+		!Ext.Array.some(bootorder, x => x.name === key))
+	    {
+		disabled.push(key);
+	    }
+	});
+	disabled.sort();
+	disabled.forEach(dev => bootorder.push({name: dev, enabled: false}));
+
+	// add descriptions
+	bootorder.forEach(entry => {
+	    entry.desc = me.vmconfig[entry.name];
+	});
+
+	me.store.insert(0, bootorder);
+	me.store.fireEvent("update");
+    },
+
+    calculateValue: function() {
+	let me = this;
+	return me.store.getData().items
+	    .filter(x => x.data.enabled)
+	    .map(x => x.data.name)
+	    .join(';');
+    },
+
+    onGetValues: function() {
+	let me = this;
+	// Note: we allow an empty value, so no 'delete' option
+	let val = { order: me.calculateValue() };
+	let res = { boot: PVE.Parser.printPropertyString(val) };
 	return res;
     },
 
-    recomputeSelection: function(combobox, newVal, oldVal) {
-	var me = this.up('#inputpanel');
-	me.selection = [];
-	me.comboboxes.forEach(function(item) {
-	    var val = item.getValue();
-
-	    // when selecting an already selected item,
-	    // switch it around
-	    if ((val === newVal || (me.isBootDisk(val) && me.isBootDisk(newVal))) &&
-		item.name !== combobox.name &&
-		newVal !== '__none__') {
-		// swap items
-		val = oldVal;
-	    }
-
-	    // push 'c','d' or 'n' in the array
-	    if (me.isBootDisk(val)) {
-		me.selection.push('c');
-		me.bootdisk = val;
-	    } else if (val === 'd' ||
-		       val === 'n') {
-		me.selection.push(val);
-	    }
-	});
-
-	me.recomputeList();
-    },
-
-    recomputeList: function(){
-	var me = this;
-	// set the correct values in the kvcomboboxes
-	var cnt = 0;
-	me.comboboxes.forEach(function(item) {
-	    if (cnt === 0) {
-		// never show 'none' on first combobox
-		item.store.loadData(me.list.slice(0, me.list.length-1));
-	    } else {
-		item.store.loadData(me.list);
-	    }
-	    item.suspendEvent('change');
-	    if (cnt < me.selection.length) {
-		item.setValue((me.selection[cnt] !== 'c')?me.selection[cnt]:me.bootdisk);
-	    } else if (cnt === 0){
-		item.setValue('');
-	    } else {
-		item.setValue('__none__');
-	    }
-	    cnt++;
-	    item.resumeEvent('change');
-	    item.validate();
-	});
-    },
-
-    initComponent : function() {
-	var me = this;
-
-	// this has to be done here, because of
-	// the way our inputPanel class handles items
-	me.comboboxes = [
-		Ext.createWidget('proxmoxKVComboBox', {
-		fieldLabel: gettext('Boot device') + " 1",
-		labelWidth: 120,
-		name: 'bd1',
-		allowBlank: false,
-		listeners: {
-		    change: me.recomputeSelection
+    items: [
+	{
+	    xtype: 'grid',
+	    reference: 'grid',
+	    margin: '0 0 5 0',
+	    columns: [
+		{
+		    header: '',
+		    renderer: () => "<i class='fa fa-reorder cursor-move'></i>",
+		    width: 30,
+		    sortable: false,
+		    hideable: false,
+		    draggable: false,
+		},
+		{
+		    header: '#',
+		    width: 30,
+		    sortable: false,
+		    hideable: false,
+		    draggable: false,
+		    renderer: (value, metaData, record, rowIndex) => {
+			let idx = (rowIndex + 1).toString();
+			if (record.get('enabled')) {
+			    return idx;
+			} else {
+			    return "<span class='faded'>" + idx + "</span>";
+			}
+		    },
+		},
+		{
+		    xtype: 'checkcolumn',
+		    header: gettext('Enabled'),
+		    dataIndex: 'enabled',
+		    width: 70,
+		    sortable: false,
+		    hideable: false,
+		    draggable: false,
+		},
+		{
+		    header: gettext('Device'),
+		    dataIndex: 'name',
+		    width: 70,
+		    sortable: false,
+		    hideable: false,
+		    draggable: false,
+		},
+		{
+		    header: gettext('Description'),
+		    dataIndex: 'desc',
+		    flex: true,
+		    sortable: false,
+		    hideable: false,
+		    draggable: false,
+		},
+	    ],
+	    viewConfig: {
+		plugins: {
+		    ptype: 'gridviewdragdrop',
+		    dragText: gettext('Drag and drop to reorder'),
 		}
-	    }),
-		Ext.createWidget('proxmoxKVComboBox', {
-		fieldLabel: gettext('Boot device') + " 2",
-		labelWidth: 120,
-		name: 'bd2',
-		allowBlank: false,
-		listeners: {
-		    change: me.recomputeSelection
+	    },
+	    listeners: {
+		drop: function() {
+		    // doesn't fire automatically on reorder
+		    this.getStore().fireEvent("update");
 		}
-	    }),
-		Ext.createWidget('proxmoxKVComboBox', {
-		fieldLabel: gettext('Boot device') + " 3",
-		labelWidth: 120,
-		name: 'bd3',
-		allowBlank: false,
-		listeners: {
-		    change: me.recomputeSelection
+	    },
+	},
+	{
+	    xtype: 'component',
+	    html: gettext('Drag and drop to reorder'),
+	},
+	{
+	    xtype: 'displayfield',
+	    reference: 'emptyWarning',
+	    userCls: 'pmx-hint',
+	    value: gettext('Warning: No devices selected, the VM will probably not boot!'),
+	},
+	{
+	    // for dirty marking and 'reset' function
+	    xtype: 'field',
+	    reference: 'marker',
+	    hidden: true,
+	    setValue: function(val) {
+		let me = this;
+		let panel = me.up('pveQemuBootOrderPanel');
+
+		// on form reset, go back to original state
+		if (!panel.inUpdate) {
+		    panel.setVMConfig(panel.vmconfig);
 		}
-	    })
-	];
-	Ext.apply(me, { items: me.comboboxes });
+
+		// not a subclass, so no callParent; just do it manually
+		me.setRawValue(me.valueToRaw(val));
+		return me.mixins.field.setValue.call(me, val);
+	    }
+	},
+    ],
+
+    initComponent: function() {
+	let me = this;
+
 	me.callParent();
+
+	let controller = me.getController();
+
+	let grid = controller.lookup('grid');
+	let marker = controller.lookup('marker');
+	let emptyWarning = controller.lookup('emptyWarning');
+
+	marker.originalValue = undefined;
+
+	me.store = Ext.create('Ext.data.Store', {
+	    model: 'pve-boot-order-entry',
+	    listeners: {
+		update: function() {
+		    this.commitChanges();
+		    let val = me.calculateValue();
+		    if (marker.originalValue === undefined) {
+			marker.originalValue = val;
+		    }
+		    me.inUpdate = true;
+		    marker.setValue(val);
+		    me.inUpdate = false;
+		    marker.checkDirty();
+		    emptyWarning.setHidden(val !== '');
+		    grid.getView().refresh();
+		}
+	    }
+	});
+	grid.setStore(me.store);
     }
 });
 
@@ -157,9 +257,10 @@ Ext.define('PVE.qemu.BootOrderEdit', {
     }],
 
     subject: gettext('Boot Order'),
+    width: 600,
 
     initComponent : function() {
-	var me = this;
+	let me = this;
 	me.callParent();
 	me.load({
 	    success: function(response, options) {
diff --git a/www/manager6/qemu/Options.js b/www/manager6/qemu/Options.js
index 20f6ffbb..1f07d81a 100644
--- a/www/manager6/qemu/Options.js
+++ b/www/manager6/qemu/Options.js
@@ -92,27 +92,45 @@ Ext.define('PVE.qemu.Options', {
 		editor: caps.vms['VM.Config.Disk'] ? 'PVE.qemu.BootOrderEdit' : undefined,
 		multiKey: ['boot', 'bootdisk'],
 		renderer: function(order, metaData, record, rowIndex, colIndex, store, pending) {
+		    if (/^\s*$/.test(order)) {
+			return gettext('(No boot device selected)');
+		    }
+		    let boot = PVE.Parser.parsePropertyString(order, "legacy");
+		    if (boot.order) {
+			let list = boot.order.split(';');
+			let ret = '';
+			let i = 1;
+			list.forEach(dev => {
+			    if (ret) {
+				ret += ', ';
+			    }
+			    ret += dev;
+			});
+			return ret;
+		    }
+
+		    // legacy style and fallback
 		    var i;
 		    var text = '';
 		    var bootdisk = me.getObjectValue('bootdisk', undefined, pending);
-		    order = order || 'cdn';
+		    order = boot.legacy || 'cdn';
 		    for (i = 0; i < order.length; i++) {
-			var sel = order.substring(i, i + 1);
 			if (text) {
 			    text += ', ';
 			}
+			var sel = order.substring(i, i + 1);
 			if (sel === 'c') {
 			    if (bootdisk) {
-				text += "Disk '" + bootdisk + "'";
+				text += bootdisk;
 			    } else {
-				text += "Disk";
+				text += gettext('(no bootdisk)');
 			    }
 			} else if (sel === 'n') {
-			    text += 'Network';
+			    text += gettext('any net');
 			} else if (sel === 'a') {
-			    text += 'Floppy';
+			    text += gettext('Floppy');
 			} else if (sel === 'd') {
-			    text += 'CD-ROM';
+			    text += gettext('any CD-ROM');
 			} else {
 			    text += sel;
 			}
-- 
2.20.1

[pve-devel] applied-series: [PATCH v2 0/7] Improve boot device/order configuration

[Thomas Lamprecht]

On 06.10.20 15:32, Stefan Reiter wrote:
> The goal of the series is to modernize boot order selection by allowing the user
> to specify arbitrary VM disks and devices in any order. Fixes #3010 and #3011.
> 
> First patch is a standalone cleanup. Tests are seperated into extra patches.
> 
> v2:
> * split qemu-server changes into seperate patches (hence changelog only here)
> * use order= property in boot: instead of standalone bootorder:
> ** update API and GUI to match
> * fix get_default_bootdevices and bootorder_from_legacy to *actually* match the
> old behaviour, as is now verified with the test
> => see GUI patch for changes there
> 
> 
> qemu-server: Stefan Reiter (6):
>   fix indentation
>   cfg2cmd: add test for legacy-style bootorder
>   add new 'boot' property format and introduce legacy conversion helpers
>   fix #3010: add 'bootorder' parameter for better control of boot
>     devices
>   api: add handling for new boot order format
>   cfg2cmd: add tests for new boot order property
> 
>  PVE/API2/Qemu.pm                       |  37 ++++-
>  PVE/CLI/qm.pm                          |   4 +-
>  PVE/QemuServer.pm                      | 207 ++++++++++++++++++++-----
>  PVE/QemuServer/Drive.pm                |  32 +++-
>  PVE/QemuServer/PCI.pm                  |   3 +-
>  PVE/QemuServer/USB.pm                  |  14 +-
>  test/cfg2cmd/bootorder-empty.conf      |  16 ++
>  test/cfg2cmd/bootorder-empty.conf.cmd  |  38 +++++
>  test/cfg2cmd/bootorder-legacy.conf     |  17 ++
>  test/cfg2cmd/bootorder-legacy.conf.cmd |  38 +++++
>  test/cfg2cmd/bootorder.conf            |  16 ++
>  test/cfg2cmd/bootorder.conf.cmd        |  38 +++++
>  12 files changed, 398 insertions(+), 62 deletions(-)
>  create mode 100644 test/cfg2cmd/bootorder-empty.conf
>  create mode 100644 test/cfg2cmd/bootorder-empty.conf.cmd
>  create mode 100644 test/cfg2cmd/bootorder-legacy.conf
>  create mode 100644 test/cfg2cmd/bootorder-legacy.conf.cmd
>  create mode 100644 test/cfg2cmd/bootorder.conf
>  create mode 100644 test/cfg2cmd/bootorder.conf.cmd
> 
> manager: Stefan Reiter (1):
>   ui: improve boot order editor with 'bootorder' support
> 
>  www/css/ext6-pve.css               |   4 +
>  www/manager6/qemu/BootOrderEdit.js | 355 ++++++++++++++++++-----------
>  www/manager6/qemu/Options.js       |  32 ++-
>  3 files changed, 257 insertions(+), 134 deletions(-)
> 



applied series, thanks!

I did a few smaller followups for the webinterface though, nothing big,
the only thing you may want to check out for the future is the use of
the defaults property for defaults covering all/most items:
https://docs.sencha.com/extjs/6.0.1/classic/Ext.grid.Panel.html#cfg-defaults

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

[Thomas Lamprecht]

On 06.10.20 15:32, Stefan Reiter wrote:
> @@ -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};

Seeing just now, never declare a variable conditionally, this is dangerous in perl,
like a lot! If the check above fails, the variable will operate with the value it was
last set too, and not be undef.

You added this pattern also when doing the RNG stuff, so it's clearly something
you're not aware off but like to use. Please stop doing so, as this led us already
once to a long bug hunt, which I'd like to avoid again.


> +    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++;
> +	}
> +    }
> +