Re: [PATCH v2 7/7] tools: allow to limit xenstore features via guest config

[Anthony PERARD <anthony@xxxxxxxxxxxxxx>]

On Fri, Jul 25, 2025 at 03:19:28PM +0200, Juergen Gross wrote:
> diff --git a/tools/libs/light/libxl_dom.c b/tools/libs/light/libxl_dom.c
> index a61085ca3b..2a7923533f 100644
> --- a/tools/libs/light/libxl_dom.c
> +++ b/tools/libs/light/libxl_dom.c
> @@ -494,6 +494,18 @@ retry_transaction:
>      if (!xs_transaction_end(ctx->xsh, t, 0))
>          if (errno == EAGAIN)
>              goto retry_transaction;
> +
> +    if (info->xenstore_feature_mask != ~0U) {
> +        unsigned int features;
> +
> +        if (xs_get_features_supported(ctx->xsh, &features) &&
> +            !xs_set_features_domain(ctx->xsh, domid,
> +                                    features & info->xenstore_feature_mask)) 
> {
> +            LOG(ERROR, "Failed to set Xenstore features");

Surly xs_{get,set}* set errno on failure, and we know the domid, can you
use LOGED for the error message?

> +            return ERROR_FAIL;

Unfortunately, this function does an allocation that isn't collected in
GC, `vm_path` is leaked. Could you replace that by `rc=ERROR_FAIL; goto
out;`, then place out: at the right place and return `rc` at the end of
the function?

> +        }
> +    }
> +
>      xs_introduce_domain(ctx->xsh, domid, state->store_mfn, 
> state->store_port);
>      free(vm_path);
>      return 0;

Thanks,

-- 
Anthony PERARD