[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


  • To: Stefano Stabellini <sstabellini@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Fri, 25 Jul 2025 07:38:09 +0200
  • Autocrypt: addr=jbeulich@xxxxxxxx; keydata= xsDiBFk3nEQRBADAEaSw6zC/EJkiwGPXbWtPxl2xCdSoeepS07jW8UgcHNurfHvUzogEq5xk hu507c3BarVjyWCJOylMNR98Yd8VqD9UfmX0Hb8/BrA+Hl6/DB/eqGptrf4BSRwcZQM32aZK 7Pj2XbGWIUrZrd70x1eAP9QE3P79Y2oLrsCgbZJfEwCgvz9JjGmQqQkRiTVzlZVCJYcyGGsD /0tbFCzD2h20ahe8rC1gbb3K3qk+LpBtvjBu1RY9drYk0NymiGbJWZgab6t1jM7sk2vuf0Py O9Hf9XBmK0uE9IgMaiCpc32XV9oASz6UJebwkX+zF2jG5I1BfnO9g7KlotcA/v5ClMjgo6Gl MDY4HxoSRu3i1cqqSDtVlt+AOVBJBACrZcnHAUSuCXBPy0jOlBhxPqRWv6ND4c9PH1xjQ3NP nxJuMBS8rnNg22uyfAgmBKNLpLgAGVRMZGaGoJObGf72s6TeIqKJo/LtggAS9qAUiuKVnygo 3wjfkS9A3DRO+SpU7JqWdsveeIQyeyEJ/8PTowmSQLakF+3fote9ybzd880fSmFuIEJldWxp Y2ggPGpiZXVsaWNoQHN1c2UuY29tPsJgBBMRAgAgBQJZN5xEAhsDBgsJCAcDAgQVAggDBBYC AwECHgECF4AACgkQoDSui/t3IH4J+wCfQ5jHdEjCRHj23O/5ttg9r9OIruwAn3103WUITZee e7Sbg12UgcQ5lv7SzsFNBFk3nEQQCACCuTjCjFOUdi5Nm244F+78kLghRcin/awv+IrTcIWF hUpSs1Y91iQQ7KItirz5uwCPlwejSJDQJLIS+QtJHaXDXeV6NI0Uef1hP20+y8qydDiVkv6l IreXjTb7DvksRgJNvCkWtYnlS3mYvQ9NzS9PhyALWbXnH6sIJd2O9lKS1Mrfq+y0IXCP10eS FFGg+Av3IQeFatkJAyju0PPthyTqxSI4lZYuJVPknzgaeuJv/2NccrPvmeDg6Coe7ZIeQ8Yj t0ARxu2xytAkkLCel1Lz1WLmwLstV30g80nkgZf/wr+/BXJW/oIvRlonUkxv+IbBM3dX2OV8 AmRv1ySWPTP7AAMFB/9PQK/VtlNUJvg8GXj9ootzrteGfVZVVT4XBJkfwBcpC/XcPzldjv+3 HYudvpdNK3lLujXeA5fLOH+Z/G9WBc5pFVSMocI71I8bT8lIAzreg0WvkWg5V2WZsUMlnDL9 mpwIGFhlbM3gfDMs7MPMu8YQRFVdUvtSpaAs8OFfGQ0ia3LGZcjA6Ik2+xcqscEJzNH+qh8V m5jjp28yZgaqTaRbg3M/+MTbMpicpZuqF4rnB0AQD12/3BNWDR6bmh+EkYSMcEIpQmBM51qM EKYTQGybRCjpnKHGOxG0rfFY1085mBDZCH5Kx0cl0HVJuQKC+dV2ZY5AqjcKwAxpE75MLFkr wkkEGBECAAkFAlk3nEQCGwwACgkQoDSui/t3IH7nnwCfcJWUDUFKdCsBH/E5d+0ZnMQi+G0A nAuWpQkjM1ASeQwSHEeAWPgskBQL
  • Cc: Penny Zheng <Penny.Zheng@xxxxxxx>, ray.huang@xxxxxxx, Julien Grall <julien@xxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>, Michal Orzel <michal.orzel@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Anthony PERARD <anthony.perard@xxxxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, "Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Fri, 25 Jul 2025 05:38:15 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 25.07.2025 03:21, Stefano Stabellini wrote:
> On Thu, 24 Jul 2025, Jan Beulich wrote:
>> On 23.07.2025 22:30, Stefano Stabellini wrote:
>>> 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.
>>
>> Then again (and going against the thought of making SYSCTL depend on DOMCTL)
>> there may be a desire to query / alter certain properties of the system as
>> a whole, without also having that need for individual domains. But yes,
>> covering both with a single control also is an option to consider.
> 
> If making SYSCTL depend on DOMCTL and/or a single kconfig for both
> SYSCTL and DOMCTL are both way forward, then we can take this patch as
> is?

In both of the named cases this patch simply wouldn't be needed. Once the
conversion work was done, that is. And to be frank, I'm not happy to see
the function move out and then back in.

Jan



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.