Re: [Qemu-devel] [PATCH 1/2] block: Fix too early free in multiwrite

[Stefan Hajnoczi]

On Thu, Jul 1, 2010 at 3:31 PM, Kevin Wolf <address@hidden> wrote:
> bdrv_aio_writev may call the callback immediately (and it will commonly do so
> in error cases). If num_requests doesn't have its final value yet,
> multiwrite_cb will falsely detect that all requests are completed and frees
> the mcb. However, the mcb is still used by other requests that are started 
> only
> afterwards. When all requests are completed, it is freed for the second time.
>
> Fix this by setting the right num_requests from the beginning.

Looks good to me.

>
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
>  block.c |    6 ++----
>  1 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/block.c b/block.c
> index c40dd2c..9719649 100644
> --- a/block.c
> +++ b/block.c
> @@ -2198,6 +2198,7 @@ int bdrv_aio_multiwrite(BlockDriverState *bs, 
> BlockRequest *reqs, int num_reqs)
>     num_reqs = multiwrite_merge(bs, reqs, num_reqs, mcb);
>
>     // Run the aio requests
> +    mcb->num_requests = num_reqs;
>     for (i = 0; i < num_reqs; i++) {
>         acb = bdrv_aio_writev(bs, reqs[i].sector, reqs[i].qiov,
>             reqs[i].nb_sectors, multiwrite_cb, mcb);
> @@ -2206,16 +2207,13 @@ int bdrv_aio_multiwrite(BlockDriverState *bs, 
> BlockRequest *reqs, int num_reqs)
>             // We can only fail the whole thing if no request has been
>             // submitted yet. Otherwise we'll wait for the submitted AIOs to
>             // complete and report the error in the callback.
> -            if (mcb->num_requests == 0) {
> +            if (i == 0) {
>                 reqs[i].error = -EIO;
>                 goto fail;
>             } else {
> -                mcb->num_requests++;
>                 multiwrite_cb(mcb, -EIO);
>                 break;
>             }
> -        } else {
> -            mcb->num_requests++;
>         }
>     }
>
> --
> 1.6.6.1
>
>
>

Stefan