[pve-devel] [PATCH v2 qemu-server 04/12] Adapt CPUConfig to handle custom models

[Thomas Lamprecht]

On 9/30/19 12:58 PM, Stefan Reiter wrote:
> Turn CPUConfig into a SectionConfig with parsing/writing support for
> custom CPU models. IO is handled using cfs.
> 
> The "custom" parameter provides differentiation between custom and
> default types, even if the name is the same ('namespacing').
> 
> Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
> ---
>  PVE/QemuServer/CPUConfig.pm | 59 ++++++++++++++++++++++++++++++++++---
>  1 file changed, 55 insertions(+), 4 deletions(-)
> 
> diff --git a/PVE/QemuServer/CPUConfig.pm b/PVE/QemuServer/CPUConfig.pm
> index c994228..3fde987 100644
> --- a/PVE/QemuServer/CPUConfig.pm
> +++ b/PVE/QemuServer/CPUConfig.pm
> @@ -3,6 +3,8 @@ package PVE::QemuServer::CPUConfig;
>  use strict;
>  use warnings;
>  use PVE::JSONSchema;
> +use PVE::Cluster qw(cfs_register_file cfs_read_file);
> +use base qw(PVE::SectionConfig);
>  
>  use base 'Exporter';
>  our @EXPORT_OK = qw(
> @@ -11,6 +13,15 @@ get_cpu_options
>  qemu_machine_feature_enabled
>  );
>  
> +my $default_filename = "cpu-models.conf";
> +cfs_register_file($default_filename,
> +		  sub { PVE::QemuServer::CPUConfig->parse_config(@_); },
> +		  sub { PVE::QemuServer::CPUConfig->write_config(@_); });
> +
> +sub load_custom_model_conf {
> +    return cfs_read_file($default_filename);
> +}
> +
>  my $cpu_vendor_list = {
>      # Intel CPUs
>      486 => 'GenuineIntel',
> @@ -80,11 +91,27 @@ my $cpu_flag = qr/[+-](@{[join('|', @supported_cpu_flags)]})/;
>  
>  our $cpu_fmt = {
>      cputype => {
> -	description => "Emulated CPU type.",
> +	description => "Emulated CPU type. Can be default or custom name.",
>  	type => 'string',
> -	enum => [ sort { "\L$a" cmp "\L$b" } keys %$cpu_vendor_list ],
> +	format_description => 'string',
>  	default => 'kvm64',
>  	default_key => 1,
> +	optional => 1,
> +    },
> +    custom => {
> +	description => "True if 'cputype' is a custom model, false if otherwise.",

maybe negate it and call it built-in? We use that as name for Permission roles which
are generated by code, we probably want to call this also built-in in the UI, and be
it only to re-use translations ;)

> +	type => 'boolean',
> +	default => 0,
> +	optional => 1,
> +    },
> +    'reported-model' => {
> +	description => "CPU model and vendor to report to the guest. Must be a QEMU/KVM supported model."
> +		     . " Only valid for custom CPU model definitions, default models will always report themselves to the guest OS.",
> +	type => 'string',
> +	enum => [ sort { "\L$a" cmp "\L$b" } keys %$cpu_vendor_list ],

we do not use \L at all in our code, AFAICT, just use the a bit more
expressive, or at least already used all over the place, lc()

> +	default => 'kvm64',
> +	custom_only => 1,
.     	^^^^^^^^^^^
That's a fake JSONSchema attribute, we do not do those normally, either
some mechanism gets added to JSONSchema to really represent this in general
or just omit it.

> +	optional => 1,
>      },
>      hidden => {
>  	description => "Do not identify as a KVM virtual machine.",
> @@ -102,14 +129,34 @@ our $cpu_fmt = {
>      flags => {
>  	description => "List of additional CPU flags separated by ';'."
>  		     . " Use '+FLAG' to enable, '-FLAG' to disable a flag."
> -		     . " Currently supported flags: @{[join(', ', @supported_cpu_flags)]}.",
> +		     . " Custom CPU models can specify any flag supported by"
> +		     . " QEMU/KVM, VM-specific flags must be from the following"
> +		     . " set for security reasons: @{[join(', ', @supported_cpu_flags)]}.",

is the last part still true if the pattern restriction to $cpu_flag are lifted
below?

>  	format_description => '+FLAG[;-FLAG...]',
>  	type => 'string',
> -	pattern => qr/$cpu_flag(;$cpu_flag)*/,
> +	pattern => qr/[+-][a-zA-Z0-9\-_\.]+(;[+-][a-zA-Z0-9\-_\.]+)*/,
>  	optional => 1,
>      },
>  };
>  
> +# Section config settings
> +my $defaultData = {
> +    # shallow copy, since SectionConfig modifies propertyList internally
> +    propertyList => { %$cpu_fmt },
> +};
> +
> +sub private {
> +    return $defaultData;
> +}
> +
> +sub options {
> +    return { %$cpu_fmt };
> +}
> +
> +sub type {
> +    return 'cpu-model';
> +}
> +
>  # Print a QEMU device node for a given VM configuration for hotplugging CPUs
>  sub print_cpu_device {
>      my ($conf, $id) = @_;
> @@ -248,4 +295,8 @@ sub qemu_machine_feature_enabled {
>  		 $current_minor >= $version_minor);
>  }
>  
> +
> +__PACKAGE__->register();
> +__PACKAGE__->init();
> +
>  1;
>