Re: [Qemu-devel] [PATCH v2 16/25] block/raw-win32: create one QEMUWin32AIOState per BDRVRawState

[Paolo Bonzini <pbonzini@redhat.com>]

Il 07/05/2014 12:27, Stefan Hajnoczi ha scritto:
> Each QEMUWin32AIOState event notifier is associated with an AioContext.
> Since BlockDriverState instances can use different AioContexts we cannot
> continue to use a global QEMUWin32AIOState.
>
> Let each BDRVRawState have its own QEMUWin32AIOState and free it when
> BDRVRawState is closed.
>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  block/raw-aio.h   |  1 +
>  block/raw-win32.c | 29 ++++++++++++++++-------------
>  block/win32-aio.c |  8 ++++++++
>  3 files changed, 25 insertions(+), 13 deletions(-)
>
> diff --git a/block/raw-aio.h b/block/raw-aio.h
> index 55e0ccc..6269f3d 100644
> --- a/block/raw-aio.h
> +++ b/block/raw-aio.h
> @@ -45,6 +45,7 @@ void laio_attach_aio_context(void *s, AioContext *new_context);
>  #ifdef _WIN32
>  typedef struct QEMUWin32AIOState QEMUWin32AIOState;
>  QEMUWin32AIOState *win32_aio_init(void);
> +void win32_aio_cleanup(QEMUWin32AIOState *aio);
>  int win32_aio_attach(QEMUWin32AIOState *aio, HANDLE hfile);
>  BlockDriverAIOCB *win32_aio_submit(BlockDriverState *bs,
>          QEMUWin32AIOState *aio, HANDLE hfile,
> diff --git a/block/raw-win32.c b/block/raw-win32.c
> index 064ea31..cf1e9ce 100644
> --- a/block/raw-win32.c
> +++ b/block/raw-win32.c
> @@ -36,8 +36,6 @@
>  #define FTYPE_CD     1
>  #define FTYPE_HARDDISK 2
>
> -static QEMUWin32AIOState *aio;
> -
>  typedef struct RawWin32AIOData {
>      BlockDriverState *bs;
>      HANDLE hfile;
> @@ -300,15 +298,6 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags,
>
>      raw_parse_flags(flags, &access_flags, &overlapped);
>
> -    if ((flags & BDRV_O_NATIVE_AIO) && aio == NULL) {
> -        aio = win32_aio_init();
> -        if (aio == NULL) {
> -            error_setg(errp, "Could not initialize AIO");
> -            ret = -EINVAL;
> -            goto fail;
> -        }
> -    }
> -
>      if (filename[0] && filename[1] == ':') {
>          snprintf(s->drive_path, sizeof(s->drive_path), "%c:\\", filename[0]);
>      } else if (filename[0] == '\\' && filename[1] == '\\') {
> @@ -335,13 +324,21 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags,
>      }
>
>      if (flags & BDRV_O_NATIVE_AIO) {
> -        ret = win32_aio_attach(aio, s->hfile);
> +        s->aio = win32_aio_init();
> +        if (s->aio == NULL) {
> +            CloseHandle(s->hfile);
> +            error_setg(errp, "Could not initialize AIO");
> +            ret = -EINVAL;
> +            goto fail;
> +        }
> +
> +        ret = win32_aio_attach(s->aio, s->hfile);
>          if (ret < 0) {
> +            win32_aio_cleanup(s->aio);
>              CloseHandle(s->hfile);
>              error_setg_errno(errp, -ret, "Could not enable AIO");
>              goto fail;
>          }
> -        s->aio = aio;
>      }
>
>      raw_probe_alignment(bs);
> @@ -389,6 +386,12 @@ static BlockDriverAIOCB *raw_aio_flush(BlockDriverState *bs,
>  static void raw_close(BlockDriverState *bs)
>  {
>      BDRVRawState *s = bs->opaque;
> +
> +    if (s->aio) {
> +        win32_aio_cleanup(s->aio);
> +        s->aio = NULL;
> +    }
> +
>      CloseHandle(s->hfile);
>      if (bs->open_flags & BDRV_O_TEMPORARY) {
>          unlink(bs->filename);
> diff --git a/block/win32-aio.c b/block/win32-aio.c
> index 5d1d199..b43b166 100644
> --- a/block/win32-aio.c
> +++ b/block/win32-aio.c
> @@ -204,3 +204,11 @@ out_free_state:
>      g_free(s);
>      return NULL;
>  }
> +
> +void win32_aio_cleanup(QEMUWin32AIOState *aio)
> +{
> +    qemu_aio_set_event_notifier(&aio->e, NULL);
> +    CloseHandle(aio->hIOCP);
> +    event_notifier_cleanup(&aio->e);
> +    g_free(aio);
> +}
>

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>