On October 7, 2019 2:47 pm, Stefan Reiter wrote: > To avoid hardcoding even more CPU-flag related things for custom CPU > models, introduce a dynamic approach to resolving flags. > > resolve_cpu_flags takes a list of hashes (as documented in the > comment) and resolves them to a valid "-cpu" argument without > duplicates. This also helps by providing a reason why specific CPU flags > have been added, and thus allows for useful warning messages should a > flag be overwritten by another. > > Signed-off-by: Stefan Reiter <s.rei...@proxmox.com> > --- > PVE/QemuServer/CPUConfig.pm | 67 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 67 insertions(+) > > diff --git a/PVE/QemuServer/CPUConfig.pm b/PVE/QemuServer/CPUConfig.pm > index fb8e852..405ad86 100644 > --- a/PVE/QemuServer/CPUConfig.pm > +++ b/PVE/QemuServer/CPUConfig.pm > @@ -298,6 +298,73 @@ sub print_cpu_device { > return > "$cpu-x86_64-cpu,id=cpu$id,socket-id=$current_socket,core-id=$current_core,thread-id=0"; > } > > +# Resolves multiple arrays of hashes representing CPU flags with metadata to > a > +# single string in QEMU "-cpu" compatible format. Later arrays have higher > +# priority. > +# > +# Hashes take the following format: > +# { > +# aes => { > +# op => "+", # defaults to "" if undefined > +# reason => "to support AES acceleration", # for override warnings > +# value => "" # needed for kvm=off (value: off) etc... > +# }, > +# ... > +# } > +sub resolve_cpu_flags { > + my $flag_hashes = \@_;
why? it's only used once two lines below, where you immediately derefence it again. > + > + my $flags = {}; > + > + for my $hash (@$flag_hashes) { > + for my $flag_name (keys %$hash) { > + my $flag = $hash->{$flag_name}; > + my $old_flag = $flags->{$flag_name}; > + > + $flag->{op} //= ""; > + > + if ($old_flag) { > + my $value_changed = defined($flag->{value}) != > defined($old_flag->{value}); > + if (!$value_changed && defined($flag->{value})) { > + $value_changed = $flag->{value} eq $old_flag->{value}; > + } eq should be ne, right? either the definedness changed, or it was and is defined and the value changed? AFAICT, this is in fact the following nested conditional: my $value_changed = (defined($flag->{value}) != defined($old_flag->{value})) || (defined($flag->{value}) && $flag->{value} ne $old_flag->{value}); which is more readable IMHO, but you could also expand it more with additional variables. > + > + if ($old_flag->{op} eq $flag->{op} && !$value_changed) { > + $flags->{$flag_name}->{reason} .= " & $flag->{reason}"; > + next; > + } > + > + my $old = print_cpuflag_hash($flag_name, $flags->{$flag_name}); > + my $new = print_cpuflag_hash($flag_name, $flag); > + warn "warning: CPU flag/setting $new overwrites $old\n"; > + } > + > + $flags->{$flag_name} = $flag; > + } > + } > + > + my $flag_str = ''; > + # sort for command line stability > + for my $flag_name (sort keys %$flags) { > + $flag_str .= ','; > + $flag_str .= $flags->{$flag_name}->{op}; > + $flag_str .= $flag_name; > + $flag_str .= "=$flags->{$flag_name}->{value}" > + if $flags->{$flag_name}->{value}; > + } > + > + return $flag_str; > +} > + > +sub print_cpuflag_hash { > + my ($flag_name, $flag) = @_; > + my $formatted = "'$flag->{op}$flag_name"; > + $formatted .= "=$flag->{value}" if defined($flag->{value}); > + $formatted .= "'"; > + $formatted .= " ($flag->{reason})" if defined($flag->{reason}); > + return $formatted; > +} > + > # Calculate QEMU's '-cpu' argument from a given VM configuration > sub get_cpu_options { > my ($conf, $arch, $kvm, $machine_type, $kvm_off, $kvmver, $winversion, > $gpu_passthrough) = @_; > -- > 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