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

["Roger Pau Monné" <roger.pau@citrix.com>]

On Fri, Apr 22, 2022 at 01:56:17PM +0200, Jan Beulich wrote:
> 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@suse.com>
> >> ---
> >> 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.

Yes, that would be fine if there's no need to expose the new function
for external callers.

Thanks, Roger.