Re: [Qemu-devel] [PATCH] block: Drop bdrv_ioctl_bh_cb

[Paolo Bonzini <pbonzini@redhat.com>]

On 01/06/2016 03:52, Fam Zheng wrote:
> Similar to the "!drv || !drv->bdrv_aio_ioctl" case above, here it is
> okay to set co.ret and return. As pointed out by Paolo, a BH will be
> created as necessary by the caller (bdrv_co_maybe_schedule_bh).
> Besides, as pointed out by Kevin, "data" was leaked before.
> 
> Reported-by: Kevin Wolf <kwolf@redhat.com>
> Reported-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  block/io.c | 20 ++------------------
>  1 file changed, 2 insertions(+), 18 deletions(-)
> 
> diff --git a/block/io.c b/block/io.c
> index 2d832aa..c32f5b7 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -2309,19 +2309,6 @@ int bdrv_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors)
>      return rwco.ret;
>  }
>  
> -typedef struct {
> -    CoroutineIOCompletion *co;
> -    QEMUBH *bh;
> -} BdrvIoctlCompletionData;
> -
> -static void bdrv_ioctl_bh_cb(void *opaque)
> -{
> -    BdrvIoctlCompletionData *data = opaque;
> -
> -    bdrv_co_io_em_complete(data->co, -ENOTSUP);
> -    qemu_bh_delete(data->bh);
> -}
> -
>  static int bdrv_co_do_ioctl(BlockDriverState *bs, int req, void *buf)
>  {
>      BlockDriver *drv = bs->drv;
> @@ -2339,11 +2326,8 @@ static int bdrv_co_do_ioctl(BlockDriverState *bs, int req, void *buf)
>  
>      acb = drv->bdrv_aio_ioctl(bs, req, buf, bdrv_co_io_em_complete, &co);
>      if (!acb) {
> -        BdrvIoctlCompletionData *data = g_new(BdrvIoctlCompletionData, 1);
> -        data->bh = aio_bh_new(bdrv_get_aio_context(bs),
> -                                bdrv_ioctl_bh_cb, data);
> -        data->co = &co;
> -        qemu_bh_schedule(data->bh);
> +        co.ret = -ENOTSUP;
> +        goto out;
>      }
>      qemu_coroutine_yield();
>  out:
> 

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