Re: [PATCH RFC] x86+libxl: correct p2m (shadow) memory pool size calculation

[Jan Beulich <jbeulich@xxxxxxxx>]

On 22.04.2022 13:14, Roger Pau Monné wrote:
> On Fri, Apr 22, 2022 at 12:57:03PM +0200, Jan Beulich wrote:
>> The reference "to shadow the resident processes" is applicable to
>> domains (potentially) running in shadow mode only. Adjust the
>> calculations accordingly.
>>
>> In dom0_paging_pages() also take the opportunity and stop open-coding
>> DIV_ROUND_UP().
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> ---
>> RFC: I'm pretty sure I can't change a public libxl function (deprecated
>>      or not) like this, but I also don't know how I should go about
>>      doing so (short of introducing a brand new function and leaving the
>>      existing one broken).
> 
> You have to play with LIBXL_API_VERSION, see for example:
> 
> 1e3304005e libxl: Make libxl_retrieve_domain_configuration async
> 
>>
>> --- a/tools/include/libxl_utils.h
>> +++ b/tools/include/libxl_utils.h
>> @@ -23,7 +23,10 @@ const
>>  #endif
>>  char *libxl_basename(const char *name); /* returns string from strdup */
>>  
>> -unsigned long libxl_get_required_shadow_memory(unsigned long maxmem_kb, 
>> unsigned int smp_cpus);
>> +unsigned long libxl_get_required_shadow_memory(unsigned long maxmem_kb,
>> +                                               unsigned int smp_cpus,
>> +                                               libxl_domain_type type,
>> +                                               bool hap);
> 
> Iff we are to change this anyway, we might as well rename the
> function and introduce a proper
> libxl_get_required_{paging,p2m}_memory?
> 
> It seems wrong to have a function explicitly named 'shadow' that takes
> a 'hap' parameter.
> 
> If you introduce a new function there's no need to play with the
> LIBXL_API_VERSION and you can just add a new LIBXL_HAVE_FOO define.

With the original function deprecated, I don't see why I'd need to
make a new public function - my fallback plan was (as also suggested
by Jürgen) to make a new internal function.

Jan