[libvirt] [PATCH v2] util: eventpoll: Survive EBADF on macOS

[Michal Prívozník]

On 08/23/2018 10:49 AM, Roman Bolshakov wrote:
> Fixes:
> https://www.redhat.com/archives/libvir-list/2017-January/msg00978.html
> 
> QEMU is probed through monitor fd to check capabilities during libvirtd init.
> The monitor fd is closed after probing by virQEMUCapsInitQMPCommandFree
> that calls virQEMUCapsInitQMPCommandAbort that calls qemuMonitorClose,
> the latter one notifies the event loop via an interrupt handle in
> qemuMonitorUnregister and after then closes monitor fd.
> 
> There could be a case when interrupt is sent after eventLoop is unlocked
> but before virEventPollRunOnce blocks in poll, shortly before file
> descriptor is closed by qemuMonitorClose. Then poll receives closed monitor
> fd in fdset and returns EBADF.
> 
> EBADF is not mentioned as a valid errno on macOS poll man-page but such
> behaviour can appear release-to-release, according to cpython:
> https://github.com/python/cpython/blob/master/Modules/selectmodule.c#L1161
> 
> The change also fixes the issue in qemucapabilitiestest. It returns
> Bad file descriptor message 25 times without the fix.
> 
> Signed-off-by: Roman Bolshakov <r.bolshakov at yadro.com>
> ---
> 
> Changes since v1:
> * Don't go through dispatch code on EBADF
> 
>  src/util/vireventpoll.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/src/util/vireventpoll.c b/src/util/vireventpoll.c
> index 13d278df13..7d0ffd4113 100644
> --- a/src/util/vireventpoll.c
> +++ b/src/util/vireventpoll.c
> @@ -643,6 +643,12 @@ int virEventPollRunOnce(void)
>          EVENT_DEBUG("Poll got error event %d", errno);
>          if (errno == EINTR || errno == EAGAIN)
>              goto retry;
> +#ifdef __APPLE__
> +        if (errno == EBADF) {
> +            virMutexLock(&eventLoop.lock);
> +            goto stop;
> +        }
> +#endif
>          virReportSystemError(errno, "%s",
>                               _("Unable to poll on file handles"));
>          return -1;
> @@ -660,6 +666,7 @@ int virEventPollRunOnce(void)
>      virEventPollCleanupTimeouts();
>      virEventPollCleanupHandles();
>  
> + stop:

This will cause a warning about unused label on everything else than
__APPLE__. Also, I think it should be named cleanup to match pattern
from other functions of ours.

>      eventLoop.running = 0;
>      virMutexUnlock(&eventLoop.lock);
>      return 0;
> 

Fixed, ACKed and pushed.

Congratulations on your first libvirt contribution!

Michal