From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id B69611FF145 for ; Thu, 05 Feb 2026 09:48:32 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id D920694CE; Thu, 5 Feb 2026 09:49:03 +0100 (CET) Message-ID: Date: Thu, 5 Feb 2026 09:48:59 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta Subject: Re: [PATCH qemu-server v2 1/2] tests: improve multiarch build support To: Thomas Lamprecht , pve-devel@lists.proxmox.com References: <20260204100425.1303295-1-d.csapak@proxmox.com> <88e10fbe-3231-40da-8ea2-d95fc1576715@proxmox.com> <7fd01f54-4148-4cb4-8d22-bf68d2e9855e@proxmox.com> <7573bcbe-af00-41ed-9204-fb4db63ab634@proxmox.com> Content-Language: en-US From: Dominik Csapak In-Reply-To: <7573bcbe-af00-41ed-9204-fb4db63ab634@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1770281261882 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.031 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 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. 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: WILKML7RIJAQQQOHILVHNUYXNN6MMAVS X-Message-ID-Hash: WILKML7RIJAQQQOHILVHNUYXNN6MMAVS X-MailFrom: d.csapak@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: On 2/5/26 9:26 AM, Thomas Lamprecht wrote: > 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. sure, makes sense > >> 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.. ok, complicated was the wrong choice of words here, what i meant was 'more code and a longer diff', but yeah this is often not the best metric for such things. if we'd want to be able to reinitialize (for the tests) we can't make it really 'my sub', or did you mean that for the tests we override the getter instead? (just to summarize) the actual reason we need this to be changable/ overwriteable is that the 'host' cpu type model only exists for the host architecture. This can't normally change after the module is loaded, except when running tests, since the host arch can change between them. or is there a way in perl to call a 'my sub' from external or can we 'reload' a module in a way the variables are 'cleared'? > > 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). I can refactor the models into their own module of course, I'd just like to settle on a way to do things. I'd either * like you first suggested, do a common initialization helper that can be called externally (for the tests) and a getter for the cpu_models_by_arch with an arch parameter and a getter for 'all_cpu_models' * have a small (perl?) script that generates the perl module during build with the correct cpu models that provides either constants to use, or similar getters as above. in that case each test case would need to regenerate and reload that perl module for each test somehow (e.g. interpolating the output directly from the generation script?) I'd tend to the first one since it sounds easier to do and as you said, having getters is often better, or did I misunderstand your suggestions?