Re: [XEN PATCH v3 RESEND] tools/lsevtchn: Use errno macro to handle hypercall error cases

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Mon, Jul 15, 2024 at 04:36:03PM +0100, Matthew Barnes wrote:
> Currently, lsevtchn aborts its event channel enumeration when it hits
> its first hypercall error, namely:
> * When an event channel doesn't exist at the specified port
> * When the event channel is owned by Xen
> 
> lsevtchn does not distinguish between different hypercall errors, which
> results in lsevtchn missing potential relevant event channels with
> higher port numbers.
> 
> Use the errno macro to distinguish between hypercall errors, and
> continue event channel enumeration if the hypercall error is not
> critical to enumeration.
> 
> Signed-off-by: Matthew Barnes <matthew.barnes@xxxxxxxxx>
> ---
>  tools/xcutils/lsevtchn.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/xcutils/lsevtchn.c b/tools/xcutils/lsevtchn.c
> index d1710613ddc5..e4b3f88fe4ec 100644
> --- a/tools/xcutils/lsevtchn.c
> +++ b/tools/xcutils/lsevtchn.c
> @@ -24,7 +25,18 @@ int main(int argc, char **argv)
>          status.port = port;
>          rc = xc_evtchn_status(xch, &status);
>          if ( rc < 0 )
> -            break;
> +        {
> +            if ( errno == ESRCH )
> +            {
> +                fprintf(stderr, "Domain ID '%d' does not correspond to valid 
> domain.\n", domid);
> +                break;
> +            }
> +
> +            if ( errno == EINVAL )
> +                break;
> +
> +            continue;

Usually, when there's an error, we want to deal with it properly, and
not blindly ignore them. Instead, could you check for error that are
non-fatal, like described in the patch description?

Also, the code change doesn't reflect the patch description. The
description says "continue if error not fatal", but the implementation
is "exit on few known fatal error".

One other potentially useful thing to do would be to print why
xc_evtchn_status() failed like you did for ESRCH, but with perror() (or
err() or warn()) which print a translation of the errno value into text.
Of course, only in case where we stop the enumeration.

Thanks,

-- 

Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech