[pve-devel] [PATCH v7 qemu-server 03/10] Adapt CPUConfig to handle custom models

Thomas Lamprecht t.lamprecht at proxmox.com
Mon Jan 27 09:46:26 CET 2020


On 1/16/20 4:40 PM, Stefan Reiter wrote:
> Turn CPUConfig into a SectionConfig with parsing/writing support for
> custom CPU models. IO is handled using cfs.
> 
> Namespacing will be provided using "custom-" prefix for custom model
> names (in VM config only, cpu-models.conf will contain unprefixed
> names).
> 
> Includes two overrides to avoid writing redundant information to the
> config file, additionally get_custom_model is used to retrieve a custom
> model configuration by name.
> 
> Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
> ---
> 
> Depends on updated pve-cluster.
> 

looks OK, a few nits inline - do not warrant a v8 but if other things
warrant one you could address them too.

> 
>  PVE/QemuServer/CPUConfig.pm | 109 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 106 insertions(+), 3 deletions(-)
> 
> diff --git a/PVE/QemuServer/CPUConfig.pm b/PVE/QemuServer/CPUConfig.pm
> index 86febe8..36a9439 100644
> --- a/PVE/QemuServer/CPUConfig.pm
> +++ b/PVE/QemuServer/CPUConfig.pm
> @@ -4,15 +4,26 @@ use strict;
>  use warnings;
>  
>  use PVE::JSONSchema;
> +use PVE::Cluster qw(cfs_register_file cfs_read_file);
>  use PVE::QemuServer::Helpers qw(min_version);
>  
> -use base qw(Exporter);
> +use base qw(PVE::SectionConfig Exporter);
>  
>  our @EXPORT_OK = qw(
>  print_cpu_device
>  get_cpu_options
>  );
>  
> +mkdir "/etc/pve/virtual-guest";
> +my $default_filename = "virtual-guest/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',
> @@ -84,11 +95,20 @@ 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 (custom model names must be prefixed with 'custom-').",
>  	type => 'string',
> -	enum => [ sort { "\L$a" cmp "\L$b" } keys %$cpu_vendor_list ],
> +	format_description => 'string',
>  	default => 'kvm64',
>  	default_key => 1,
> +	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 { lc("$a") cmp lc("$b") } keys %$cpu_vendor_list ],
> +	default => 'kvm64',
> +	optional => 1,
>      },
>      hidden => {
>  	description => "Do not identify as a KVM virtual machine.",
> @@ -114,6 +134,86 @@ our $cpu_fmt = {
>      },
>  };
>  
> +# 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';
> +}
> +
> +sub parse_section_header {
> +    my ($class, $line) = @_;
> +
> +    my ($type, $sectionId, $errmsg, $config) =
> +	$class->SUPER::parse_section_header($line);
> +
> +    return undef if !$type;
> +    return ($type, $sectionId, $errmsg, {
> +	# to avoid duplicate model name in config file, parse id as cputype
> +	# models parsed from file are by definition always custom

comment is a bit hard to understand, maybe just add
"# cfg models are custom by definition" to the same line below?

> +	cputype => "custom-$sectionId",
> +    });
> +}
> +
> +sub write_config {
> +    my ($class, $filename, $cfg) = @_;
> +
> +    for my $model (keys %{$cfg->{ids}}) {
> +	my $model_conf = $cfg->{ids}->{$model};
> +
> +	die "internal error: tried saving built-in CPU model (or missing prefix)\n"

I always try to include the "bad" values which triggered this exception, so we've
get have something to work with if a user gets this message in a, for they our us,
unexpected situation

> +	    if !is_custom_model($model_conf->{cputype});
> +
> +	die "internal error: tried saving custom cpumodel with cputype (ignoring prefix) not equal to \$cfg->ids entry\n"

same here, even if it gets a bit long then.

> +	    if "custom-$model" ne $model_conf->{cputype};
> +
> +	# saved in section header
> +	delete $model_conf->{cputype};
> +    }
> +
> +    $class->SUPER::write_config($filename, $cfg);
> +}
> +
> +sub is_custom_model {
> +    my ($cputype) = @_;
> +    return $cputype =~ m/^custom-/;
> +}
> +
> +# Use this to get a single model in the format described by $cpu_fmt.
> +# Allows names with and without custom- prefix.
> +sub get_custom_model {
> +    my ($name, $noerr) = @_;
> +
> +    $name =~ s/^custom-//;
> +    my $conf = load_custom_model_conf();
> +
> +    my $entry = $conf->{ids}->{$name};
> +    if (!defined($entry)) {
> +	die "Custom cputype '$name' not found\n" if !$noerr;
> +	return undef;
> +    }
> +
> +    my $model = {};
> +    for my $property (keys %$cpu_fmt) {
> +	if (my $value = $entry->{$property}) {
> +	    $model->{$property} = $value;
> +	}
> +    }
> +
> +    return $model;
> +}
> +
>  # Print a QEMU device node for a given VM configuration for hotplugging CPUs
>  sub print_cpu_device {
>      my ($conf, $id) = @_;
> @@ -229,4 +329,7 @@ sub add_hyperv_enlightenments {
>      }
>  }
>  
> +__PACKAGE__->register();
> +__PACKAGE__->init();
> +
>  1;
> 




More information about the pve-devel mailing list