From 3948f8d26573975fb201e91f49a2f6d2643cf306 Mon Sep 17 00:00:00 2001
From: Fiona Ebner <f.ebner@proxmox.com>
Date: Mon, 27 Jan 2025 12:29:08 +0100
Subject: [PATCH 01/16] migration: remove unused variable

The cloudinit config variable has been unused since commit 898e9296
("migrate: drop outdated PVE 7.2 check guarding cloudinit config
section").

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Link: https://lore.proxmox.com/20250127112923.31703-2-f.ebner@proxmox.com
---
 PVE/QemuMigrate.pm | 1 -
 1 file changed, 1 deletion(-)

diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
index ed5ede30..625fd1f7 100644
--- a/PVE/QemuMigrate.pm
+++ b/PVE/QemuMigrate.pm
@@ -181,7 +181,6 @@ sub prepare {
     my $conf = $self->{vmconf} = PVE::QemuConfig->load_config($vmid);
 
     my $version = PVE::QemuServer::Helpers::get_node_pvecfg_version($self->{node});
-    my $cloudinit_config = $conf->{cloudinit};
 
     my $repl_conf = PVE::ReplicationConfig->new();
     $self->{replication_jobcfg} = $repl_conf->find_local_replication_job($vmid, $self->{node});

From 27f38b00f327c74798dd2af9b881d2985226060d Mon Sep 17 00:00:00 2001
From: Fiona Ebner <f.ebner@proxmox.com>
Date: Mon, 27 Jan 2025 12:29:09 +0100
Subject: [PATCH 02/16] test: avoid duplicate mock module in restore config
 test

The duplication is there, because two independet fixes for a test
failure where applied, namely commits:
75c430ce ("test: unbreak restore_config_test")
cc1cdadb ("test: fix restore config test as unprivileged user")

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Link: https://lore.proxmox.com/20250127112923.31703-3-f.ebner@proxmox.com
---
 test/run_qemu_restore_config_tests.pl | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/test/run_qemu_restore_config_tests.pl b/test/run_qemu_restore_config_tests.pl
index 1e1e8072..1566ddf3 100755
--- a/test/run_qemu_restore_config_tests.pl
+++ b/test/run_qemu_restore_config_tests.pl
@@ -7,7 +7,6 @@ use lib qw(..);
 
 use Test::MockModule;
 use Test::More;
-use Test::MockModule;
 
 use File::Basename;
 
@@ -17,18 +16,11 @@ use PVE::Tools qw(dir_glob_foreach file_get_contents);
 my $INPUT_DIR = './restore-config-input';
 my $EXPECTED_DIR = './restore-config-expected';
 
-my $pve_cluster_module = Test::MockModule->new('PVE::Cluster');
-$pve_cluster_module->mock(
-    cfs_read_file => sub {
-	return {};
-    },
-);
-
 # NOTE update when you add/remove tests
 plan tests => 4;
 
-my $cfs_mock = Test::MockModule->new("PVE::Cluster");
-$cfs_mock->mock(
+my $pve_cluster_module = Test::MockModule->new("PVE::Cluster");
+$pve_cluster_module->mock(
     cfs_read_file => sub {
 	my ($file) = @_;
 

From 60b9ff0fd9c14d9c4d47937aa9183b1f64d8b239 Mon Sep 17 00:00:00 2001
From: Fiona Ebner <f.ebner@proxmox.com>
Date: Mon, 27 Jan 2025 12:29:10 +0100
Subject: [PATCH 03/16] test: add parse config tests

Tests for parsing and writing VM configuration files. The parsing part
is already covered by the config2command test too, but that only
focuses on the main section, not other section types and does not also
test parsing in strict mode.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Link: https://lore.proxmox.com/20250127112923.31703-4-f.ebner@proxmox.com
---
 test/Makefile                                 |   7 +-
 .../verify-snapshot.conf                      |  36 ++++
 .../verify-snapshot.conf.strict.error         |   1 +
 test/parse-config-input/locked.conf           |  16 ++
 test/parse-config-input/plain.conf            |  15 ++
 test/parse-config-input/regular-vm-efi.conf   |  16 ++
 test/parse-config-input/sections.conf         |  44 ++++
 test/parse-config-input/snapshots.conf        | 189 ++++++++++++++++++
 test/parse-config-input/verify-snapshot.conf  |  37 ++++
 test/run_parse_config_tests.pl                |  92 +++++++++
 10 files changed, 451 insertions(+), 2 deletions(-)
 create mode 100644 test/parse-config-expected/verify-snapshot.conf
 create mode 100644 test/parse-config-expected/verify-snapshot.conf.strict.error
 create mode 100644 test/parse-config-input/locked.conf
 create mode 100644 test/parse-config-input/plain.conf
 create mode 100644 test/parse-config-input/regular-vm-efi.conf
 create mode 100644 test/parse-config-input/sections.conf
 create mode 100644 test/parse-config-input/snapshots.conf
 create mode 100644 test/parse-config-input/verify-snapshot.conf
 create mode 100755 test/run_parse_config_tests.pl

diff --git a/test/Makefile b/test/Makefile
index 65ed7bc4..f7372e2e 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -1,6 +1,6 @@
 all: test
 
-test: test_snapshot test_cfg_to_cmd test_pci_addr_conflicts test_qemu_img_convert test_migration test_restore_config
+test: test_snapshot test_cfg_to_cmd test_pci_addr_conflicts test_qemu_img_convert test_migration test_restore_config test_parse_config
 
 test_snapshot: run_snapshot_tests.pl
 	./run_snapshot_tests.pl
@@ -25,6 +25,9 @@ $(MIGRATION_TEST_TARGETS):
 test_restore_config: run_qemu_restore_config_tests.pl
 	./run_qemu_restore_config_tests.pl
 
+test_parse_config: run_parse_config_tests.pl
+	./run_parse_config_tests.pl
+
 .PHONY: clean
 clean:
-	rm -rf MigrationTest/run
+	rm -rf MigrationTest/run parse-config-output
diff --git a/test/parse-config-expected/verify-snapshot.conf b/test/parse-config-expected/verify-snapshot.conf
new file mode 100644
index 00000000..cd503f86
--- /dev/null
+++ b/test/parse-config-expected/verify-snapshot.conf
@@ -0,0 +1,36 @@
+boot: order=scsi0
+cores: 2
+cpu: x86-64-v2-AES
+ide2: lvm:vm-120-cloudinit,media=cdrom
+ipconfig0: ip6=dhcp
+memory: 4096
+meta: creation-qemu=9.0.2,ctime=1725975013
+name: deb1223
+net0: vmxnet3=BC:24:11:2C:69:EC,bridge=vnet0,firewall=1
+numa: 0
+ostype: l26
+parent: snap
+scsi0: nfs:120/vm-120-disk-0.qcow2,iothread=1,size=4G
+scsihw: virtio-scsi-single
+smbios1: uuid=b3247ab1-1fe6-428e-965b-08a1b64a8746
+sockets: 1
+unused0: rbd:vm-120-disk-0
+vmgenid: 7079e97c-50e3-4079-afe7-23e67566b946
+
+[snap]
+boot: order=scsi0
+cores: 2
+cpu: x86-64-v2-AES
+ide2: lvm:vm-120-cloudinit,media=cdrom
+ipconfig0: ip6=dhcp
+memory: 4096
+meta: creation-qemu=9.0.2,ctime=1725975013
+name: deb1223
+net0: vmxnet3=BC:24:11:2C:69:EC,bridge=vnet0,firewall=1
+ostype: l26
+scsi0: nfs:120/vm-120-disk-0.qcow2,iothread=1,size=4G
+scsihw: virtio-scsi-single
+smbios1: uuid=b3247ab1-1fe6-428e-965b-08a1b64a8746
+snaptime: 1737549549
+sockets: 1
+vmgenid: 7079e97c-50e3-4079-afe7-23e67566b946
diff --git a/test/parse-config-expected/verify-snapshot.conf.strict.error b/test/parse-config-expected/verify-snapshot.conf.strict.error
new file mode 100644
index 00000000..1a77d4a5
--- /dev/null
+++ b/test/parse-config-expected/verify-snapshot.conf.strict.error
@@ -0,0 +1 @@
+vm 8006 - unable to parse value of 'numa' - type check ('boolean') failed - got 'verify meee~ :)'
diff --git a/test/parse-config-input/locked.conf b/test/parse-config-input/locked.conf
new file mode 100644
index 00000000..38b6e36c
--- /dev/null
+++ b/test/parse-config-input/locked.conf
@@ -0,0 +1,16 @@
+# locked
+bootdisk: scsi0
+cores: 1
+ide2: none,media=cdrom
+lock: backup
+memory: 512
+name: apache
+net0: virtio=92:38:11:FD:ED:87,bridge=vmbr0,firewall=1
+numa: 0
+ostype: l26
+scsi0: mydir:1422/vm-1422-disk-0.qcow2,size=4G
+scsihw: virtio-scsi-pci
+smbios1: uuid=ddf91b3f-a597-42be-9a7e-fb6421dcd5cd
+sockets: 1
+unused7: mydir:1422/vm-1422-disk-8.qcow2
+vmgenid: 0
diff --git a/test/parse-config-input/plain.conf b/test/parse-config-input/plain.conf
new file mode 100644
index 00000000..63449b9e
--- /dev/null
+++ b/test/parse-config-input/plain.conf
@@ -0,0 +1,15 @@
+# plain VM
+bootdisk: scsi0
+cores: 1
+ide2: none,media=cdrom
+memory: 512
+name: apache
+net0: virtio=92:38:11:FD:ED:87,bridge=vmbr0,firewall=1
+numa: 0
+ostype: l26
+scsi0: mydir:142/vm-142-disk-0.qcow2,size=4G
+scsihw: virtio-scsi-pci
+smbios1: uuid=ddf91b3f-a597-42be-9a7e-fb6421dcd5cd
+sockets: 1
+tags: foo bar
+vmgenid: 0
diff --git a/test/parse-config-input/regular-vm-efi.conf b/test/parse-config-input/regular-vm-efi.conf
new file mode 100644
index 00000000..9d75fff2
--- /dev/null
+++ b/test/parse-config-input/regular-vm-efi.conf
@@ -0,0 +1,16 @@
+# regular VM with an EFI disk
+bios: ovmf
+boot: order=scsi0;ide2;net0
+cores: 1
+efidisk0: mydir:139/vm-139-disk-0.qcow2,size=128K
+ide2: local:iso/debian-10.6.0-amd64-netinst.iso,media=cdrom
+memory: 2048
+name: eficloneclone
+net0: virtio=7A:6C:A5:8B:11:93,bridge=vmbr0,firewall=1
+numa: 0
+ostype: l26
+scsi0: rbdkvm:vm-139-disk-1,size=4G
+scsihw: virtio-scsi-pci
+smbios1: uuid=21a7e7bc-3cd2-4232-a009-a41f4ee992ae
+sockets: 1
+vmgenid: 0
diff --git a/test/parse-config-input/sections.conf b/test/parse-config-input/sections.conf
new file mode 100644
index 00000000..6329c33a
--- /dev/null
+++ b/test/parse-config-input/sections.conf
@@ -0,0 +1,44 @@
+boot: order=scsi0
+cores: 2
+cpu: x86-64-v2-AES
+ide2: lvm:vm-120-cloudinit,media=cdrom
+ipconfig0: ip6=dhcp
+memory: 4096
+meta: creation-qemu=9.0.2,ctime=1725975013
+name: deb1223
+net0: vmxnet3=BC:24:11:2C:69:EC,bridge=vnet0,firewall=1
+numa: 0
+ostype: l26
+parent: foo
+scsi0: nfs:120/vm-120-disk-0.qcow2,iothread=1,size=4G
+scsihw: virtio-scsi-single
+smbios1: uuid=b3247ab1-1fe6-428e-965b-08a1b64a8746
+sockets: 1
+unused0: rbd:vm-120-disk-0
+vmgenid: 7079e97c-50e3-4079-afe7-23e67566b946
+
+[PENDING]
+bios: ovmf
+
+[special:cloudinit]
+ipconfig0: ip=dhcp,ip6=dhcp
+name: deb122
+
+[foo]
+boot: order=scsi0
+cores: 2
+cpu: x86-64-v2-AES
+ide2: lvm:vm-120-cloudinit,media=cdrom
+ipconfig0: ip=dhcp,ip6=dhcp
+memory: 4096
+meta: creation-qemu=9.0.2,ctime=1725975013
+name: deb1223
+net0: vmxnet3=BC:24:11:2C:69:EC,bridge=vnet0,firewall=1
+numa: 0
+ostype: l26
+scsi0: nfs:120/vm-120-disk-0.qcow2,iothread=1,size=4G
+scsihw: virtio-scsi-single
+smbios1: uuid=b3247ab1-1fe6-428e-965b-08a1b64a8746
+snaptime: 1737548747
+sockets: 1
+vmgenid: 7079e97c-50e3-4079-afe7-23e67566b946
diff --git a/test/parse-config-input/snapshots.conf b/test/parse-config-input/snapshots.conf
new file mode 100644
index 00000000..4f4f8675
--- /dev/null
+++ b/test/parse-config-input/snapshots.conf
@@ -0,0 +1,189 @@
+boot: order=scsi1;ide2;net0;ide1
+cores: 4
+cpu: x86-64-v2-AES
+ide0: dir:111/vm-111-disk-2.qcow2,size=1G
+ide1: sani:iso/virtio-win-0.1.266.iso,media=cdrom,size=707456K
+ide2: sani:iso/Win2019-evaluation.iso,media=cdrom,size=4985424K
+machine: pc-i440fx-9.1
+memory: 4096
+meta: creation-qemu=9.1.2,ctime=1736349024
+name: win-machine-ver
+net0: virtio=BC:24:11:A3:DA:B1,bridge=vnet0,firewall=1
+net1: e1000=BC:24:11:79:D5:65,bridge=vnet0,firewall=1
+numa: 0
+ostype: win10
+parent: win19_5_2_plus_stuff
+scsi0: dir:111/vm-111-disk-1.qcow2,iothread=1,size=1G
+scsi1: lvmthinbig:vm-111-disk-0,iothread=1,size=32G
+scsihw: virtio-scsi-single
+smbios1: uuid=2c4a2cda-712b-44ab-8728-51f5e734b658
+sockets: 1
+unused0: rbd:vm-111-disk-0
+vga: qxl
+virtio0: dir:111/vm-111-disk-0.qcow2,iothread=1,size=1G
+vmgenid: 713da648-38a6-489e-b0b2-dd9cef419f33
+
+[machine_version_5_1]
+boot: order=ide0;ide2;net0
+cores: 4
+cpu: x86-64-v2-AES
+ide0: lvmthinbig:vm-111-disk-0,size=32G
+ide2: sani:iso/Win2016-1616-evaluation.ISO,media=cdrom,size=5198078K
+memory: 4096
+meta: creation-qemu=9.1.2,ctime=1736349024
+name: win-machine-ver
+net0: e1000=BC:24:11:A3:DA:B1,bridge=vnet0,firewall=1
+numa: 0
+ostype: win10
+scsihw: virtio-scsi-single
+smbios1: uuid=2c4a2cda-712b-44ab-8728-51f5e734b658
+snaptime: 1736939109
+sockets: 1
+vmgenid: 1f314a76-50a3-4b92-9307-c8c6e313d3ca
+
+[machine_version_5_1_with_virtio]
+boot: order=ide0;ide2;net0;ide1
+cores: 4
+cpu: x86-64-v2-AES
+ide0: lvmthinbig:vm-111-disk-0,size=32G
+ide1: sani:iso/virtio-win-0.1.266.iso,media=cdrom,size=707456K
+ide2: sani:iso/Win2016-1616-evaluation.ISO,media=cdrom,size=5198078K
+memory: 4096
+meta: creation-qemu=9.1.2,ctime=1736349024
+name: win-machine-ver
+net0: virtio=BC:24:11:A3:DA:B1,bridge=vnet0,firewall=1
+numa: 0
+ostype: win10
+parent: machine_version_5_1
+scsi0: dir:111/vm-111-disk-1.qcow2,iothread=1,size=1G
+scsihw: virtio-scsi-single
+smbios1: uuid=2c4a2cda-712b-44ab-8728-51f5e734b658
+snaptime: 1736940462
+sockets: 1
+virtio0: dir:111/vm-111-disk-0.qcow2,iothread=1,size=1G
+vmgenid: 4f602356-cb9c-45ad-a554-d76d95c7c0f8
+
+[ovmf_machine_version_5_1]
+bios: ovmf
+boot: order=ide0;ide2;net0;ide1
+cores: 4
+cpu: x86-64-v2-AES
+efidisk0: rbd:vm-111-disk-0,efitype=4m,pre-enrolled-keys=1,size=1M
+ide0: lvmthinbig:vm-111-disk-0,size=32G
+ide1: sani:iso/virtio-win-0.1.266.iso,media=cdrom,size=707456K
+ide2: sani:iso/Win2016-1616-evaluation.ISO,media=cdrom,size=5198078K
+machine: pc-q35-5.1
+memory: 4096
+meta: creation-qemu=9.1.2,ctime=1736349024
+name: win-machine-ver
+net0: e1000=BC:24:11:A3:DA:B1,bridge=vnet0,firewall=1
+numa: 0
+ostype: win10
+parent: machine_version_5_1_with_virtio
+scsi0: dir:111/vm-111-disk-1.qcow2,iothread=1,size=1G
+scsihw: virtio-scsi-single
+smbios1: uuid=2c4a2cda-712b-44ab-8728-51f5e734b658
+snaptime: 1736943308
+sockets: 1
+virtio0: dir:111/vm-111-disk-0.qcow2,iothread=1,size=1G
+vmgenid: 4f602356-cb9c-45ad-a554-d76d95c7c0f8
+
+[ovmf_machine_version_5_1_virtio]
+bios: ovmf
+boot: order=ide0;ide2;net0;ide1
+cores: 4
+cpu: x86-64-v2-AES
+efidisk0: rbd:vm-111-disk-0,efitype=4m,pre-enrolled-keys=1,size=1M
+ide0: lvmthinbig:vm-111-disk-0,size=32G
+ide1: sani:iso/virtio-win-0.1.266.iso,media=cdrom,size=707456K
+ide2: sani:iso/Win2016-1616-evaluation.ISO,media=cdrom,size=5198078K
+machine: pc-q35-5.1
+memory: 4096
+meta: creation-qemu=9.1.2,ctime=1736349024
+name: win-machine-ver
+net0: virtio=BC:24:11:A3:DA:B1,bridge=vnet0,firewall=1
+numa: 0
+ostype: win10
+parent: ovmf_machine_version_5_1
+scsi0: dir:111/vm-111-disk-1.qcow2,iothread=1,size=1G
+scsihw: virtio-scsi-single
+smbios1: uuid=2c4a2cda-712b-44ab-8728-51f5e734b658
+snaptime: 1736944525
+sockets: 1
+virtio0: dir:111/vm-111-disk-0.qcow2,iothread=1,size=1G
+vmgenid: 00b95468-4f34-4faa-b0af-b214ff5bbcdf
+
+[static-network]
+bios: ovmf
+boot: order=ide0;ide2;net0;ide1
+cores: 4
+cpu: x86-64-v2-AES
+efidisk0: rbd:vm-111-disk-0,efitype=4m,pre-enrolled-keys=1,size=1M
+ide0: lvmthinbig:vm-111-disk-0,size=32G
+ide1: sani:iso/virtio-win-0.1.266.iso,media=cdrom,size=707456K
+ide2: sani:iso/Win2016-1616-evaluation.ISO,media=cdrom,size=5198078K
+machine: pc-q35-5.1
+memory: 4096
+meta: creation-qemu=9.1.2,ctime=1736349024
+name: win-machine-ver
+net0: virtio=BC:24:11:A3:DA:B1,bridge=vnet0,firewall=1
+numa: 0
+ostype: win10
+parent: ovmf_machine_version_5_1_virtio
+scsi0: dir:111/vm-111-disk-1.qcow2,iothread=1,size=1G
+scsihw: virtio-scsi-single
+smbios1: uuid=2c4a2cda-712b-44ab-8728-51f5e734b658
+snaptime: 1736945713
+sockets: 1
+virtio0: dir:111/vm-111-disk-0.qcow2,iothread=1,size=1G
+vmgenid: 5d65fc62-2cb1-4945-9641-631b37c265a5
+
+[win19_5_2]
+boot: order=scsi1;ide2;net0;ide1
+cores: 4
+cpu: x86-64-v2-AES
+ide1: sani:iso/virtio-win-0.1.266.iso,media=cdrom,size=707456K
+ide2: sani:iso/Win2019-evaluation.iso,media=cdrom,size=4985424K
+machine: pc-i440fx-5.2
+memory: 4096
+meta: creation-qemu=9.1.2,ctime=1736349024
+name: win-machine-ver
+net0: virtio=BC:24:11:A3:DA:B1,bridge=vnet0,firewall=1
+net1: e1000=BC:24:11:79:D5:65,bridge=vnet0,firewall=1
+numa: 0
+ostype: win10
+parent: machine_version_5_1_with_virtio
+scsi0: dir:111/vm-111-disk-1.qcow2,iothread=1,size=1G
+scsi1: lvmthinbig:vm-111-disk-0,iothread=1,size=32G
+scsihw: virtio-scsi-single
+smbios1: uuid=2c4a2cda-712b-44ab-8728-51f5e734b658
+snaptime: 1736950690
+sockets: 1
+virtio0: dir:111/vm-111-disk-0.qcow2,iothread=1,size=1G
+vmgenid: f259de06-fa08-4ff7-8ba9-b1233a726ac4
+
+[win19_5_2_plus_stuff]
+boot: order=scsi1;ide2;net0;ide1
+cores: 4
+cpu: x86-64-v2-AES
+ide0: dir:111/vm-111-disk-2.qcow2,size=1G
+ide1: sani:iso/virtio-win-0.1.266.iso,media=cdrom,size=707456K
+ide2: sani:iso/Win2019-evaluation.iso,media=cdrom,size=4985424K
+machine: pc-i440fx-5.2
+memory: 4096
+meta: creation-qemu=9.1.2,ctime=1736349024
+name: win-machine-ver
+net0: virtio=BC:24:11:A3:DA:B1,bridge=vnet0,firewall=1
+net1: e1000=BC:24:11:79:D5:65,bridge=vnet0,firewall=1
+numa: 0
+ostype: win10
+parent: win19_5_2
+scsi0: dir:111/vm-111-disk-1.qcow2,iothread=1,size=1G
+scsi1: lvmthinbig:vm-111-disk-0,iothread=1,size=32G
+scsihw: virtio-scsi-single
+smbios1: uuid=2c4a2cda-712b-44ab-8728-51f5e734b658
+snaptime: 1736951300
+sockets: 1
+vga: qxl
+virtio0: dir:111/vm-111-disk-0.qcow2,iothread=1,size=1G
+vmgenid: 713da648-38a6-489e-b0b2-dd9cef419f33
diff --git a/test/parse-config-input/verify-snapshot.conf b/test/parse-config-input/verify-snapshot.conf
new file mode 100644
index 00000000..5f52272d
--- /dev/null
+++ b/test/parse-config-input/verify-snapshot.conf
@@ -0,0 +1,37 @@
+boot: order=scsi0
+cores: 2
+cpu: x86-64-v2-AES
+ide2: lvm:vm-120-cloudinit,media=cdrom
+ipconfig0: ip6=dhcp
+memory: 4096
+meta: creation-qemu=9.0.2,ctime=1725975013
+name: deb1223
+net0: vmxnet3=BC:24:11:2C:69:EC,bridge=vnet0,firewall=1
+numa: 0
+ostype: l26
+parent: snap
+scsi0: nfs:120/vm-120-disk-0.qcow2,iothread=1,size=4G
+scsihw: virtio-scsi-single
+smbios1: uuid=b3247ab1-1fe6-428e-965b-08a1b64a8746
+sockets: 1
+unused0: rbd:vm-120-disk-0
+vmgenid: 7079e97c-50e3-4079-afe7-23e67566b946
+
+[snap]
+boot: order=scsi0
+cores: 2
+cpu: x86-64-v2-AES
+ide2: lvm:vm-120-cloudinit,media=cdrom
+ipconfig0: ip6=dhcp
+memory: 4096
+meta: creation-qemu=9.0.2,ctime=1725975013
+name: deb1223
+net0: vmxnet3=BC:24:11:2C:69:EC,bridge=vnet0,firewall=1
+numa: verify meee~ :)
+ostype: l26
+scsi0: nfs:120/vm-120-disk-0.qcow2,iothread=1,size=4G
+scsihw: virtio-scsi-single
+smbios1: uuid=b3247ab1-1fe6-428e-965b-08a1b64a8746
+snaptime: 1737549549
+sockets: 1
+vmgenid: 7079e97c-50e3-4079-afe7-23e67566b946
diff --git a/test/run_parse_config_tests.pl b/test/run_parse_config_tests.pl
new file mode 100755
index 00000000..d071f355
--- /dev/null
+++ b/test/run_parse_config_tests.pl
@@ -0,0 +1,92 @@
+#!/usr/bin/perl
+
+# Tests parsing and writing VM configuration files.
+# The parsing part is already covered by the config2command test too, but that only focuses on the
+# main section, not other section types and does not also test parsing in strict mode.
+#
+# If no expected file exists, the input is assumed to be equal to the expected output.
+# If $file.strict.error (respectively $file.non-strict.error) exists, it is assumed to be the
+# expected error when parsing the config in strict (respectively non-strict) mode.
+
+use strict;
+use warnings;
+
+use lib qw(..);
+
+use File::Path qw(make_path remove_tree);
+
+use Test::MockModule;
+use Test::More;
+
+use PVE::QemuServer;
+use PVE::Tools;
+
+my $INPUT_DIR = './parse-config-input';
+my $OUTPUT_DIR = './parse-config-output';
+my $EXPECTED_DIR = './parse-config-expected';
+
+# NOTE update when you add/remove tests
+plan tests => 2 * 6;
+
+sub run_tests {
+    my ($strict) = @_;
+
+    PVE::Tools::dir_glob_foreach('./parse-config-input', '.*\.conf', sub {
+	my ($file) = @_;
+
+	my $strict_mode = $strict ? 'strict' : 'non-strict';
+
+	my $expected_err_file = "${EXPECTED_DIR}/${file}.${strict_mode}.error";
+	my $expected_err;
+	$expected_err = PVE::Tools::file_get_contents($expected_err_file) if -f $expected_err_file;
+
+	my $fake_config_fn ="$file/qemu-server/8006.conf";
+	my $input_file = "${INPUT_DIR}/${file}";
+	my $input = PVE::Tools::file_get_contents($input_file);
+	my $conf = eval {
+	    PVE::QemuServer::parse_vm_config($fake_config_fn, $input, $strict);
+	};
+	if (my $err = $@) {
+	    if ($expected_err) {
+		is($err, $expected_err, $file);
+	    } else {
+		note("got unexpected error '$err'");
+		fail($file);
+	    }
+	    return;
+	}
+
+	if ($expected_err) {
+	    note("expected error for strict mode did not occur: '$expected_err'");
+	    fail($file);
+	    return;
+	}
+
+	my $output = eval { PVE::QemuServer::write_vm_config($fake_config_fn, $conf); };
+	if (my $err = $@) {
+	    note("got unexpected error '$err'");
+	    fail($file);
+	    return;
+	}
+
+	my $output_file = "${OUTPUT_DIR}/${file}";
+	PVE::Tools::file_set_contents($output_file, $output);
+
+	my $expected_file = "${EXPECTED_DIR}/${file}";
+	$expected_file = $input_file if !-f $expected_file;
+
+	my $cmd = ['diff', '-u', $expected_file, $output_file];
+	if (system(@$cmd) == 0) {
+	    pass($file);
+	} else {
+	    fail($file);
+	}
+    });
+}
+
+make_path(${OUTPUT_DIR});
+run_tests(0);
+run_tests(1);
+remove_tree(${OUTPUT_DIR}) or die "failed to remove output directory\n";
+
+done_testing();

From df6d255b0fc25c6854b13ffbf72893f616c70509 Mon Sep 17 00:00:00 2001
From: Fiona Ebner <f.ebner@proxmox.com>
Date: Mon, 27 Jan 2025 12:29:11 +0100
Subject: [PATCH 04/16] parse config: be precise about section type checks

There are checks for custom parsing behavior inside certain sections
relying only on the section name. While the name 'pending' cannot be
used by snapshots, the name 'cloudinit' can. Introduce an associated
section type to make the checks precise.

The test was not added in a separate commit, because it would fail
when writing the config before the fix, and failure in writing is
never expected by the test script. So there is no easy way to
highlight just the difference in behavior together with the fix and
the git history should stay bisectable.

Compare with the verify-snapshot.conf testcase without the actual fix
applied to see the difference.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Link: https://lore.proxmox.com/20250127112923.31703-5-f.ebner@proxmox.com
---
 PVE/QemuServer.pm                             | 18 ++++----
 .../cloudinit-snapshot.conf                   | 40 ++++++++++++++++++
 .../cloudinit-snapshot.conf.strict.error      |  1 +
 .../cloudinit-snapshot.conf                   | 41 +++++++++++++++++++
 test/run_parse_config_tests.pl                |  2 +-
 5 files changed, 92 insertions(+), 10 deletions(-)
 create mode 100644 test/parse-config-expected/cloudinit-snapshot.conf
 create mode 100644 test/parse-config-expected/cloudinit-snapshot.conf.strict.error
 create mode 100644 test/parse-config-input/cloudinit-snapshot.conf

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 808c0e1c..d8bb21d6 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -2229,27 +2229,27 @@ sub parse_vm_config {
 	}
 	$descr = undef;
     };
-    my $section = '';
+    my $section = { name => '', type => 'main' };
 
     my @lines = split(/\n/, $raw);
     foreach my $line (@lines) {
 	next if $line =~ m/^\s*$/;
 
 	if ($line =~ m/^\[PENDING\]\s*$/i) {
-	    $section = 'pending';
+	    $section = { name => 'pending', type => 'pending' };
 	    $finish_description->();
-	    $conf = $res->{$section} = {};
+	    $conf = $res->{$section->{name}} = {};
 	    next;
 	} elsif ($line =~ m/^\[special:cloudinit\]\s*$/i) {
-	    $section = 'cloudinit';
+	    $section = { name => 'cloudinit', type => 'special' };
 	    $finish_description->();
-	    $conf = $res->{$section} = {};
+	    $conf = $res->{$section->{name}} = {};
 	    next;
 
 	} elsif ($line =~ m/^\[([a-z][a-z0-9_\-]+)\]\s*$/i) {
-	    $section = $1;
+	    $section = { name => $1, type => 'snapshot' };
 	    $finish_description->();
-	    $conf = $res->{snapshots}->{$section} = {};
+	    $conf = $res->{snapshots}->{$section->{name}} = {};
 	    next;
 	}
 
@@ -2270,7 +2270,7 @@ sub parse_vm_config {
 	    $conf->{$key} = $value;
 	} elsif ($line =~ m/^delete:\s*(.*\S)\s*$/) {
 	    my $value = $1;
-	    if ($section eq 'pending') {
+	    if ($section->{name} eq 'pending' && $section->{type} eq 'pending') {
 		$conf->{delete} = $value; # we parse this later
 	    } else {
 		$handle_error->("vm $vmid - property 'delete' is only allowed in [PENDING]\n");
@@ -2278,7 +2278,7 @@ sub parse_vm_config {
 	} elsif ($line =~ m/^([a-z][a-z_\-]*\d*):\s*(.+?)\s*$/) {
 	    my $key = $1;
 	    my $value = $2;
-	    if ($section eq 'cloudinit') {
+	    if ($section->{name} eq 'cloudinit' && $section->{type} eq 'special') {
 		# ignore validation only used for informative purpose
 		$conf->{$key} = $value;
 		next;
diff --git a/test/parse-config-expected/cloudinit-snapshot.conf b/test/parse-config-expected/cloudinit-snapshot.conf
new file mode 100644
index 00000000..bc01f975
--- /dev/null
+++ b/test/parse-config-expected/cloudinit-snapshot.conf
@@ -0,0 +1,40 @@
+boot: order=scsi0
+cores: 2
+cpu: x86-64-v2-AES
+ide2: lvm:vm-120-cloudinit,media=cdrom
+ipconfig0: ip6=dhcp
+memory: 4096
+meta: creation-qemu=9.0.2,ctime=1725975013
+name: deb1223
+net0: vmxnet3=BC:24:11:2C:69:EC,bridge=vnet0,firewall=1
+numa: 0
+ostype: l26
+parent: cloudinit
+scsi0: nfs:120/vm-120-disk-0.qcow2,iothread=1,size=4G
+scsihw: virtio-scsi-single
+smbios1: uuid=b3247ab1-1fe6-428e-965b-08a1b64a8746
+sockets: 1
+unused0: rbd:vm-120-disk-0
+vmgenid: 7079e97c-50e3-4079-afe7-23e67566b946
+
+[special:cloudinit]
+ipconfig0: ip=dhcp,ip6=dhcp
+name: deb122
+
+[cloudinit]
+boot: order=scsi0
+cores: 2
+cpu: x86-64-v2-AES
+ide2: lvm:vm-120-cloudinit,media=cdrom
+ipconfig0: ip6=dhcp
+memory: 4096
+meta: creation-qemu=9.0.2,ctime=1725975013
+name: deb1223
+net0: vmxnet3=BC:24:11:2C:69:EC,bridge=vnet0,firewall=1
+ostype: l26
+scsi0: nfs:120/vm-120-disk-0.qcow2,iothread=1,size=4G
+scsihw: virtio-scsi-single
+smbios1: uuid=b3247ab1-1fe6-428e-965b-08a1b64a8746
+snaptime: 1737549549
+sockets: 1
+vmgenid: 7079e97c-50e3-4079-afe7-23e67566b946
diff --git a/test/parse-config-expected/cloudinit-snapshot.conf.strict.error b/test/parse-config-expected/cloudinit-snapshot.conf.strict.error
new file mode 100644
index 00000000..1a77d4a5
--- /dev/null
+++ b/test/parse-config-expected/cloudinit-snapshot.conf.strict.error
@@ -0,0 +1 @@
+vm 8006 - unable to parse value of 'numa' - type check ('boolean') failed - got 'verify meee~ :)'
diff --git a/test/parse-config-input/cloudinit-snapshot.conf b/test/parse-config-input/cloudinit-snapshot.conf
new file mode 100644
index 00000000..9be05b1c
--- /dev/null
+++ b/test/parse-config-input/cloudinit-snapshot.conf
@@ -0,0 +1,41 @@
+boot: order=scsi0
+cores: 2
+cpu: x86-64-v2-AES
+ide2: lvm:vm-120-cloudinit,media=cdrom
+ipconfig0: ip6=dhcp
+memory: 4096
+meta: creation-qemu=9.0.2,ctime=1725975013
+name: deb1223
+net0: vmxnet3=BC:24:11:2C:69:EC,bridge=vnet0,firewall=1
+numa: 0
+ostype: l26
+parent: cloudinit
+scsi0: nfs:120/vm-120-disk-0.qcow2,iothread=1,size=4G
+scsihw: virtio-scsi-single
+smbios1: uuid=b3247ab1-1fe6-428e-965b-08a1b64a8746
+sockets: 1
+unused0: rbd:vm-120-disk-0
+vmgenid: 7079e97c-50e3-4079-afe7-23e67566b946
+
+[special:cloudinit]
+ipconfig0: ip=dhcp,ip6=dhcp
+name: deb122
+
+[cloudinit]
+boot: order=scsi0
+cores: 2
+cpu: x86-64-v2-AES
+ide2: lvm:vm-120-cloudinit,media=cdrom
+ipconfig0: ip6=dhcp
+memory: 4096
+meta: creation-qemu=9.0.2,ctime=1725975013
+name: deb1223
+net0: vmxnet3=BC:24:11:2C:69:EC,bridge=vnet0,firewall=1
+numa: verify meee~ :)
+ostype: l26
+scsi0: nfs:120/vm-120-disk-0.qcow2,iothread=1,size=4G
+scsihw: virtio-scsi-single
+smbios1: uuid=b3247ab1-1fe6-428e-965b-08a1b64a8746
+snaptime: 1737549549
+sockets: 1
+vmgenid: 7079e97c-50e3-4079-afe7-23e67566b946
diff --git a/test/run_parse_config_tests.pl b/test/run_parse_config_tests.pl
index d071f355..51f87ae5 100755
--- a/test/run_parse_config_tests.pl
+++ b/test/run_parse_config_tests.pl
@@ -26,7 +26,7 @@ my $OUTPUT_DIR = './parse-config-output';
 my $EXPECTED_DIR = './parse-config-expected';
 
 # NOTE update when you add/remove tests
-plan tests => 2 * 6;
+plan tests => 2 * 7;
 
 sub run_tests {
     my ($strict) = @_;

From c80f968cc1d34bf5f57e54432d8efd2c8e430655 Mon Sep 17 00:00:00 2001
From: Fiona Ebner <f.ebner@proxmox.com>
Date: Mon, 27 Jan 2025 12:29:12 +0100
Subject: [PATCH 05/16] test: add test case exposing issue with unknown
 sections

While unknown sections do lead to an error in strict mode, in
non-strict mode the line is just skipped, meaning that key-value
pairs from the unknown section will override the key-value pairs from
the previous section.

Fixed by the next commit.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Link: https://lore.proxmox.com/20250127112923.31703-6-f.ebner@proxmox.com
---
 .../unknown-sections.conf                     | 44 ++++++++++++++
 .../unknown-sections.conf.strict.error        |  1 +
 test/parse-config-input/unknown-sections.conf | 57 +++++++++++++++++++
 test/run_parse_config_tests.pl                |  2 +-
 4 files changed, 103 insertions(+), 1 deletion(-)
 create mode 100644 test/parse-config-expected/unknown-sections.conf
 create mode 100644 test/parse-config-expected/unknown-sections.conf.strict.error
 create mode 100644 test/parse-config-input/unknown-sections.conf

diff --git a/test/parse-config-expected/unknown-sections.conf b/test/parse-config-expected/unknown-sections.conf
new file mode 100644
index 00000000..08f1a3e2
--- /dev/null
+++ b/test/parse-config-expected/unknown-sections.conf
@@ -0,0 +1,44 @@
+boot: order=scsi0
+cores: 2
+cpu: x86-64-v2-AES
+ide2: lvm:vm-120-cloudinit,media=cdrom
+ipconfig0: ip6=dhcp
+memory: 4096
+meta: creation-qemu=9.0.2,ctime=1725975013
+name: foo
+net0: vmxnet3=BC:24:11:2C:69:EC,bridge=vnet0,firewall=1
+numa: 0
+ostype: l26
+parent: foo
+scsi0: nfs:120/vm-120-disk-0.qcow2,iothread=1,size=4G
+scsihw: virtio-scsi-single
+smbios1: uuid=b3247ab1-1fe6-428e-965b-08a1b64a8746
+sockets: 1
+unused0: rbd:vm-120-disk-0
+vmgenid: 7079e97c-50e3-4079-afe7-23e67566b946
+
+[PENDING]
+bios: seabios
+
+[special:cloudinit]
+ipconfig0: ip=dhcp,ip6=dhcp
+name: bar
+
+[foo]
+boot: order=scsi0
+cores: 2
+cpu: x86-64-v2-AES
+ide2: lvm:vm-120-cloudinit,media=cdrom
+ipconfig0: ip=dhcp,ip6=dhcp
+memory: 4096
+meta: creation-qemu=9.0.2,ctime=1725975013
+name: baz
+net0: vmxnet3=BC:24:11:2C:69:EC,bridge=vnet0,firewall=1
+numa: 0
+ostype: l26
+scsi0: nfs:120/vm-120-disk-0.qcow2,iothread=1,size=4G
+scsihw: virtio-scsi-single
+smbios1: uuid=b3247ab1-1fe6-428e-965b-08a1b64a8746
+snaptime: 1737548747
+sockets: 1
+vmgenid: 7079e97c-50e3-4079-afe7-23e67566b946
diff --git a/test/parse-config-expected/unknown-sections.conf.strict.error b/test/parse-config-expected/unknown-sections.conf.strict.error
new file mode 100644
index 00000000..e7004dc9
--- /dev/null
+++ b/test/parse-config-expected/unknown-sections.conf.strict.error
@@ -0,0 +1 @@
+vm 8006 - unable to parse config: [special:unknown123]
diff --git a/test/parse-config-input/unknown-sections.conf b/test/parse-config-input/unknown-sections.conf
new file mode 100644
index 00000000..0dcd5951
--- /dev/null
+++ b/test/parse-config-input/unknown-sections.conf
@@ -0,0 +1,57 @@
+boot: order=scsi0
+cores: 2
+cpu: x86-64-v2-AES
+ide2: lvm:vm-120-cloudinit,media=cdrom
+ipconfig0: ip6=dhcp
+memory: 4096
+meta: creation-qemu=9.0.2,ctime=1725975013
+name: deb1223
+net0: vmxnet3=BC:24:11:2C:69:EC,bridge=vnet0,firewall=1
+numa: 0
+ostype: l26
+parent: foo
+scsi0: nfs:120/vm-120-disk-0.qcow2,iothread=1,size=4G
+scsihw: virtio-scsi-single
+smbios1: uuid=b3247ab1-1fe6-428e-965b-08a1b64a8746
+sockets: 1
+unused0: rbd:vm-120-disk-0
+vmgenid: 7079e97c-50e3-4079-afe7-23e67566b946
+
+[special:unknown123]
+name: foo
+
+[PENDING]
+bios: ovmf
+
+[special:unknown124]
+bios: seabios
+
+[special:cloudinit]
+ipconfig0: ip=dhcp,ip6=dhcp
+name: deb122
+
+[special:unknown125]
+name: bar
+
+[foo]
+boot: order=scsi0
+cores: 2
+cpu: x86-64-v2-AES
+ide2: lvm:vm-120-cloudinit,media=cdrom
+ipconfig0: ip=dhcp,ip6=dhcp
+memory: 4096
+meta: creation-qemu=9.0.2,ctime=1725975013
+name: deb1223
+net0: vmxnet3=BC:24:11:2C:69:EC,bridge=vnet0,firewall=1
+numa: 0
+ostype: l26
+scsi0: nfs:120/vm-120-disk-0.qcow2,iothread=1,size=4G
+scsihw: virtio-scsi-single
+smbios1: uuid=b3247ab1-1fe6-428e-965b-08a1b64a8746
+snaptime: 1737548747
+sockets: 1
+vmgenid: 7079e97c-50e3-4079-afe7-23e67566b946
+
+[:3]
+name: baz
+cat: nya~
diff --git a/test/run_parse_config_tests.pl b/test/run_parse_config_tests.pl
index 51f87ae5..b1a9a0c1 100755
--- a/test/run_parse_config_tests.pl
+++ b/test/run_parse_config_tests.pl
@@ -26,7 +26,7 @@ my $OUTPUT_DIR = './parse-config-output';
 my $EXPECTED_DIR = './parse-config-expected';
 
 # NOTE update when you add/remove tests
-plan tests => 2 * 7;
+plan tests => 2 * 8;
 
 sub run_tests {
     my ($strict) = @_;

From 248349f25e33e83eb55ed4da48a704c2db1de4af Mon Sep 17 00:00:00 2001
From: Fiona Ebner <f.ebner@proxmox.com>
Date: Mon, 27 Jan 2025 12:29:13 +0100
Subject: [PATCH 06/16] parse config: skip unknown sections and warn about
 their presence

Currently, keys in an unknown section will be interpreted as still
belonging to the last section and might erroneously overwrite values
in that way. Explicitly ignore unknown sections to avoid this while
warning the user.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Link: https://lore.proxmox.com/20250127112923.31703-7-f.ebner@proxmox.com
---
 PVE/QemuServer.pm                                         | 8 ++++++++
 test/parse-config-expected/unknown-sections.conf          | 8 ++++----
 .../unknown-sections.conf.strict.error                    | 2 +-
 3 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index d8bb21d6..e0cca0e4 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -2251,8 +2251,16 @@ sub parse_vm_config {
 	    $finish_description->();
 	    $conf = $res->{snapshots}->{$section->{name}} = {};
 	    next;
+	} elsif ($line =~ m/^\[([^\]]*)\]\s*$/i) {
+	    my $unknown_section = $1;
+	    $section = undef;
+	    $finish_description->();
+	    $handle_error->("vm $vmid - skipping unknown section: '$unknown_section'\n");
+	    next;
 	}
 
+	next if !defined($section);
+
 	if ($line =~ m/^\#(.*)$/) {
 	    $descr = '' if !defined($descr);
 	    $descr .= PVE::Tools::decode_text($1) . "\n";
diff --git a/test/parse-config-expected/unknown-sections.conf b/test/parse-config-expected/unknown-sections.conf
index 08f1a3e2..6329c33a 100644
--- a/test/parse-config-expected/unknown-sections.conf
+++ b/test/parse-config-expected/unknown-sections.conf
@@ -5,7 +5,7 @@ ide2: lvm:vm-120-cloudinit,media=cdrom
 ipconfig0: ip6=dhcp
 memory: 4096
 meta: creation-qemu=9.0.2,ctime=1725975013
-name: foo
+name: deb1223
 net0: vmxnet3=BC:24:11:2C:69:EC,bridge=vnet0,firewall=1
 numa: 0
 ostype: l26
@@ -18,11 +18,11 @@ unused0: rbd:vm-120-disk-0
 vmgenid: 7079e97c-50e3-4079-afe7-23e67566b946
 
 [PENDING]
-bios: seabios
+bios: ovmf
 
 [special:cloudinit]
 ipconfig0: ip=dhcp,ip6=dhcp
-name: bar
+name: deb122
 
 [foo]
 boot: order=scsi0
@@ -32,7 +32,7 @@ ide2: lvm:vm-120-cloudinit,media=cdrom
 ipconfig0: ip=dhcp,ip6=dhcp
 memory: 4096
 meta: creation-qemu=9.0.2,ctime=1725975013
-name: baz
+name: deb1223
 net0: vmxnet3=BC:24:11:2C:69:EC,bridge=vnet0,firewall=1
 numa: 0
 ostype: l26
diff --git a/test/parse-config-expected/unknown-sections.conf.strict.error b/test/parse-config-expected/unknown-sections.conf.strict.error
index e7004dc9..7f921a70 100644
--- a/test/parse-config-expected/unknown-sections.conf.strict.error
+++ b/test/parse-config-expected/unknown-sections.conf.strict.error
@@ -1 +1 @@
-vm 8006 - unable to parse config: [special:unknown123]
+vm 8006 - skipping unknown section: 'special:unknown123'

From d4208c7cc62a18c85b24b3d55dc1d22d3bae52d8 Mon Sep 17 00:00:00 2001
From: Fiona Ebner <f.ebner@proxmox.com>
Date: Mon, 27 Jan 2025 12:29:14 +0100
Subject: [PATCH 07/16] vzdump: anchor matches for pending and special sections

Otherwise, a snapshot with a name that includes "pending" will be
misinterpreted as the pending section.

Only affects the warning messages, but still confusing.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Link: https://lore.proxmox.com/20250127112923.31703-8-f.ebner@proxmox.com
---
 PVE/VZDump/QemuServer.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/PVE/VZDump/QemuServer.pm b/PVE/VZDump/QemuServer.pm
index 44b68ae9..72a76e65 100644
--- a/PVE/VZDump/QemuServer.pm
+++ b/PVE/VZDump/QemuServer.pm
@@ -237,9 +237,9 @@ sub assemble {
 	next if $line =~ m/^\#vzdump\#/; # just to be sure
 	next if $line =~ m/^\#qmdump\#/; # just to be sure
 	if ($line =~ m/^\[(.*)\]\s*$/) {
-	    if ($1 =~ m/PENDING/i) {
+	    if ($1 =~ m/^PENDING$/i) {
 		$found_pending = 1;
-	    } elsif ($1 =~ m/special:cloudinit/) {
+	    } elsif ($1 =~ m/^special:cloudinit$/) {
 		$found_cloudinit = 1;
 	    } else {
 		$found_snapshot = 1;

From d295ea9a28d0682a6e946a29c070f5ec35fff173 Mon Sep 17 00:00:00 2001
From: Fiona Ebner <f.ebner@proxmox.com>
Date: Mon, 27 Jan 2025 12:29:15 +0100
Subject: [PATCH 08/16] vzdump: skip all special sections

Also log an informational message just like for pending and snapshot
sections.

Add a comment about it to parse_vm_config() in the hope that the
behavior is noted when introducing a future special section.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Link: https://lore.proxmox.com/20250127112923.31703-9-f.ebner@proxmox.com
---
 PVE/QemuServer.pm        |  1 +
 PVE/VZDump/QemuServer.pm | 12 ++++++++----
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index e0cca0e4..e1c9d3f5 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -2198,6 +2198,7 @@ sub parse_vm_config {
 
     return if !defined($raw);
 
+    # note that pending, snapshot and special sections are currently skipped when a backup is taken
     my $res = {
 	digest => Digest::SHA::sha1_hex($raw),
 	snapshots => {},
diff --git a/PVE/VZDump/QemuServer.pm b/PVE/VZDump/QemuServer.pm
index 72a76e65..cdaaa3a2 100644
--- a/PVE/VZDump/QemuServer.pm
+++ b/PVE/VZDump/QemuServer.pm
@@ -232,20 +232,21 @@ sub assemble {
 
     my $found_snapshot;
     my $found_pending;
-    my $found_cloudinit;
+    my $found_special;
     while (defined (my $line = <$conffd>)) {
 	next if $line =~ m/^\#vzdump\#/; # just to be sure
 	next if $line =~ m/^\#qmdump\#/; # just to be sure
 	if ($line =~ m/^\[(.*)\]\s*$/) {
 	    if ($1 =~ m/^PENDING$/i) {
 		$found_pending = 1;
-	    } elsif ($1 =~ m/^special:cloudinit$/) {
-		$found_cloudinit = 1;
+	    } elsif ($1 =~ m/^special:.*$/) {
+		$found_special = 1;
 	    } else {
 		$found_snapshot = 1;
 	    }
 	}
-	next if $found_snapshot || $found_pending || $found_cloudinit; # skip all snapshots,pending changes and cloudinit config data
+	# skip all snapshots, pending changes and special sections
+	next if $found_snapshot || $found_pending || $found_special;
 
 	if ($line =~ m/^unused\d+:\s*(\S+)\s*/) {
 	    $self->loginfo("skip unused drive '$1' (not included into backup)");
@@ -266,6 +267,9 @@ sub assemble {
 	}
     }
 
+    if ($found_special) {
+	$self->loginfo("special config section found (not included into backup)");
+    }
     if ($found_snapshot) {
 	$self->loginfo("snapshots found (not included into backup)");
     }

From 63567c0c424e9935b56878dafa4e4b78c3b3ea18 Mon Sep 17 00:00:00 2001
From: Fiona Ebner <f.ebner@proxmox.com>
Date: Mon, 27 Jan 2025 12:29:16 +0100
Subject: [PATCH 09/16] config: make special section handling generic

Collect special sections below a common 'special-sections' key in
preparation to introduce a new special section.

The special 'cloudinit' section was added in the top-level of the
configuration structure, but it's cleaner to group special sections
more similar to snapshots.

The 'cloudinit' key was already initialized, so having the new
'special-sections' key be always initialized should not cause issues
after checking and adapting all usages of 'cloudinit' which this patch
attempts to do.

Add compat handling for remote migration which might receive the
configuration hash from a node that does not yet have the changes.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Link: https://lore.proxmox.com/20250127112923.31703-10-f.ebner@proxmox.com
---
 PVE/API2/Qemu.pm            | 19 ++++++++++++++++---
 PVE/QemuConfig.pm           |  2 +-
 PVE/QemuServer.pm           | 30 +++++++++++++++++-------------
 PVE/QemuServer/Cloudinit.pm |  8 ++++++--
 4 files changed, 40 insertions(+), 19 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 295260e7..084e4efb 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -1654,7 +1654,7 @@ __PACKAGE__->register_method({
 	my $vmid = $param->{vmid};
 	my $conf = PVE::QemuConfig->load_config($vmid);
 
-	my $ci = $conf->{cloudinit};
+	my $ci = $conf->{'special-sections'}->{cloudinit};
 
 	$conf->{cipassword} = '**********' if exists $conf->{cipassword};
 	$ci->{cipassword} = '**********' if exists $ci->{cipassword};
@@ -5952,9 +5952,22 @@ __PACKAGE__->register_method({
 		    # not handled by update_vm_api
 		    my $vmgenid = delete $new_conf->{vmgenid};
 		    my $meta = delete $new_conf->{meta};
-		    my $cloudinit = delete $new_conf->{cloudinit}; # this is informational only
+
+		    my $special_sections = delete $new_conf->{'special-sections'} // {};
+
 		    $new_conf->{skip_cloud_init} = 1; # re-use image from source side
 
+		    # TODO PVE 10 - remove backwards-compat handling?
+		    my $cloudinit = delete $new_conf->{cloudinit};
+		    if ($cloudinit) {
+			if ($special_sections->{cloudinit}) {
+			    warn "config has duplicate special 'cloudinit' sections - skipping"
+				." legacy variant\n";
+			} else {
+			    $special_sections->{cloudinit} = $cloudinit;
+			}
+		    }
+
 		    $new_conf->{vmid} = $state->{vmid};
 		    $new_conf->{node} = $node;
 
@@ -5976,7 +5989,7 @@ __PACKAGE__->register_method({
 		    $conf->{lock} = 'migrate';
 		    $conf->{vmgenid} = $vmgenid if defined($vmgenid);
 		    $conf->{meta} = $meta if defined($meta);
-		    $conf->{cloudinit} = $cloudinit if defined($cloudinit);
+		    $conf->{'special-sections'} = $special_sections;
 		    PVE::QemuConfig->write_config($state->{vmid}, $conf);
 
 		    $state->{lock} = 'migrate';
diff --git a/PVE/QemuConfig.pm b/PVE/QemuConfig.pm
index ffdf9f03..3d57a0a8 100644
--- a/PVE/QemuConfig.pm
+++ b/PVE/QemuConfig.pm
@@ -541,7 +541,7 @@ sub load_current_config {
     my ($class, $vmid, $current) = @_;
 
     my $conf = $class->SUPER::load_current_config($vmid, $current);
-    delete $conf->{cloudinit};
+    delete $conf->{'special-sections'};
     return $conf;
 }
 
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index e1c9d3f5..5a55e70e 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -1849,7 +1849,7 @@ sub vmconfig_register_unused_drive {
     if (drive_is_cloudinit($drive)) {
 	eval { PVE::Storage::vdisk_free($storecfg, $drive->{file}) };
 	warn $@ if $@;
-	delete $conf->{cloudinit};
+	delete $conf->{'special-sections'}->{cloudinit};
     } elsif (!drive_is_cdrom($drive)) {
 	my $volid = $drive->{file};
 	if (vm_is_volid_owner($storecfg, $vmid, $volid)) {
@@ -2203,7 +2203,7 @@ sub parse_vm_config {
 	digest => Digest::SHA::sha1_hex($raw),
 	snapshots => {},
 	pending => {},
-	cloudinit => {},
+	'special-sections' => {},
     };
 
     my $handle_error = sub {
@@ -2230,6 +2230,9 @@ sub parse_vm_config {
 	}
 	$descr = undef;
     };
+
+    my $special_sections_re_1 = qr/(cloudinit)/;
+
     my $section = { name => '', type => 'main' };
 
     my @lines = split(/\n/, $raw);
@@ -2241,10 +2244,10 @@ sub parse_vm_config {
 	    $finish_description->();
 	    $conf = $res->{$section->{name}} = {};
 	    next;
-	} elsif ($line =~ m/^\[special:cloudinit\]\s*$/i) {
-	    $section = { name => 'cloudinit', type => 'special' };
+	} elsif ($line =~ m/^\[special:$special_sections_re_1\]\s*$/i) {
+	    $section = { name => $1, type => 'special' };
 	    $finish_description->();
-	    $conf = $res->{$section->{name}} = {};
+	    $conf = $res->{'special-sections'}->{$section->{name}} = {};
 	    next;
 
 	} elsif ($line =~ m/^\[([a-z][a-z0-9_\-]+)\]\s*$/i) {
@@ -2349,7 +2352,7 @@ sub write_vm_config {
 
 	foreach my $key (keys %$cref) {
 	    next if $key eq 'digest' || $key eq 'description' || $key eq 'snapshots' ||
-		$key eq 'snapstate' || $key eq 'pending' || $key eq 'cloudinit';
+		$key eq 'snapstate' || $key eq 'pending' || $key eq 'special-sections';
 	    my $value = $cref->{$key};
 	    if ($key eq 'delete') {
 		die "propertry 'delete' is only allowed in [PENDING]\n"
@@ -2403,7 +2406,7 @@ sub write_vm_config {
 	}
 
 	foreach my $key (sort keys %$conf) {
-	    next if $key =~ /^(digest|description|pending|cloudinit|snapshots)$/;
+	    next if $key =~ /^(digest|description|pending|snapshots|special-sections)$/;
 	    $raw .= "$key: $conf->{$key}\n";
 	}
 	return $raw;
@@ -2416,9 +2419,10 @@ sub write_vm_config {
 	$raw .= &$generate_raw_config($conf->{pending}, 1);
     }
 
-    if (scalar(keys %{$conf->{cloudinit}}) && PVE::QemuConfig->has_cloudinit($conf)){
-	$raw .= "\n[special:cloudinit]\n";
-	$raw .= &$generate_raw_config($conf->{cloudinit});
+    for my $special (sort keys $conf->{'special-sections'}->%*) {
+	next if $special eq 'cloudinit' && !PVE::QemuConfig->has_cloudinit($conf);
+	$raw .= "\n[special:$special]\n";
+	$raw .= &$generate_raw_config($conf->{'special-sections'}->{$special});
     }
 
     foreach my $snapname (sort keys %{$conf->{snapshots}}) {
@@ -4767,7 +4771,7 @@ sub vmconfig_hotplug_pending {
 	my ($conf, $opt, $old, $new) = @_;
 	return if !$cloudinit_pending_properties->{$opt};
 
-	my $ci = ($conf->{cloudinit} //= {});
+	my $ci = ($conf->{'special-sections'}->{cloudinit} //= {});
 
 	my $recorded = $ci->{$opt};
 	my %added = map { $_ => 1 } PVE::Tools::split_list(delete($ci->{added}) // '');
@@ -5157,7 +5161,7 @@ sub vmconfig_apply_pending {
     if ($generate_cloudinit) {
 	if (PVE::QemuServer::Cloudinit::apply_cloudinit_config($conf, $vmid)) {
 	    # After successful generation and if there were changes to be applied, update the
-	    # config to drop the {cloudinit} entry.
+	    # config to drop the 'cloudinit' special section.
 	    PVE::QemuConfig->write_config($vmid, $conf);
 	}
     }
@@ -5588,7 +5592,7 @@ sub vm_start_nolock {
     if (!$migratedfrom) {
 	if (PVE::QemuServer::Cloudinit::apply_cloudinit_config($conf, $vmid)) {
 	    # FIXME: apply_cloudinit_config updates $conf in this case, and it would only drop
-	    # $conf->{cloudinit}, so we could just not do this?
+	    # $conf->{'special-sections'}->{cloudinit}, so we could just not do this?
 	    # But we do it above, so for now let's be consistent.
 	    $conf = PVE::QemuConfig->load_config($vmid); # update/reload
 	}
diff --git a/PVE/QemuServer/Cloudinit.pm b/PVE/QemuServer/Cloudinit.pm
index f1143aeb..001022e6 100644
--- a/PVE/QemuServer/Cloudinit.pm
+++ b/PVE/QemuServer/Cloudinit.pm
@@ -657,7 +657,11 @@ my $cloudinit_methods = {
 sub has_changes {
     my ($conf) = @_;
 
-    return !!$conf->{cloudinit}->%*;
+    if (my $cloudinit = $conf->{'special-sections'}->{cloudinit}) {
+	return !!$cloudinit->%*;
+    }
+
+    return;
 }
 
 sub generate_cloudinit_config {
@@ -689,7 +693,7 @@ sub apply_cloudinit_config {
     my $has_changes = generate_cloudinit_config($conf, $vmid);
 
     if ($has_changes) {
-	delete $conf->{cloudinit};
+	delete $conf->{'special-sections'}->{cloudinit};
 	PVE::QemuConfig->write_config($vmid, $conf);
 	return 1;
     }

From 90ae915305ae795a54553e9ce2e38977d4534210 Mon Sep 17 00:00:00 2001
From: Fiona Ebner <f.ebner@proxmox.com>
Date: Mon, 27 Jan 2025 12:29:17 +0100
Subject: [PATCH 10/16] test: parse config: test config with duplicate sections

Add a test case to witness how duplicate sections are handled.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Link: https://lore.proxmox.com/20250127112923.31703-11-f.ebner@proxmox.com
---
 .../duplicate-sections.conf                   | 43 ++++++++++++
 .../duplicate-sections.conf                   | 68 +++++++++++++++++++
 test/run_parse_config_tests.pl                |  2 +-
 3 files changed, 112 insertions(+), 1 deletion(-)
 create mode 100644 test/parse-config-expected/duplicate-sections.conf
 create mode 100644 test/parse-config-input/duplicate-sections.conf

diff --git a/test/parse-config-expected/duplicate-sections.conf b/test/parse-config-expected/duplicate-sections.conf
new file mode 100644
index 00000000..1cb7a88a
--- /dev/null
+++ b/test/parse-config-expected/duplicate-sections.conf
@@ -0,0 +1,43 @@
+boot: order=scsi0
+cores: 2
+cpu: x86-64-v2-AES
+ide2: lvm:vm-120-cloudinit,media=cdrom
+ipconfig0: ip=dhcp,ip6=dhcp
+memory: 4096
+meta: creation-qemu=9.0.2,ctime=1725975013
+name: deb122
+net0: vmxnet3=BC:24:11:2C:69:EC,bridge=vnet0,firewall=1
+numa: 0
+ostype: l26
+parent: foo
+scsi0: nfs:120/vm-120-disk-0.qcow2,iothread=1,size=4G
+scsihw: virtio-scsi-single
+smbios1: uuid=b3247ab1-1fe6-428e-965b-08a1b64a8746
+sockets: 1
+unused0: rbd:vm-120-disk-0
+vmgenid: 7079e97c-50e3-4079-afe7-23e67566b946
+
+[PENDING]
+vga: qxl
+
+[special:cloudinit]
+name: deb123
+
+[foo]
+boot: order=scsi0
+cores: 4
+cpu: host
+ide2: lvm:vm-120-cloudinit,media=cdrom
+ipconfig0: ip=dhcp,ip6=dhcp
+memory: 4096
+meta: creation-qemu=9.0.2,ctime=1725975013
+name: deb1223
+net0: vmxnet3=BC:24:11:2C:69:EC,bridge=vnet0,firewall=1
+numa: 0
+ostype: l26
+scsi0: nfs:120/vm-120-disk-0.qcow2,iothread=1,size=4G
+scsihw: virtio-scsi-single
+smbios1: uuid=b3247ab1-1fe6-428e-965b-08a1b64a8746
+snaptime: 1737548747
+sockets: 1
+vmgenid: 7079e97c-50e3-4079-afe7-23e67566b946
diff --git a/test/parse-config-input/duplicate-sections.conf b/test/parse-config-input/duplicate-sections.conf
new file mode 100644
index 00000000..41e90e37
--- /dev/null
+++ b/test/parse-config-input/duplicate-sections.conf
@@ -0,0 +1,68 @@
+boot: order=scsi0
+cores: 2
+cpu: x86-64-v2-AES
+ide2: lvm:vm-120-cloudinit,media=cdrom
+ipconfig0: ip=dhcp,ip6=dhcp
+memory: 4096
+meta: creation-qemu=9.0.2,ctime=1725975013
+name: deb122
+net0: vmxnet3=BC:24:11:2C:69:EC,bridge=vnet0,firewall=1
+numa: 0
+ostype: l26
+parent: foo
+scsi0: nfs:120/vm-120-disk-0.qcow2,iothread=1,size=4G
+scsihw: virtio-scsi-single
+smbios1: uuid=b3247ab1-1fe6-428e-965b-08a1b64a8746
+sockets: 1
+unused0: rbd:vm-120-disk-0
+vmgenid: 7079e97c-50e3-4079-afe7-23e67566b946
+
+[PENDING]
+bios: ovmf
+
+[PENDING]
+vga: qxl
+
+[special:cloudinit]
+name: deb12
+
+[special:cloudinit]
+name: deb123
+
+[foo]
+boot: order=scsi0
+cores: 2
+cpu: x86-64-v2-AES
+ide2: lvm:vm-120-cloudinit,media=cdrom
+ipconfig0: ip=dhcp,ip6=dhcp
+memory: 4096
+meta: creation-qemu=9.0.2,ctime=1725975013
+name: deb1223
+net0: vmxnet3=BC:24:11:2C:69:EC,bridge=vnet0,firewall=1
+numa: 0
+ostype: l26
+scsi0: nfs:120/vm-120-disk-0.qcow2,iothread=1,size=4G
+scsihw: virtio-scsi-single
+smbios1: uuid=b3247ab1-1fe6-428e-965b-08a1b64a8746
+snaptime: 1737548747
+sockets: 1
+vmgenid: 7079e97c-50e3-4079-afe7-23e67566b946
+
+[foo]
+boot: order=scsi0
+cores: 4
+cpu: host
+ide2: lvm:vm-120-cloudinit,media=cdrom
+ipconfig0: ip=dhcp,ip6=dhcp
+memory: 4096
+meta: creation-qemu=9.0.2,ctime=1725975013
+name: deb1223
+net0: vmxnet3=BC:24:11:2C:69:EC,bridge=vnet0,firewall=1
+numa: 0
+ostype: l26
+scsi0: nfs:120/vm-120-disk-0.qcow2,iothread=1,size=4G
+scsihw: virtio-scsi-single
+smbios1: uuid=b3247ab1-1fe6-428e-965b-08a1b64a8746
+snaptime: 1737548747
+sockets: 1
+vmgenid: 7079e97c-50e3-4079-afe7-23e67566b946
diff --git a/test/run_parse_config_tests.pl b/test/run_parse_config_tests.pl
index b1a9a0c1..bc7ad86e 100755
--- a/test/run_parse_config_tests.pl
+++ b/test/run_parse_config_tests.pl
@@ -26,7 +26,7 @@ my $OUTPUT_DIR = './parse-config-output';
 my $EXPECTED_DIR = './parse-config-expected';
 
 # NOTE update when you add/remove tests
-plan tests => 2 * 8;
+plan tests => 2 * 9;
 
 sub run_tests {
     my ($strict) = @_;

From 68eabc42c1d367cdd65f2344711af4076412ff05 Mon Sep 17 00:00:00 2001
From: Fiona Ebner <f.ebner@proxmox.com>
Date: Mon, 27 Jan 2025 12:29:18 +0100
Subject: [PATCH 11/16] parse config: warn about duplicate sections

Currently, a duplicate section will quietly override the previous
instance of the section with the same identifier. Keep the current
behavior of preferring later entries, but issue a warning or die when
parsing strictly.

The entry for 'pending' in the result needs to start out as undefined
for the check to also work in presence of empty sections. Avoid
changing the returned value itself, by making sure to initialize the
entry before returning.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Link: https://lore.proxmox.com/20250127112923.31703-12-f.ebner@proxmox.com
---
 PVE/QemuServer.pm                                      | 10 +++++++++-
 .../duplicate-sections.conf.strict.error               |  1 +
 2 files changed, 10 insertions(+), 1 deletion(-)
 create mode 100644 test/parse-config-expected/duplicate-sections.conf.strict.error

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 5a55e70e..8c26fbc3 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -2202,7 +2202,7 @@ sub parse_vm_config {
     my $res = {
 	digest => Digest::SHA::sha1_hex($raw),
 	snapshots => {},
-	pending => {},
+	pending => undef,
 	'special-sections' => {},
     };
 
@@ -2242,17 +2242,23 @@ sub parse_vm_config {
 	if ($line =~ m/^\[PENDING\]\s*$/i) {
 	    $section = { name => 'pending', type => 'pending' };
 	    $finish_description->();
+	    $handle_error->("vm $vmid - duplicate section: $section->{name}\n")
+		if defined($res->{$section->{name}});
 	    $conf = $res->{$section->{name}} = {};
 	    next;
 	} elsif ($line =~ m/^\[special:$special_sections_re_1\]\s*$/i) {
 	    $section = { name => $1, type => 'special' };
 	    $finish_description->();
+	    $handle_error->("vm $vmid - duplicate special section: $section->{name}\n")
+		if defined($res->{'special-sections'}->{$section->{name}});
 	    $conf = $res->{'special-sections'}->{$section->{name}} = {};
 	    next;
 
 	} elsif ($line =~ m/^\[([a-z][a-z0-9_\-]+)\]\s*$/i) {
 	    $section = { name => $1, type => 'snapshot' };
 	    $finish_description->();
+	    $handle_error->("vm $vmid - duplicate snapshot section: $section->{name}\n")
+		if defined($res->{snapshots}->{$section->{name}});
 	    $conf = $res->{snapshots}->{$section->{name}} = {};
 	    next;
 	} elsif ($line =~ m/^\[([^\]]*)\]\s*$/i) {
@@ -2322,6 +2328,8 @@ sub parse_vm_config {
     $finish_description->();
     delete $res->{snapstate}; # just to be sure
 
+    $res->{pending} = {} if !defined($res->{pending});
+
     return $res;
 }
 
diff --git a/test/parse-config-expected/duplicate-sections.conf.strict.error b/test/parse-config-expected/duplicate-sections.conf.strict.error
new file mode 100644
index 00000000..f7aabfd7
--- /dev/null
+++ b/test/parse-config-expected/duplicate-sections.conf.strict.error
@@ -0,0 +1 @@
+vm 8006 - duplicate section: pending

From 6eb92d31bad83664a779a66ac93a971d7eb96e61 Mon Sep 17 00:00:00 2001
From: Fiona Ebner <f.ebner@proxmox.com>
Date: Mon, 27 Jan 2025 12:29:19 +0100
Subject: [PATCH 12/16] check type: require schema as an argument

In preparation to re-use the helper with a dedicated schema for a
'fleecing' special section.

No functional change intended.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Link: https://lore.proxmox.com/20250127112923.31703-13-f.ebner@proxmox.com
---
 PVE/QemuServer.pm | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 8c26fbc3..7711014d 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -2081,11 +2081,13 @@ sub cloudinit_pending_properties {
 }
 
 sub check_type {
-    my ($key, $value) = @_;
+    my ($key, $value, $schema) = @_;
 
-    die "unknown setting '$key'\n" if !$confdesc->{$key};
+    die "check_type: no schema defined\n" if !$schema;
 
-    my $type = $confdesc->{$key}->{type};
+    die "unknown setting '$key'\n" if !$schema->{$key};
+
+    my $type = $schema->{$key}->{type};
 
     if (!defined($value)) {
 	die "got undefined value\n";
@@ -2106,7 +2108,7 @@ sub check_type {
         return $value if $value =~ m/^(\d+)(\.\d+)?$/;
         die "type check ('number') failed - got '$value'\n";
     } elsif ($type eq 'string') {
-	if (my $fmt = $confdesc->{$key}->{format}) {
+	if (my $fmt = $schema->{$key}->{format}) {
 	    PVE::JSONSchema::check_format($fmt, $value);
 	    return $value;
 	}
@@ -2301,7 +2303,7 @@ sub parse_vm_config {
 		$conf->{$key} = $value;
 		next;
 	    }
-	    eval { $value = check_type($key, $value); };
+	    eval { $value = check_type($key, $value, $confdesc); };
 	    if ($@) {
 		$handle_error->("vm $vmid - unable to parse value of '$key' - $@");
 	    } else {
@@ -2368,7 +2370,7 @@ sub write_vm_config {
 		# fixme: check syntax?
 		next;
 	    }
-	    eval { $value = check_type($key, $value); };
+	    eval { $value = check_type($key, $value, $confdesc); };
 	    die "unable to parse value of '$key' - $@" if $@;
 
 	    $cref->{$key} = $value;

From a82c4555e30da69717f4066119da6970404d6312 Mon Sep 17 00:00:00 2001
From: Fiona Ebner <f.ebner@proxmox.com>
Date: Mon, 27 Jan 2025 12:29:20 +0100
Subject: [PATCH 13/16] config: add fleecing section

Will be used for improved cleanup of fleecing images.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Link: https://lore.proxmox.com/20250127112923.31703-14-f.ebner@proxmox.com
---
 PVE/API2/Qemu.pm                              |  3 ++
 PVE/QemuServer.pm                             | 29 ++++++++++++++-----
 test/parse-config-input/fleecing-section.conf | 20 +++++++++++++
 test/run_parse_config_tests.pl                |  2 +-
 4 files changed, 46 insertions(+), 8 deletions(-)
 create mode 100644 test/parse-config-input/fleecing-section.conf

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 084e4efb..0bda4426 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -5955,6 +5955,9 @@ __PACKAGE__->register_method({
 
 		    my $special_sections = delete $new_conf->{'special-sections'} // {};
 
+		    # fleecing state is specific to source side
+		    delete $special_sections->{fleecing};
+
 		    $new_conf->{skip_cloud_init} = 1; # re-use image from source side
 
 		    # TODO PVE 10 - remove backwards-compat handling?
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 7711014d..dba95975 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -2195,6 +2195,16 @@ sub destroy_vm {
     }
 }
 
+my $fleecing_section_schema = {
+    'fleecing-images' => {
+	type => 'string',
+	format => 'pve-volume-id-list',
+	description => "For internal use only. List of fleecing images allocated during backup."
+	   ." If no backup is running, these are left-overs that failed to be removed.",
+	optional => 1,
+    },
+};
+
 sub parse_vm_config {
     my ($filename, $raw, $strict) = @_;
 
@@ -2233,23 +2243,28 @@ sub parse_vm_config {
 	$descr = undef;
     };
 
-    my $special_sections_re_1 = qr/(cloudinit)/;
+    my $special_schemas = {
+	cloudinit => $confdesc, # not actually used right now, see below
+	fleecing => $fleecing_section_schema,
+    };
+    my $special_sections_re_string = join('|', keys $special_schemas->%*);
+    my $special_sections_re_1 = qr/($special_sections_re_string)/;
 
-    my $section = { name => '', type => 'main' };
+    my $section = { name => '', type => 'main', schema => $confdesc };
 
     my @lines = split(/\n/, $raw);
     foreach my $line (@lines) {
 	next if $line =~ m/^\s*$/;
 
 	if ($line =~ m/^\[PENDING\]\s*$/i) {
-	    $section = { name => 'pending', type => 'pending' };
+	    $section = { name => 'pending', type => 'pending', schema => $confdesc };
 	    $finish_description->();
 	    $handle_error->("vm $vmid - duplicate section: $section->{name}\n")
 		if defined($res->{$section->{name}});
 	    $conf = $res->{$section->{name}} = {};
 	    next;
 	} elsif ($line =~ m/^\[special:$special_sections_re_1\]\s*$/i) {
-	    $section = { name => $1, type => 'special' };
+	    $section = { name => $1, type => 'special', schema => $special_schemas->{$1} };
 	    $finish_description->();
 	    $handle_error->("vm $vmid - duplicate special section: $section->{name}\n")
 		if defined($res->{'special-sections'}->{$section->{name}});
@@ -2257,7 +2272,7 @@ sub parse_vm_config {
 	    next;
 
 	} elsif ($line =~ m/^\[([a-z][a-z0-9_\-]+)\]\s*$/i) {
-	    $section = { name => $1, type => 'snapshot' };
+	    $section = { name => $1, type => 'snapshot', schema => $confdesc };
 	    $finish_description->();
 	    $handle_error->("vm $vmid - duplicate snapshot section: $section->{name}\n")
 		if defined($res->{snapshots}->{$section->{name}});
@@ -2303,12 +2318,12 @@ sub parse_vm_config {
 		$conf->{$key} = $value;
 		next;
 	    }
-	    eval { $value = check_type($key, $value, $confdesc); };
+	    eval { $value = check_type($key, $value, $section->{schema}); };
 	    if ($@) {
 		$handle_error->("vm $vmid - unable to parse value of '$key' - $@");
 	    } else {
 		$key = 'ide2' if $key eq 'cdrom';
-		my $fmt = $confdesc->{$key}->{format};
+		my $fmt = $section->{schema}->{$key}->{format};
 		if ($fmt && $fmt =~ /^pve-qm-(?:ide|scsi|virtio|sata)$/) {
 		    my $v = parse_drive($key, $value);
 		    if (my $volid = filename_to_volume_id($vmid, $v->{file}, $v->{media})) {
diff --git a/test/parse-config-input/fleecing-section.conf b/test/parse-config-input/fleecing-section.conf
new file mode 100644
index 00000000..ee89dc56
--- /dev/null
+++ b/test/parse-config-input/fleecing-section.conf
@@ -0,0 +1,20 @@
+boot: order=scsi0
+cores: 2
+cpu: x86-64-v2-AES
+ide2: lvm:vm-120-cloudinit,media=cdrom
+ipconfig0: ip6=dhcp
+memory: 4096
+meta: creation-qemu=9.0.2,ctime=1725975013
+name: deb1223
+net0: vmxnet3=BC:24:11:2C:69:EC,bridge=vnet0,firewall=1
+numa: 0
+ostype: l26
+scsi0: nfs:120/vm-120-disk-0.qcow2,iothread=1,size=4G
+scsihw: virtio-scsi-single
+smbios1: uuid=b3247ab1-1fe6-428e-965b-08a1b64a8746
+sockets: 1
+unused0: rbd:vm-120-disk-0
+vmgenid: 7079e97c-50e3-4079-afe7-23e67566b946
+
+[special:fleecing]
+fleecing-images: zfs:vm-120-fleece-0
diff --git a/test/run_parse_config_tests.pl b/test/run_parse_config_tests.pl
index bc7ad86e..b353ea19 100755
--- a/test/run_parse_config_tests.pl
+++ b/test/run_parse_config_tests.pl
@@ -26,7 +26,7 @@ my $OUTPUT_DIR = './parse-config-output';
 my $EXPECTED_DIR = './parse-config-expected';
 
 # NOTE update when you add/remove tests
-plan tests => 2 * 9;
+plan tests => 2 * 10;
 
 sub run_tests {
     my ($strict) = @_;

From a39866732fdd3cf6c7e98101fabf9a0d4fcda144 Mon Sep 17 00:00:00 2001
From: Fiona Ebner <f.ebner@proxmox.com>
Date: Mon, 27 Jan 2025 12:29:21 +0100
Subject: [PATCH 14/16] fix #5440: vzdump: better cleanup fleecing images after
 hard errors
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

By recording the allocated fleecing images in the VM config, they
are not immediately orphaned, should a hard error occur during
backup that prevents cleanup.

They are attempted to be cleaned up during the next backup run.

In the cleanup helper, check if fleecing images are still attached in
QEMU and detach them. This allows recovering from more failure
scenarios. However, to avoid a deadlock, a left-over backup job needs
to be canceled first. While canceling a left-over backup already
happens when cleanup is done for a subsquent backup, it is required
for other cases that like cleanup before migration (to be added in a
following commit).

Suggested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Link: https://lore.proxmox.com/20250127112923.31703-15-f.ebner@proxmox.com
---
 PVE/QemuConfig.pm        | 82 ++++++++++++++++++++++++++++++++++++++++
 PVE/VZDump/QemuServer.pm | 36 +++++++++++++++---
 2 files changed, 112 insertions(+), 6 deletions(-)

diff --git a/PVE/QemuConfig.pm b/PVE/QemuConfig.pm
index 3d57a0a8..92747165 100644
--- a/PVE/QemuConfig.pm
+++ b/PVE/QemuConfig.pm
@@ -13,6 +13,7 @@ use PVE::QemuServer::Monitor qw(mon_cmd);
 use PVE::QemuServer;
 use PVE::QemuServer::Machine;
 use PVE::QemuServer::Memory qw(get_current_memory);
+use PVE::RESTEnvironment qw(log_warn);
 use PVE::Storage;
 use PVE::Tools;
 use PVE::Format qw(render_bytes render_duration);
@@ -578,4 +579,85 @@ sub has_cloudinit {
     return $found;
 }
 
+# Caller is expected to deal with volumes from an already existing 'fleecing' special section in the
+# configuration first.
+sub record_fleecing_images {
+    my ($vmid, $volids) = @_;
+
+    return if scalar($volids->@*) == 0;
+
+    PVE::QemuConfig->lock_config($vmid, sub {
+	my $conf = PVE::QemuConfig->load_config($vmid);
+	$conf->{'special-sections'}->{fleecing}->{'fleecing-images'} = join(',', $volids->@*);
+	PVE::QemuConfig->write_config($vmid, $conf);
+    });
+}
+
+# Will also cancel a running backup job inside QEMU. Not doing so can lead to a deadlock when
+# attempting to detach the fleecing image.
+sub cleanup_fleecing_images {
+    my ($vmid, $storecfg, $log_func) = @_;
+
+    if (!$log_func) {
+	$log_func = sub {
+	    my ($level, $line) = @_;
+	    chomp($line);
+	    if ($level eq 'info') {
+		print "$line\n";
+	    } else {
+		log_warn($line);
+	    }
+	};
+    }
+
+    my $volids = [];
+    my $failed = [];
+
+    # cancel left-over backup job and detach any left-over images from a running VM
+    if (PVE::QemuServer::Helpers::vm_running_locally($vmid)) {
+	eval {
+	    if (my $status = mon_cmd($vmid, 'query-backup')) {
+		if ($status->{status} && $status->{status} eq 'active') {
+		    $log_func->('warn', "left-over backup job still running inside QEMU - canceling now");
+		    mon_cmd($vmid, 'backup-cancel');
+		}
+	    }
+	};
+	$log_func->('warn', "checking/canceling old backup job failed - $@") if $@;
+
+	my $block_info = mon_cmd($vmid, "query-block");
+	for my $info ($block_info->@*) {
+	    my $device_id = $info->{device};
+	    next if $device_id !~ m/-fleecing$/;
+
+	    $log_func->('info', "detaching (old) fleecing image for '$device_id'");
+	    $device_id =~ s/^drive-//; # re-added by qemu_drivedel()
+	    eval { PVE::QemuServer::qemu_drivedel($vmid, $device_id) };
+	    $log_func->('warn', "error detaching (old) fleecing image '$device_id' - $@") if $@;
+	}
+    }
+
+    PVE::QemuConfig->lock_config($vmid, sub {
+	my $conf = PVE::QemuConfig->load_config($vmid);
+	my $special = $conf->{'special-sections'};
+	if (my $fleecing = $special->{fleecing}) {
+	    $volids = [PVE::Tools::split_list($fleecing->{'fleecing-images'})];
+	    delete $fleecing->{'fleecing-images'};
+	    delete $special->{fleecing} if !scalar(keys $fleecing->%*);
+	    PVE::QemuConfig->write_config($vmid, $conf);
+	}
+    });
+
+    for my $volid ($volids->@*) {
+	$log_func->('info', "removing (old) fleecing image '$volid'");
+	eval { PVE::Storage::vdisk_free($storecfg, $volid); };
+	if (my $err = $@) {
+	    $log_func->('warn', "error removing fleecing image '$volid' - $err");
+	    push $failed->@*, $volid;
+	}
+    }
+
+    record_fleecing_images($vmid, $failed);
+}
+
 1;
diff --git a/PVE/VZDump/QemuServer.pm b/PVE/VZDump/QemuServer.pm
index cdaaa3a2..55325217 100644
--- a/PVE/VZDump/QemuServer.pm
+++ b/PVE/VZDump/QemuServer.pm
@@ -533,15 +533,25 @@ sub get_and_check_pbs_encryption_config {
     die "internal error - unhandled case for getting & checking PBS encryption ($keyfile, $master_keyfile)!";
 }
 
+# Helper is intended to be called from allocate_fleecing_images() only. Otherwise, fleecing volids
+# have already been recorded in the configuration and PVE::QemuConfig::cleanup_fleecing_images()
+# should be used instead.
 my sub cleanup_fleecing_images {
-    my ($self, $disks) = @_;
+    my ($self, $vmid, $disks) = @_;
+
+    my $failed = [];
 
     for my $di ($disks->@*) {
 	if (my $volid = $di->{'fleece-volid'}) {
 	    eval { PVE::Storage::vdisk_free($self->{storecfg}, $volid); };
-	    $self->log('warn', "error removing fleecing image '$volid' - $@") if $@;
+	    if (my $err = $@) {
+		$self->log('warn', "error removing fleecing image '$volid' - $err");
+		push $failed->@*, $volid;
+	    }
 	}
     }
+
+    PVE::QemuConfig::record_fleecing_images($vmid, $failed);
 }
 
 my sub allocate_fleecing_images {
@@ -549,8 +559,7 @@ my sub allocate_fleecing_images {
 
     die "internal error - no fleecing storage specified\n" if !$fleecing_storeid;
 
-    # TODO what about potential left-over images from a failed attempt? Just
-    # auto-remove? While unlikely, could conflict with manually created image from user...
+    my $fleece_volids = [];
 
     eval {
 	my $n = 0; # counter for fleecing image names
@@ -567,6 +576,8 @@ my sub allocate_fleecing_images {
 		$di->{'fleece-volid'} = PVE::Storage::vdisk_alloc(
 		    $self->{storecfg}, $fleecing_storeid, $vmid, $format, $name, $size);
 
+		push $fleece_volids->@*, $di->{'fleece-volid'};
+
 		$n++;
 	    } else {
 		die "implement me (type '$di->{type}')";
@@ -574,9 +585,11 @@ my sub allocate_fleecing_images {
 	}
     };
     if (my $err = $@) {
-	cleanup_fleecing_images($self, $disks);
+	cleanup_fleecing_images($self, $vmid, $disks);
 	die $err;
     }
+
+    PVE::QemuConfig::record_fleecing_images($vmid, $fleece_volids);
 }
 
 my sub detach_fleecing_images {
@@ -636,6 +649,13 @@ my sub check_and_prepare_fleecing {
 	$use_fleecing = 0;
     }
 
+    # clean up potential left-overs from a previous attempt
+    eval {
+	PVE::QemuConfig::cleanup_fleecing_images(
+	    $vmid, $self->{storecfg}, sub { $self->log($_[0], $_[1]); });
+    };
+    $self->log('warn', "attempt to clean up left-over fleecing images failed - $@") if $@;
+
     if ($use_fleecing) {
 	my ($default_format, $valid_formats) = PVE::Storage::storage_default_format(
 	    $self->{storecfg}, $fleecing_opts->{storage});
@@ -1132,7 +1152,11 @@ sub cleanup {
     # If VM was started only for backup, it is already stopped now.
     if (PVE::QemuServer::Helpers::vm_running_locally($vmid)) {
 	$detach_tpmstate_drive->($task, $vmid);
-	detach_fleecing_images($task->{disks}, $vmid) if $task->{'use-fleecing'};
+	if ($task->{'use-fleecing'}) {
+	    detach_fleecing_images($task->{disks}, $vmid);
+	    PVE::QemuConfig::cleanup_fleecing_images(
+		$vmid, $self->{storecfg}, sub { $self->log($_[0], $_[1]); });
+	}
     }
 
     cleanup_fleecing_images($self, $task->{disks}) if $task->{'use-fleecing'};

From 4e659fcac6fea9c7767e41b014ceecffedbfedca Mon Sep 17 00:00:00 2001
From: Fiona Ebner <f.ebner@proxmox.com>
Date: Mon, 27 Jan 2025 12:29:22 +0100
Subject: [PATCH 15/16] migration: attempt to clean up potential left-over
 fleecing images
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Clean up left-over fleecing images before the guest is migrated to a
different node and they'd really become orphaned.

Suggested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Link: https://lore.proxmox.com/20250127112923.31703-16-f.ebner@proxmox.com
---
 PVE/QemuMigrate.pm | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
index 625fd1f7..f02360ad 100644
--- a/PVE/QemuMigrate.pm
+++ b/PVE/QemuMigrate.pm
@@ -177,6 +177,13 @@ sub prepare {
 
     my $storecfg = $self->{storecfg} = PVE::Storage::config();
 
+    # updates the configuration, so ordered before saving the configuration in $self
+    eval {
+	PVE::QemuConfig::cleanup_fleecing_images(
+	    $vmid, $storecfg, sub { $self->log($_[0], $_[1]); });
+    };
+    $self->log('warn', "attempt to clean up left-over fleecing images failed - $@") if $@;
+
     # test if VM exists
     my $conf = $self->{vmconf} = PVE::QemuConfig->load_config($vmid);
 

From a5185ba114d1530f15e88aee1476b1446b12cf63 Mon Sep 17 00:00:00 2001
From: Fiona Ebner <f.ebner@proxmox.com>
Date: Mon, 27 Jan 2025 12:29:23 +0100
Subject: [PATCH 16/16] destroy vm: clean up potential left-over fleecing
 images
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Avoids that any left-over fleecing images become orphaned.

Suggested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Link: https://lore.proxmox.com/20250127112923.31703-17-f.ebner@proxmox.com
---
 PVE/QemuServer.pm | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index dba95975..6babb1df 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -2122,6 +2122,9 @@ sub check_type {
 sub destroy_vm {
     my ($storecfg, $vmid, $skiplock, $replacement_conf, $purge_unreferenced) = @_;
 
+    eval { PVE::QemuConfig::cleanup_fleecing_images($vmid, $storecfg) };
+    log_warn("attempt to clean up left-over fleecing images failed - $@") if $@;
+
     my $conf = PVE::QemuConfig->load_config($vmid);
 
     if (!$skiplock && !PVE::QemuConfig->has_lock($conf, 'suspended')) {