Re: [PATCH v1] xen: move getdomaininfo() to domain.c

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 23 Jul 2025, Jan Beulich wrote:
> 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 <jbeulich@xxxxxxxx>
> >>> Signed-off-by: Penny Zheng <Penny.Zheng@xxxxxxx>
> >>
> >> 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?

I think it is OK.

I also think "having SYSCTL depend on DOMCTL" is certainly worth
thinking about. In terms of privilege and potential for interference
with other domains sysctl and domctl don't seem different so it is
unlikely one would want to disable one but not the other.

Another idea is to have a single kconfig for both SYSCTL and DOMCTL: we
don't necessarily need to offer individual kconfig for every feature.
>From a safety point of view, we want to disable them both.