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

[Anthony PERARD <anthony.perard@citrix.com>]

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