Re: [PATCH 5/7] tools: Modify single-domid callers of xc_domain_getinfolist

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 26/04/2023 3:59 pm, Alejandro Vallejo wrote:
> diff --git a/tools/libs/light/libxl_dom.c b/tools/libs/light/libxl_dom.c
> index 25fb716084..482e04b38c 100644
> --- a/tools/libs/light/libxl_dom.c
> +++ b/tools/libs/light/libxl_dom.c
> @@ -70,15 +70,10 @@ int libxl__domain_cpupool(libxl__gc *gc, uint32_t domid)
>      xc_domaininfo_t info;
>      int ret;
>  
> -    ret = xc_domain_getinfolist(CTX->xch, domid, 1, &info);
> -    if (ret != 1)
> +    ret = xc_domain_getinfo_single(CTX->xch, domid, &info);
> +    if (ret < 0)
>      {
> -        LOGE(ERROR, "getinfolist failed %d", ret);
> -        return ERROR_FAIL;
> -    }
> -    if (info.domain != domid)
> -    {
> -        LOGE(ERROR, "got info for dom%d, wanted dom%d\n", info.domain, 
> domid);
> +        LOGE(ERROR, "getinfo_single failed %d", ret);

These are vaguely for human consumption.  This one wants to be

LOGED(ERROR, domid, "get dominfo failed: %d", ret);

I think.  (This code quite possibly predates LOGED() being introduced.)

> diff --git a/tools/libs/light/libxl_domain.c b/tools/libs/light/libxl_domain.c
> index 7f0986c185..33ac8e9ce8 100644
> --- a/tools/libs/light/libxl_domain.c
> +++ b/tools/libs/light/libxl_domain.c
> @@ -349,16 +349,12 @@ int libxl_domain_info(libxl_ctx *ctx, libxl_dominfo 
> *info_r,
>      int ret;
>      GC_INIT(ctx);
>  
> -    ret = xc_domain_getinfolist(ctx->xch, domid, 1, &xcinfo);
> +    ret = xc_domain_getinfo_single(ctx->xch, domid, &xcinfo);
>      if (ret<0) {
> -        LOGED(ERROR, domid, "Getting domain info list");
> +        LOGED(ERROR, domid, "Getting domain info single");

Swapping list for single really isn't very helpful here.  "Getting
domain info" would be better than either of these, but all of these
ought to be updated to print ret, because right now I don't think
there's any qualifying information.

Interpreting -ESRCH is the important thing here, because that's the
common "your domain doesn't (/no longer) exists" case.

> diff --git a/tools/xenpaging/xenpaging.c b/tools/xenpaging/xenpaging.c
> index 6e5490315d..023f2bf295 100644
> --- a/tools/xenpaging/xenpaging.c
> +++ b/tools/xenpaging/xenpaging.c
> @@ -169,10 +169,10 @@ static int xenpaging_get_tot_pages(struct xenpaging 
> *paging)
>      xc_domaininfo_t domain_info;
>      int rc;
>  
> -    rc = xc_domain_getinfolist(xch, paging->vm_event.domain_id, 1, 
> &domain_info);
> -    if ( rc != 1 )
> +    rc = xc_domain_getinfo_single(xch, paging->vm_event.domain_id, 
> &domain_info);
> +    if ( rc < 0 )
>      {
> -        PERROR("Error getting domain info");
> +        PERROR("Error getting domain info single");

These messages I'd just be tempted to leave as-are.  xenpaging hasn't
left experimental status...

~Andrew