[PATCH v2] tests: commandhelper: Accept POLLNVAL on macOS

Roman Bolshakov posted 1 patch 2 weeks ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20201008140207.80362-1-r.bolshakov@yadro.com
tests/commandhelper.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)

[PATCH v2] tests: commandhelper: Accept POLLNVAL on macOS

Posted by Roman Bolshakov 2 weeks ago
commandhelper hangs indefinitely in poll() on macOS on commandtest test2
and later because POLLNVAL is returned on revents for input file
descriptor opened from /dev/null, i.e this hangs:

  $ tests/commandhelper < /dev/null
  BEGIN STDOUT
  BEGIN STDERR
  ^C

But it works fine with regular stdin:

  $ tests/commandhelper <<< test
  BEGIN STDOUT
  BEGIN STDERR
  test
  test
  END STDOUT
  END STDERR

The issue is mentioned in poll(2):

  BUGS
    The poll() system call currently does not support devices.

With the change all 28 cases in commandtest pass.

Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
Chnges since v1:
 - Extract expected events into a variable (Andrea)
 - Provide upstream issue # and fix a typo (Ján)

 tests/commandhelper.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/tests/commandhelper.c b/tests/commandhelper.c
index 7c260c4e13..ba5681b715 100644
--- a/tests/commandhelper.c
+++ b/tests/commandhelper.c
@@ -190,7 +190,17 @@ int main(int argc, char **argv) {
         }
 
         for (i = 0; i < numpollfds; i++) {
-            if (fds[i].revents & (POLLIN | POLLHUP | POLLERR)) {
+            short revents = POLLIN | POLLHUP | POLLERR;
+
+# ifdef __APPLE__
+            /*
+             * poll() on /dev/null will return POLLNVAL
+             * Apple-Feedback: FB8785208
+             */
+            revents |= POLLNVAL;
+# endif
+
+            if (fds[i].revents & revents) {
                 fds[i].revents = 0;
 
                 got = read(fds[i].fd, buf, sizeof(buf));
-- 
2.28.0


Re: [PATCH v2] tests: commandhelper: Accept POLLNVAL on macOS

Posted by Andrea Bolognani 2 weeks ago
On Thu, 2020-10-08 at 17:02 +0300, Roman Bolshakov wrote:
> With the change all 28 cases in commandtest pass.
> 
> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
> ---
> Chnges since v1:
>  - Extract expected events into a variable (Andrea)
>  - Provide upstream issue # and fix a typo (Ján)
> 
>  tests/commandhelper.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)

  Reviewed-by: Andrea Bolognani <abologna@redhat.com>

and pushed.

Since you seem interested in getting libvirt to work better on macOS,
you might want to take a look at

  https://gitlab.com/libvirt/libvirt/-/issues/58

which contains additional information about the current issues with
the test suite in particular.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH v2] tests: commandhelper: Accept POLLNVAL on macOS

Posted by Roman Bolshakov 2 weeks ago
On Thu, Oct 08, 2020 at 05:12:24PM +0200, Andrea Bolognani wrote:
> 
> Since you seem interested in getting libvirt to work better on macOS,
> you might want to take a look at
> 
>   https://gitlab.com/libvirt/libvirt/-/issues/58
> 
> which contains additional information about the current issues with
> the test suite in particular.
> 

Thanks for the reference, Andrea.

I'll chime in and I'm happy to see a number of people is involved.

Best regards,
Roman