Re: [PATCH v2 20/21] libs/{light,guest}: implement xc_cpuid_apply_policy in libxl

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Tue, Apr 13, 2021 at 04:01:38PM +0200, Roger Pau Monne wrote:
> diff --git a/tools/libs/light/libxl_cpuid.c b/tools/libs/light/libxl_cpuid.c
> index 539fc4869e6..cadc8b2a05e 100644
> --- a/tools/libs/light/libxl_cpuid.c
> +++ b/tools/libs/light/libxl_cpuid.c
> @@ -436,6 +438,42 @@ int libxl__cpuid_legacy(libxl_ctx *ctx, uint32_t domid, 
> bool restore,
>       */
>      bool nested_virt = info->nested_hvm.val > 0;
>  
> +    policy = xc_cpu_policy_init();
> +    if (!policy) {
> +        LOGE(ERROR, "Failed to init CPU policy");
> +        rc = ERROR_FAIL;
> +        goto out;
> +    }
> +
> +    rc = xc_cpu_policy_get_domain(ctx->xch, domid, policy);

libxc return values should be stored in 'r'.

> +    if (rc) {
> +        LOGE(ERROR, "Failed to fetch domain %u CPU policy", domid);

It's probably better to use LOGED(ERROR, domid, ...)  to log the domid.
libvirt might be able to use that domid information, but I'm not sure
about that. At least, using LOG*D macros would make logging the domid
consistent with the rest of libxl.

Also use the LOG*D variant for other LOG*, to record the domid
every time.

> +        rc = ERROR_FAIL;
> +        goto out;
> +    }
> +
> +    /*
> +     * Account for feature which have been disabled by default since Xen 
> 4.13,

Why the 's' from "features" has been removed? Also, if it must be
removed then I'm pretty sure that we should also change "have been"
to "has been".


Thanks,

-- 
Anthony PERARD