From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 76DD91FF138 for ; Wed, 18 Feb 2026 11:41:13 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 1BD7A1955B; Wed, 18 Feb 2026 11:42:13 +0100 (CET) Message-ID: Date: Wed, 18 Feb 2026 11:42:05 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH qemu-server] cpu config: Move CPU flags utils to CPUConfig module To: Arthur Bied-Charreton , pve-devel@lists.proxmox.com References: <20260217105050.194368-1-a.bied-charreton@proxmox.com> Content-Language: en-US From: Fiona Ebner In-Reply-To: <20260217105050.194368-1-a.bied-charreton@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1771411318157 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.015 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: ACOTOS4CVE4CW4GS6MNGYII4Z2KEFNII X-Message-ID-Hash: ACOTOS4CVE4CW4GS6MNGYII4Z2KEFNII X-MailFrom: f.ebner@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 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/176174696524.1776006.10760334798456181474.b4-ty@proxmox.com/