Am 17.02.26 um 11:51 AM schrieb Arthur Bied-Charreton:
> ...where they fit better alongside other CPU-related utilities.
>
> Signed-off-by: Arthur Bied-Charreton <[email protected]>
When sending multiple patches, please always use a cover letter [0].
It's really helpful for tooling like b4 and to keep a better overview.
> ---
> src/PVE/QemuServer.pm | 131 +-------------------------------
> src/PVE/QemuServer/CPUConfig.pm | 131 +++++++++++++++++++++++++++++++-
> 2 files changed, 131 insertions(+), 131 deletions(-)
>
> diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
> index 5d2dbe03..2121f7c8 100644
> --- a/src/PVE/QemuServer.pm
> +++ b/src/PVE/QemuServer.pm
> @@ -46,8 +46,7 @@ use PVE::SafeSyslog;
> use PVE::Storage;
> use PVE::SysFSTools;
> use PVE::Systemd;
> -use PVE::Tools
> - qw(run_command file_read_firstline file_get_contents dir_glob_foreach
> get_host_arch $IPV6RE);
> +use PVE::Tools qw(run_command file_read_firstline file_get_contents
> dir_glob_foreach $IPV6RE);
>
> use PVE::QMPClient;
> use PVE::QemuConfig;
> @@ -2911,134 +2910,6 @@ sub vga_conf_has_spice {
> return $1 || 1;
> }
>
> -# To use query_supported_cpu_flags and query_understood_cpu_flags to get
> flags
> -# to use in a QEMU command line (-cpu element), first array_intersect the
> result
> -# of query_supported_ with query_understood_. This is necessary because:
> -#
> -# a) query_understood_ returns flags the host cannot use and
> -# b) query_supported_ (rather the QMP call) doesn't actually return CPU
> -# flags, but CPU settings - with most of them being flags. Those settings
> -# (and some flags, curiously) cannot be specified as a "-cpu" argument.
> -#
> -# query_supported_ needs to start up to 2 temporary VMs and is therefore
> rather
> -# expensive. If you need the value returned from this, you can get it much
> -# cheaper from pmxcfs using PVE::Cluster::get_node_kv('cpuflags-$accel') with
> -# $accel being 'kvm' or 'tcg'.
> -#
> -# pvestatd calls this function on startup and whenever the QEMU/KVM version
> -# changes, automatically populating pmxcfs.
> -#
> -# Returns: { kvm => [ flagX, flagY, ... ], tcg => [ flag1, flag2, ... ] }
> -# since kvm and tcg machines support different flags
> -#
> -sub query_supported_cpu_flags {
If you remove it here, it will require having a versioned Breaks in
qemu-server's d/control towards older pve-manager. I'd keep a wrapper
here, which we can drop in the future.
---snip 8<---
> diff --git a/src/PVE/QemuServer/CPUConfig.pm b/src/PVE/QemuServer/CPUConfig.pm
> index 32ec4954..e53f0978 100644
> --- a/src/PVE/QemuServer/CPUConfig.pm
> +++ b/src/PVE/QemuServer/CPUConfig.pm
> @@ -9,7 +9,8 @@ use PVE::JSONSchema qw(json_bool);
> use PVE::Cluster qw(cfs_register_file cfs_read_file);
> use PVE::ProcFSTools;
> use PVE::RESTEnvironment qw(log_warn);
> -use PVE::Tools qw(run_command);
> +use PVE::Tools qw(run_command get_host_arch);
> +use PVE::QemuServer::Monitor qw(mon_cmd);
>
> use PVE::QemuServer::Helpers qw(min_version);
>
> @@ -579,6 +580,134 @@ sub add_cpu_json_properties {
> return $prop;
> }
>
> +# To use query_supported_cpu_flags and query_understood_cpu_flags to get
> flags
> +# to use in a QEMU command line (-cpu element), first array_intersect the
> result
> +# of query_supported_ with query_understood_. This is necessary because:
> +#
> +# a) query_understood_ returns flags the host cannot use and
> +# b) query_supported_ (rather the QMP call) doesn't actually return CPU
> +# flags, but CPU settings - with most of them being flags. Those settings
> +# (and some flags, curiously) cannot be specified as a "-cpu" argument.
> +#
> +# query_supported_ needs to start up to 2 temporary VMs and is therefore
> rather
> +# expensive. If you need the value returned from this, you can get it much
> +# cheaper from pmxcfs using PVE::Cluster::get_node_kv('cpuflags-$accel') with
> +# $accel being 'kvm' or 'tcg'.
> +#
> +# pvestatd calls this function on startup and whenever the QEMU/KVM version
> +# changes, automatically populating pmxcfs.
> +#
> +# Returns: { kvm => [ flagX, flagY, ... ], tcg => [ flag1, flag2, ... ] }
> +# since kvm and tcg machines support different flags
> +#
> +sub query_supported_cpu_flags {
> + my ($arch) = @_;
> +
> + $arch //= get_host_arch();
> + my $default_machine =
> PVE::QemuServer::Machine::default_machine_for_arch($arch);
> +
> + my $flags = {};
> +
> + # FIXME: Once this is merged, the code below should work for ARM as well:
> + # https://lists.nongnu.org/archive/html/qemu-devel/2019-06/msg04947.html
> + die "QEMU/KVM cannot detect CPU flags on ARM (aarch64)\n"
> + if $arch eq "aarch64";
> +
> + my $kvm_supported = defined(PVE::QemuServer::kvm_version());
Ah, we cannot depend on PVE::QemuServer here, because that is a cyclic
dependency. That helper could be moved to the Helpers module anyways,
but...[continued below]
> + my $qemu_cmd = PVE::QemuServer::Helpers::get_command_for_arch($arch);
> + my $fakevmid = -1;
> + my $pidfile = PVE::QemuServer::Helpers::vm_pidfile_name($fakevmid);
> +
> + # Start a temporary (frozen) VM with vmid -1 to allow sending a QMP
> command
> + my $query_supported_run_qemu = sub {
> + my ($kvm) = @_;
> +
> + my $flags = {};
> + my $cmd = [
> + $qemu_cmd,
> + '-machine',
> + $default_machine,
> + '-display',
> + 'none',
> + '-chardev',
> +
> "socket,id=qmp,path=/var/run/qemu-server/$fakevmid.qmp,server=on,wait=off",
> + '-mon',
> + 'chardev=qmp,mode=control',
> + '-pidfile',
> + $pidfile,
> + '-S',
> + '-daemonize',
> + ];
> +
> + if (!$kvm) {
> + push @$cmd, '-accel', 'tcg';
> + }
> +
> + my $rc = run_command($cmd, noerr => 1, quiet => 0);
> + die "QEMU flag querying VM exited with code " . $rc if $rc;
> +
> + eval {
> + my $cmd_result = mon_cmd(
> + $fakevmid,
> + 'query-cpu-model-expansion',
> + type => 'full',
> + model => { name => 'host' },
> + );
> +
> + my $props = $cmd_result->{model}->{props};
> + foreach my $prop (keys %$props) {
> + next if $props->{$prop} ne '1';
> + # QEMU returns some flags multiple times, with '_', '.' or
> '-'
> + # (e.g. lahf_lm and lahf-lm; sse4.2, sse4-2 and sse4_2; ...).
> + # We only keep those with underscores, to match /proc/cpuinfo
> + $prop =~ s/\.|-/_/g;
> + $flags->{$prop} = 1;
> + }
> + };
> + my $err = $@;
> +
> + # force stop with 10 sec timeout and 'nocheck', always stop, even if
> QMP failed
> + PVE::QemuServer::vm_stop(undef, $fakevmid, 1, 1, 10, 0, 1);
...the vm_stop() call will be harder to deal with. The right module for
it would be the RunState module, but this requires more untangling
first. In particular, vm_stop_cleanup() calls vmconfig_apply_pending()
so that would also need to move to another module first. This is out of
scope for the feature you are working on. I'm also wondering if a
Capabilities::CPU module would not be better fitting than the CPUConfig
too. Let's just keep the functions in QemuServer.pm for now.
> +
> + die $err if $err;
> +
> + return [sort keys %$flags];
> + };
> +
> + # We need to query QEMU twice, since KVM and TCG have different
> supported flags
> + PVE::QemuConfig->lock_config(
You would also need to include the QemuConfig module, but this would not
be a dealbreaker from a first glance.
> + $fakevmid,
> + sub {
> + $flags->{tcg} = eval { $query_supported_run_qemu->(0) };
> + warn "warning: failed querying supported tcg flags: $@\n" if $@;
> +
> + if ($kvm_supported) {
> + $flags->{kvm} = eval { $query_supported_run_qemu->(1) };
> + warn "warning: failed querying supported kvm flags: $@\n" if
> $@;
> + }
> + },
> + );
> +
> + return $flags;
> +}
> +
> +# Understood CPU flags are written to a file at 'pve-qemu' compile time
> +my $understood_cpu_flag_dir = "/usr/share/kvm";
> +
> +sub query_understood_cpu_flags {
> + my $arch = get_host_arch();
> + my $filepath = "$understood_cpu_flag_dir/recognized-CPUID-flags-$arch";
> +
> + die "Cannot query understood QEMU CPU flags for architecture: $arch
> (file not found)\n"
> + if !-e $filepath;
> +
> + my $raw = PVE::Tools::file_get_contents($filepath);
> + $raw =~ s/^\s+|\s+$//g;
> + my @flags = split(/\s+/, $raw);
> +
> + return \@flags;
> +}
> +
> sub get_cpu_models {
> my ($include_custom, $arch) = @_;
>
[0]:
https://lore.proxmox.com/all/[email protected]/
