[pve-devel] [PATCH v2 qemu-server 06/12] Verify VM-specific CPU configs seperately

[Thomas Lamprecht]

On 9/30/19 12:58 PM, Stefan Reiter wrote:
> $cpu_fmt is being reused for custom CPUs as well as VM-specific CPU
> settings. The "pve-vm-cpu-conf" format is introduced to verify a config
> specifically for use as VM-specific settings.
> 
> Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
> ---
>  PVE/QemuServer.pm           |  2 +-
>  PVE/QemuServer/CPUConfig.pm | 68 +++++++++++++++++++++++++++++++++++--
>  2 files changed, 67 insertions(+), 3 deletions(-)
> 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index a95006f..abf5578 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -505,7 +505,7 @@ EODESCR
>  	optional => 1,
>  	description => "Emulated CPU type.",
>  	type => 'string',
> -	format => $PVE::QemuServer::CPUConfig::cpu_fmt,
> +	format => 'pve-vm-cpu-conf',
>      },
>      parent => get_standard_option('pve-snapshot-name', {
>  	optional => 1,
> diff --git a/PVE/QemuServer/CPUConfig.pm b/PVE/QemuServer/CPUConfig.pm
> index 125b636..61bd024 100644
> --- a/PVE/QemuServer/CPUConfig.pm
> +++ b/PVE/QemuServer/CPUConfig.pm
> @@ -87,9 +87,9 @@ my @supported_cpu_flags = (
>      'hv-evmcs',
>      'aes'
>  );
> -my $cpu_flag = qr/[+-](@{[join('|', @supported_cpu_flags)]})/;
> +my $cpu_flag_re = qr/([+-])(@{[join('|', @supported_cpu_flags)]})/;
>  
> -our $cpu_fmt = {
> +my $cpu_fmt = {
>      cputype => {
>  	description => "Emulated CPU type. Can be default or custom name.",
>  	type => 'string',
> @@ -139,6 +139,70 @@ our $cpu_fmt = {
>      },
>  };
>  
> +# $cpu_fmt describes both the CPU config passed as part of a VM config, as well
> +# as the definition of a custom CPU model. There are some slight differences
> +# though, which we catch in the custom verification function below.
> +sub verify_cpu_conf_basic {
s/verify/parse/ ?

> +    my ($cpu, $noerr) = @_;
> +
> +    eval {
> +        $cpu = PVE::JSONSchema::parse_property_string($cpu_fmt, $cpu);
> +    };

Two things:

* I do not really like variable re-use, especially in such a weak typed
  and not to much "compiler" checked language as perl.. So please call the
  parameter $cpu_str or the like to make the difference clear.
* I find it a bit easier to read:
    my $cpu = eval { PVE::JSONSchema::parse_property_string($cpu_fmt, $cpu_str) };
  but that's rather personal taste, so see as nit ;)

> +    if ($@) {
> +        die $@ if !$noerr;
> +        return undef;
> +    }
> +
> +    # required, but can't be made "optional => 0" since it's not included as a
> +    # seperate property in config file
> +    if (!$cpu->{cputype}) {
> +	die "CPU is missing cputype\n" if !$noerr;
> +	return undef;
> +    }
> +
> +    return $cpu;
> +}
> +
> +PVE::JSONSchema::register_format('pve-vm-cpu-conf', \&verify_vm_cpu_conf);
> +sub verify_vm_cpu_conf {
> +    my ($cpu, $noerr) = @_;
> +
> +    $cpu = verify_cpu_conf_basic($cpu, $noerr);
> +    return undef if !$cpu;
> +
> +    my $cputype = $cpu->{cputype};
> +
> +    # Model must exist
> +    if ($cpu->{custom}) {
> +	my $config = load_custom_model_conf();
> +	return $cpu if $config && defined($config->{ids}->{$cputype});
> +
> +	die "Custom cputype '$cputype' not found\n" if !$noerr;
> +	return undef;
> +    } else {
> +	return $cpu if defined($cpu_vendor_list->{$cputype});
> +
> +	die "Default cputype '$cputype' not found\n" if !$noerr;
> +	return undef;
> +    }
> +
> +    if ($cpu->{flags} && $cpu->{flags} !~ m/$cpu_flag_re(;$cpu_flag_re)*/) {
> +	die "VM-specific CPU flags must be a subset of: @{[join(', ', @supported_cpu_flags)]}\n"
> +	    if !$noerr;
> +	return undef;
> +    }
> +
> +    for my $prop (keys %$cpu) {
> +	if ($cpu->{$prop}->{custom_only}) {
> +	    die "Property '$prop' not allowed in VM-specific CPU config.\n"
> +		if !$noerr;
> +	    return undef;
> +	}
> +    }
> +
> +    return $cpu;
> +}
> +
>  # Section config settings
>  my $defaultData = {
>      # shallow copy, since SectionConfig modifies propertyList internally
>