On September 2, 2019 4:27 pm, Stefan Reiter wrote:
> Inherits from SectionConfig to provide base parsing infrastructure.
>
> Use with helper functions:
> * config_from_file gives bless'd config
> * get_model_by_name returns a "formatted" hash for a single CPU model
> * config_to_file writes changes back
>
> File reads are cached in a local hash.
high-level:
use cfs_register/write/read_file please (it's our mechanism for handling
config files on pmxcfs after all ;))
is there a reason you need a class-like interface here? we usually use
that if we want to have multiple implementations of a certain interface
with a common shared code base, but that is not the case here..
>
> Signed-off-by: Stefan Reiter <s.rei...@proxmox.com>
> ---
>
> This will definitely require some sort of versioning mechanism, otherwise CPU
> definitions could be changed after starting a VM, thus breaking live-migration
> by starting the migration-target with different parameters.
>
> Hints, ideas, recommendations?
versioning is one possibility (cumbersome with SectionConfig though).
another would be to retrieve the CPU model from the running Qemu
instance, and convert that back to a '-cpu ...' string for starting the
target instance. if nothing hot-pluggable can change the generated -cpu
string, we could also record that somewhere (or take it from
/proc/$pid/cmdline) and override the target instance with that.
> PVE/QemuServer/CustomCPUConfig.pm | 129 ++++++++++++++++++++++++++++++
> PVE/QemuServer/Makefile | 1 +
> 2 files changed, 130 insertions(+)
> create mode 100644 PVE/QemuServer/CustomCPUConfig.pm
>
> diff --git a/PVE/QemuServer/CustomCPUConfig.pm
> b/PVE/QemuServer/CustomCPUConfig.pm
> new file mode 100644
> index 0000000..87ba9e6
> --- /dev/null
> +++ b/PVE/QemuServer/CustomCPUConfig.pm
> @@ -0,0 +1,129 @@
> +package PVE::QemuServer::CustomCPUConfig;
> +
> +use strict;
> +use warnings;
> +use PVE::Tools qw(file_get_contents file_set_contents);
> +
> +use base qw(PVE::SectionConfig);
> +
> +my $defaultData = {
> + propertyList => {
> + basemodel => {
> + description => "Emulated CPU type to inherit defaults from.",
> + type => 'string',
> + format_description => 'string',
> + default => 'kvm64',
should have the same pattern/format as QemuServer's $cpu_fmt->{cputype}.
in other words, it probably makes sense to extract and re-use that ;)
> + },
> + flags => {
> + description => "List of additional CPU flags separated by ';'."
> + . " Use '+FLAG' to enable, '-FLAG' to disable a flag."
> + . " Supports all flags supported by QEMU/KVM.",
> + format_description => '+FLAG[;-FLAG...]',
> + type => 'string',
> + pattern => qr/[+-][a-zA-Z0-9\-_\.]+(;[+-][a-zA-Z0-9\-_\.]+)*/,
same here (different description, please just use a single definition
for both instances)
> + optional => 1,
> + },
> + 'phys-bits' => {
> + type => 'integer',
> + minimum => 1,
> + maximum => 64,
> + optional => 1,
> + description => "The physical memory bits that are reported to the
> guest OS. Must be smaller or equal to the host.",
this one might make sense to also allow in the regular, per-VM 'cpu'
option (and then we could again re-use the definition)
> + },
> + 'host-phys-bits' => {
> + type => 'boolean',
> + default => 0,
> + description => "Whether to report the host's physical memory bits.
> Overrides 'phys-bits' when set.",
> + },
same
> + vendor => {
> + type => 'string',
> + enum => [qw(default AuthenticAMD GenuineIntel)],
> + default => 'default',
> + description => "The CPU vendor to report. 'default' uses the
> host's.",
> + },
> + }
this probably only makes sense for the custom types. the default value
means that you never inherit the vendor from your basemodel - is that
intended?
there are two more from the regular one that we might want to include in
custom ones as well:
hidden
hv-vendor-id
I am not sure whether just moving some format definitions, or moving
all/most CPU related stuff to a new module (e.g.,
QemuServer/CPUConfig.pm instead of QemuServer/CustomCPUConfig.pm) is
more clean / feasible. the latter probably would require moving some
helpers from QemuServer.pm to another new module as well, as they'd be
used by the CPU module and QemuServer.pm
I think we all agree that QemuServer.pm is way too big, so a patch
(series) that adds new features and reduces that bloat would be more
than welcome :D but if you think it's too much for this series, we can
also do it as a follow-up since it's mostly about moving code and format
definitions.
> +};
> +
> +sub private {
> + return $defaultData;
> +}
> +
> +sub options {
> + return {
> + basemodel => { optional => 1 },
> + flags => { optional => 1 },
> + 'phys-bits' => { optional => 1 },
> + 'host-phys-bits' => { optional => 1 },
> + vendor => { optional => 1 }
> + };
> +}
> +
> +sub type {
> + return 'cpu-model';
> +}
> +
> +# helpers for reading/parsing and writing/formatting of config files
> +
> +my $default_filename = "/etc/pve/cpu-models.conf";
> +my $cfg_cache = {};
> +
> +sub config_from_file {
> + my ($filename) = @_;
> +
> + $filename //= $default_filename;
> +
> + if (my $cached = $cfg_cache->{$filename}) {
> + return $cached;
> + }
> +
> + return undef if ! -e $filename;
> +
> + my $raw = file_get_contents($filename);
> + my $cfg = __PACKAGE__->parse_config($filename, $raw);
> +
> + bless $cfg, qw(PVE::QemuServer::CustomCPUConfig);
> + $cfg_cache->{$filename} = $cfg;
> +
> + return $cfg;
> +}
> +
> +sub config_to_file {
> + my ($class, $filename) = @_;
> +
> + $filename //= $default_filename;
> +
> + $cfg_cache->{$filename} = $class;
> +
> + my $raw = $class->write_config($filename);
> + file_set_contents($filename, $raw);
> +}
> +
> +# Use this to get a single model in the format described by $defaultData
> above
> +# Includes the name as an additional property and sets default values for
> undefs
> +# Returns undef for unknown $name
> +sub get_model_by_name {
> + my ($class, $name) = @_;
> +
> + return undef if !defined($class->{ids}->{$name});
> +
> + my $model = {
> + name => $name
> + };
> +
> + for my $property (keys %{$defaultData->{propertyList}}) {
> + next if $property eq 'type';
> +
> + if (my $value = $class->{ids}->{$name}->{$property}) {
> + $model->{$property} = $value;
> + } elsif (my $default =
> $defaultData->{propertyList}->{$property}->{default}) {
> + $model->{$property} = $default;
> + }
> + }
> +
> + return $model;
> +}
> +
> +__PACKAGE__->register();
> +__PACKAGE__->init();
> +
> +1;
> diff --git a/PVE/QemuServer/Makefile b/PVE/QemuServer/Makefile
> index afc39a3..41b4cb6 100644
> --- a/PVE/QemuServer/Makefile
> +++ b/PVE/QemuServer/Makefile
> @@ -5,6 +5,7 @@ SOURCES=PCI.pm \
> OVF.pm \
> Cloudinit.pm \
> Agent.pm \
> + CustomCPUConfig.pm \
>
> .PHONY: install
> install: ${SOURCES}
> --
> 2.20.1
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
