Thanks for the review, I'll prepare a v3 :)
Just some clarifications inline for this patch.
On 10/2/19 7:49 AM, Thomas Lamprecht wrote:
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.rei...@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()
TBH I just moved that part from the existing "cputype" enum, but I can
change it in v3 - it sure looks clearer with 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?
Yes, because it is checked in verify_vm_cpu_conf from patch 06/12. I
admit it's not entirely clear from just looking at this property
definition, but it was the only way I could think of to somewhat cleanly
handle the re-use of $cpu_fmt for VM-specific settings and custom models
- maybe I'll add a comment.
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;
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
