Re: [Xen-devel] [XEN PATCH for-4.13 5/6] libxl: Move shadow_memkb and iommu_memkb defaulting into libxl

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Fri, Oct 04, 2019 at 04:17:06PM +0100, Ian Jackson wrote:
> @@ -862,6 +864,30 @@ static void domcreate_destruction_cb(libxl__egc *egc,
>                                       libxl__domain_destroy_state *dds,
>                                       int rc);
>  
> +static _Bool ok_to_default_memkb_in_create(libxl__gc *gc)

Is there a reason to use _Bool instead of `bool'? It would be the first
_Bool in libxl.

> +{
> +    /*
> +     * This is a fudge.  We are trying to find whether the caller
> +     * calls the old version of libxl_domain_need_memory.  If they do
> +     * then, because it only gets the b_info, and because it can't
> +     * update the b_info (because it's const), it will base its
> +     * calculations on defaulting shadow_memkb and iommu_memkb to 0
> +     * In that case we probably shouldn't default them differently
> +     * during libxl_domain_create.
> +     *
> +     * The result is that the behaviour with old callers is the same
> +     * as in 4.13: no additional memory is allocated for shadow and
> +     * iommu (unless the caller set shadow_memkb, eg from a call to
> +     * libxl_get_required_shadow_memory).
> +     */
> +    return CTX->libxl_domain_need_memory_0x041200_called &&
> +          !CTX->libxl_domain_need_memory_called;

I think the logic here is inverted.

With this patch applied, we have:
xl
    calls libxl_domain_need_memory()
        libxl__domain_config_setdefault()
            because shadow_memkb is default and that function return 0
            then shadow_memkb is still default
    create_domain()
        libxl__domain_config_setdefault()
            same thing, shadow_memkb is untouch, so still default
            libxl__domain_build_info_setdefault()
                shadow_memkb is default
                    -> shadow_memkb = 0

This is different what is expected by the commit message.
> For xl, no change.  The code moves from xl to libxl.

Instead, if xl calls libxl_domain_need_memory_0x041200():
    create_domain()
        libxl__domain_config_setdefault()
            if shadow_memkb is default; ok_to_default_memkb_in_create is 1
                so shadow_memkb is set to a value different that the one
                calculated by need_memory_0x04100.

Or did I miss something?

-- 
Anthony PERARD

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