* [pve-devel] applied: [PATCH v3 qemu-server 3/3] fix #2671: include CPU format in man page again
[not found] ` <1594028311.osgnixtap1.astroid@nora.none>
@ 2020-07-08 8:29 ` Fabian Grünbichler
0 siblings, 0 replies; 2+ messages in thread
From: Fabian Grünbichler @ 2020-07-08 8:29 UTC (permalink / raw)
To: PVE development discussion
On July 6, 2020 11:38 am, Fabian Grünbichler wrote:
> this looks good, waiting for pve-common bump before applying this with a
> versioned dependency.
done now ;)
>
> On June 25, 2020 1:35 pm, Stefan Reiter wrote:
>> Use the new register_format(3) call to use a validator (instead of a
>> parser) for 'pve-(vm-)?cpu-conf'. This way the $cpu_fmt hash can be used for
>> generating the documentation, while still applying the same verification
>> rules as before.
>>
>> Since the function no longer parses but only verifies, the parsing in
>> print_cpu_device/get_cpu_options has to go via JSONSchema directly.
>>
>> Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
>> ---
>> PVE/QemuServer/CPUConfig.pm | 56 ++++++++++++-------------------------
>> 1 file changed, 18 insertions(+), 38 deletions(-)
>>
>> diff --git a/PVE/QemuServer/CPUConfig.pm b/PVE/QemuServer/CPUConfig.pm
>> index 6250591..8ed898c 100644
>> --- a/PVE/QemuServer/CPUConfig.pm
>> +++ b/PVE/QemuServer/CPUConfig.pm
>> @@ -153,6 +153,7 @@ my $cpu_fmt = {
>> 'phys-bits' => {
>> type => 'string',
>> format => 'pve-phys-bits',
>> + format_description => '8-64|host',
>> description => "The physical memory address bits that are reported to"
>> . " the guest OS. Should be smaller or equal to the host's."
>> . " Set to 'host' to use value from host CPU, but note that"
>> @@ -182,57 +183,36 @@ sub parse_phys_bits {
>>
>> # $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.
>> -PVE::JSONSchema::register_format('pve-cpu-conf', \&parse_cpu_conf_basic);
>> -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;
>> - }
>> +# though, which we catch in the custom validation functions below.
>> +PVE::JSONSchema::register_format('pve-cpu-conf', $cpu_fmt, \&validate_cpu_conf);
>> +sub validate_cpu_conf {
>> + my ($cpu) = @_;
>>
>> # required, but can't be forced in schema since it's encoded in section
>> # header for custom models
>> - if (!$cpu->{cputype}) {
>> - die "CPU is missing cputype\n" if !$noerr;
>> - return undef;
>> - }
>> -
>> - return $cpu;
>> + die "CPU is missing cputype\n" if !$cpu->{cputype};
>> }
>> +PVE::JSONSchema::register_format('pve-vm-cpu-conf', $cpu_fmt, \&validate_vm_cpu_conf);
>> +sub validate_vm_cpu_conf {
>> + my ($cpu) = @_;
>>
>> -PVE::JSONSchema::register_format('pve-vm-cpu-conf', \&parse_vm_cpu_conf);
>> -sub parse_vm_cpu_conf {
>> - my ($cpu_str, $noerr) = @_;
>> -
>> - my $cpu = parse_cpu_conf_basic($cpu_str, $noerr);
>> - return undef if !$cpu;
>> + validate_cpu_conf($cpu);
>>
>> my $cputype = $cpu->{cputype};
>>
>> # a VM-specific config is only valid if the cputype exists
>> if (is_custom_model($cputype)) {
>> - eval { get_custom_model($cputype); };
>> - if ($@) {
>> - die $@ if !$noerr;
>> - return undef;
>> - }
>> + # dies on unknown model
>> + get_custom_model($cputype);
>> } else {
>> - if (!defined($cpu_vendor_list->{$cputype})) {
>> - die "Built-in cputype '$cputype' is not defined (missing 'custom-' prefix?)\n" if !$noerr;
>> - return undef;
>> - }
>> + die "Built-in cputype '$cputype' is not defined (missing 'custom-' prefix?)\n"
>> + if !defined($cpu_vendor_list->{$cputype});
>> }
>>
>> # in a VM-specific config, certain properties are limited/forbidden
>>
>> - if ($cpu->{flags} && $cpu->{flags} !~ m/$cpu_flag_supported_re(;$cpu_flag_supported_re)*/) {
>> - die "VM-specific CPU flags must be a subset of: @{[join(', ', @supported_cpu_flags)]}\n"
>> - if !$noerr;
>> - return undef;
>> - }
>> + die "VM-specific CPU flags must be a subset of: @{[join(', ', @supported_cpu_flags)]}\n"
>> + if ($cpu->{flags} && $cpu->{flags} !~ m/$cpu_flag_supported_re(;$cpu_flag_supported_re)*/);
>>
>> die "Property 'reported-model' not allowed in VM-specific CPU config.\n"
>> if defined($cpu->{'reported-model'});
>> @@ -369,7 +349,7 @@ sub print_cpu_device {
>> my $kvm = $conf->{kvm} // 1;
>> my $cpu = $kvm ? "kvm64" : "qemu64";
>> if (my $cputype = $conf->{cpu}) {
>> - my $cpuconf = parse_cpu_conf_basic($cputype)
>> + my $cpuconf = PVE::JSONSchema::parse_property_string('pve-vm-cpu-conf', $cputype)
>> or die "Cannot parse cpu description: $cputype\n";
>> $cpu = $cpuconf->{cputype};
>>
>> @@ -481,7 +461,7 @@ sub get_cpu_options {
>> my $custom_cpu;
>> my $hv_vendor_id;
>> if (my $cpu_prop_str = $conf->{cpu}) {
>> - $cpu = parse_vm_cpu_conf($cpu_prop_str)
>> + $cpu = PVE::JSONSchema::parse_property_string('pve-vm-cpu-conf', $cpu_prop_str)
>> or die "Cannot parse cpu description: $cpu_prop_str\n";
>>
>> $cputype = $cpu->{cputype};
>> --
>> 2.20.1
>>
>>
>> _______________________________________________
>> pve-devel mailing list
>> pve-devel@pve.proxmox.com
>> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>>
>>
>
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [pve-devel] [PATCH v3 qemu-server 3/3] fix #2671: include CPU format in man page again
[not found] ` <20200625113541.16684-4-s.reiter@proxmox.com>
[not found] ` <1594028311.osgnixtap1.astroid@nora.none>
@ 2020-07-09 12:29 ` Thomas Lamprecht
1 sibling, 0 replies; 2+ messages in thread
From: Thomas Lamprecht @ 2020-07-09 12:29 UTC (permalink / raw)
To: PVE development discussion, Stefan Reiter
On 25.06.20 13:35, Stefan Reiter wrote:
> Use the new register_format(3) call to use a validator (instead of a
> parser) for 'pve-(vm-)?cpu-conf'. This way the $cpu_fmt hash can be used for
> generating the documentation, while still applying the same verification
> rules as before.
>
> Since the function no longer parses but only verifies, the parsing in
> print_cpu_device/get_cpu_options has to go via JSONSchema directly.
>
> Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
> ---
> PVE/QemuServer/CPUConfig.pm | 56 ++++++++++++-------------------------
> 1 file changed, 18 insertions(+), 38 deletions(-)
>
> diff --git a/PVE/QemuServer/CPUConfig.pm b/PVE/QemuServer/CPUConfig.pm
> index 6250591..8ed898c 100644
> --- a/PVE/QemuServer/CPUConfig.pm
> +++ b/PVE/QemuServer/CPUConfig.pm
> @@ -153,6 +153,7 @@ my $cpu_fmt = {
> 'phys-bits' => {
> type => 'string',
> format => 'pve-phys-bits',
> + format_description => '8-64|host',
> description => "The physical memory address bits that are reported to"
> . " the guest OS. Should be smaller or equal to the host's."
> . " Set to 'host' to use value from host CPU, but note that"
> @@ -182,57 +183,36 @@ sub parse_phys_bits {
>
> # $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.
> -PVE::JSONSchema::register_format('pve-cpu-conf', \&parse_cpu_conf_basic);
> -sub parse_cpu_conf_basic {
This method is still used in PVE/QemuMigrate.pm:236 and breaks live-migration with
cpu type = host..
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-07-09 12:30 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20200625113541.16684-1-s.reiter@proxmox.com>
[not found] ` <20200625113541.16684-4-s.reiter@proxmox.com>
[not found] ` <1594028311.osgnixtap1.astroid@nora.none>
2020-07-08 8:29 ` [pve-devel] applied: [PATCH v3 qemu-server 3/3] fix #2671: include CPU format in man page again Fabian Grünbichler
2020-07-09 12:29 ` [pve-devel] " Thomas Lamprecht
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal