On Mon, Jan 12, 2026 at 10:56:54AM +0000, Hector CAO via Devel wrote:
Thanks Andreas,
Really appreciate your feedback and suggestions,
I took a look at your revised patch, it looks great, I have however a question:
+if not get_option('msr_module_load').disabled() + if host_machine.system() != 'linux' + if get_option('msr_module_load').enabled() + error('msr module loading can be enabled on Linux only') + endif + endif + if host_machine.cpu_family() not in [ 'x86', 'x86_64' ] + if get_option('msr_module_load').enabled() + error('msr module loading can be enabled on x86 only') + endif + endif + conf.set('WITH_MSR_MODULE_LOAD', 1) +endif
Do we need the second "if get_option('msr_module_load').enabled()" since we already check it in the outer if ?
Yes. The outer check is there to skip the entire block if the user explicitly disabled the feature, while the inner ones are to ensure that we error out if the user explicitly asked for it to be enabled but it's not possible to do so (wrong OS or architecture). But I have actually gotten it wrong, because as it is it doesn't ensure that installation of the file is automatically disabled if the OS is not Linux or the architecture is not x86. This is what it should look like instead: if not get_option('msr_module_load').disabled() msr_module_load = true if host_machine.system() != 'linux' if get_option('msr_module_load').enabled() error('msr module loading can be enabled on Linux only') else msr_module_load = false endif endif if host_machine.cpu_family() not in [ 'x86', 'x86_64' ] if get_option('msr_module_load').enabled() error('msr module loading can be enabled on x86 only') else msr_module_load = false endif endif if msr_module_load conf.set('WITH_MSR_MODULE_LOAD', 1) endif endif I believe it's correct this time around O:-) -- Andrea Bolognani / Red Hat / Virtualization