On 23.07.2025 02:46, Stefano Stabellini wrote:
> On Tue, 22 Jul 2025, Jan Beulich wrote:
>> On 22.07.2025 07:04, Penny Zheng wrote:
>>> Function getdomaininfo() is not only invoked by domctl-op, but also
>>> sysctl-op,
>>> so it shall better live in domain.c, rather than domctl.c. Which is also
>>> applied for arch_get_domain_info(). Style corrections shall be applied at
>>> the same time while moving these functions, such as converting u64 to
>>> uint64_t.
>>>
>>> The movement could also fix CI error of a randconfig picking both SYSCTL=y
>>> and PV_SHIM_EXCLUSIVE=y results in sysctl.c being built, but domctl.c not
>>> being built, which leaves getdomaininfo() undefined, causing linking to
>>> fail.
>>>
>>> Fixes: 34317c508294 ("xen/sysctl: wrap around sysctl hypercall")
>>> Reported-by: Jan Beulich <jbeul...@suse.com>
>>> Signed-off-by: Penny Zheng <penny.zh...@amd.com>
>>
>> I'm not convinced of this approach. In the longer run this would mean
>> wrapping
>> everything you move in "#if defined(CONFIG_SYSCTL) ||
>> defined(CONFIG_DOMCTL)",
>> which I consider undesirable. Without DOMCTL, the usefulness of
>> XEN_SYSCTL_getdomaininfolist is at least questionable. Therefore adding more
>> isolated "#ifdef CONFIG_DOMCTL" just there may be an option. Similarly, as
>> mentioned on the other thread, having SYSCTL depend on DOMCTL is an approach
>> which imo wants at least considering. And there surely are further options.
>>
>> As indicated elsewhere, my preference goes towards reverting the final one or
>> two patches of that series. They can be re-applied once the dependencies were
>> properly sorted, which may (as per above) involve properly introducing a
>> DOMCTL Kconfig setting first.
>
> I don't think this is a good idea.
And implicitly you say that what I put under question in the first paragraph
is a good way forward?
Jan
> The patch that Penny wrote addresses
> this specific issue effectively and allows us to move forward. It also
> helps us continue identifying problems in GitLab CI and prevents
> regressions with CONFIG_SYSCTL.
>
> I am also aware that Penny has already more patches to send, including
> the DOMCTL Kconfig you are asking.
>
> I think this patch is fine as is:
> Reviewed-by: Stefano Stabellini <sstabell...@kernel.org>
>
