All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Anthony PERARD" <anthony.perard@vates.tech>
To: "Teddy Astie" <teddy.astie@vates.tech>
Cc: xen-devel@lists.xenproject.org, "Juergen Gross" <jgross@suse.com>
Subject: Re: [PATCH v8 1/2] libxc: Report consistent errors in xc_resource_op
Date: Mon, 02 Mar 2026 15:12:53 +0000	[thread overview]
Message-ID: <aaWo9FfgQMYvy_2I@l14> (raw)
In-Reply-To: <ec92e54a493a16f5bc085738e943b3778c3a0231.1772211384.git.teddy.astie@vates.tech>

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@vates.tech>
> ---
> 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



  parent reply	other threads:[~2026-03-02 15:13 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-27 17:00 [PATCH v8 1/2] libxc: Report consistent errors in xc_resource_op Teddy Astie
2026-02-27 17:00 ` [PATCH v8 2/2] xenpm: Add get-core-temp subcommand Teddy Astie
2026-03-02 16:50   ` Jan Beulich
2026-03-03 10:50     ` Teddy Astie
2026-03-03 11:54       ` Jan Beulich
2026-03-11 16:45         ` Anthony PERARD
2026-03-02 15:12 ` Anthony PERARD [this message]
2026-03-03 10:36   ` [PATCH v8 1/2] libxc: Report consistent errors in xc_resource_op Teddy Astie

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aaWo9FfgQMYvy_2I@l14 \
    --to=anthony.perard@vates.tech \
    --cc=jgross@suse.com \
    --cc=teddy.astie@vates.tech \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.