On October 7, 2019 2:47 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.rei...@proxmox.com> > --- > > v3 -> v4: > * use is_custom_model > > v2 -> v3: > * move $cpu_fmt->{flags} changes here, to avoid having broken checks between > patches > > > PVE/QemuServer.pm | 2 +- > PVE/QemuServer/CPUConfig.pm | 70 ++++++++++++++++++++++++++++++++++--- > 2 files changed, 67 insertions(+), 5 deletions(-) > > diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm > index 58e2944..04ab1c2 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 9876757..fb8e852 100644 > --- a/PVE/QemuServer/CPUConfig.pm > +++ b/PVE/QemuServer/CPUConfig.pm > @@ -90,9 +90,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)]})/;
maybe it makes sense to pull out both REs? > > -our $cpu_fmt = { > +my $cpu_fmt = { > cputype => { > description => "Emulated CPU type. Can be default or custom name > (custom model names must be prefixed with 'custom-').", > type => 'string', > @@ -125,14 +125,76 @@ 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)]}.", > format_description => '+FLAG[;-FLAG...]', > type => 'string', > - pattern => qr/$cpu_flag(;$cpu_flag)*/, > + pattern => qr/[+-][a-zA-Z0-9\-_\.]+(;[+-][a-zA-Z0-9\-_\.]+)*/, > optional => 1, > }, > }; > > +# $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 parse_cpu_conf_basic { > + my ($cpu_str, $noerr) = @_; > + > + my $cpu = eval { PVE::JSONSchema::parse_property_string($cpu_fmt, > $cpu_str) }; > + 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 required, but only encoded in section header? > + 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_str, $noerr) = @_; > + > + my $cpu = parse_cpu_conf_basic($cpu_str, $noerr); > + return undef if !$cpu; > + > + my $cputype = $cpu->{cputype}; > + > + if (is_custom_model($cputype)) { > + # Custom model only has to exist, all props are allowed > + my $config = load_custom_model_conf(); > + my $cputype_id = $cputype; > + $cputype_id =~ s/^custom-//; > + return $cpu if $config && defined($config->{ids}->{$cputype_id}); this is very similar to get_model_by_name, see my other comments there and think about re-using it here.. > + > + die "Custom cputype '$cputype_id' not found\n" if !$noerr; > + return undef; > + } > + > + # Only built-in models get here > + if (!defined($cpu_vendor_list->{$cputype})) { > + die "Default cputype '$cputype' not found\n" if !$noerr; s/Default/Built-in/ or even "(Built-in) cputype '$cputype' is not defined (missing 'custom-' prefix?)\n" > + 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; > + } > + > + die "Property 'reported-model' not allowed in VM-specific CPU config.\n" > + if defined($cpu->{'reported-model'}); > + > + return $cpu; > +} > + > # Section config settings > my $defaultData = { > # shallow copy, since SectionConfig modifies propertyList internally > -- > 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