[pve-devel] [PATCH v4 qemu-server 07/12] Add overrides and convenience functions to CPUConfig

[Fabian Grünbichler]

On October 7, 2019 2:47 pm, Stefan Reiter wrote:
> Add two overrides to avoid writing redundant information to the config
> file.
> 
> get_model_by_name is used to return a cpu config with default values
> filled out.
> 
> Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
> ---
> 
> v3 -> v4:
> * add is_custom_model
> 
> v2 -> v3:
> * add validity checks to write_config
> 
> 
>  PVE/QemuServer/CPUConfig.pm | 65 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 65 insertions(+)
> 
> diff --git a/PVE/QemuServer/CPUConfig.pm b/PVE/QemuServer/CPUConfig.pm
> index 6ea5811..9876757 100644
> --- a/PVE/QemuServer/CPUConfig.pm
> +++ b/PVE/QemuServer/CPUConfig.pm
> @@ -151,6 +151,71 @@ 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
> +	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"
> +	    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"
> +	    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.
> +# Fills in defaults from schema if value is not set in config.
> +# Returns undef for unknown $name, allows names with and without custom- prefix.
> +sub get_model_by_name {
> +    my ($conf, $name) = @_;

see latter patches as well.

I'd rename this to get_custom_model, and inline the call to 
load_custom_model_conf since there is only one call-site which does the 
load right before just to call this, and we are only ever interested in 
a single model anyway..

adding $noerr

> +
> +    $name =~ s/^custom-//;
> +
> +    my $entry = $conf->{ids}->{$name};
> +    return undef if !defined($entry);

and a 'die' here would also be a good idea I think.
> +
> +    my $propList = $defaultData->{propertyList};
> +
> +    my $model = {};
> +    for my $property (keys %$propList) {
> +	next if $property eq 'type';
> +
> +	if (my $value = $entry->{$property}) {
> +	    $model->{$property} = $value;
> +	} elsif (my $default = $propList->{$property}->{default}) {
> +	    $model->{$property} = $default;
> +	}

does this really make sense here? for 'hidden' it's problematic, for 
'host-phys-bits' it has no effect anyway since the default is 0, 
reported-model is only accessed once so we could move the default 
application there, and cputype is non-optional for all intents and 
purposes, so we should never need to apply the default here?

> +    }
> +
> +    return $model;
> +}
> +
>  # Print a QEMU device node for a given VM configuration for hotplugging CPUs
>  sub print_cpu_device {
>      my ($conf, $id) = @_;
> -- 
> 2.20.1
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
>