On Mon, Sep 04, 2023 at 04:07:54PM +0000, Tage Johansson wrote:
> From: Eric Blake <ebl...@redhat.com>
> 
> CI shows our async handle fails to build on FreeBSD and MacOS (where
> epoll() is not available as a syscall, and therefore not available as
> a Rust crate).  We can instead accomplish the same level-probing
> effects by doing a zero-timeout poll with mio (which defers under the
> hood to epoll on Linux, and kqueue on BSD).
> 
> Fixes: 223a9965 ("rust: async: Create an async friendly handle type")
> CC: Tage Johansson <tage.j.li...@posteo.net>
> Signed-off-by: Eric Blake <ebl...@redhat.com>
> ---
>  rust/Cargo.toml          |  3 ++-
>  rust/src/async_handle.rs | 40 +++++++++++++++++++++++-----------------
>  2 files changed, 25 insertions(+), 18 deletions(-)
> 
> -        // Use epoll to check the current read/write availability on the fd.
> +        // Use mio poll to check the current read/write availability on the 
> fd.
>          // This is needed because Tokio supports only edge-triggered
>          // notifications but Libnbd requires level-triggered notifications.
> -        let mut revent = epoll::Event { data: 0, events: 0 };
>          // Setting timeout to 0 means that it will return immediately.
> -        epoll::wait(epfd, 0, std::slice::from_mut(&mut revent))?;
> -        let revents = Events::from_bits(revent.events).unwrap();
> -        if !revents.contains(Events::EPOLLIN) {
> -            ready_guard.clear_ready_matching(IoReady::READABLE);
> -        }
> -        if !revents.contains(Events::EPOLLOUT) {
> -            ready_guard.clear_ready_matching(IoReady::WRITABLE);
> +        // mio states that it is OS-dependent on whether a single event
> +        // can be both readable and writable, but we survive just fine
> +        // if we only see one direction even when both are available.
> +        poll.registry().register(
> +            &mut SourceFd(&fd),
> +            Token(0),
> +            MioInterest::READABLE | MioInterest::WRITABLE,
> +        )?;
> +        poll.poll(&mut events, Some(Duration::ZERO))?;

Why do we want 'poll.poll()?;', that is, to fail this function if the
poll returns an error?  We _expect_ poll to sometimes return an error
(namely, the fact that it timed out) if there is nothing pending on
the fd, at which point we WANT to successfully clear the ready_guard
for both read and write, rather than to error out of this function.

> +        for event in &events {
> +            if !event.is_readable() {
> +                ready_guard.clear_ready_matching(IoReady::READABLE);
> +            }
> +            if !event.is_writable() {
> +                ready_guard.clear_ready_matching(IoReady::WRITABLE);
> +            }
>          }
>          ready_guard.retain_ready();
> +        poll.registry().deregister(&mut SourceFd(&fd))?;

Furthermore, if the regsiter()/deregister() should always occur in
pairs, the early poll()? exit breaks that assumption (I don't know if
Rust has enough smarts to automatically clean up the unpaired
registration).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Reply via email to