[pve-devel] [PATCH qemu-server 0/2] fix missing lock-type 'clone' in schema-definition

[Stoiko Ivanov]

Defining the return-properties for vm_config exposed this bug:

When cloning a VM, the config gets written with 'lock: clone', and it gets
updated, for every mirrored blockdevice, however 'clone' is not in the enum
for lock (QemuServer.pm).

The first patch adds 'lock' to the enum.

After a short discussion with Dominik off-list, he suggested, that we should
check the config in general more thoroughly.

The second patch adds a check for enum-types in our schema to check_type,
which is called when writing the config into pmxcfs (via cfs_register_file and
write_vm_config).

The same check could be added (at least) for containers and node configs, so I'd
suggest maybe a helper in pve-common/src/PVE/JSONSchema.pm, used for the
check_type subs.


Stoiko Ivanov (2):
add 'clone' to lock schema definition/confdesc.
check_type: compare value to defined enum

PVE/QemuServer.pm | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)

[Stoiko Ivanov]

check_type is invoked while writing the configuration to the cfs, to validate
the config. This patch adds a check, that the value is in the enum for the
key, if such an enum is defined.

Signed-off-by: Stoiko Ivanov <***@proxmox.com>
---
PVE/QemuServer.pm | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 1832b71..8db4e7e 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -2438,6 +2438,15 @@ sub check_type {
die "property contains a line feed\n";
}

+ if (my $enum_array = $confdesc->{$key}->{enum}) {
+ foreach my $enum_value (@$enum_array) {
+ if ($value eq $enum_value) {
+ return $value;
+ }
+ }
+ die "type check ('enum') failed - '$value' is not in the enumeration for $key\n";
+ }
+
if ($type eq 'boolean') {
return 1 if ($value eq '1') || ($value =~ m/^(on|yes|true)$/i);
return 0 if ($value eq '0') || ($value =~ m/^(off|no|false)$/i);

[Stoiko Ivanov]

Signed-off-by: Stoiko Ivanov <***@proxmox.com>
---
PVE/QemuServer.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index a162db9..1832b71 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -277,7 +277,7 @@ my $confdesc = {
optional => 1,
type => 'string',
description => "Lock/unlock the VM.",
- enum => [qw(migrate backup snapshot rollback)],
+ enum => [qw(migrate backup snapshot rollback clone)],
},
cpulimit => {
optional => 1,

[Wolfgang Bumiller]

applied, and added the other missing values, too ;-)

Post by Stoiko Ivanov
---
PVE/QemuServer.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index a162db9..1832b71 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -277,7 +277,7 @@ my $confdesc = {
optional => 1,
type => 'string',
description => "Lock/unlock the VM.",
- enum => [qw(migrate backup snapshot rollback)],
+ enum => [qw(migrate backup snapshot rollback clone)],
},
cpulimit => {
optional => 1,
--
2.11.0