[pve-devel] [PATCH qemu-server v5 04/16] parse config: be precise about section type checks

From: Fiona Ebner <f.ebner@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH qemu-server v5 04/16] parse config: be precise about section type checks
Date: Mon, 27 Jan 2025 12:29:11 +0100	[thread overview]
Message-ID: <20250127112923.31703-5-f.ebner@proxmox.com> (raw)
In-Reply-To: <20250127112923.31703-1-f.ebner@proxmox.com>

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

