Re: [Xen-devel] [PATCH-for-4.13 v2 0/2] libxl: fix assertion failure

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Thu, Oct 03, 2019 at 02:30:31PM +0100, Ian Jackson wrote:
> Paul Durrant writes ("Re: [Xen-devel] [PATCH-for-4.13 v2 0/2] libxl: fix 
> assertion failure"):
> > On Wed, 2 Oct 2019 at 17:04, Ian Jackson <ian.jackson@xxxxxxxxxx> wrote:
> > > I am continuing to look at the defaulting and config management here
> > > with a view to getting rid of some of the duplicated code and moving
> > > it all into libxl.
> > 
> > That would indeed be beneficial for the likes of libvirt.
> 
> I propose the following plan for 4.13:
> 
>  * Move the default calculations of b_info->shadow_memkb and
>    b_info->iommu_memkb from xl_vmcontrol.c into libxl, in a new
>    function libxl__need_memory_setdefault, called from
>    initiate_domain_create.  That has access to the whole of c_info and
>    b_info.
> 
>  * Change the API/ABI for libxl_domain_need_memory to take a
>    libxl_domain_config.  Internally, this will call an implementation
>    function libxl__domain_need_memory which takes the b_info and
>    c_info separately, and which calls libxl__need_memory_setdefault.
>    (This is the only other call site for
>    libxl__domain_build_info_setdefault.)
> 
>  * There will be the usual backward compatible arrangement: here, a
>    function libxl_domain_need_memory_0x040c00, which will pass NULL
>    for c_info.  The code in libxl__need_memory_setdefault will use 0
>    for the two additional memory amounts when c_info is NULL.
> 
>  * The overall effect is that old callers will get the old behaviour.
>    New callers get the new right behaviour.  This is the same as the
>    present libxl 4.13 code.  Note that libxl_domain_need_memory
>    already has an API stability caveat.
> 
>  * Consequently, the need for libxl_get_required_shadow_memory and
>    libxl_get_required_iommu_memory goes away.  Delete them (they have
>    not been in any release so we can just do this).

libxl_get_required_shadow_memory is old, and libvirt is using.
Only libxl_get_required_iommu_memory is new.

>  * Invent a new value for c_info->passthrough "enabled".  Defaulting
>    will be 1. turn "unknown" into "disabled" or "enabled" according to
>    the current logic based on pcidevs/dtdefs; 2. turn "enabled" into
>    something specific according to the current logic based on type,
>    hap_pt_share, etc.  Make sure this is all correct inside libxl.
> 
>  * Delete the defaulting code in xl.  xl can just leave settings not
>    specified by the user as blank, and libxl will DTRT with them.
> 
> What do people think ?  I really want to fix this for 4.13 because the
> current 4.13 API is not one I want to support.

That plan sound fine to me.

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel