Re: [PATCH v8 1/2] libxc: Report consistent errors in xc_resource_op

["Anthony PERARD" <anthony.perard@xxxxxxxxxx>]

On Fri, Feb 27, 2026 at 05:00:05PM +0000, Teddy Astie wrote:
> xc_report_op doesn't update errno in some error conditions.
> Make sure it reports -ENOMEM in out of memory errors and -EINVAL
> in invalid usages errors.
> 
> Signed-off-by: Teddy Astie <teddy.astie@xxxxxxxxxx>
> ---
> v7: Introduced
> v8: Use errno to report errors
> 
>  tools/libs/ctrl/xc_resource.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/tools/libs/ctrl/xc_resource.c b/tools/libs/ctrl/xc_resource.c
> index cb6a97202b..ac1524d1bd 100644
> --- a/tools/libs/ctrl/xc_resource.c
> +++ b/tools/libs/ctrl/xc_resource.c
> @@ -28,7 +28,10 @@ static int xc_resource_op_one(xc_interface *xch, 
> xc_resource_op_t *op)
>                                  XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
>  
>      if ( xc_hypercall_bounce_pre(xch, entries) )
> +    {
> +        errno = ENOMEM;

Looking at xc_hypercall_bounce_pre(), it's looks like `errno` should
already be set. On Linux, that would be `mmap()` or `madvise()` updating
it.

>          return -1;
> +    }
>  
>      platform_op.cmd = XENPF_resource_op;
>      platform_op.u.resource_op.nr_entries = op->nr_entries;
> @@ -54,11 +57,15 @@ static int xc_resource_op_multi(xc_interface *xch, 
> uint32_t nr_ops, xc_resource_
>      call_list = xc_hypercall_buffer_alloc(xch, call_list,
>                                            sizeof(*call_list) * nr_ops);
>      if ( !call_list )
> +    {
> +        errno = ENOMEM;

Here, xc_hypercall_buffer_alloc() should already have updated `errno`.
(It's a function called by xc_hypercall_bounce_pre(), so we've got the
same culprit updating `errno`.)

>          return -1;
> +    }
>  
>      platform_ops = xc_hypercall_buffer_array_create(xch, nr_ops);
>      if ( !platform_ops )
>      {
> +        errno = ENOMEM;

Here, xc_hypercall_buffer_array_create() calls `calloc()` and `malloc()`
which will update `errno`.

>          rc = -1;
>          goto out;
>      }
> @@ -66,6 +73,7 @@ static int xc_resource_op_multi(xc_interface *xch, uint32_t 
> nr_ops, xc_resource_
>      entries_list = xc_hypercall_buffer_array_create(xch, nr_ops);
>      if ( !entries_list )
>      {
> +        errno = ENOMEM;

Same as above.

>          rc = -1;
>          goto out;
>      }
> @@ -81,6 +89,7 @@ static int xc_resource_op_multi(xc_interface *xch, uint32_t 
> nr_ops, xc_resource_
>                          platform_op, sizeof(xen_platform_op_t));
>          if ( !platform_op )
>          {
> +            errno = ENOMEM;

With xc_hypercall_buffer_array_alloc(), `errno` is updated by `mmap` or
`madvise`, like the first case.

>              rc = -1;
>              goto out;
>          }
> @@ -90,6 +99,7 @@ static int xc_resource_op_multi(xc_interface *xch, uint32_t 
> nr_ops, xc_resource_
>                     entries, entries_size);
>          if ( !entries)
>          {
> +            errno = ENOMEM;

Same as above.

>              rc = -1;
>              goto out;
>          }
> @@ -137,6 +147,7 @@ int xc_resource_op(xc_interface *xch, uint32_t nr_ops, 
> xc_resource_op_t *ops)
>      if ( nr_ops > 1 )
>          return xc_resource_op_multi(xch, nr_ops, ops);
>  
> +    errno = EINVAL;

Ha! This one was missing indeed.

The patch description will need to be updated with all the chunk be one
been dropped.

Thanks,


--
Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech