Currently, lsevtchn aborts its event channel enumeration when it hits
an event channel that 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@cloud.com>
---
Changes in v6:
- Add blank space before label

Changes in v5:
- Code style changes to switch statement

Changes in v4:
- Catch non-critical errors and fail on everything else, instead of
  catching few known critical errors and ignoring everything else
- Use 'perror' to describe miscellaneous critical errors
- Return appropriate error code from lsevtchn tool
- Tweak commit description
---
 tools/xcutils/lsevtchn.c | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/tools/xcutils/lsevtchn.c b/tools/xcutils/lsevtchn.c
index d1710613ddc5..29504c9d2077 100644
--- a/tools/xcutils/lsevtchn.c
+++ b/tools/xcutils/lsevtchn.c
@@ -3,6 +3,7 @@
 #include <stdint.h>
 #include <string.h>
 #include <stdio.h>
+#include <errno.h>
 
 #include <xenctrl.h>
 
@@ -24,7 +25,28 @@ int main(int argc, char **argv)
         status.port = port;
         rc = xc_evtchn_status(xch, &status);
         if ( rc < 0 )
-            break;
+        {
+            switch ( errno )
+            {
+            case EACCES: /* Xen-owned evtchn */
+                continue;
+
+            case EINVAL: /* Port enumeration has ended */
+                rc = 0;
+                break;
+
+            case ESRCH:
+                perror("Domain ID does not correspond to valid domain");
+                rc = ESRCH;
+                break;
+
+            default:
+                perror(NULL);
+                rc = errno;
+                break;
+            }
+            goto out;
+        }
 
         if ( status.status == EVTCHNSTAT_closed )
             continue;
@@ -58,7 +80,8 @@ int main(int argc, char **argv)
         printf("\n");
     }
 
+ out:
     xc_interface_close(xch);
 
-    return 0;
+    return rc;
 }
-- 
2.34.1

On Mon, Jul 29, 2024 at 04:02:41PM +0100, Matthew Barnes wrote:
> diff --git a/tools/xcutils/lsevtchn.c b/tools/xcutils/lsevtchn.c
> index d1710613ddc5..29504c9d2077 100644
> --- a/tools/xcutils/lsevtchn.c
> +++ b/tools/xcutils/lsevtchn.c
> @@ -24,7 +25,28 @@ int main(int argc, char **argv)
>          status.port = port;
>          rc = xc_evtchn_status(xch, &status);
>          if ( rc < 0 )
> -            break;
> +        {
> +            switch ( errno )
> +            {
> +            case EACCES: /* Xen-owned evtchn */
> +                continue;
> +
> +            case EINVAL: /* Port enumeration has ended */
> +                rc = 0;
> +                break;
> +
> +            case ESRCH:
> +                perror("Domain ID does not correspond to valid domain");

I think this is going to print:
    "Domain ID does not correspond to valid domain: No such process"

> +                rc = ESRCH;

So, `rc` is going to be the value returned by main(), that is the exit
value of the program `lsevtchn`. Having different exit value might
be useful sometime to distinguish between different errors but that
would at least need to be written in some document. Can we just return
"1" here on error? (I mean "rc = 1")

> +                break;
> +
> +            default:
> +                perror(NULL);

It would be slightly more useful to print which function fails, simply
with:
    perror("xc_evtchn_status");


> +                rc = errno;

Same here, just "rc=1" should be good enough. Then if someone want to
know why `lsevtchn` failed, they can read the error messages.

At least, errno is likely to be between 0 and 255, but still, usually
not very useful as an exit value for a program.

> +                break;
> +            }
> +            goto out;
> +        }
>  
>          if ( status.status == EVTCHNSTAT_closed )
>              continue;
> @@ -58,7 +80,8 @@ int main(int argc, char **argv)
>          printf("\n");
>      }
>  
> + out:
>      xc_interface_close(xch);
>  
> -    return 0;
> +    return rc;
>  }

Thanks,

-- 

Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech