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

Matthew Barnes posted 1 patch 1 month, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/cce46da98274f84a3cea16d0e1e34b56d4d09410.1722011581.git.matthew.barnes@cloud.com
There is a newer version of this series
tools/xcutils/lsevtchn.c | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)
[XEN PATCH v4] tools/lsevtchn: Use errno macro to handle hypercall error cases
Posted by Matthew Barnes 1 month, 1 week ago
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 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 | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/tools/xcutils/lsevtchn.c b/tools/xcutils/lsevtchn.c
index d1710613ddc5..16f5e3def023 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,23 @@ int main(int argc, char **argv)
         status.port = port;
         rc = xc_evtchn_status(xch, &status);
         if ( rc < 0 )
-            break;
+        {
+            switch ( errno ) {
+                case EACCES: continue; /* Xen-owned evtchn */
+                case EINVAL: /* Port enumeration has ended */
+                    rc = 0;
+                    goto out;
+
+                case ESRCH:
+                    perror("Domain ID does not correspond to valid domain");
+                    rc = ESRCH;
+                    goto out;
+                default:
+                    perror(NULL);
+                    rc = errno;
+                    goto out;
+            }
+        }
 
         if ( status.status == EVTCHNSTAT_closed )
             continue;
@@ -58,7 +75,8 @@ int main(int argc, char **argv)
         printf("\n");
     }
 
+out:
     xc_interface_close(xch);
 
-    return 0;
+    return rc;
 }
-- 
2.34.1
Re: [XEN PATCH v4] tools/lsevtchn: Use errno macro to handle hypercall error cases
Posted by Jan Beulich 1 month, 1 week ago
On 26.07.2024 18:40, Matthew Barnes wrote:
> @@ -24,7 +25,23 @@ int main(int argc, char **argv)
>          status.port = port;
>          rc = xc_evtchn_status(xch, &status);
>          if ( rc < 0 )
> -            break;
> +        {
> +            switch ( errno ) {
> +                case EACCES: continue; /* Xen-owned evtchn */
> +                case EINVAL: /* Port enumeration has ended */
> +                    rc = 0;
> +                    goto out;
> +
> +                case ESRCH:
> +                    perror("Domain ID does not correspond to valid domain");
> +                    rc = ESRCH;
> +                    goto out;
> +                default:
> +                    perror(NULL);
> +                    rc = errno;
> +                    goto out;
> +            }
> +        }

There are a number of style violations here: Opening figure brace
placement, indentation of the case labels, placement of the
"continue", lack of blank lines between non-fall-through case blocks.
Also why three "goto out" when one would do?

Jan
Re: [XEN PATCH v4] tools/lsevtchn: Use errno macro to handle hypercall error cases
Posted by Matthew Barnes 1 month, 1 week ago
On Mon, Jul 29, 2024 at 09:58:46AM +0200, Jan Beulich wrote:
> On 26.07.2024 18:40, Matthew Barnes wrote:
> > @@ -24,7 +25,23 @@ int main(int argc, char **argv)
> >          status.port = port;
> >          rc = xc_evtchn_status(xch, &status);
> >          if ( rc < 0 )
> > -            break;
> > +        {
> > +            switch ( errno ) {
> > +                case EACCES: continue; /* Xen-owned evtchn */
> > +                case EINVAL: /* Port enumeration has ended */
> > +                    rc = 0;
> > +                    goto out;
> > +
> > +                case ESRCH:
> > +                    perror("Domain ID does not correspond to valid domain");
> > +                    rc = ESRCH;
> > +                    goto out;
> > +                default:
> > +                    perror(NULL);
> > +                    rc = errno;
> > +                    goto out;
> > +            }
> > +        }
> 
> There are a number of style violations here: Opening figure brace
> placement, indentation of the case labels, placement of the
> "continue", lack of blank lines between non-fall-through case blocks.
> Also why three "goto out" when one would do?

There's no particular reason why three "goto out"s were used.

I will tweak these style decisions in patch v5.

Matt