Re: [PATCH] Fix errno check, prevent spurious errors under heavy load

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 08/08/2012 02:59 PM, Peter Feiner wrote:
>>From man poll(2), poll does not set errno=EAGAIN, however it does set
> errno=EINTR. Have libvirt retry on the appropriate errno.

man poll(2) is slightly wrong - according to POSIX, poll may _also_ set
EAGAIN when "The allocation of internal data structures failed but a
subsequent request may succeed."  But you are correct that poll()
interrupted by a signal must be EINTR.

I went looking (git grep -A3 '\bpoll *([a-zA-Z]'), and some of our code
already checks for _both_ EAGAIN and EINTR, so I'd feel safer updating
ALL the code to check for both instead of one.

> 
> Under heavy load, a program of mine kept getting libvirt errors 'poll on
> socket
> failed: Interrupted system call'. The signals were SIGCHLD from processes
> forked
> by threads unrelated to those using libvirt.
> 
> diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
> index 2530ffa..f2a6922 100644
> --- a/src/rpc/virnetclient.c
> +++ b/src/rpc/virnetclient.c
> @@ -1374,7 +1374,7 @@ static int virNetClientIOEventLoop(virNetClientPtr
> client,
> 
>      repoll:
>          ret = poll(fds, ARRAY_CARDINALITY(fds), timeout);
> -        if (ret < 0 && errno == EAGAIN)
> +        if (ret < 0 && errno == EINTR)
>              goto repoll;
> 
>          ignore_value(pthread_sigmask(SIG_SETMASK, &oldmask, NULL));
> 

I squashed this into your change, and pushed.  I also added you to
AUTHORS; let me know if you prefer an alternate spelling of name or email.

diff --git i/src/rpc/virnetclient.c w/src/rpc/virnetclient.c
index 2530ffa..42bdc54 100644
--- i/src/rpc/virnetclient.c
+++ w/src/rpc/virnetclient.c
@@ -1,7 +1,7 @@
 /*
  * virnetclient.c: generic network RPC client
  *
- * Copyright (C) 2006-2011 Red Hat, Inc.
+ * Copyright (C) 2006-2012 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -651,7 +651,7 @@ int virNetClientSetTLSSession(virNetClientPtr client,

     repoll:
         ret = poll(fds, ARRAY_CARDINALITY(fds), -1);
-        if (ret < 0 && errno == EAGAIN)
+        if (ret < 0 && (errno == EAGAIN || errno == EINTR))
             goto repoll;

         ignore_value(pthread_sigmask(SIG_BLOCK, &oldmask, NULL));
@@ -675,7 +675,7 @@ int virNetClientSetTLSSession(virNetClientPtr client,

     repoll2:
     ret = poll(fds, ARRAY_CARDINALITY(fds), -1);
-    if (ret < 0 && errno == EAGAIN)
+    if (ret < 0 && (errno == EAGAIN || errno == EINTR))
         goto repoll2;

     ignore_value(pthread_sigmask(SIG_BLOCK, &oldmask, NULL));
@@ -1374,7 +1374,7 @@ static int virNetClientIOEventLoop(virNetClientPtr
client,

     repoll:
         ret = poll(fds, ARRAY_CARDINALITY(fds), timeout);
-        if (ret < 0 && errno == EAGAIN)
+        if (ret < 0 && (errno == EAGAIN || errno == EINTR))
             goto repoll;

         ignore_value(pthread_sigmask(SIG_SETMASK, &oldmask, NULL));
diff --git i/src/util/event_poll.c w/src/util/event_poll.c
index 1bc1d41..0e7d1c1 100644
--- i/src/util/event_poll.c
+++ w/src/util/event_poll.c
@@ -1,7 +1,7 @@
 /*
  * event.c: event loop for monitoring file handles
  *
- * Copyright (C) 2007, 2010-2011 Red Hat, Inc.
+ * Copyright (C) 2007, 2010-2012 Red Hat, Inc.
  * Copyright (C) 2007 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -615,7 +615,7 @@ int virEventPollRunOnce(void) {
     ret = poll(fds, nfds, timeout);
     if (ret < 0) {
         EVENT_DEBUG("Poll got error event %d", errno);
-        if (errno == EINTR) {
+        if (errno == EINTR || errno == EAGAIN) {
             goto retry;
         }
         virReportSystemError(errno, "%s",


-- 
Eric Blake   eblake@xxxxxxxxxx    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]