* [pve-devel] [PATCH-SERIES qemu-server v5 00/16] more robust handling of fleecing images
@ 2025-01-27 11:29 Fiona Ebner
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 01/16] migration: remove unused variable Fiona Ebner
` (15 more replies)
0 siblings, 16 replies; 17+ messages in thread
From: Fiona Ebner @ 2025-01-27 11:29 UTC (permalink / raw)
To: pve-devel
Changes in v5:
* everything new in v5 except the last 3 patches
* new approach, use special config section instead of config key
* add tests and some fixes for configuration handling
* make special section handling more generic
* also check for and cancel left-over running backup job before
detaching the fleecing images to prevent a deadlock in some
scenarios
Record the created fleecing images in the VM configuration to be able
to remove left-overs after hard failures.
Adds a new special configuration section 'fleecing', making special
section handling more generic as preparation, as well as fixing some
corner cases in configuration parsing and adding tests.
Fiona Ebner (16):
migration: remove unused variable
test: avoid duplicate mock module in restore config test
test: add parse config tests
parse config: be precise about section type checks
test: add test case exposing issue with unknown sections
parse config: skip unknown sections and warn about their presence
vzdump: anchor matches for pending and special sections
vzdump: skip all special sections
config: make special section handling generic
test: parse config: test config with duplicate sections
parse config: warn about duplicate sections
check type: require schema as an argument
config: add fleecing section
fix #5440: vzdump: better cleanup fleecing images after hard errors
migration: attempt to clean up potential left-over fleecing images
destroy vm: clean up potential left-over fleecing images
PVE/API2/Qemu.pm | 22 +-
PVE/QemuConfig.pm | 84 +++++++-
PVE/QemuMigrate.pm | 8 +-
PVE/QemuServer.pm | 97 ++++++---
PVE/QemuServer/Cloudinit.pm | 8 +-
PVE/VZDump/QemuServer.pm | 50 ++++-
test/Makefile | 7 +-
.../cloudinit-snapshot.conf | 40 ++++
.../cloudinit-snapshot.conf.strict.error | 1 +
.../duplicate-sections.conf | 43 ++++
.../duplicate-sections.conf.strict.error | 1 +
.../unknown-sections.conf | 44 ++++
.../unknown-sections.conf.strict.error | 1 +
.../verify-snapshot.conf | 36 ++++
.../verify-snapshot.conf.strict.error | 1 +
.../cloudinit-snapshot.conf | 41 ++++
.../duplicate-sections.conf | 68 +++++++
test/parse-config-input/fleecing-section.conf | 20 ++
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/unknown-sections.conf | 57 ++++++
test/parse-config-input/verify-snapshot.conf | 37 ++++
test/run_parse_config_tests.pl | 92 +++++++++
test/run_qemu_restore_config_tests.pl | 12 +-
27 files changed, 992 insertions(+), 58 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-expected/duplicate-sections.conf
create mode 100644 test/parse-config-expected/duplicate-sections.conf.strict.error
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-expected/verify-snapshot.conf
create mode 100644 test/parse-config-expected/verify-snapshot.conf.strict.error
create mode 100644 test/parse-config-input/cloudinit-snapshot.conf
create mode 100644 test/parse-config-input/duplicate-sections.conf
create mode 100644 test/parse-config-input/fleecing-section.conf
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/unknown-sections.conf
create mode 100644 test/parse-config-input/verify-snapshot.conf
create mode 100755 test/run_parse_config_tests.pl
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* [pve-devel] [PATCH qemu-server v5 01/16] migration: remove unused variable
2025-01-27 11:29 [pve-devel] [PATCH-SERIES qemu-server v5 00/16] more robust handling of fleecing images Fiona Ebner
@ 2025-01-27 11:29 ` Fiona Ebner
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 02/16] test: avoid duplicate mock module in restore config test Fiona Ebner
` (14 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Fiona Ebner @ 2025-01-27 11:29 UTC (permalink / raw)
To: pve-devel
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>
---
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});
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* [pve-devel] [PATCH qemu-server v5 02/16] test: avoid duplicate mock module in restore config test
2025-01-27 11:29 [pve-devel] [PATCH-SERIES qemu-server v5 00/16] more robust handling of fleecing images Fiona Ebner
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 01/16] migration: remove unused variable Fiona Ebner
@ 2025-01-27 11:29 ` Fiona Ebner
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 03/16] test: add parse config tests Fiona Ebner
` (13 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Fiona Ebner @ 2025-01-27 11:29 UTC (permalink / raw)
To: pve-devel
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>
---
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) = @_;
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* [pve-devel] [PATCH qemu-server v5 03/16] test: add parse config tests
2025-01-27 11:29 [pve-devel] [PATCH-SERIES qemu-server v5 00/16] more robust handling of fleecing images Fiona Ebner
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 01/16] migration: remove unused variable Fiona Ebner
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 02/16] test: avoid duplicate mock module in restore config test Fiona Ebner
@ 2025-01-27 11:29 ` Fiona Ebner
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 04/16] parse config: be precise about section type checks Fiona Ebner
` (12 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Fiona Ebner @ 2025-01-27 11:29 UTC (permalink / raw)
To: pve-devel
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>
---
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();
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* [pve-devel] [PATCH qemu-server v5 04/16] parse config: be precise about section type checks
2025-01-27 11:29 [pve-devel] [PATCH-SERIES qemu-server v5 00/16] more robust handling of fleecing images Fiona Ebner
` (2 preceding siblings ...)
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 03/16] test: add parse config tests Fiona Ebner
@ 2025-01-27 11:29 ` Fiona Ebner
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 05/16] test: add test case exposing issue with unknown sections Fiona Ebner
` (11 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Fiona Ebner @ 2025-01-27 11:29 UTC (permalink / raw)
To: pve-devel
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>
---
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) = @_;
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* [pve-devel] [PATCH qemu-server v5 05/16] test: add test case exposing issue with unknown sections
2025-01-27 11:29 [pve-devel] [PATCH-SERIES qemu-server v5 00/16] more robust handling of fleecing images Fiona Ebner
` (3 preceding siblings ...)
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 04/16] parse config: be precise about section type checks Fiona Ebner
@ 2025-01-27 11:29 ` Fiona Ebner
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 06/16] parse config: skip unknown sections and warn about their presence Fiona Ebner
` (10 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Fiona Ebner @ 2025-01-27 11:29 UTC (permalink / raw)
To: pve-devel
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>
---
.../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) = @_;
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* [pve-devel] [PATCH qemu-server v5 06/16] parse config: skip unknown sections and warn about their presence
2025-01-27 11:29 [pve-devel] [PATCH-SERIES qemu-server v5 00/16] more robust handling of fleecing images Fiona Ebner
` (4 preceding siblings ...)
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 05/16] test: add test case exposing issue with unknown sections Fiona Ebner
@ 2025-01-27 11:29 ` Fiona Ebner
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 07/16] vzdump: anchor matches for pending and special sections Fiona Ebner
` (9 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Fiona Ebner @ 2025-01-27 11:29 UTC (permalink / raw)
To: pve-devel
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>
---
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'
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* [pve-devel] [PATCH qemu-server v5 07/16] vzdump: anchor matches for pending and special sections
2025-01-27 11:29 [pve-devel] [PATCH-SERIES qemu-server v5 00/16] more robust handling of fleecing images Fiona Ebner
` (5 preceding siblings ...)
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 06/16] parse config: skip unknown sections and warn about their presence Fiona Ebner
@ 2025-01-27 11:29 ` Fiona Ebner
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 08/16] vzdump: skip all " Fiona Ebner
` (8 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Fiona Ebner @ 2025-01-27 11:29 UTC (permalink / raw)
To: pve-devel
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>
---
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;
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* [pve-devel] [PATCH qemu-server v5 08/16] vzdump: skip all special sections
2025-01-27 11:29 [pve-devel] [PATCH-SERIES qemu-server v5 00/16] more robust handling of fleecing images Fiona Ebner
` (6 preceding siblings ...)
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 07/16] vzdump: anchor matches for pending and special sections Fiona Ebner
@ 2025-01-27 11:29 ` Fiona Ebner
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 09/16] config: make special section handling generic Fiona Ebner
` (7 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Fiona Ebner @ 2025-01-27 11:29 UTC (permalink / raw)
To: pve-devel
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>
---
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)");
}
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* [pve-devel] [PATCH qemu-server v5 09/16] config: make special section handling generic
2025-01-27 11:29 [pve-devel] [PATCH-SERIES qemu-server v5 00/16] more robust handling of fleecing images Fiona Ebner
` (7 preceding siblings ...)
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 08/16] vzdump: skip all " Fiona Ebner
@ 2025-01-27 11:29 ` Fiona Ebner
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 10/16] test: parse config: test config with duplicate sections Fiona Ebner
` (6 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Fiona Ebner @ 2025-01-27 11:29 UTC (permalink / raw)
To: pve-devel
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>
---
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;
}
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* [pve-devel] [PATCH qemu-server v5 10/16] test: parse config: test config with duplicate sections
2025-01-27 11:29 [pve-devel] [PATCH-SERIES qemu-server v5 00/16] more robust handling of fleecing images Fiona Ebner
` (8 preceding siblings ...)
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 09/16] config: make special section handling generic Fiona Ebner
@ 2025-01-27 11:29 ` Fiona Ebner
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 11/16] parse config: warn about " Fiona Ebner
` (5 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Fiona Ebner @ 2025-01-27 11:29 UTC (permalink / raw)
To: pve-devel
Add a test case to witness how duplicate sections are handled.
Signed-off-by: Fiona Ebner <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) = @_;
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* [pve-devel] [PATCH qemu-server v5 11/16] parse config: warn about duplicate sections
2025-01-27 11:29 [pve-devel] [PATCH-SERIES qemu-server v5 00/16] more robust handling of fleecing images Fiona Ebner
` (9 preceding siblings ...)
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 10/16] test: parse config: test config with duplicate sections Fiona Ebner
@ 2025-01-27 11:29 ` Fiona Ebner
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 12/16] check type: require schema as an argument Fiona Ebner
` (4 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Fiona Ebner @ 2025-01-27 11:29 UTC (permalink / raw)
To: pve-devel
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>
---
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
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* [pve-devel] [PATCH qemu-server v5 12/16] check type: require schema as an argument
2025-01-27 11:29 [pve-devel] [PATCH-SERIES qemu-server v5 00/16] more robust handling of fleecing images Fiona Ebner
` (10 preceding siblings ...)
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 11/16] parse config: warn about " Fiona Ebner
@ 2025-01-27 11:29 ` Fiona Ebner
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 13/16] config: add fleecing section Fiona Ebner
` (3 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Fiona Ebner @ 2025-01-27 11:29 UTC (permalink / raw)
To: pve-devel
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>
---
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;
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* [pve-devel] [PATCH qemu-server v5 13/16] config: add fleecing section
2025-01-27 11:29 [pve-devel] [PATCH-SERIES qemu-server v5 00/16] more robust handling of fleecing images Fiona Ebner
` (11 preceding siblings ...)
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 12/16] check type: require schema as an argument Fiona Ebner
@ 2025-01-27 11:29 ` Fiona Ebner
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 14/16] fix #5440: vzdump: better cleanup fleecing images after hard errors Fiona Ebner
` (2 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Fiona Ebner @ 2025-01-27 11:29 UTC (permalink / raw)
To: pve-devel
Will be used for improved cleanup of fleecing images.
Signed-off-by: Fiona Ebner <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) = @_;
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* [pve-devel] [PATCH qemu-server v5 14/16] fix #5440: vzdump: better cleanup fleecing images after hard errors
2025-01-27 11:29 [pve-devel] [PATCH-SERIES qemu-server v5 00/16] more robust handling of fleecing images Fiona Ebner
` (12 preceding siblings ...)
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 13/16] config: add fleecing section Fiona Ebner
@ 2025-01-27 11:29 ` Fiona Ebner
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 15/16] migration: attempt to clean up potential left-over fleecing images Fiona Ebner
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 16/16] destroy vm: " Fiona Ebner
15 siblings, 0 replies; 17+ messages in thread
From: Fiona Ebner @ 2025-01-27 11:29 UTC (permalink / raw)
To: pve-devel
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>
---
Changes in v5:
* cancel backup job if still running before detaching
* use special config section instead of config key
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'};
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* [pve-devel] [PATCH qemu-server v5 15/16] migration: attempt to clean up potential left-over fleecing images
2025-01-27 11:29 [pve-devel] [PATCH-SERIES qemu-server v5 00/16] more robust handling of fleecing images Fiona Ebner
` (13 preceding siblings ...)
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 14/16] fix #5440: vzdump: better cleanup fleecing images after hard errors Fiona Ebner
@ 2025-01-27 11:29 ` Fiona Ebner
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 16/16] destroy vm: " Fiona Ebner
15 siblings, 0 replies; 17+ messages in thread
From: Fiona Ebner @ 2025-01-27 11:29 UTC (permalink / raw)
To: pve-devel
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>
---
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);
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* [pve-devel] [PATCH qemu-server v5 16/16] destroy vm: clean up potential left-over fleecing images
2025-01-27 11:29 [pve-devel] [PATCH-SERIES qemu-server v5 00/16] more robust handling of fleecing images Fiona Ebner
` (14 preceding siblings ...)
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 15/16] migration: attempt to clean up potential left-over fleecing images Fiona Ebner
@ 2025-01-27 11:29 ` Fiona Ebner
15 siblings, 0 replies; 17+ messages in thread
From: Fiona Ebner @ 2025-01-27 11:29 UTC (permalink / raw)
To: pve-devel
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>
---
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')) {
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2025-01-27 11:31 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-01-27 11:29 [pve-devel] [PATCH-SERIES qemu-server v5 00/16] more robust handling of fleecing images Fiona Ebner
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 01/16] migration: remove unused variable Fiona Ebner
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 02/16] test: avoid duplicate mock module in restore config test Fiona Ebner
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 03/16] test: add parse config tests Fiona Ebner
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 04/16] parse config: be precise about section type checks Fiona Ebner
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 05/16] test: add test case exposing issue with unknown sections Fiona Ebner
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 06/16] parse config: skip unknown sections and warn about their presence Fiona Ebner
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 07/16] vzdump: anchor matches for pending and special sections Fiona Ebner
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 08/16] vzdump: skip all " Fiona Ebner
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 09/16] config: make special section handling generic Fiona Ebner
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 10/16] test: parse config: test config with duplicate sections Fiona Ebner
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 11/16] parse config: warn about " Fiona Ebner
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 12/16] check type: require schema as an argument Fiona Ebner
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 13/16] config: add fleecing section Fiona Ebner
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 14/16] fix #5440: vzdump: better cleanup fleecing images after hard errors Fiona Ebner
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 15/16] migration: attempt to clean up potential left-over fleecing images Fiona Ebner
2025-01-27 11:29 ` [pve-devel] [PATCH qemu-server v5 16/16] destroy vm: " Fiona Ebner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox