Re: [Xen-devel] [PATCH] libxl: fix libxl_domain_need_memory after 899433f149d

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Fri, May 17, 2019 at 06:05:55PM +0100, Wei Liu wrote:
> After 899433f149d libxl needs to know the content of d_config to
> determine which QEMU is used. The code is changed such that
> libxl__domain_set_device_model needs to be called before
> libxl__domain_build_info_setdefault.
> 
> This is fine for libxl code, but it is problematic for
> libxl_domain_need_memory, which is the only public API that takes a
> build_info. To avoid break its users, provide a compatibility setting
> inside that function.
> 
> Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>

Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

With one nit below.

> ---
> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> Cc: Olaf Hering <olaf@xxxxxxxxx>
> Cc: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> Cc: Juergen Gross <jgross@xxxxxxxx>
> 
> Please test.
> ---
>  tools/libxl/libxl_mem.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/tools/libxl/libxl_mem.c b/tools/libxl/libxl_mem.c
> index 448a2af8fd..fe1f9c2ff8 100644
> --- a/tools/libxl/libxl_mem.c
> +++ b/tools/libxl/libxl_mem.c
> @@ -457,6 +457,24 @@ int libxl_domain_need_memory(libxl_ctx *ctx,
>      libxl_domain_build_info_init(b_info);
>      libxl_domain_build_info_copy(ctx, b_info, b_info_in);
>  
> +    /*
> +     * It has become a requirement that to figure out which QEMU to
> +     * use, libxl will need to peek d_config's content. The code has
> +     * been changed such that one will need to call
> +     * libxl__domain_set_device_model before calling
> +     * libxl__domain_build_info_setdefault inside libxl.
> +     *
> +     * This (problematic) public API is the only one which takes a
> +     * b_info, imposing the same requirement on the users of this
> +     * public API will break them.
> +     *
> +     * Provide a compatibility setting for this function. The
> +     * calculation doesn't really care which QEMU is set here, so we
> +     * go with the upstream default.
> +     */
> +    if (!b_info->device_model_version)
> +     b_info->device_model_version = LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;

There's a hard tab in the line above.

Thanks, Roger.

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