public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Dominik Csapak <d.csapak@proxmox.com>, pve-devel@lists.proxmox.com
Subject: Re: [PATCH qemu-server v2 1/2] tests: improve multiarch build support
Date: Thu, 5 Feb 2026 09:27:23 +0100	[thread overview]
Message-ID: <7573bcbe-af00-41ed-9204-fb4db63ab634@proxmox.com> (raw)
In-Reply-To: <7fd01f54-4148-4cb4-8d22-bf68d2e9855e@proxmox.com>

Am 05.02.26 um 08:50 schrieb Dominik Csapak:
> On 2/4/26 4:45 PM, Thomas Lamprecht wrote:
>> Am 04.02.26 um 11:04 schrieb Dominik Csapak:
> [snip]
>>>   +initialize_cpu_models();
>>
>> this now still always does this on module load, would be nicer to actually
>> only pay for that if needed by adding getter methods for each variable, like
>>
>> sub get_all_cpu_models {
>>     initialize_cpu_models() if !defined($all_cpu_models);
>>     return $all_cpu_models;
>> }
>>
>> Same with a get_cpu_models_by_arch getter.
> 
> not sure if that gains us anything, since we need the 'all_cpu_models' hash statically for the 'reported-model' enum of $cpu_fmt, so even if i put it in a getter, it would still get initialized on module load...

It still nicer to have, especially if this would be decoupled in the future.
Else we can stop clean separation and just always initialize everything
globally everywhere, to exaggerate for the points sake.

> also not sure if having two seperate getters make sense, since
> the 'all_cpu_models' one depends on the cpu_models_by_arch one.

I'm not sure if I see what a data dependency has to do with not having
a cleaner getter interface to better encapsulate that local variable off
and hedge against someone just making it an "our" shared variable in the
future. A shared initialization code path doesn't IMO mean that one has
to couple using the result of that together.

> So in that case we'd have to initialize both anyway (again, on module load).

Yes, but that's just a detail of the current implementation, not–to over
play the point–making it ugly because it doesn't matter *now* for the
*current* use case. Even there it's nicer to not have a module wide
variable used directly, as for all but small scripts that seldomly makes
code more readable.

> so this would make code a bit more complicated, but I don't really see the gain here.

How is having a getter and making the initialization a local "my sub"
complicated? I basically provided the getter code for one variable
already, the other one is basically just a copy of that, and adaption
to using these getter's should be straight forward..

btw., now that I'm thinking more of this, might be even nicer to clean
this up even slightly more and produce a minimal CPUModels perl module
on build time, as the info won't ever change during run time, and for
testing it should make it even easier to override things there.

And then these could be constants, where I would not care that much
anymore about directly accessing them, but won't help testing and
using constants over getters is IMO not really cleaner for non-scalar
values most of the time (as always there certainly are exceptions).




  parent reply	other threads:[~2026-02-05  8:27 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-04 10:01 Dominik Csapak
2026-02-04 10:01 ` [PATCH qemu-server v2 2/2] tests: cfg2cmd: add some architecture tests Dominik Csapak
2026-02-04 15:46 ` [PATCH qemu-server v2 1/2] tests: improve multiarch build support Thomas Lamprecht
2026-02-05  7:51   ` Dominik Csapak
2026-02-05  8:26     ` Dominik Csapak
2026-02-05  8:27     ` Thomas Lamprecht [this message]
2026-02-05  8:48       ` Dominik Csapak
2026-02-05 14:25 ` superseded: " Dominik Csapak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7573bcbe-af00-41ed-9204-fb4db63ab634@proxmox.com \
    --to=t.lamprecht@proxmox.com \
    --cc=d.csapak@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal