Re: [Qemu-devel] [QEMU-devel][PATCH v3] aio-posix: Fix concurrent aio_poll/set_fd_handler.

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1549 bytes --]

On Mon, Dec 17, 2018 at 05:48:47PM +0100, remy.noel@blade-group.com wrote:
> From: Remy Noel <remy.noel@blade-group.com>
> 
> It is possible for an io_poll callback to be concurrently executed along
> with an aio_set_fd_handlers. This can cause all sorts of problems, like
> a NULL callback or a bad opaque pointer.
> 
> This changes set_fd_handlers so that it no longer modify existing handlers
> entries and instead, always insert those after having proper initialisation.
> 
> Also, we do not call aio_epoll_update for deleted handlers as this has
> no impact whatsoever.
> 
> Signed-off-by: Remy Noel <remy.noel@blade-group.com>
> ---

Please include a changelog in future patches.  For example:

v3:
 * Don't drop revents when a handler is modified [Stefan]

That way reviewers know what to look for and which issues you have
addressed.

>  util/aio-posix.c | 86 ++++++++++++++++++++++++++++--------------------
>  util/aio-win32.c | 67 ++++++++++++++++---------------------
>  2 files changed, 79 insertions(+), 74 deletions(-)
> 
> diff --git a/util/aio-posix.c b/util/aio-posix.c
> index 51c41ed3c9..d658cf3007 100644
> --- a/util/aio-posix.c
> +++ b/util/aio-posix.c

Thanks!  The worst case I can now imagine is if an fd is handled twice
due to a concurrent aio_set_fd_handler() call, but spurious
->io_read()/->io_write() should not cause problems.

I will wait for Paolo to review this because he is most familiar with
the lockcnt abstraction.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]